[FFmpeg-devel] [PATCH 9/9] ffplay: add -af option

[Clément Bœsch]

On Fri, Jun 22, 2012 at 12:11:23PM +0200, Stefano Sabatini wrote:
> ---
>  doc/ffplay.texi |    6 +++
>  ffplay.c        |  133 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 130 insertions(+), 9 deletions(-)
> 
> diff --git a/doc/ffplay.texi b/doc/ffplay.texi
> index e2bded7..8ea3b25 100644
> --- a/doc/ffplay.texi
> +++ b/doc/ffplay.texi
> @@ -83,6 +83,12 @@ the input video.
>  Use the option "-filters" to show all the available filters (including
>  also sources and sinks).
>  
> + at item -af @var{filter_graph}
> + at var{filter_graph} is a description of the filter graph to apply to
> +the input audio.
> +Use the option "-filters" to show all the available filters (including
> +sources and sinks).
> +
>  @item -i @var{input_file}
>  Read @var{input_file}.
>  @end table
> diff --git a/ffplay.c b/ffplay.c
> index cb234f3..c8663f6 100644
> --- a/ffplay.c
> +++ b/ffplay.c
> @@ -110,6 +110,7 @@ typedef struct VideoPicture {
>  
>  #if CONFIG_AVFILTER
>      AVFilterBufferRef *picref;
> +    AVFilterBufferRef *samplesref;
>  #endif
>  } VideoPicture;
>  
> @@ -233,6 +234,10 @@ typedef struct VideoState {
>      AVFilterGraph *graph;
>      int use_dr1;
>      FrameBuffer *buffer_pool;
> +
> +    AVFilterContext *in_audio_filter;           ///<the first filter in the audio chain
> +    AVFilterContext *out_audio_filter;          ///<the last filter in the audio chain
> +    AVFilterGraph *agraph;
>  #endif
>  
>      int refresh;
> @@ -289,6 +294,7 @@ static const char *video_codec_name;
>  static int rdftspeed = 20;
>  #if CONFIG_AVFILTER
>  static char *vfilters = NULL;
> +static char *afilters = NULL;
>  #endif
>  
>  /* current context */
> @@ -1634,6 +1640,65 @@ static int configure_video_filters(VideoState *is, const char *vfilters)
>      return ret;
>  }
>  
> +static int configure_audio_filters(VideoState *is)
> +{
> +    static const enum PixelFormat sample_fmts[] = { AV_SAMPLE_FMT_S16, PIX_FMT_NONE };
> +    int64_t layouts[] = { 0, PIX_FMT_NONE };

layouts and pixfmt looks weird.

> +    AVFilterContext *filt_asrc = NULL, *filt_asink = NULL;
> +    AVCodecContext *avctx = is->audio_st->codec;
> +    char abuffer_args[256];
> +    AVABufferSinkParams *abuffersink_params = av_abuffersink_params_alloc();
> +    int ret;
> +
> +    is->agraph = avfilter_graph_alloc();
> +
> +    if (!avctx->channel_layout)
> +        avctx->channel_layout = av_get_default_channel_layout(avctx->channels);
> +    layouts[0] = avctx->channel_layout;
> +
> +    snprintf(abuffer_args, sizeof(abuffer_args), "sample_rate=%d:sample_fmt=%s:channel_layout=0x%"PRIx64,
> +             avctx->sample_rate,
> +             av_get_sample_fmt_name(avctx->sample_fmt),
> +             avctx->channel_layout);
> +    ret = avfilter_graph_create_filter(&filt_asrc,
> +                                       avfilter_get_by_name("abuffer"), "ffplay_abuffer",
> +                                       abuffer_args, NULL, is->agraph);
> +    if (ret < 0) goto fail;
> +
> +    abuffersink_params->sample_fmts     = sample_fmts;
> +    abuffersink_params->channel_layouts = layouts;
> +
> +    ret = avfilter_graph_create_filter(&filt_asink,
> +                                       avfilter_get_by_name("abuffersink"), "ffplay_abuffersink",
> +                                       NULL, abuffersink_params, is->agraph);
> +    if (ret < 0) goto fail;
> +
> +    if (afilters) {
> +        AVFilterInOut *inputs  = av_malloc(sizeof(AVFilterInOut));
> +        AVFilterInOut *outputs = av_malloc(sizeof(AVFilterInOut));
> +
> +        *inputs  = (AVFilterInOut){ av_strdup("out"), filt_asink, 0, NULL };
> +        *outputs = (AVFilterInOut){ av_strdup("in" ), filt_asrc, 0, NULL };
> +

It seems you tried to workaround a very verbose requirement of
libavfilter, but I'm not sure that's for the better here:

 - there is no allocation check; I know we tend to forget them often but
   at least the av_malloc() should be checked. If the names are NULL maybe
   it doesn't break so it might not be worth checking them.
 - why not using avfilter_inout_alloc()? It's unlikely AVFilterInOut will
   become private, but adding an entry at the end will break here (it is
   not zero-ed)

Maybe this would be enough:

   AVFilterInOut *inputs  = avfilter_inout_alloc();
   AVFilterInOut *outputs = avfilter_inout_alloc();
   if (!inputs || !outputs)
       return AVERROR(ENOMEM);
   inputs ->name = av_strdup("out"); inputs ->filter_ctx = filt_asink;
   outputs->name = av_strdup("in");  outputs->filter_ctx = filt_asrc;

Another solution might be to add an helper for this.

   AVFilterInOut *inputs, outputs;

   ret = avfilter_init_inout(&inputs, &outputs, filt_asink, filt_asrc);
   if (ret < 0)
       return ret;

It would automatically add "in" and "out", etc. And it will IMO clarifies
things.

[...]
> +#if CONFIG_AVFILTER
> +        avfilter_graph_free(&is->agraph);
> +#endif
>          break;
>      case AVMEDIA_TYPE_VIDEO:
>          packet_queue_abort(&is->videoq);
> @@ -3002,6 +3116,7 @@ static const OptionDef options[] = {
>      { "window_title", OPT_STRING | HAS_ARG, { (void*)&window_title }, "set window title", "window title" },
>  #if CONFIG_AVFILTER
>      { "vf", OPT_STRING | HAS_ARG, { (void*)&vfilters }, "video filters", "filter list" },
> +    { "af", OPT_STRING | HAS_ARG, {(void*)&afilters}, "audio filters", "filter list" },

nit: space consistency.

I'm not familiar with ffplay so I can't comment more.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120623/69d29427/attachment.asc>