Hi, On 08/26/2011 12:55 PM, Vladimir Pantelic wrote: > On 08/26/2011 05:43 PM, John Stebbins wrote: >> Hi, >> >> On 08/26/2011 05:48 AM, Vladimir Pantelic wrote: > >>> int ticks = st->st->codec->ticks_per_frame + (ist->st->parser ? >>> ist->st->parser->repeat_pict : 0); >>> >> >> Well, after all that, I did some more code reading and spelunking of mailing >> list archives and discovered >> the reason that the code is the way it is. The decoders define repeat_pict >> == 0 to mean one frame time because >> a decoder always returns frames. The parser defines repeat_pict == 0 to >> mean one field time because >> the parser can return after parsing only a field. So when the parser sees a >> frame, it must add 1 >> to repeat pict so that the caller can distinguish field from frame. I think >> this is rather obtuse, but >> it is what it is. Is anybody in favor of adding a field to >> AVCodecParserContext to explicitly flag field >> vs. frame? This option was discussed and rejected in the original mailing >> list discussion. >> This was all discussed in the context of this thread back in feb 2009. >> >> http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/84007/focus=84344 >> > > then maybe at least add some comment lines, otherwise somebody else > will "discover" that again in 2013... >
I'll throw together a patch that improves the comments in avcodec.h for now.
Longer term, I'm investigating what it would take to do this right so it works
correctly with h.264 VFR video. The code currently makes assumptions all
over the place that ticks per field == 1 (when there can be fields) and that
ticks_per_frame == ticks per field + 1. It should instead assume that
ticks_per_frame can be anything and that ticks per field == ticks_per_frame / 2.
I can do this using the current definitions of repeat_pict, or I can add
a flag that explicitly signals when the parser is outputting a field.
I prefer that latter since it removes the obfuscation and provides
additional information that might actually be useful. But I would
like some opinions before I continue with that.
Changing those incorrect assumptions means code that does this:
duration = time_base
if (repeat_pict)
duration *= (1 + repeat_pict)
must become (if we keep the current definitions of repeat_pict)
duration = (1 + repeat_pict) * time_base * ticks_per_frame /
((ticks_per_frame > 1) + 1)
or if we used a field flag to designate fields vs frames:
duration = (1 + !field + repeat_pict) * time_base * ticks_per_frame /
((ticks_per_frame > 1) + 1)
This satisfies the case where there are fields and ticks_per_frame > 1.
repeat_pict specifies the
number of additional field times.
And it satisfies the case where there are not fields and ticks_per_frame == 1.
repeat_pict specifies
the number of additional frame times.
signature.asc
Description: OpenPGP digital signature
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
