[FFmpeg-devel] [PATCH] libavfilter-soc: Make overlay handle still images

[Vitor Sessak]

Martin Storsj? wrote:
> On Thu, 7 May 2009, Vitor Sessak wrote:
> 
>> It does indeed a improves EOF handling, but the serious problem I told you in
>> my previous email remains (and there is no much point in improving EOF
>> handling of broke code). I'll try to explain a little better how the filter
>> library is supposed to work so you can see where the bug is. Suppose your
>> client application wants to feed two inputs to a filter chain and get one
>> output (that it prints to the screen, for example). It will do something like:
>>
>> while (1) {
>>     if (filter chain consumed input 1) {
>>          feed input 1 to the filter chain
>>          if (eof(file1))
>>              tell input filter1 to return AVERROR_EOF
>>     }
>>
>>     if (filter chain consumed input 2) {
>>          feed input 2 to the filter chain
>>          if (eof(file2))
>>              tell input filter2 to return AVERROR_EOF
>>     }
>>
>>     if (avfilter_poll_frame(output_filter)) {
>>          // We have a frame ready for consumption!
>>          if (!avfilter_request_frame(output_filter))
>>              break; // No more frames to output, EOF
>>          else
>>              show(output_filter->current_pic);
>>     }
>> }
>>
>> notice that if the video chain is just an overlay filter, every time
>> request_frame() is called, there are just _one_ frame available from each
>> input. That means that request_frame() in vf_overlay.c can only call
>> avfilter_request_frame() at most once for every input. And the current code
>> (even after your patch) may call it more than once for the first output frame.
> 
> Ok - here's a new attempt at solving this. Now it initially pulls one 
> frame from each input. At subsequent calls, it tries to pull from each 
> input, unless it already has got a frame from an input which it hasn't 
> used at all yet.
> 
> So at the first call, it will pull one frame from each. At the second call 
> it will try to pull one frame from each but will return EOF only if none 
> of them succeed. If it has received a frame from one input, but not yet 
> used it, it won't try to pull anything more from that input until the 
> queued frame is used.
> 
> Does this seem better? Any obvious (or non-obvious) problems in this 
> approach? Hopefully the code comments are enough for understanding the 
> logic behind this.

I like it.

[...]

A nit:

> +    if (!over->pics[0][0] || !over->pics[1][0]) {
> +        /* The first time through, pull a frame from each input */
> +        if (!over->pics[0][0] && avfilter_request_frame(link->src->inputs[0]))
                ^^^^^^^^^^^^^^^^
> +            return AVERROR_EOF;
> +        if (!over->pics[1][0] && avfilter_request_frame(link->src->inputs[1]))
                ^^^^^^^^^^^^^^^^
> +            return AVERROR_EOF;

Those checks are useless (you can assert(!over->pics[0][0] && 
!over->pics[1][0]) after the if() if you wanted to double-check).

> +        /* Try pulling a new candidate from each input unless we already
> +           have one */
> +        if (!over->pics[0][1])
> +            avfilter_request_frame(link->src->inputs[0]);
> +        if (!over->pics[1][1])
> +            avfilter_request_frame(link->src->inputs[1]);
> +        if (!over->pics[0][1] && !over->pics[1][1])
> +            return AVERROR_EOF; /* No new candidates in any input; EOF */

I think it is better to check for the return values of 
avfilter_request_frame() and after (if not both EOF) 
assert(!over->pics[0][1] || !over->pics[1][1]).

> +
> +        if (over->pics[0][1] && over->pics[1][1]) {
> +            /* Candidates in both queues, check which one to use */

/* Neither one of the inputs has finished */

-Vitor