[FFmpeg-devel] [PATCH] lavd/pulse_audio_enc: add buffer length control options

[Timothy Gu]

On Nov 23, 2013 6:32 AM, "Lukasz Marek" <lukasz.m.luki at gmail.com> wrote:
>
> Add options to control the length of the PulseAudio buffer.
>
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  doc/outdevs.texi              |    8 ++++++++
>  libavdevice/pulse_audio_enc.c |   21 +++++++++++++++++++++
>  libavdevice/version.h         |    2 +-
>  3 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/doc/outdevs.texi b/doc/outdevs.texi
> index c737225..0e7309e 100644
> --- a/doc/outdevs.texi
> +++ b/doc/outdevs.texi
> @@ -164,6 +164,14 @@ by default it is set to the specified output name.
>  Specify the device to use. Default device is used when not provided.
>  List of output devices can be obtained with command @command{pactl list
sinks}.
>
> + at item buffer_length
> + at item buffer_duration
> +Controls the length of the PulseAudio buffer. Small buffer gives more
control,
> +but requires more frequent updates.
> +buffer_length specifies length in bytes while buffer_duration specifies
length in miliseconds.
> +When both options are provided then higher value is used.
> +By default PulseAudio set buffer length to around 2 seconds.

Add:
If both options are set, it will print out a warning and
@option{buffer_duration} is used.

> +
>  @end table
>
>  @subsection Examples

[...]

> +    else if (s->buffer_length) {
> +        attr.tlength = s->buffer_length;
> +        if (s->buffer_duration)
> +            av_log(s, AV_LOG_WARNING, "buffer_duration parameter is
ignored.\n");

A few problems here:
a) it is an else if, which means that this will not execute if
   buffer_duration is set, and this will not override buffer_duration
b) even if you changed this to plain if, if user sets both options,
   the code spent time calculating buffer_duration, but at
   the end it is overridden. So you should move buffer_length to the front
of buffer_duration.

> +    }
> +
>      ss.format = ff_codec_id_to_pulse_format(st->codec->codec_id);
>      ss.rate = st->codec->sample_rate;
>      ss.channels = st->codec->channels;
[...]

> +    { "buffer_duration", "set buffer length in msecs",
OFFSET(buffer_duration), AV_OPT_TYPE_INT, {.i64 = 0}, 0, UINT_MAX, E },

AV_OPT_TYPE_DURATION?

[...]

Timothy