[FFmpeg-devel] [PATCH] examples/filtering_audio: get rid of AVABufferSinkParams

[Nicolas George]

L'octidi 28 germinal, an CCXXI, pkoshevoy at gmail.com a écrit :
> From: Pavel Koshevoy <pkoshevoy at gmail.com>
> 
> AVABufferSinkParams are ignored by avfilter_graph_create_filter,
> therefore the example is misleading.  Use av_opt_set_int_list to
> configure abuffersink directly.

Thanks for the patch. Comments follow below.

> 
> Also, make the example a bit more interesting.

I believe the two changes belong in different patches.

> 
> Signed-off-by: Pavel Koshevoy <pkoshevoy at gmail.com>
> ---
>  doc/examples/filtering_audio.c |   45 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 37 insertions(+), 8 deletions(-)
> 
> diff --git a/doc/examples/filtering_audio.c b/doc/examples/filtering_audio.c
> index 67588aa..c891dfe 100644
> --- a/doc/examples/filtering_audio.c
> +++ b/doc/examples/filtering_audio.c
> @@ -36,9 +36,19 @@
>  #include <libavfilter/avcodec.h>
>  #include <libavfilter/buffersink.h>
>  #include <libavfilter/buffersrc.h>
> +#include <libavutil/opt.h>
>  
> -const char *filter_descr = "aresample=8000,aformat=sample_fmts=s16:channel_layouts=mono";
> -const char *player       = "ffplay -f s16le -ar 8000 -ac 1 -";
> +/*
> + * Example of pitch-shifting effect:
> + *
> + * 1. use atempo filter at 48KHz and increase playback tempo
> + * by a factor of 2 thus reducing number of samples per second in half.
> + *

> + * 2. use ffplay to ingest raw audio at 24KHz thus increasing playback
> + * duration by a factor of 2 and resulting in playback at a lower pitch.
> +*/
> +const char *filter_descr = "aresample=48000, atempo=2.0";
> +const char *player       = "ffplay -f s16le -ar 24000 -ac 2 -";

I believe this kind of mismatch is most likely to confuse users, and
therefore should be avoided in an example.

>  
>  static AVFormatContext *fmt_ctx;
>  static AVCodecContext *dec_ctx;
> @@ -88,8 +98,9 @@ static int init_filters(const char *filters_descr)
>      AVFilter *abuffersink = avfilter_get_by_name("abuffersink");
>      AVFilterInOut *outputs = avfilter_inout_alloc();
>      AVFilterInOut *inputs  = avfilter_inout_alloc();
> -    const enum AVSampleFormat sample_fmts[] = { AV_SAMPLE_FMT_S16, -1 };
> -    AVABufferSinkParams *abuffersink_params;
> +    const enum AVSampleFormat out_sample_fmts[] = { AV_SAMPLE_FMT_S16, -1 };
> +    const int64_t out_channel_layouts[] = { AV_CH_LAYOUT_STEREO, -1 };
> +    const int out_sample_rates[] = { 48000, -1 };
>      const AVFilterLink *outlink;
>      AVRational time_base = fmt_ctx->streams[audio_stream_index]->time_base;
>  
> @@ -110,16 +121,34 @@ static int init_filters(const char *filters_descr)
>      }
>  
>      /* buffer audio sink: to terminate the filter chain. */
> -    abuffersink_params = av_abuffersink_params_alloc();
> -    abuffersink_params->sample_fmts     = sample_fmts;
>      ret = avfilter_graph_create_filter(&buffersink_ctx, abuffersink, "out",
> -                                       NULL, abuffersink_params, filter_graph);
> -    av_free(abuffersink_params);
> +                                       NULL, NULL, filter_graph);
>      if (ret < 0) {
>          av_log(NULL, AV_LOG_ERROR, "Cannot create audio buffer sink\n");
>          return ret;
>      }
>  

> +    ret = av_opt_set_int_list(buffersink_ctx, "sample_fmts", out_sample_fmts, -1,
> +                              AV_OPT_SEARCH_CHILDREN);
> +    if (ret < 0) {
> +        av_log(NULL, AV_LOG_ERROR, "Cannot set output sample format\n");
> +        return ret;
> +    }
> +
> +    ret = av_opt_set_int_list(buffersink_ctx, "channel_layouts", out_channel_layouts, -1,
> +                              AV_OPT_SEARCH_CHILDREN);
> +    if (ret < 0) {
> +        av_log(NULL, AV_LOG_ERROR, "Cannot set output channel layout\n");
> +        return ret;
> +    }
> +
> +    ret = av_opt_set_int_list(buffersink_ctx, "sample_rates", out_sample_rates, -1,
> +                              AV_OPT_SEARCH_CHILDREN);
> +    if (ret < 0) {
> +        av_log(NULL, AV_LOG_ERROR, "Cannot set output sample rate\n");
> +        return ret;
> +    }
> +

IMHO, since the lists are available as local arrays, av_opt_set_bin and
sizeof are more elegant. Possibly, an example should show both
possibilities.

>      /* Endpoints for the filter graph. */
>      outputs->name       = av_strdup("in");
>      outputs->filter_ctx = buffersrc_ctx;

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130417/caa9b197/attachment.asc>