[FFmpeg-devel] ffmpeg -af, libavfilter
Clément Bœsch
ubitux at gmail.com
Sat Feb 25 09:45:48 CET 2012
On Fri, Feb 24, 2012 at 07:13:33PM +0100, Stefano Sabatini wrote:
> On date Tuesday 2012-02-21 17:44:28 +0100, Clément Bœsch encoded:
> > Hi folks,
> >
> > This is the 3rd attempt to get the -af option in ffmpeg (Hemanth → Stefano →
> > Clément). And guess what, it's still a work in progress because there are still
>
> You forget Mina ;).
>
Oh, on ffmpeg too? Sorry.
> > a few issues to fix:
> >
> > * -async option is now disabled
> > * -map_channel is now disabled
> > * audio resampling filter doesn't flush
> > * on-the-fly resampling won't work anymore
> >
> > The main reason of all of this is that the current do_audio_out() in ffmpeg.c
> > implements its own resampling, audio compensation and related stuff. And this
> > code is using a lot the audio decode context. This is a problem because the
> > audio buffer in the decoded frame now always contain the data which goes out the
> > filtergraph, and not the data just after the decode.
> >
> > Example with a channel layout change:
> >
> > Current:
> >
> > dec do_audio_out() enc
> > [6ch] --- [6ch ~~~~ 2ch] --- [2ch]
> >
> > With filtergraph system (always active, even without specifying -af):
> >
> > dec filtergraph do_audio_out() enc
> > [6ch] --- [6ch ~~~~ 2ch] --- [2ch ~~~~ 2ch] --- [2ch]
> >
> > So now, do_audio_out() is supposed to receive a sanitized input (which means
> > exactly the same sample rate, number of channels, and format as the output) all
> > the time, and thus can't do the audio sync anymore, as well as -map_channel;
> > the heuristics in do_audio_out() make excessive use of the decode context.
> >
> > I can deal with -map_channel: I would "just" have to insert af_pan filters (and
> > af_amerge to finally support the merge) at the end of the filtergraph, just like
> > we could do for the volume option (with af_volume). So I'll be working on this
> > in the next days.
>
>
> > On the other hand, I don't think I'll be able to correctly deal with the -async
> > option, which we should somehow integrates to libavfilter; anyone wants to do
> > that?
>
> What should the filter exactly do? (I suppose it would be a A|V -> A|V
> filter).
>
It should mainly do this:
http://git.videolan.org/?p=ffmpeg.git;a=blob;f=ffmpeg.c;h=8d727a3a52686601854f2c4a6b6c97db13bccf91;hb=HEAD#l1220
AFAICT it's not related to a/v, but only i/o audio streams ts.
BTW, since it's adding or removing audio samples, it might have its place
in libswresample.
> >
> > Concerning the audio resampling filter which doesn't flush, as Michael said on
> > IRC, it's likely to be handled in a request_frame() callback in af_aresample.
> >
>
> > Last issue is the on-the-fly resampling heuristics (check resample_changed).
> > Libavfilter is not able to deal with that, right? How are we supposed to
> > workaround this?
>
> This is dynamic reconfiguration, I keep talking about this since ages
> but I never implement it, maybe we could add it to the GSoC task, some
> brainstorming on it may help so we have at least a clear design idea.
>
> Currently the abuffersrc source works by normalizing input (same for
> the source video buffer).
>
OK so, as well as -async, this is blocking the -af option.
> > Hopefully, fixing these issues will allow to replace a lot of audio "hacks" in
> > ffmpeg.c (and we should consider make a hard dependency on libavfilter to get
> > rid of all the old code).
>
> Are there still use cases when it makes sense to compile ffmpeg
> without libavfilter? Benchmarks? But I agree we should get rid of the
> non-lavfi path if keeping it is making the code more harder to
> maintain.
>
Just have a look to do_audio_out(). Mixing this code with libavfilter
method will be a hell to maintain IMO. Though as we said, it has at the
moment features libavfilter doesn't have (audio sync and sampling
reconfiguration).
[...]
> > /**
> > + * Fill an AVFrame with the information stored in samplesref.
> > + *
> > + * @param frame an already allocated AVFrame
> > + * @param samplesref an audio buffer reference
> > + * @return 0 in case of success, a negative AVERROR code in case of
> > + * failure
> > + */
> > +int avfilter_fill_frame_from_audio_buffer_ref(AVFrame *frame,
> > + const AVFilterBufferRef *samplesref);
> > +
> > +/**
> > * Fill an AVFrame with the information stored in picref.
> > *
> > * @param frame an already allocated AVFrame
> > --
> > 1.7.9
>
> OK, but maybe we should unify the A/V API and have a single
> avfilter_fill_frame_from_buffer_ref().
>
Yes, sure ok I'll add this one.
> > From 6f8dabe1fa5e3611134b7a20063d691d1c429437 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <clement.boesch at smartjog.com>
> > Date: Tue, 7 Feb 2012 09:36:40 +0100
> > Subject: [PATCH 2/2] ffmpeg/WIP: add -af option.
> >
>
> > Based on a patch by Stefano which itself is based on a patch by Hemanth.
>
> Based on a patch by Mina Nagy Zaky which was based on a patch by
> Stefano which itself is based on a patch by Hemanth.
>
OK :)
> > ---
> > ffmpeg.c | 185 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> > 1 files changed, 175 insertions(+), 10 deletions(-)
> >
> > diff --git a/ffmpeg.c b/ffmpeg.c
> > index 8d727a3..0da8461 100644
> > --- a/ffmpeg.c
> > +++ b/ffmpeg.c
> > @@ -62,6 +62,7 @@
> > # include "libavfilter/buffersink.h"
> > # include "libavfilter/buffersrc.h"
> > # include "libavfilter/vsrc_buffer.h"
> > +# include "libavfilter/asrc_abuffer.h"
> > #endif
> >
> > #if HAVE_SYS_RESOURCE_H
> > @@ -286,9 +287,13 @@ typedef struct OutputStream {
> > #if CONFIG_AVFILTER
> > AVFilterContext *output_video_filter;
> > AVFilterContext *input_video_filter;
> > + AVFilterContext *output_audio_filter;
> > + AVFilterContext *input_audio_filter;
> > AVFilterBufferRef *picref;
> > + AVFilterBufferRef *samplesref;
> > char *avfilter;
> > AVFilterGraph *graph;
> > + AVFilterGraph *agraph;
> > #endif
> >
> > int64_t sws_flags;
> > @@ -703,6 +708,96 @@ static int configure_video_filters(InputStream *ist, OutputStream *ost)
> >
> > return 0;
> > }
> > +
> > +static int configure_audio_filters(InputStream *ist, OutputStream *ost)
> > +{
> > + AVFilterContext *last_filter, *filter;
> > + AVCodecContext * const icodec = ist->st->codec;
> > + AVCodecContext * const ocodec = ost->st->codec;
> > + const AVFilterLink *outlink;
> > +
> > + const enum AVSampleFormat sample_fmts[] = { AV_SAMPLE_FMT_S16, -1 };
> > + const int packing_fmts[] = { AVFILTER_PACKED, -1 };
> > + const int64_t *chlayouts = avfilter_all_channel_layouts;
> > + AVABufferSinkParams *abuffersink_params;
> > +
> > + char args[255];
> > + int ret;
> > +
> > + ost->agraph = avfilter_graph_alloc();
> > + if (!ost->agraph)
> > + return AVERROR(ENOMEM);
> > +
> > + /* input */
> > + if (!icodec->channel_layout)
> > + icodec->channel_layout = av_get_default_channel_layout(icodec->channels);
> > + snprintf(args, sizeof(args), "%d:%d:0x%"PRIx64":packed",
> > + icodec->sample_rate, icodec->sample_fmt, icodec->channel_layout);
> > + ret = avfilter_graph_create_filter(&ost->input_audio_filter,
> > + avfilter_get_by_name("abuffer"), "asrc",
> > + args, NULL, ost->agraph);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* output */
> > + abuffersink_params = av_abuffersink_params_alloc();
> > + abuffersink_params->sample_fmts = sample_fmts;
> > + abuffersink_params->channel_layouts = chlayouts;
> > + abuffersink_params->packing_fmts = packing_fmts;
> > + ret = avfilter_graph_create_filter(&ost->output_audio_filter,
> > + avfilter_get_by_name("abuffersink"), "aout", NULL,
> > + abuffersink_params, ost->agraph);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* auto insert resampling in case of sample rate mismatch */
> > + last_filter = ost->input_audio_filter;
> > + if (icodec->sample_rate != ocodec->sample_rate) {
> > + snprintf(args, sizeof(args), "%d", ocodec->sample_rate);
> > + if ((ret = avfilter_graph_create_filter(&filter, avfilter_get_by_name("aresample"),
> > + NULL, args, NULL, ost->agraph)) < 0)
> > + return ret;
> > + if ((ret = avfilter_link(last_filter, 0, filter, 0)) < 0)
> > + return ret;
> > + last_filter = filter;
> > + }
> > +
> > + /* insert user filter (-af) */
> > + if (ost->avfilter) {
> > + AVFilterInOut *outputs = avfilter_inout_alloc();
> > + AVFilterInOut *inputs = avfilter_inout_alloc();
> > +
> > + outputs->name = av_strdup("in");
> > + outputs->filter_ctx = last_filter;
> > + outputs->pad_idx = 0;
> > + outputs->next = NULL;
> > +
> > + inputs->name = av_strdup("out");
> > + inputs->filter_ctx = ost->output_audio_filter;
> > + inputs->pad_idx = 0;
> > + inputs->next = NULL;
> > +
> > + if ((ret = avfilter_graph_parse(ost->agraph, ost->avfilter, &inputs, &outputs, NULL)) < 0)
> > + return ret;
> > + av_freep(&ost->avfilter);
> > + } else {
> > + if ((ret = avfilter_link(last_filter, 0, ost->output_audio_filter, 0)) < 0)
> > + return ret;
> > + }
> > +
> > + if ((ret = avfilter_graph_config(ost->agraph, NULL)) < 0)
> > + return ret;
> > +
> > + /* set output codec context with settings of the audio buffer sink */
> > + outlink = ost->output_audio_filter->inputs[0];
> > + ocodec->sample_rate = outlink->sample_rate;
> > + ocodec->channel_layout = outlink->channel_layout;
> > + ocodec->channels = av_get_channel_layout_nb_channels(outlink->channel_layout);
> > + ocodec->sample_fmt = outlink->format;
> > +
> > + return 0;
> > +}
>
> I wonder how much of this could be merged with the
> configure_video_filters() code (and if that would be convenient from
> the maintainance point of view).
>
Yes I wanted to avoid too much refactoring for a first working version (I
just changed video filters in previous patches so the functions are
similar enough, and thus can be factorized later) because I'm not sure I
can anticipate all the differences in handling a/v at first.
[...]
> >
> > if (!check_output_constraints(ist, ost) || !ost->encoding_needed)
> > continue;
> > +
> > +#if CONFIG_AVFILTER
> > + while (av_buffersink_poll_frame(ost->output_audio_filter)) {
> > + AVFrame *filtered_frame;
> > +
>
> > + if (av_buffersink_get_buffer_ref(ost->output_audio_filter, &ost->samplesref, 0) < 0) {
> > + av_log(NULL, AV_LOG_WARNING, "AV Filter told us it has audio samples available but failed to output some\n");
>
> This message is a bit lame (think of what "AV Filter" means to the
> average user reading the log), yes I know this is consistent with the
> video path.
>
Any suggestion? I'd be happy to change it as long as we change the one for
video.
BTW, question about this part of the code; why do we use
av_buffersink_poll_frame() at all? Why couldn't we just have:
while (av_buffersink_get_buffer_ref(...)) { ... } ?
This looks more simpler to me...
Also, this poll_frame system is *not* called at all when using lavfi
device; so for instance the recently proposed asetframesize filter can not
work with "-f lavfi -i amovie=a.wav,asetframesize". Can't we simplify all
this stuff?
I also think the callback system with poll_frame/request_frame is quite
overkill, at least for audio (I can't comment on video); couldn't we just
have filters which raise buffer ref with 0 samples, or just raise a NULL
buffer ref when they can't output any samples at some point?
(Sorry to comment this here)
[...]
>
> Note: this requires a FATE test of course, my last incomplete &
> outdated variant can be found here:
> http://gitorious.org/~saste/ffmpeg/sastes-ffmpeg/commit/48d568e058f7356de1ae733ffd53cd63e3d97c69
It will require the code to first pass all the current fate tests, which
is not the case; also, I just though there is also the flushing issue,
which will likely break a lot of tests.
I will send another patchset later.
--
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/20120225/91eba628/attachment.asc>
More information about the ffmpeg-devel
mailing list