[FFmpeg-devel] [PATCH] Fix MPEG-TS seek and frame positions in general
Tue Feb 17 11:02:03 CET 2009
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.
Yes, read_timestamp will return position of some timestamp, not
necessarily a key frame. mpegts by itself cannot know, if it is a key
frame or not, only after actually parsing the frame found at the given
position (and walking to a frame which IS a key frame).
Generic implementation would then in av_gen_search() call
read_timestamp_and_find_keyframe() instead of just read_timestamp(), and
this new function would call read_timestamp() and then walk the frames
until key frame found. I.e., the code for key frame search which I
implemented for mpegts.c would be moved to utils.c. Do I understand you
The only disadvantage is, if we have some formats which do return
timestamp of a key frame, we'd do an unnecessary read_frame. But since
seeking is IMHO not that time critical, this should be no big deal.
Shall I prepare a patch for that? Michael must also give his OK then, I
> I'm not sure if this code belongs in mpegts demuxer since mpegps also
> suffer from the same problem, using read_timestamps.
Possibly. I didn't check mpegps, since I only want to process mpegts
> I think this could be handled in a generic way in av_seek_frame_binary.
> Maybe mpegts and mpegps demuxer could use AVFMT_GENERIC_INDEX to
> populate index entries with key flag set which will be set by parser, it
> seems current mpegps demuxer use index but consider all frames as key
Currently, it seems like the index is not populated for mpegts. Do you
mean, setting this flag will do it then automagically? I'll try it.
More information about the ffmpeg-devel