[FFmpeg-devel] [PATCH] Fix MPEG-TS seek and frame positions in general
Tue Feb 24 21:13:24 CET 2009
Michael Niedermayer wrote:
> On Mon, Feb 16, 2009 at 06:14:01PM -0800, Baptiste Coudurier wrote:
>> 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×tamp. 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.
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?
Anyway, even if finding keyframe is done in generic way, seeking will
work in O(key_frame_distance * log filesize). But I don't see this as a
major problem. At least it will work :-)
Further, as a prerequisite we still need to correct frame position as
returned from av_read_frame (discussed in another subthread), but I
didn't get to clean it up yet.
> 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...
More information about the ffmpeg-devel