av_seek_frame() without AVSEEK_FLAG_ANY on mpg files returns success without seeking to a keyframe

[mbradshaw]

The documentation of av_seek_frame() makes a contract that it seeks to a keyframe if AVSEEK_FLAG_ANY is not set. mpg files break this contract by not seeking to a keyframe and still returning success.

Updated Edit:
It seems that any AVInputFormat that doesn't have it's own read_seek() or read_seek2() functions defined will default to either ff_seek_frame_binary() or ff_gen_search() (with priority given to ff_seek_frame_binary()). ff_seek_frame_binary() doesn't respect seeking by keyframe, and will break the keyframe contract made by av_seek_frame() when AVSEEK_FLAG_ANY is not set.

Proposed solutions:
1)
Seeking in mpg should require AVSEEK_FLAG_ANY to be set in order for success to be returned (return failure if it's not specified). This can be done by requiring AVSEEK_FLAG_ANY to be set before calling ff_seek_frame_binary().

2) (in response to Don's suggestions)
Change ff_seek_frame_binary() so that it respects seeking by keyframe if AVSEEK_FLAG_ANY is not set. This can be done by sequentially reading (either forwards or backwards (I would prefer backwards)) up to a keyframe after the binary search is complete.

For context on how this came about, see this thread: ​http://ffmpeg.org/pipermail/libav-user/2012-July/002468.html

I originally opened ticket #1575 but Don Moir pointed out this is more of a bug than a lack of documentation.

[DonMoir]

I don't think the above is the solution and in fact it may break a lot of applications.

I think the real solution is to make seeking to a timestamp work as it is supposed to, which is seeking to a keyframe. This would involve fixing/adding seek code for mpeg related code. I think now it just uses the generic seek code.

Work arounds for this are clumsy where it probably is fairly easy to fix for someone knowledgeable in this area. Also, fixing it once in the place it should be fixed means many others don't have to spend time on it.

[mbradshaw]

I think that's a good point. One option is to add custom seek code to mpg. Another option is to add some new code to ff_seek_frame_binary() (which is what I believe mpg defaults to using, judging by the code I'm reading). ff_seek_frame_binary() could be changed so that if AVSEEK_FLAG_ANY is not set, after it finds its frame, and if that frame is not a keyframe, it reads backwards until it finds a keyframe. Otherwise, if AVSEEK_FLAG_ANY is set it just returns the frame it finds.

Thoughts?

[DonMoir]

mpeg2video failing sample file (6 MB):

​http://sms.pangolin.com/temp/mpeg2video_seek_to_non_keyframe.mpg

[Carl Eugen Hoyos]

Replying to DonMoir:

mpeg2video failing sample file (6 MB):

​http://sms.pangolin.com/temp/mpeg2video_seek_to_non_keyframe.mpg

I failed to extract a non-keyframe out of this sample with ffmpeg.
If it is possible, please provide the command together with complete, uncut console output.

[mbradshaw]

Replying to cehoyos:

Replying to DonMoir:

mpeg2video failing sample file (6 MB):

​http://sms.pangolin.com/temp/mpeg2video_seek_to_non_keyframe.mpg

I failed to extract a non-keyframe out of this sample with ffmpeg.
If it is possible, please provide the command together with complete, uncut console output.

Can you describe how you were able to extract only keyframes out of this sample? Because I'm afraid we may not be on the same page (again). If av_seek_frame() seeks to a keyframe, then the first packet out of av_read_frame() should be a keyframe packet and the first call to avcodec_decode_video2() with this packet should return a full decoded frame. This is not the case. av_seek_frame() on this file (and other mpg files) seeks to a non-keyframe, and the first several packets out of av_read_frame() are not keyframe packets, and several calls must be made to avcodec_decode_video2() before a full decoded frame is returned.

The problem isn't that you get a junk frame out (the decoder makes sure you don't). The problem is that av_seek_frame() doesn't seek to a keyframe packet (despite the fact that that's what it claims it does), and several calls must be made to av_read_frame() before finally finding a keyframe packet.

[DonMoir]

This is best looked at by a developer. Trying to produce something using ffmpeg or ffplay may be difficult to visualize for you. Sometimes recovery is quick and is not as noticeable. If a developer will try to seek on the above file and look at the first packet read after the seek, most likely it will not be a key packet. Sometimes you get lucky and you will land on a keyframe but mostly not. Since in general you do not land on keyframe after a seek for mpeg2video files, there will be a delay before animation begins. This can be a 2 second delay and just depends on the file and the time you attempted to seek to.

The seek for mpeg2video files uses ff_seek_frame_binary and it's just not smart enough to to do keyframe seeking. Any format that is using the generic seek code may also have the same problem. Best thing is to add specialized seeking for mpeg2video like most of the other formats that work have.

I put in bold some hints at the bottom of this ffplay output.

ffplay -ss 5 mpeg2video_seek_to_non_keyframe.mpg
ffplay version N-43206-gf857465 Copyright (c) 2003-2012 the FFmpeg developers

built on Aug 4 2012 16:12:33 with gcc 4.7.1 (GCC)
configuration: --disable-static --enable-shared --enable-gpl --enable-version3

--disable-w32threads --enable-runtime-cpudetect --enable-avisynth --enable-bzlib
--enable-frei0r --enable-libass --enable-libcelt --enable-libopencore-amrnb
--enable-libopencore-amrwb --enable-libfreetype --enable-libgsm --enable-libmp3lame
--enable-libnut --enable-libopenjpeg --enable-librtmp --enable-libschroedinger
--enable-libspeex --enable-libtheora --enable-libutvideo --enable-libvo-aacenc
--enable-libvo-amrwbenc --enable-libvorbis --enable-libvpx --enable-libx264
--enable-libxavs --enable-libxvid --enable-zlib

libavutil 51. 66.100 / 51. 66.100
libavcodec 54. 49.100 / 54. 49.100
libavformat 54. 22.100 / 54. 22.100
libavdevice 54. 2.100 / 54. 2.100
libavfilter 3. 5.102 / 3. 5.102
libswscale 2. 1.100 / 2. 1.100
libswresample 0. 15.100 / 0. 15.100
libpostproc 52. 0.100 / 52. 0.100

[mpeg @ 012a0520] max_analyze_duration 5000000 reached at 5024000
Input #0, mpeg, from 'mpeg2video_seek_to_non_keyframe.mpg':

Duration: 00:00:09.85, start: 1.320000, bitrate: 4998 kb/s

Stream #0:0[0x1e0]: Video: mpeg2video (Main), yuv420p,

720x576 [SAR 16:15 DAR 4:3], 8000 kb/s, 25 fps, 25 tbr, 90k tbn, 50 tbc

Stream #0:1[0x80]: Audio: ac3, 48000 Hz, stereo, s16, 192 kb/s

[mpeg2video @ 012ae6e0] warning: first frame is no keyframe
[ac3 @ 012a7c20] frame sync error aq= 8KB vq= 5KB sq= 0B f=0/0
Frame changed from size:0x0 to size:720x576

25.71 A-V: 6.834 fd= 3 aq= 0KB vq= 0KB sq= 0B f=0/0 f=0/0

[DonMoir]

If you are just looking visually at the above file you may not notice too much of a problem. Some files are much worse. The above file is just the smallest I have that will reproduce the problem but a developer needs to look at it. The other thing is that since ff_seek_frame_binary is being used without any index entries, it should be obvious to any developer why this is a problem and is most likely already known.

[DonMoir]

There are no index entries for these files and the seek just falls into ff_gen_search with the times that are originally passed in. Now maybe if index entries were added, this could be made to work without adding special seek code.

I suppose it would need a read_seek which would create the index entries and then most likely could call ff_seek_frame_binary with success.

[Carl Eugen Hoyos]

Replying to mbradshaw:

Replying to cehoyos:

Replying to DonMoir:

mpeg2video failing sample file (6 MB):

​http://sms.pangolin.com/temp/mpeg2video_seek_to_non_keyframe.mpg

I failed to extract a non-keyframe out of this sample with ffmpeg.
If it is possible, please provide the command together with complete, uncut console output.

Can you describe how you were able to extract only keyframes out of this sample?

$ ffmpeg -ss 1 -i mpeg2video_seek_to_non_keyframe.mpg out.png
$ ffmpeg -ss 2 -i mpeg2video_seek_to_non_keyframe.mpg out.png
$ ffmpeg -ss 3 -i mpeg2video_seek_to_non_keyframe.mpg out.png
$ ffmpeg -ss 4 -i mpeg2video_seek_to_non_keyframe.mpg out.png
$ ffmpeg -ss 5 -i mpeg2video_seek_to_non_keyframe.mpg out.png
$ ffmpeg -ss 6 -i mpeg2video_seek_to_non_keyframe.mpg out.png
$ ffmpeg -ss 7 -i mpeg2video_seek_to_non_keyframe.mpg out.png

The problem isn't that you get a junk frame out (the decoder makes sure you don't).

I wonder what this sentence means: There are many samples that return gray frames for above command line (including some sample that you pointed to).

[Carl Eugen Hoyos]

And there is a second possibility how not to extract non-keyframes:
$ ffmpeg -ss 1 -i mpeg2video_seek_to_non_keyframe.mpg -vcodec copy -vframes 1 out.m2v
$ ffmpeg -ss 2 -i mpeg2video_seek_to_non_keyframe.mpg -vcodec copy -vframes 1 out.m2v
$ ffmpeg -ss 3 -i mpeg2video_seek_to_non_keyframe.mpg -vcodec copy -vframes 1 out.m2v
$ ffmpeg -ss 4 -i mpeg2video_seek_to_non_keyframe.mpg -vcodec copy -vframes 1 out.m2v
$ ffmpeg -ss 5 -i mpeg2video_seek_to_non_keyframe.mpg -vcodec copy -vframes 1 out.m2v
$ ffmpeg -ss 6 -i mpeg2video_seek_to_non_keyframe.mpg -vcodec copy -vframes 1 out.m2v
$ ffmpeg -ss 7 -i mpeg2video_seek_to_non_keyframe.mpg -vcodec copy -vframes 1 out.m2v
$ ffmpeg -ss 8 -i mpeg2video_seek_to_non_keyframe.mpg -vcodec copy -vframes 1 out.m2v
$ ffmpeg -ss 9 -i mpeg2video_seek_to_non_keyframe.mpg -vcodec copy -vframes 1 out.m2v

[mbradshaw]

Replying to cehoyos:

Replying to mbradshaw:

The problem isn't that you get a junk frame out (the decoder makes sure you don't).

I wonder what this sentence means: There are many samples that return gray frames for above command line (including some sample that you pointed to).

Really? I don't get any gray frames with those commands. I get good looking frames out, but that's because the decoder swallows the non-keyframes after the seek and ffmpeg keeps reading until it gets a keyframe packet and the decoder finally returns a full frame. I get:

./ffmpeg -ss 1 -i ~/mpeg2video_seek_to_non_keyframe.mpg ~/out.png
ffmpeg version N-43294-g8993c25 Copyright (c) 2000-2012 the FFmpeg developers
  built on Aug  7 2012 12:02:23 with gcc 4.2.1 (GCC) (Apple Inc. build 5666) (dot 3)
  configuration: 
  libavutil      51. 66.101 / 51. 66.101
  libavcodec     54. 50.100 / 54. 50.100
  libavformat    54. 22.101 / 54. 22.101
  libavdevice    54.  2.100 / 54.  2.100
  libavfilter     3.  5.102 /  3.  5.102
  libswscale      2.  1.101 /  2.  1.101
  libswresample   0. 15.100 /  0. 15.100
[mpeg @ 0x10180ec00] max_analyze_duration 5000000 reached at 5024000
Input #0, mpeg, from '/Users/mjbshaw/mpeg2video_seek_to_non_keyframe.mpg':
  Duration: 00:00:09.85, start: 1.320000, bitrate: 4998 kb/s
    Stream #0:0[0x1e0]: Video: mpeg2video (Main), yuv420p, 720x576 [SAR 16:15 DAR 4:3], 8000 kb/s, 25 fps, 25 tbr, 90k tbn, 50 tbc
    Stream #0:1[0x80]: Audio: ac3, 48000 Hz, stereo, s16, 192 kb/s
Output #0, image2, to '/Users/mjbshaw/out.png':
  Metadata:
    encoder         : Lavf54.22.101
    Stream #0:0: Video: png, rgb24, 720x576 [SAR 16:15 DAR 4:3], q=2-31, 200 kb/s, 90k tbn, 25 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (mpeg2video -> png)
Press [q] to stop, [?] for help
[mpeg2video @ 0x10180f200] warning: first frame is no keyframe
[image2 @ 0x10180f800] Could not get frame filename number 2 from pattern '/Users/mjbshaw/out.png'
av_interleaved_write_frame(): Invalid argument

The output image looks okay though. The key here is the same thing Don points out (the [mpeg2video @ 0x10180f200] warning: first frame is no keyframe part). If you're able to get gray frames out with that command then something's different between our ffmpegs...

@Don: I don't think (someone please correct me if I'm wrong) that specialized code can really be written for the mpeg's AVInputFormat seek function because of the structure of mpeg's. I think it would just be better to fix ff_seek_frame_binary(), because if custom code was written for mpeg's it would end up being so similar to ff_seek_frame_binary(). IIUC, mpeg's are impossible to properly seek by keyframes reliably, and the best solution would be to do a binary search for a frame, and then after finding it, sequentially reading (either forwards or backwards) to the next keyframe.

[Carl Eugen Hoyos]

Replying to mbradshaw:

Replying to cehoyos:

Replying to mbradshaw:

The problem isn't that you get a junk frame out (the decoder makes sure you don't).

I wonder what this sentence means: There are many samples that return gray frames for above command line (including some sample that you pointed to).

Really? I don't get any gray frames with those commands.

Assuming you mean with the sample from this ticket: That is what I wrote.

I tried to explain that Don opened many tickets, some of them contain samples that produced gray frames with above command line(s) or very similar ones (most of them should be fixed in current git head).

[DonMoir]

The reason ff_seek_frame_binary is not effective in this case is because there is no index table that marks keyframes. So ff_seek_frame_binary basically does nothing and falls into ff_gen_search with the original requested timestamp. If an index table could be built then ff_seek_frame_binary would probably be fine as it is. Some other formats have a simple read_seek that builds the index table and then calls ff_seek_frame_binary. So Michael Bradshaw says it is not possible to build an index table in this case.

Special seek code can always be available for any format but in this case it just defaults to the generic seek code and ends up in ff_gen_search. ff_gen_search is currently blind about any keyframes as far as I can tell in this case. Your requested timestamp is unlikely to fall on a keyframe and since there were no index entries. ff_gen_search does work if you have seeked to a known keyframe timestamp.

ff_gen_search is the code that actually closes in on the timestamp and it does appear to work for timestamps that represent actual keyframes. So you probably don't want to modify ff_gen_search to do extra work in the case when a timestamp is passed in that is known to represent a keyframe. These known timestamps are typically retrieved from an index table.

ff_gen_search seems to be more of a subroutine that works off whatever you pass it and used for reading the timestamp position.

So now we are back to needing a specialized read_seek function in this case. This is because you probably don't want to modify the existing generic seek functions. It is completely normal for various formats to have a specialized read_seek function. The read_seek could be a modified version of ff_gen_search or whatever that does the right thing.

Would be good if someone that knew for sure would comment on all this.