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

[Stefano Sabatini]

On date Sunday 2013-11-24 02:35:33 +0100, Lukasz Marek encoded:
> Add options to control the length of the PulseAudio buffer.
> 
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  doc/outdevs.texi              |    9 +++++++++
>  libavdevice/pulse_audio_enc.c |   17 +++++++++++++++++
>  libavdevice/version.h         |    2 +-
>  3 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/outdevs.texi b/doc/outdevs.texi
> index c737225..1255d30 100644
> --- a/doc/outdevs.texi
> +++ b/doc/outdevs.texi
> @@ -164,6 +164,15 @@ 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
> +Control the length and duration of the PulseAudio buffer. A small buffer
> +gives more control, but requires more frequent updates.

> +buffer_length specifies length in bytes while buffer_duration specifies length in milliseconds.

@option{buffer_length} ... @option{buffer_duration} ...

> +When both options are provided then the highest value is used (duration is recalculated to bytes using stream parameters).
> +If they are set to 0 (which is default), the device will use the default PulseAudio duration value.
> +By default PulseAudio set buffer duration to around 2 seconds.
> +
>  @end table
>  
>  @subsection Examples
> diff --git a/libavdevice/pulse_audio_enc.c b/libavdevice/pulse_audio_enc.c
> index a2abc4a..192a046 100644
> --- a/libavdevice/pulse_audio_enc.c
> +++ b/libavdevice/pulse_audio_enc.c
> @@ -35,6 +35,8 @@ typedef struct PulseData {
>      const char *device;
>      pa_simple *pa;
>      int64_t timestamp;
> +    unsigned buffer_length;
> +    unsigned buffer_duration;
>  } PulseData;
>  
>  static av_cold int pulse_write_header(AVFormatContext *h)
> @@ -59,6 +61,19 @@ static av_cold int pulse_write_header(AVFormatContext *h)
>              stream_name = "Playback";
>      }
>  
> +    if (s->buffer_duration) {
> +        int64_t bytes = s->buffer_duration;
> +        bytes *= st->codec->channels * st->codec->sample_rate *
> +                 av_get_bytes_per_sample(st->codec->sample_fmt);
> +        bytes /= 1000;
> +        attr.tlength = FFMAX(s->buffer_length, av_clip64(bytes, 0, 0xFFFFFFFE));

0xFFFFFFFE -> UINT32_MAX-1 ?

why -1?

> +        av_log(s, AV_LOG_DEBUG,
> +               "Buffer duration: %ums recalculated into %"PRId64" bytes buffer.\n",
> +               s->buffer_duration, bytes);
> +        av_log(s, AV_LOG_DEBUG, "Real buffer length is %u bytes\n", attr.tlength);
> +    } else if (s->buffer_length)
> +        attr.tlength = s->buffer_length;
> +
>      ss.format = ff_codec_id_to_pulse_format(st->codec->codec_id);
>      ss.rate = st->codec->sample_rate;
>      ss.channels = st->codec->channels;
> @@ -142,6 +157,8 @@ static const AVOption options[] = {
>      { "name",          "set application name",   OFFSET(name),        AV_OPT_TYPE_STRING, {.str = LIBAVFORMAT_IDENT},  0, 0, E },
>      { "stream_name",   "set stream description", OFFSET(stream_name), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, E },
>      { "device",        "set device name",        OFFSET(device),      AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, E },

> +    { "buffer_length",   "set buffer length in bytes", OFFSET(buffer_length),   AV_OPT_TYPE_INT, {.i64 = 0}, 0, 0xFFFFFFFE, E },

nit++: buffer_size if probably more clear, since "length" is usually
associated to a string length.

Also 0xFFFFFFFE should be replaced by a symbolic constants for better
readability. Also probably if you want to store an uint32_t you should
probably use TYPE_INT64.

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

could overflow in case value > INT_MAX (in general with TYPE_INT you
should use max <= INT_MAX).
-- 
FFmpeg = Fostering Fascinating MultiPurpose Ecletic Generator