[FFmpeg-devel] [PATCH] Frame rate emulation
Michael Niedermayer
michaelni
Sun Jun 17 12:43:39 CEST 2007
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
>
> 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
> 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
[...]
> 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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070617/4349ab61/attachment.pgp>
More information about the ffmpeg-devel
mailing list