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

[Michael Niedermayer]

On Sun, Mar 01, 2009 at 07:16:50PM +0100, Ivan Schreter wrote:
> Michael Niedermayer wrote:
> > On Sun, Mar 01, 2009 at 05:58:30PM +0100, Ivan Schreter wrote:
> >   
> >> Michael Niedermayer wrote:
> >>     
> >>> On Sun, Mar 01, 2009 at 02:31:27PM +0100, Ivan Schreter wrote:
> >>>   
> >>> [...]
> >>>       
> >>>> Why do we need the array with several timestamps? I'd say the code can be 
> >>>> simplified to (names possibly suboptimal):
> >>>>
> >>>> 1) set cur_pts, cur_dts, cur_pos to pts/dts/pos of currently-arriving 
> >>>> (packet) buffer,
> >>>> 2) if frame_pts, frame_dts and frame_pos is unset, set them to cur_pts, 
> >>>> cur_dts and cur_pos,
> >>>> 3) unset next_pts, next_dts and next_pos,
> >>>> 4) call actual parser,
> >>>> 5) if the parser returns a buffer, frame_pts, frame_dts and frame_pos are 
> >>>> actual pts/dts/pos for the parsed buffer, so return them in pts, dts and 
> >>>> pos,
> >>>> 6) reset frame_pts, frame_dts and frame_pos to next_pts, next_dts and 
> >>>> next_pos,
> >>>> 7) reset next_pts, next_dts and next_pos to unset.
> >>>>     
> >>>>         
> >>> this will not work, not at all
> >>> example
> >>> packet1 50% of frame1 pts=0 (iam just writing one timestamp due to lazyness)
> >>> (1) cur_pts=0
> >>> (2) frame_pts=0
> >>> (3) next_pts=-
> >>> (45) parser says nothing yet
> >>> (6) frame_pts=-
> >>> (7) next_pts=-
> >>> packet2 50% of frame1 pts=-
> >>> (1) cur_pts=-
> >>> (2) frame_pts=-
> >>> (3) next_pts=-
> >>> (45) parser returns the packet, correct pts is 0 but you return nothing
> >>>
> >>> this is just the most trivial case, a frame split in 2 packets and detected
> >>> in the second
> >>>   
> >>>       
> >> Uhm, I think this is a misunderstanding of (2). For packet 2, (2) will 
> >> NOT set frame_pts to (unset) cur_pts, but leave it set.
> >>     
> >
> > frame_pts has been already wiped out in (6) of the previous packet.
> > (2) is setting it to unset because it is unset already. its a NOP
> >
> >
> >   
> Sorry I messed it up, 6 and 7 were meant to be executed also only if 
> buffer returned, as:
> 
> 1) set cur_pts, cur_dts, cur_pos to pts/dts/pos of currently-arriving 
> (packet) buffer,
> 2) if frame_pts, frame_dts and frame_pos is unset, set them to cur_pts, 
> cur_dts and cur_pos,
> 3) unset next_pts, next_dts and next_pos,
> 4) call actual parser,
> 5) if the parser returns a buffer:
>    5a) frame_pts, frame_dts and frame_pos are 
>        actual pts/dts/pos for the parsed buffer, so return them in
>        pts, dts and pos,
>    5b) reset frame_pts, frame_dts and frame_pos to next_pts,
>        next_dts and next_pos,
>    5c) reset next_pts, next_dts and next_pos to unset.


packet1 50% of frame1 pts=0
(1) cur_pts=0
(2) frame_pts=0
(3) next_pts=-
(45) parser says nothing yet
packet2 50% of frame1 pts=-
(1) cur_pts=-
(2) frame_pts=0
(3) next_pts=-
(45) parser says nothing yet (it has a complete frame but doesnt know yet)
packet3 50% of frame2 pts=1
(1) cur_pts=1
(2) frame_pts=0
(3) next_pts=-
(45) parser returns packet correctly with pts0
(5b) frame_pts=-
at this point you have lost the timestamp of the next packet


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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- 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/20090301/479301f0/attachment.pgp>