[FFmpeg-devel] [PATCH] Frame rate emulation

Ramiro Ribeiro Polla ramiro
Sun Jun 17 23:59:30 CEST 2007


Michael Niedermayer wrote:
> Hi
> On Sat, Jun 16, 2007 at 11:10:07PM -0300, Ramiro Ribeiro Polla wrote:
>   
>> Hello,
>>
>> Michael Niedermayer wrote:
>>     
>>> Hi
>>>
>>> On Fri, Jun 15, 2007 at 05:57:31PM -0300, Ramiro Ribeiro Polla wrote:
>>>  
>>>       
>>>> Ramiro Ribeiro Polla wrote:
>>>>    
>>>>         
>>>>> Michael Niedermayer wrote:
>>>>>      
>>>>>           
>>>>>> Hi
>>>>>>
>>>>>> On Mon, Jun 11, 2007 at 11:01:50AM -0300, Ramiro Ribeiro Polla wrote:
>>>>>>
>>>>>>        
>>>>>>             
>>>>>>> Michael Niedermayer wrote:
>>>>>>>
>>>>>>>   
>>>>>>>          
>>>>>>>               
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Wed, Jun 06, 2007 at 03:31:03PM -0300, Ramiro Ribeiro Polla wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>     
>>>>>>>>            
>>>>>>>>                 
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> x11grab provides its own frame rate emulation inside its 
>>>>>>>>> read_frame function. Many other grab formats will need it (GDI, 
>>>>>>>>> VFW...), so it would be best to make a format level frame rate 
>>>>>>>>> emulation in FFmpeg, and not each demuxer (also I believe they 
>>>>>>>>> don't belong in the demuxers themselves).
>>>>>>>>>
>>>>>>>>> FFmpeg already provides a frame rate emulation (added for DV), but 
>>>>>>>>> it works depending on a codec's demand, and not at a format level, 
>>>>>>>>> like avcodec.h says:
>>>>>>>>>   /**
>>>>>>>>>    * Frame rate emulation. If not zero, the lower layer (i.e. 
>>>>>>>>> format handler)
>>>>>>>>>    * has to read frames at native frame rate.
>>>>>>>>>    * - encoding: Set by user.
>>>>>>>>>    * - decoding: unused
>>>>>>>>>    */
>>>>>>>>>
>>>>>>>>> It's in Fabrice's TODO "suppress rate_emu from AVCodecContext".
>>>>>>>>>
>>>>>>>>> I don't know DV's needs for rate emulation, but couldn't it be 
>>>>>>>>> passed to ffmpeg.c's AVOutputStream instead of AVCodecContext? It 
>>>>>>>>> would depend on the format then, and not the codec.
>>>>>>>>>
>>>>>>>>> Also, I plan to add that to AVInputStream, right before 
>>>>>>>>> av_read_frame, where it will be useful for grabbing demuxers.
>>>>>>>>>
>>>>>>>>> Can there be 2 places where rate emulation occurs (reading from 
>>>>>>>>> the format, and writing to the codec)?
>>>>>>>>> Is it ok for DV for the rate emulation to occur at writing to the 
>>>>>>>>> output format, so it's checked in AVOutputStream?
>>>>>>>>>  
>>>>>>>>>        
>>>>>>>>>              
>>>>>>>>>                   
>>>>>>>> first, AVInputStream / AVOutputStream are private structs of ffmpeg.c
>>>>>>>> libav* is used by more than just ffmpeg.c thus moving essential 
>>>>>>>> functionality into ffmpeg.c is not ok
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>      
>>>>>>>>            
>>>>>>>>                 
>>>>>>> But ffmpeg.c is currently responsible for the actual frame rate 
>>>>>>> emulation.
>>>>>>> Anyways, it's your decision, so I won't bother with output frame 
>>>>>>> rate emulation anymore.
>>>>>>> I'll just leave that in Fabrice's TODO =)
>>>>>>>
>>>>>>>   
>>>>>>>          
>>>>>>>               
>>>>>>>> second, its the input formats / demuxers job to read the frames 
>>>>>>>> properly
>>>>>>>> that includes reading them at the proper time for realtime stuff, sane
>>>>>>>> capture APIs should provide the needed functionality to capture at 
>>>>>>>> a specific
>>>>>>>> time. simply waiting in ffmpeg.c and then calling the demuxer is a 
>>>>>>>> incredibly
>>>>>>>> inaccurate way to do it, keep in mind ffmpeg does encoding, 
>>>>>>>> writeing and
>>>>>>>> others too, using a seperate thread would probably be needed ...
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>      
>>>>>>>>            
>>>>>>>>                 
>>>>>>> To avoid duplicating the same code in all grabbing formats that 
>>>>>>> require it (only X11 for the moment, but VFW and GDI will also need 
>>>>>>> it), is it ok to move the input frame rate emulation from x11grab.c 
>>>>>>> to libavformat/utils.c under the name ff_rate_emu (or something 
>>>>>>> similar)? Or should it be av_rate_emu?
>>>>>>>    
>>>>>>>          
>>>>>>>               
>>>>>> it should be moved into a new file so its not compiled if no grabing
>>>>>> interface is
>>>>>>
>>>>>>  
>>>>>>        
>>>>>>             
>>>>> Now that I looked closer and implemented it, ffmpeg.c could also 
>>>>> benefit from the same code, so it's best if unconditionally compiled 
>>>>> (= no need for a separate file that depends on grabbing interfaces). 
>>>>> Being this way, is it acceptable to put it in utils.c? If not, where?
>>>>> There are 2 different implementations of rate emulation in FFmpeg. One 
>>>>> in ffmpeg.c, and the other in x11grab.c. Which one is preferred?
>>>>> Attached patch uses ffmpeg.c's.
>>>>> Is it possible for a codec to change time_base? I don't know if it's 
>>>>> best for time_base to be passed on initialization or every call to 
>>>>> av_rate_emu.
>>>>>
>>>>>      
>>>>>           
>>>> Ping.
>>>>    
>>>>         
>>> rejected
>>>
>>> this whole thing is a total mess, demuxers have no bushiness executing
>>> *sleep() from the calling thread, they either must use a seperate thread
>>> or interrupt handler and or return with EAGAIN
>>>
>>> also the new AVrateEmu struct cant be used like you do as it causes
>>> ABI issues if its changes
>>>
>>>  
>>>       
>> Attached patch does not fix most problems you spotted, so it's more an 
>> update of my work-in-progress and further questions to solve those 
>> problems than a review for inclusion.
>>
>> The rate emulation code from x11grab.c was copied from grab.c, so new 
>> patch also removed that one.
>>
>> Now I wonder... Why is waiting not a problem in v4l2.c? Around line 334, 
>> it whiles around if the error is EAGAIN. Isn't that also blocking FFmpeg?
>>     
>
> if v4l2.c does that its broken too
>
>
>   

Luca, if you're reading this thread, can you take a look at this? It 
should be ok to make it return EAGAIN as soon this patch is reviewed 
into acceptance and applied.

>> I made av_rate_emu return EAGAIN instead of sleeping. Now x11grab and 
>> video4linux return EAGAIN immediately, not blocking ffmpeg.c, which now 
>> checks for this error and continues to other demuxers.
>>     
>
> good
>
>
>   
>> 1. Could you be more specific on how AVRateEmu would cause ABI issues? I 
>> failed to see it, so in this patch I still left it like the previous one.
>>     
>
> well your application (ffmpeg.c here but thats no excuse) uses
> AVRateEmu directly not a pointer to it, so if a future libav* adds a variable
> to AVRateEmu the size of AVRateEmu increases but the application was
> compiled with the assumtation of the old size so the space reserved for
> AVRateEmu in te struct in which AVRateEmu was put is now too small
>
> the result would be that any change to AVRateEmu would require us to bump
> the major version number
>
>   

Thanks for the explanation. Now, av_init_rate_emu allocates the memory 
and returns a pointer.

>   
>> 2. Even with EAGAIN, I left "-re" blocking on ffmpeg.c. I still don't 
>> know how to test it, so current patch just simplifies an existing hack, 
>> but does not fix it. Since I don't plan to fix this hack in ffmpeg.c 
>> (but want to fix the rate emulation for grab devices), should I just 
>> leave it as is?
>>     
>
> yes it should be in a seperate patch anyway
>   

Ok.

>
> [...]
>   
>> Index: libavformat/utils.c
>> ===================================================================
>> --- libavformat/utils.c	(revision 9347)
>> +++ libavformat/utils.c	(working copy)
>> @@ -2900,3 +2900,21 @@
>>      }
>>      f->num = num;
>>  }
>> +
>> +int av_rate_emu(AVRateEmu *rate_emu)
>> +{
>> +    int64_t pts = av_rescale((int64_t) rate_emu->frame * rate_emu->time_base.num, 1000000, rate_emu->time_base.den);
>> +    int64_t now = av_gettime() - rate_emu->start;
>> +    if (pts > now)
>> +        return AVERROR(EAGAIN);
>>     
>
> i think it should return the number of (micro/milli/nano whatever) seconds
> left
>
>   
Done.

Patch attached.

Ramiro Polla
-------------- next part --------------
A non-text attachment was scrubbed...
Name: av_rate_emu.diff
Type: text/x-patch
Size: 8874 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070617/6174b38e/attachment.bin>



More information about the ffmpeg-devel mailing list