[FFmpeg-devel] [PATCH] Fix MPEG-TS seek and frame positions in general

[Michael Niedermayer]

On Tue, Feb 10, 2009 at 02:44:07PM +0100, Ivan Schreter wrote:
> Michael Niedermayer wrote:
> > On Tue, Feb 10, 2009 at 11:28:01AM +0100, Ivan Schreter wrote:
> >   
> >> Michael Niedermayer wrote:
> >>     
> >>> On Mon, Feb 09, 2009 at 01:35:58AM +0100, Ivan Schreter wrote:
> >>>   
> >>>       
> >>>> So here we go, patch #4 attached.
> >>>>     
> >>>>         
> >>> ive looked more at it and it looks very wrong
> >>> the newly added variable is a duplicate of cur_pkt.pos and you override
> >>> the use of cur_pkt.pos at some places with it.
> >>> i suspect that moving got_packet: up one line has the same effect as your
> >>> rather bloated patch
> >>>
> >>>   
> >>>       
> >> Maybe it looks wrong to you, but I still believe it's very right.
> >>
> >> Moving the label one line up will address only the case where frame is 
> >> completely contained in one packet (where current code sets no position 
> >>     
> >
> > right, sorry, current code is broken no matter where that label is.
> > but yours still is nowhere near correct
> >
> >   
> I think this is way too harsh statement.

your code is broken your guessing is wrong


> 
> > the time when a frame end/start is detected can be several packets after
> > the point where the first byte of that frame was stored.
> > the obvious way to fix this is to pass pos into av_parser_parse()
> > and reorder it like the timestamps.
> >   
> The only objection could be (to see the problem you are mentioning), if 
> packets inside one AVStream (e.g., single video stream) would be 
> interleaved. I cannot really imagine why would someone do it, but the 

RTFS before guessing
there are things called startcodes they are 4 bytes long and they are
used to detect the end and start of a frame.
As it is with these, they can (and are in reality as well) be split across
packets.
thats mpeg ...
EAC3 is more complex ...


> generic solution would not work here correctly. For such case, the 
> solution with passing of position to the parser and letting the parser 
> do the reordering would be needed.
> 
> I feel 99-100% of the formats won't need this and would unneccessarily 
> have to implement position storage by themselves, when it can be done 
> generically.
> 

> My solution is maybe not 100% complete, but for real-world is definitely 
> better than no solution.

your solution is not going to work for real world and no solution is better
than something that is fundamentally wrong (aka not a step toward working
code)


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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090211/0b5de591/attachment.pgp>