[FFmpeg-devel] [PATCH 3/5] ffmpeg: flush and drain video filters.

[Michael Niedermayer]

On Tue, Mar 13, 2012 at 11:56:54AM +0100, Nicolas George wrote:
> Le quartidi 24 ventôse, an CCXX, Michael Niedermayer a écrit :
> > > Yes, but the problem here is FIFO. It does not satisfy this:
> > > > > Pushing a balanced flux unrequested frames on its inputs (what balanced
> > > > > means being filter-dependant) should not cause frames to accumulate anywhere
> > > > > before or inside the filter.
> > If you push a balanced mix into a box and take nothing out it will
> > accumulate inside
> > if you push in and pull out then the above filter graph does not
> > have an accumulation problem
> 
> You are being too laconic, and I can not decide whether you are describing
> things as they are or as they should be.

I described your axiom only.


> 
> Things, currently, are not like that. Most filters, in fact, do not have
> pull methods at all, and behave exactly like I stated: when you push to
> them, they push to the next.
> 
> My point is that if all filters were made to behave like that (instead of
> most, as it is currently), it would make the code simpler (less duplication
> between poll_frame and request_frame) and move all scheduling problems to
> the sinks, where they are simpler to solve (no need to recurse).

My point is that you have not stated clearly what you propose but the
way i understand your suggestion would render the filter framework
useless or at least severly crippled in terms of what a user can do.


1. your axiom requires zero accumulation inside filters
   Many practical filter frameworks a end user will want to use
   need accumulation inside the graph.
   examples would be filters working with synchronizing 2 or more
   streams, some hardsub filter, ... Theres need to delay/accumulate
   on at least one stream to sync.
   Also there are good reasons why a filter graph itself should be
   a filter that can be part of another filter graph. But if you
   allow accumulate at the end of a graph and not inside a filter graph
   then its not possible for a generic filter graph to be a filter

2. If a push always causes a push on the output then
   filters with multiple inputs would increase pushes and
   feedback loops in the graph would infinitly increase pushes leading
   to infinite lopps
   Allowing loops and feedback was a design goal and is supposed to
   be supported currently.


> 
> > For this to work an application must run its demuxer and protocol in
> > a seperate thread. Not all applications do this, some are single
> > threaded.
> > 
> > With a single theraded application at the filters input a request_frame
> > can either call the demuxer and decoder or not call the demuxer to get
> > more data
> 
> The same reasoning applied to poll_frame too exactly as well. And to prove
> my point, imagine you can write a poll_frame that behaves as you want. Then
> I can write:
> 
> int request_frame() {
>     if what_you_did_in_poll_frame == 0
> 	return EAGAIN
>     rest_of_request_frame
> }
> 
> and I have a request_frame that behaves as I want.
> 
> The difference is that with that design, the behaviour does not change
> whether the surrounding code has called avfilter_poll_frame or not. For the
> record, we currently have exactly that problem in the vf_thumbnail, that is
> what patch 3/6 in the other thread fixes.
> 
> > With poll_frame() we can find out which output of a filter graph has
> > data buffered inside the filter graph. With a request_frame() that
> > returns EAGAIN on an empty filter graph you have a infinite loop
> 
> Why would we have an infinite loop? EAGAIN breaks out of the loop exactly as
> poll_frame returning false would do. Confer the code of the patch: it does
> exactly that. The code is either:
> 
> while(poll_frame()) {
>     request_frame();
>     do_something;
> }
> 
> or:
> 
> while(request_frame() != AVERROR(EAGAIN)) {
>   do_something;
> }
> 
> The difference is that the filters themselves can be made simpler. I know
> for sure for having implemented vf_tile for both versions:
> http://ffmpeg.org/pipermail/ffmpeg-devel/2012-March/121738.html
> http://ffmpeg.org/pipermail/ffmpeg-devel/2012-February/121341.html
> The same applies to vf_thumbnail: with this patchset applied, its poll_frame
> can simply be removed and it will work just as well. I believe the same
> could go for yadif and tinterlace, but I am less familiar with their
> workings. This is even worst vf_select: not only could poll_frame go away,
> but also could all the buffering logic. 
> 
> I am sure so much code simplification can only be appealing to you.

I see 2 problems with droping poll_frame()
1. filters with multiple inputs need to support a failing
   request_frame() at any point, store state and be able to resume.
2. with buffering in the middle of filter graphs the EAGAIN system
   seems to have issues (current poll_frame too has them but it seems
   easier to fix with poll_frame)

Either way, if you can simplify libavfilter while allowing arbitrary
filter graphs to be filters, allowing feedback loops, allowing delays
for syncing streams, and allowing avfilter to be efficiently used in
a wide varity of applications (they may be more push or pull oriented
and may be single theaded)
then iam certainly in favor of such simplifications but i dont want to
loose signifciant features ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120313/5451f17e/attachment.asc>