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

[Michael Niedermayer]

On Tue, Jul 24, 2012 at 09:20:39PM +0200, Reimar Döffinger wrote:
> 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.

split changing the pos argument out into a seperate patch

applied

thanks for reveiwing

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There seems to be only one solution to NIH syndrom, ... a shooting squad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120726/05c8fa28/attachment.asc>