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

[Vitor Sessak]

Martin Storsj? wrote:
> Hi Vitor,
> 
> On Wed, 6 May 2009, Vitor Sessak wrote:
> 
>> Just a question: how are you testing those changes?
> 
> I'm testing it with the following command line:
> 
> ffplay in.mov -vfilters "movie=0:png:overlay.png, scale=0:0 [over1], [in] [over1] overlay=10:10 [out]"
> 
> and similarly with ffmpeg.
> 
> 
>>> +        oldpts = over->pics[idx][1]->pts;
>>>          if(avfilter_request_frame(link->src->inputs[idx]))
>>>              return -1;
>>> +        if(over->pics[idx][1]->pts == oldpts) {
>>> +            /* no new pts, we're probably at eof, pull a frame
>>> +               from the other queue to keep it moving */
>> The correct way to know if the video is over is to check if request_frame()
>> returned -1.
> 
> That was my initial guess, too, but the movie source filter provides the 
> last frame repeatedly instead of returning -1.
> 
> This can easily be fixed by the first attached patch. (It contains checks 
> both before and after movie_get_frame, to avoid unnecessary calls to the 
> underlying layers after reaching eof.)

I like this patch. The only nit is that I think that just one check 
should be enough. Please change that and follow Michael's suggestion of 
AVERROR_EOF and I'll commit it (unless V?ctor wants to comment).

> When this is applied, the eof handling in the overlay filter can be fixed 
> in a few other ways, named -overlay-eof2 and 4.
> 
> The first one, -eof2, alters the frame pulling logic only slightly. The 
> request_frame function returns -1 when unable to pull frames from both 
> streams. Since a stream may contain only one frame, it requires the image 
> copying to try to check over->pics[idx][1] if over->pics[idx][0] is null.
> 
> The second way of solving it, -eof4, moves the last frame from 
> over->pics[idx][1] into over->pics[idx][0] by a 
> start_frame(link->src->inputs[idx], NULL) if there still is a frame in 
> over->pics[idx][1]. This way, all input frames should be output too. (In 
> the original overlay filter logic, the last input frame wouldn't be 
> output.)
> 
> Do these look sensible?
> 
> This doesn't really address all issues in the overlay filter, but is at 
> least a step in the right direction, and fixes the eof issue for me.

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.

Sorry if everything looks very complicated, but overlay is certainly the 
most complex filter API-wise...

-Vitor

PS: I will look at your other, less complicated, patches soon...