[FFmpeg-devel] [GSoC] Qualification Task

[Mina Nagy Zaki]

On Thursday 24 March 2011 19:33:11 Stefano Sabatini wrote:
[...]
> On date Thursday 2011-03-24 10:35:53 +0200, Mina Nagy Zaki encoded:
> > Ok, I'm still a bit confused. So first request_frame is called on my
> > output pad, at which point I need to pass on the request, and the chain
> > of events eventually leads to filter_samples being called on my input
> > pad _OR_ I get EOF on my request_frame.
> 
> Yes.
> 
> > So essentially that means I do filtering in filter_samples
> > and draining in request_frames (where I know it's EOF), and when I'm done
> > with draining I need to actually send down the samples through
> > avfilter_filter_samples to the rest of the chain, right?
> 
> Yes.
> 
> In other words suppose that a filter receives a request_frame() (for
> example, aout pulling filtered samples).
> 
> If you don't need to buffer samples for filtering, you can simply
> handle the incoming samples in filter_samples() and send them
> immediately to the output filter(s).
> 
> If you need to buffer samples, then you'll implement a request_frame()
> callback, which will buffer samples by calling avfilter_request_frame
> until you filled your buffer and can proceed with the actual
> filtering.
> 
> > I haven't actually found a filter that employs both
> > draw_slice/filter_samples and request_frame callbacks in such a manner.
> 
> IIRC fifo works this way.
> 
> > > This is essentially what is done in the case of video, check for
> > > example how it is done in fifo (and of course you need to implement it
> > > in a source). I don't know if this is a mess, but don't worry, we'll
> > > fix it ;-).
> > 
> > Yes, vf_fifo does pass on frames through avfilter_draw_slice from inside
> > its request_frame callback, and actually breaks the draw_slice function
> > stack using an empty callback.
> 
> Uhm, what do you exctly mean with "actually breaks the draw_slice function
> stack"?

What I mean is each filter's draw_slice callback calls avfilter_draw_slice to 
propagated the slice up the chain (during the push phase). vf_fifo breaks this 
stack of function calls by having an empty draw_slice callback.

> > But then what's wrong with a pure pull model at least for audio (because
> > video, it seems, requires the start_frame/end_frame calls)? That is, I
> > get data on request_frame and I process it and return? I could actually
> > do that and use a null filter_frames callback to simply break the
> > function stack.
> 
> Anyway I suppose you can also implement a pure pull model.  You simply
> call filter_samples on the source, which will create the samples and
> send them to the next filter, up to the last filter in the
> filterchain.

That's pushing, not pulling.

> But note that in ffplay/ffmpeg you actually need to pull
> frames from the sink, because this is how the applications work (that
> is, they call avfilter_frame when they need data to inject to the
> output file / to SDL).

Precisely my point. Why the mixed push/pull model when in the end applications 
will pull data?

> What's exactly the problem that you're addressing?

All I'm saying is the API could be simplified by having a pure pull model for 
audio filtering. I can't quite think of an advantage to this mixed model (for 
audio; not sure about video, what with the start/end_frame calls).
IMHO it's more straightforward and filters will all work in a similar manner 
instead of having some of them do the actual filtering in filter_samples while 
others do it in request_frames while using filter_samples as a buffering 
mechanism.

> > [snip: patch comments]
> > I've applied all requested changes, added copyright notice, check
> > for 44.1kHz and docs.
> > 
> > 
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index 32f68f7..3b6bf4e 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> 
> Looks fine, I'll push it to the public branch I'm going to setup for
> audio filters, nice job! :)

Thanks!

> PS For posting patches the recommended way is attaching the patches
> obtained with:
> git format-patch REV.

Will do that from now on.

-- 
Mina