[FFmpeg-soc] Libavfilter RFC: problem with ffmpeg.c/ffplay.c integration

[Vitor Sessak]

Hi, and thanks for reviewing

Michael Niedermayer wrote:
> On Fri, Feb 01, 2008 at 10:18:27PM +0100, Vitor Sessak wrote:
>> Hi
>>
>> Michael Niedermayer wrote:
>>> On Wed, Jan 02, 2008 at 10:00:26AM -0500, Bobby Bingham wrote:
>>>> On Wed, 02 Jan 2008 12:49:24 +0100
>>>> Vitor Sessak <vitor1001 at gmail.com> wrote:
>>>>
>>>>> Hi all,
>>>>>
>>>>> I've spotted a problem with the ffmpeg.c integration using the command
>>>>>
>>>>> ffmpeg -vfilters fps=1 -i input.avi output.avi
>>>>>
>> [...]
>>
>>>>> What is your opinion on that?
>>>> I'll take a closer look at this tonight, but I think solution 3 sounds
>>>> reasonable.
>>> As nothing has been commited yet to solve this IIRC ...
>>> what about
>>>     /**
>>>      * Frame poll callback.  This returns the number of immedeately 
>>> available
>>>      * frames. That is if it returns 1 or larger than the next 
>>> request_frame()
>>>      * is guranteed to return one frame (with no delay)
>>>      *
>>>      * Output video pads only.
>>>      */
>>>     int (*poll_frame)(AVFilterLink *link);
>>> with that we could just check each output stream (there could be more than 
>>> 1)
>>> if a frame could be output. And if yes run a request_frame() on that. If 
>>> no
>>> stream has a >0 return for poll_frame() than another input packet could be
>>> demuxed and decoded (from a file which caused a poll()==0) and send it to 
>>> one
>>> of the input vf_filo
>>> poll_frame() could have a trivial default implementation which just polls
>>> its input filters and returns that, this would be sufficient for 90% of
>>> filters.
>> Something like the following?
> 
> [...]
>> +static int link_in_poll_frame(AVFilterLink *link)
>> +{
>> +    AVFilterLink *link2 = get_extern_input_link(link);
>> +    if(!link2)
>> +        return -1;
>> +    return avfilter_poll_frame(link2);
>> +}
>> +
>>  static int link_in_config_props(AVFilterLink *link)
>>  {
>>      AVFilterLink *link2 = get_extern_input_link(link);
>> @@ -236,6 +245,15 @@
>>      return -1;
>>  }
>>  
>> +static int graph_out_poll_frame(AVFilterLink *link)
>> +{
>> +    AVFilterLink *link2 = get_intern_output_link(link);
>> +
>> +    if(link2)
>> +        return avfilter_poll_frame(link2);
>> +    return -1;
>> +}
> 
> these 2 are inconsistant, with the retuen -1 and avfilter_poll_frame() order

Fixed.

> 
> 
> [...]
>> +static int poll_frame(AVFilterLink *link)
>> +{
>> +    FPSContext *fps = link->src->priv;
>> +
>> +    if (fps->has_frame)
>> +        return 1;
>> +
>> +    if(avfilter_poll_frame(link->src->inputs[0]) &&
>> +       avfilter_request_frame(link->src->inputs[0])) {
>> +        fps->videoend = 1;
>> +        return 1;
>> +    }
>> +
>> +    fps->has_frame = !!(fps->pic && fps->pic->pts >= fps->pts);
>> +    return fps->has_frame;
>> +}
> 
> This should output values larger than 1 if its known that more can be
> output from the available frame.

The fps filter only support decreasing the fps (since it's first 
version). All it does is to drop frames. It can't output more than one 
frame then. Yes, I know, it probably is not ok for SVN like that, but my 
priority is getting avfilter.* vf_crop.c and the ffmpeg.c in a good 
enough shape to SVN. Then I could fix one filter at a time.

> 
> 
>> +
>>  static void end_frame(AVFilterLink *link)
>>  {
>>  }
>> @@ -71,14 +90,17 @@
>>  {
>>      FPSContext *fps = link->src->priv;
>>  
>> -    while(!fps->pic || fps->pic->pts < fps->pts)
>> -        if(avfilter_request_frame(link->src->inputs[0]))
>> -            return -1;
>> +    if (fps->videoend)
>> +        return -1;
>>  
>> +    fps->has_frame=0;
>>      avfilter_start_frame(link, avfilter_ref_pic(fps->pic, ~AV_PERM_WRITE));
>>      avfilter_draw_slice (link, 0, fps->pic->h);
>>      avfilter_end_frame  (link);
>>  
>> +    avfilter_unref_pic(fps->pic);
>> +    fps->pic = NULL;
>> +
>>      fps->pts += fps->timebase;
>>  
>>      return 0;
> 
> This does not look like it would work if poll_frame() isnt used.
> I did not intent to make poll_frame() mandatory to be used, but rather
> an option for cases like ffmpeg.c.
> That is pure use of request_frame() is fine as long as your inputs are
> guranteed to be able to handle it until EOF.
> I think the extra complexity for supporting both cases is quite small

Agreed. Changes made.

New version attached. Will commit in 24h if no one opposes to it or asks 
  more time for reviewing.

-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avfilter_poll2.diff
Type: text/x-patch
Size: 11543 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20080205/ed96264e/attachment.bin>