[FFmpeg-devel] ffmpeg -af, libavfilter

[Stefano Sabatini]

On date Saturday 2012-02-25 09:45:48 +0100, Clément Bœsch encoded:
> On Fri, Feb 24, 2012 at 07:13:33PM +0100, Stefano Sabatini wrote:
[...]
> > > 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.

I see, it may be tricky since it requires to be interfaced with the
ffmpeg.c internals, this could be done by passing an opaque pointer to
the filter, but then you have an application-level-entangled filter,
now I don't know if separating the logic from the ffmpeg.c code would
be even possible and how clean that would be.

[...]
> > > +static int configure_audio_filters(InputStream *ist, OutputStream *ost)
> > > +{
[...]
> > > +}
> > 
> > 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.

No high priority anyway.

> [...]
> > >  
> > >          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.

s/AV Filter/buffersink/
is more specific,

"Sink at the end of the audio filterchain told us..."
may be more intelligible to unholy non-libavfilter-adepts.

> 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?

Possible, I believe poll was required before the buffersink thing in
order to call indirectly avfilter_request_frame(), now
av_buffersink_get_buffer_ref() does that so maybe this can be
simplified.

As for asetframesize I'm not sure, avfilter_request_frame() is
required to return a buffer or fail if it can't, maybe asetframesize
should be changed instead in order to fill a buffer and return it when
avfilter_request_frame() is called (I need to double-check the
asetframesize code). The select filter for example works like that.

> 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?

Possible if this works (sorry for not being more specific).
-- 
FFmpeg = Friendly & Foolish Maxi Perennial Ecumenical Gadget