[FFmpeg-devel] [PATCH 2/3] avfiltergraph: don't query formats if filter has uninitialized inputs

[Michael Niedermayer]

On Thu, May 03, 2012 at 06:35:26PM +0200, Michael Niedermayer wrote:
> On Thu, May 03, 2012 at 04:33:17PM +0200, Nicolas George wrote:
> > Le quintidi 15 floréal, an CCXX, Matthieu Bouron a écrit :
> > > The complex filter system add vsrc_buffer and asrc_buffer (in next patch) at
> > > the end of the avfilter context array and query_formats is currently called
> > > sequentially it. It can lead to a filter initialization failure for filters
> > > with input dependencies (like amerge)
> > > 
> > > The following example will fail on the amerge filter since the two abbufer
> > > are not initialized:
> > >   [ amerge ] [ abuffer ] [ abuffer ] [ abuffersink ]
> > 
> > Well, I had guessed amerge would be somewhere there.
> > 
> > The fact is that amerge is doing something bad: it selects its format using
> > its input's formats. That has all kinds of bad consequences. The one you
> > pointed out, but not only that. For example:
> > 
> > 	[in (stereo)] -> [ resample ] -> [ amerge ] ->
> > 
> > will, unless I am mistaken, select mono as input and insert a stereo->mono
> > converter between resample and amerge.
> > 
> > (The exact problem is that amerge will select its input format as the first
> > supported by resample, which is mono, and this happens before the
> > intersection mechanism has restricted this list to only stereo.)
> > 
> > This is bad, but this is the best I could think of at the time, and still
> > now.
> > 
> > If you have a better idea...
> 
> after merging formats
> each filters output could be choosen so as to maximize the similarity
> to its input and minimize loss of information.
> 
> 
> > 
> > > I agree, this implementation is naive.
> > > 
> > > Another solution whould be to order the avfilter context array when
> > > avfilter_link is called.
> > 
> > It would not work with loops. Loops are still unsupported, but Michael says
> > they should be eventually.
> > 
> > I can suggest the following algorithm:
> > 
> > while (1) {
> >     work_to_do = 0;
> >     success = 0;
> >     for (each filter f) {
> > 	if (!all_inputs_and_outputs_configured(f)) {
> > 	    r = f->query_format();
> > 	    if (r < 0) {
> > 		if (r == AVERROR(EAGAIN))
> > 		    work_to_do = 1;
> > 		else
> > 		    return r;
> > 	    } else {
> > 		success = 1;
> > 	    }
> > 	}
> >     }
> >     if (work_to_do) {
> > 	if (!success) /* a full round and no progress */
> > 	    return AVERROR_SOMEGHING;
> > 	break;
> >     }
> > }
> > 
> > And amerge should return EAGAIN rather than EINVAL if its input is not yet
> > configured.
> 
> iam not sure i interpret this code correctly but if so 
> that loop could take exponential time i think. This could be avoided

i meant quadratic time
and maybe this isnt a real issue, i just wanted to mention it can be
done quicker

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad
-------------- 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/20120503/f9e63e28/attachment.asc>