[FFmpeg-devel] [PATCH 6/7] lavf: move generic index generation code to a later point

Reimar Döffinger Reimar.Doeffinger at gmx.de
Tue Jul 24 21:20:39 CEST 2012


On Tue, Jul 24, 2012 at 09:14:07PM +0200, Reimar Döffinger wrote:
> On Tue, Jul 24, 2012 at 06:15:38PM +0200, Michael Niedermayer wrote:
> > Note the parser internal variables are not available at the later position
> > and thus are replaced by the more correct usage of pkt->pos.
> 
> I think the commit message should say what the purpose is/what it fixes.
> Otherwise I think it is probably ok, though I think it does have a few bad
> sides:
> 1) Index entries are now added later. If the packet queue is large this
> might result in worse behaviour when seeking forward by short amounts.
> 2) Not using the parser position means that for -ES and raw mp2/mp3
> files, the index will usually point right in the middle of a packet,
> which I believe results in one packet of crap after seeking.
> Also, using .pos with those files can lead to adding multiple index
> entries with different time stamps but same pos. I think there is
> some code that tries to handle it, but I am not sure it works too well
> (I guess hacking the mp3 demuxer to return 3 MB at a time for parsing would
> probably show the issue quite strongly).
> Is there a problem with overriding out_pkt.pos to
> st->parser->frame_offset for the !PARSER_FLAG_COMPLETE_FRAMES && AVFMT_GENERIC_INDEX
> case?
> Note that the "proper" condition would probably be something like raw format + parser enabled.

Sorry, wrong order of reviewing. It seems that patch 3) actually does
exactly that.
My suggestion would be to in that patch already change the pos argument
for adding to the index.
That way it should be more obvious what causes the test changes.


More information about the ffmpeg-devel mailing list