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

[Michael Niedermayer]

On Tue, Feb 24, 2009 at 09:13:24PM +0100, Ivan Schreter wrote:
> Michael Niedermayer wrote:
> > On Mon, Feb 16, 2009 at 06:14:01PM -0800, Baptiste Coudurier wrote:
> >   
> >> Hi,
> >>
> >> Ivan Schreter wrote:
> >>     
> >>> Ivan Schreter wrote:
> >>>       
> >>>> Ivan Schreter wrote:
> >>>>   
> >>>>         
> >>>>> Ivan Schreter wrote:
> >>>>>   
> >>>>>     
> >>>>>           
> >>>>>> [...]
> >>>>>> Patch #5: mpegts seek only searches for a PES packet with correct pid 
> >>>>>> and existing DTS and relies on this being a key frame. This is not 
> >>>>>> even the case in the test file. Read frames via av_read_frame until a 
> >>>>>> key frame is found and return position/timestamp of this frame. This 
> >>>>>> needs #1 to reset the packet reader after reading the frames, so 
> >>>>>> further reads work correctly, #2 to have correct positions from mpegts 
> >>>>>> and #4 to have exact position of the packet for later file seek.
> >>>>>>     
> >>>>>>             
> >> If I understand correctly, this seems to be a general issue with
> >> read_timestamp which does not honor the key frame request.
> >>     
> >
> > originally read_timestamp() was supposed to always saerch for a keyframe
> > and only return its pos&timestamp. If it did that seeking would work alot
> > more reliable ...
> > but this had a flaw i did not fully realize originally, namely that
> > doing it that way needs O(keyframe_distance * log filesize)
> > while first searching for a timestamp and then secondly for a keyframe
> > needs O(packet_distance * log filesize + keyframe_distance)
> > so because its better and faster read_timestamp() ignores keyframes
> > sadly the 2nd step (keyframe search) is missing ...
> >
> >   
> So you mean, the second step of searching for key frame should be done 
> in generic way in utils.c? I was experimenting with it a bit.

yes


> 
> I made a simple routine av_find_keyframe() which consumes read_timestamp 
> function to position the stream to a defined position and then finds the 
> position and DTS of the key frame using av_read_frame() calls. Then, 
> I've replaced all read_timestamp calls with this. It does work, but I 
> didn't get it clean yet, so seek regression doesn't work yet (it hangs 
> for one format). I suppose, the reason is some bug in handling border case.
> 
> Shall I clean it up and prepare a patch for that?

above does not sound entirely correct.

what we need is first to let the existing read_timestamp / binary search thing
do its job to find the position in the stream with the wanted timestamp
this is O(packet_distance * log filesize)

2A.
then once this byte position in the file is found do the keyframe search using
av_read_frame() this would produce one byte position for each stream where
there is the next keyframe.
O(keyframe distance)

2B.
like 2A just backward, and because there is no backward demuxing function
search for all keyframes with av_read_frame() begining at -1024 bytes
then is none found -2048 (stoping at -1024) then -4096 (stoping at -2048)
and so on until each stream has a found keyframe.
O(keyframe distance)

3.
The new (unfinished) API provides us with a  target timestamp the user wants
to seek to and optional max/min timestamps that bound the range acceptable to
her
the timestamp from the user as well as the limits represent pressentation
times at which all streams can be decoded
the possible byte positions to seek to match the (2*number streams) keyrames
found, each of them will allow full presentation once each stream contained
a keyframe, it is its time that has to be used to select where to start.
O(number streams)

example:
target time=5 no min/max
time:        0123456789
stream1:      K    K
stream2:        K    K

if we would seek to the byte position for the first keyframe in stream 1
we could start presenting all streams at pts=3
if we would seek to the byte position for the second keyframe in stream 1
we could start presenting all streams at pts=8
if we would seek to the byte position for the first keyframe in stream 2
we could start presenting all streams at pts=6

pts=6 is closest to the requested point from the user, so seeking to the
first keyframe we found in stream 2 is the best choice
if OTOH the user provided us with a target time=5 max=5 then 
the first keyframe in the first stream would have been the correct choice



[...]
> > also for the new seek API we need to not only find one keyframe of one
> > stream but one keyframe for each active stream and in both directions
> > then from this decide where to seek to. Sounds nasty but shouldnt be
> > because it doesnt cost to consider more streams ...
> >
> >   
> When do you plan to introduce working new seeking API? Anyway, at least 
> correct positions from av_read_frame() and from containers are a 
> prerequisite here as well...

ill work on it ASAP, there should be little to do, changes to
ffplay.c should be trivial, and the rest is just a few lines here and there
assuming you implement what i wrote above :)

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

The educated differ from the uneducated as much as the living from the
dead. -- 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/20090225/aca35fe2/attachment.pgp>