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

[Stefano Sabatini]

On date Saturday 2012-06-23 18:54:28 +0200, Marton Balint encoded:
> 
> 
> On Fri, 22 Jun 2012, 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 };
> >+    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);
> 
> Is that a good idea? (setting an attribute of AVCodecContext just
> like that). I'd prefer using a temporary variable for the sanitized
> channel layout.
> 
> Fo condition's like this I like to also check if the number of
> channels matches the channel layout description. E.g.:
> 
> if (!channel_layout || nb_channels != av_get_channel_layout_nb_channels(channel_layout))
> 
> I've seen some streams where the layout was messed up.

Uhm OK, although I think this logic should be moved to the sink buffer
(allowing to specify nb_channels in case the layout is not defined,
and failing/or warning if there is nb_channels/layout mismatch).

> >+    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;
> 

> Before calling audio_open using these are fine for determining the
> normal output of the audio filters. But once the audio_open is
> called, you should enforce the settings set in
> VideoState->audio_tgt.

So what about calling audio_open() and *then* configuring the
filtergraph? This should be simpler than:

configure_filters()
audio_open()
configure_filters() again

> 
> >+
> >+    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 };
> >+
> >+        if ((ret = avfilter_graph_parse(is->agraph, afilters, &inputs, &outputs, NULL)) < 0)
> >+            goto fail;
> >+        av_freep(&afilters);
> >+    } else {
> >+        if ((ret = avfilter_link(filt_asrc, 0, filt_asink, 0)) < 0)
> >+            goto fail;
> >+    }
> >+
> >+    if ((ret = avfilter_graph_config(is->agraph, NULL)) < 0)
> >+        goto fail;
> >+    is->in_audio_filter = filt_asrc;
> >+    is->out_audio_filter = filt_asink;
> >+    return 0;
> >+
> >+fail:
> >+    avfilter_graph_free(&is->agraph);
> >+    return ret;
> >+}
> >+
> >#endif  /* CONFIG_AVFILTER */
> >
> >static int video_thread(void *arg)
> >@@ -2058,6 +2123,7 @@ static void sdl_audio_callback(void *opaque, Uint8 *stream, int len)
> >{
> >    VideoState *is = opaque;
> >    int audio_size, len1;
> >+    AVFilterBufferRef av_unused *samplesref;
> >    int bytes_per_sec;
> >    int frame_size = av_samples_get_buffer_size(NULL, is->audio_tgt.channels, 1, is->audio_tgt.fmt, 1);
> >    double pts;
> >@@ -2072,6 +2138,25 @@ static void sdl_audio_callback(void *opaque, Uint8 *stream, int len)
> >               is->audio_buf      = is->silence_buf;
> >               is->audio_buf_size = sizeof(is->silence_buf) / frame_size * frame_size;
> >           } else {
> >+#if CONFIG_AVFILTER
> >+                const AVCodecContext *avctx = is->audio_st->codec;
> >+
> >+                /* inject the buffer into the filter graph
> >+                 * note that AVFilterBufferRef stores pts with timebase 1/samplerate */
> >+                av_buffersrc_add_frame(is->in_audio_filter, is->frame, 0);
> >+
> >+                if (av_buffersink_get_buffer_ref(is->out_audio_filter,
> >+                                                 &samplesref, 0) < 0)
> >+                    return;
> >+
> >+                pts = samplesref->pts / (double)avctx->sample_rate;
> >+
> >+                is->audio_buf = samplesref->data[0];
> >+                audio_size = samplesref->audio->nb_samples *
> >+                    av_get_channel_layout_nb_channels(samplesref->audio->channel_layout) *
> >+                    av_get_bytes_per_sample(samplesref->format);
> >+#endif
> >+
> 
> You should put this into the audio_decode_frame function. For
> example the resampler there only makes sense for the avfilter
> disabled case.
> 
> You should also check for changing audio input format (channel
> number, sample rate, etc), and reconfigure the audio filters if
> necessary.

OK (again this is a long standing issue, auto-reconfiguration should
be handled in the library).

[...]
-- 
FFmpeg = Fundamental and Free MultiPurpose Energized Gymnast