[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