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

[Stefano Sabatini]

On date Saturday 2013-11-23 15:30:01 +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              |    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.

Control

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 miliseconds.

@option{buffer_length} ... @option{buffer_duration} ...
miliseconds -> milliseconds

> +When both options are provided then higher value is used.

_the_ highest value?

> +By default PulseAudio set buffer length to around 2 seconds.

Also specify something like:

If they are set to 0, the device will use the default PulseAudio
duration value.

> +
>  @end table
>  
>  @subsection Examples
> diff --git a/libavdevice/pulse_audio_enc.c b/libavdevice/pulse_audio_enc.c
> index a2abc4a..0115862 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,23 @@ 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_c(bytes, 0, 0xFFFFFFFE));

why not av_clip64()? (it will use the optimized version if available)

> +        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) {

nit: } else if (...) { on same line

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

This is not consistent with what it was specified in the docs.

[...]
-- 
FFmpeg = Formidable Fundamental Merciful Pitiless Evil Generator