Seek problems with ogg decoder

[DonMoir]

ffmpeg-git-14d94a1

built on Jan 23 2012 17:40:00 with gcc 4.6.2
configuration: --disable-static --enable-shared --enable-gpl

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

libavutil 51. 34.101 / 51. 34.101
libavcodec 53. 57.105 / 53. 57.105
libavformat 53. 30.100 / 53. 30.100
libavdevice 53. 4.100 / 53. 4.100
libavfilter 2. 59.101 / 2. 59.101
libswscale 2. 1.100 / 2. 1.100
libswresample 0. 6.100 / 0. 6.100
libpostproc 52. 0.100 / 52. 0.100

There seems to be several problems with seeking in the ogg decoder (libavformat/oggdec.c)

This file can be used to identify some of the key problems:

​http://sms.pangolin.com/temp/bad_seek_ogg_BuckBunny.zip (46 mb)

There are no issues with playback of this file and only seeking is a problem. Fails to seek properly with SMPlayer, ffplay, and anything else using ffmpeg unless special handling is applied.

In reference to the function ogg_read_seek in file oggdec.c.

1) I see it makes no attempt to create (if needbe) the AVIndexEntry table for the stream which is pretty much required to make seeking work. Most of the other formats do this when you call it's read_seek function. So I find that I need to create this table prior to calling avformat_seek_file for ogg files. This table can be created by calling av_read_packet until the timestamp of interest is reached. This is pretty quick but it should only be done when needbe. Other formats have other and quicker means to do this, but the read packet method works well enough if nothing else is available. For ogg files, once I know that the index_entries have been created as best they are going to be, I then call av_index_search_timestamp(pStream,timestamp,AVSEEK_FLAG_BACKWARD) and I use the timestamp associated with the returned index for my seek time. The AVSEEK_FLAG_BACKWARD flag makes sure the returned timestamp will be less than or equal to my requested timestamp. If -1 is returned form av_index_search_timestamp I deal with it but this should not happen if the index_entries have been built and all else is good. Now we can call avformat_seek_file with the modified timestamp and expect reasonable results. If you don't do this read_timestamp will fail in ff_seek_frame_binary which is called from ogg_read_seek. Note that av_index_search_timestamp is also pretty much worthless for ogg files if the index_entries have not been created.

2) ogg_read_seek does not do the proper cleanup after a seek. That is, there is stale information left that was in place prior to a seek. The ogg decoder uses private data that it places in AVFormatContext.priv_data. This is known to ogg as struct ogg*. Within this structure, ogg also has information for each stream. This is known to ogg as struct ogg_stream*. The stale information that I know about within the ogg_stream is lastpts and lastdts. These are not modified by a seek and whatever was in there before the seek will be used after a seek and these values will be used by the next av_read_packet. After a seek, I am having to call ogg_reset (also in oggdec.c) but this is clearly not the right thing to do. This ogg_reset is not available to higher level code but I have it hacked in for the present. The packet now will contain AV_NOPTS_VALUE for the pts and dts and this is not right either, but at least I can deal with it. Additional packet reads straighten this out. The lastdts and lastpts should of course reflect the next packet that is read. Not really sure how to clean it all up after a seek to make sure all is normal for ogg files. All the other decoders I have tested do this correctly. I think more needs to be cleaned up in addition to lastpts and lastdts but not sure.

With the above changes, I can now seek into the buckbunny sample perfectly. Would be great if someone would revisit the seek code in the oggdec.c file and make sure it does the right thing.

There are some other problems with ogg but the above 2 things goes a long way in fixing some of them. For the most part, most of my ogg files work fine after the above changes, but there are still some anoying problems with some files. It's like the audio packets are being used to create index_entries for a video stream for some files and not video packets and so this will throw the index_entries off. So while the seek works fine, it may be some time before the next video frame is available but audio is dead on. Not sure about this one and this is just for some ogg files.

I can provide more information and code samples if required.

[DonMoir]

ffmpeg -ss 420 -i bad_seek_fails_BigBuckBunny.ogv -f null -
ffmpeg version N-37063-g14d94a1 Copyright (c) 2000-2012 the FFmpeg developers
  built on Jan 23 2012 17:40:00 with gcc 4.6.2
  configuration:
    --disable-static --enable-shared --enable-gpl --enable-version3
    --disable-w32threads --enable-runtime-cpudetect --enable-avisynth 
    --enable-bzlib --enable-frei0r --enable-libopencore-amrnb 
    --enable-libopencore-amrwb --enable-libfreetype --enable-libgsm 
    --enable-libmp3lame --enable-libopenjpeg --enable-librtmp 
    --enable-libschroedinger --enable-libspeex --enable-libtheora 
    --enable-libvo-aacenc --enable-libvo-amrwbenc --enable-libvorbis 
    --enable-libvpx --enable-libx264 --enable-libxavs --enable-libxvid 
    --enable-zlib
  libavutil      51. 34.101 / 51. 34.101
  libavcodec     53. 57.105 / 53. 57.105
  libavformat    53. 30.100 / 53. 30.100
  libavdevice    53.  4.100 / 53.  4.100
  libavfilter     2. 59.101 /  2. 59.101
  libswscale      2.  1.100 /  2.  1.100
  libswresample   0.  6.100 /  0.  6.100
  libpostproc    52.  0.100 / 52.  0.100

[ogg @ 0003B000] read_timestamp() failed in the middle

    Last message repeated 1 times
bad_seek_fails_BigBuckBunny.ogv: could not seek to position 420.000
Input #0, ogg, from 'bad_seek_fails_BigBuckBunny.ogv':
  Duration: 00:09:56.45, start: 0.000000, bitrate: 641 kb/s
    Stream #0:0: Data: none
    Stream #0:1: Video: theora, yuv420p, 640x352, 24 fps, 24 tbr, 24 tbn, 24 tbc

    Stream #0:2: Audio: vorbis, 44100 Hz, stereo, s16, 44 kb/s
[buffer @ 0003FFC0] w:640 h:352 pixfmt:yuv420p tb:1/1000000 sar:0/1 sws_param:
Output #0, null, to 'pipe:':
  Metadata:
    encoder         : Lavf53.30.100
    Stream #0:0: Video: rawvideo (I420 / 0x30323449), yuv420p, 640x352, q=2-31,
200 kb/s, 90k tbn, 24 tbc
    Stream #0:1: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s
Stream mapping:
  Stream #0:1 -> #0:0 (theora -> rawvideo)
  Stream #0:2 -> #0:1 (vorbis -> pcm_s16le)

[Carl Eugen Hoyos]

Sample plays fine for ~11 seconds, -ss 1 - 5 seem to work fine, 6 and 7 behave like ticket #438, the same as for the original sample.

$ ffmpeg -ss 8 -i bad_seek_ogg_BuckBunny_cut.ogv out.png
ffmpeg version N-37208-g01fcbdf Copyright (c) 2000-2012 the FFmpeg developers
  built on Jan 27 2012 22:13:40 with gcc 4.5.3
  configuration: --cc=/usr/local/gcc-4.5.3/bin/gcc
  libavutil      51. 34.101 / 51. 34.101
  libavcodec     53. 60.100 / 53. 60.100
  libavformat    53. 31.100 / 53. 31.100
  libavdevice    53.  4.100 / 53.  4.100
  libavfilter     2. 60.100 /  2. 60.100
  libswscale      2.  1.100 /  2.  1.100
  libswresample   0.  6.100 /  0.  6.100
[ogg @ 0x13dc3a0] read_timestamp() failed in the middle
    Last message repeated 1 times
bad_seek_ogg_BuckBunny_cut.ogv: could not seek to position 8.000
Input #0, ogg, from 'bad_seek_ogg_BuckBunny_cut.ogv':
  Duration: 00:00:11.87, start: 0.000000, bitrate: 689 kb/s
    Stream #0:0: Data: none
    Stream #0:1: Video: theora, yuv420p, 640x352, 24 fps, 24 tbr, 24 tbn, 24 tbc
    Stream #0:2: Audio: vorbis, 44100 Hz, stereo, s16, 44 kb/s
Incompatible pixel format 'yuv420p' for codec 'png', auto-selecting format 'rgb24'
[buffer @ 0x13dcb60] w:640 h:352 pixfmt:yuv420p tb:1/1000000 sar:0/1 sws_param:
[buffersink @ 0x13e0580] auto-inserting filter 'auto-inserted scale 0' between the filter 'src' and the filter 'out'
[scale @ 0x13e0e60] w:640 h:352 fmt:yuv420p -> w:640 h:352 fmt:rgb24 flags:0x4
Output #0, image2, to 'out.png':
  Metadata:
    encoder         : Lavf53.31.100
    Stream #0:0: Video: png, rgb24, 640x352, q=2-31, 200 kb/s, 90k tbn, 24 tbc
Stream mapping:
  Stream #0:1 -> #0:0 (theora -> png)
Press [q] to stop, [?] for help
frame=    0 fps=  0 q=0.0 Lsize=       0kB time=00:00:00.00 bitrate=   0.0kbits/s
video:0kB audio:0kB global headers:0kB muxing overhead nan%
Output file is empty, nothing was encoded (check -ss / -t / -frames parameters if used)

[DonMoir]

Just to restate this. Early seek times are going to succeed because there are some initial AVStream.index_entries in place at start up. It's all about when you try to seek past the last entry in the index_entries table which have not been created but should be in the ogg_read_seek function. So the seek failure is all about ogg_read_seek's failure to create the index_entries. If I go ahead and make sure the index_entries are created prior to calling avformat_seek_file, then seeking works fine outside of the additional clean up notes mentioned in item 2) above. Also I remap my timestamp to the one returned by av_index_search_timestamp so I make sure I know I am at the place I need to be without second quessing.

Attempting to seek inbetween one of these index_entries could possibly be the reason an incomplete frame is being produced for ticket #438 but there is probably more to this. With my own current code setup I don't see any incomplete frames, but there is still some other odd behavior with some ogg files.

Most of the decoders either create these index_entries in their read_seek function or they don't need to because they may be stored in the file. Without these index_entries seeking is going to be bad for ogg files.

[DonMoir]

Creating the index_entries fixed the BuckBunny sample. The code I use for this is here and it begins to fix ogg_read_seek problems for all ogg files.

static void check_index_entries (
// Creates the index_entries prior to seeking if needbe.
cFFmpegPlayer* pPlayer, uint32_t streamIndex, int64_t timestamp, struct ogg_stream* pOggStream)
{
    AVFormatContext *pFmtCtx = pPlayer->pFormatCtx;
    AVPacket packet;
    AVStream* pStream;
    int lastIdx;
    if (pFmtCtx == NULL || streamIndex >= pFmtCtx->nb_streams)
        return;
    pStream = pFmtCtx->streams [streamIndex];

    lastIdx = pStream->nb_index_entries - 1;
    if (lastIdx >= 0 &&
      (pStream->index_entries [lastIdx].flags & AVINDEX_KEYFRAME) &&
      pStream->index_entries [lastIdx].timestamp >= timestamp)
        return;
    // I am assuming we don't need to seek to the lastKnownTime to start reading because
    // the file position will be before here or past it. If it is past it then the entry
    // would have already been created and we would not be here :) This may require a
    // little more thought but this doesn't hurt anything.
    while (!pPlayer->bAllDone && !pPlayer->m_bThreadStopSeek)
    {
        if (pOggStream)
            pOggStream->keyframe_seek = 1;
        if (p_av_read_frame (pFmtCtx,&packet))
            break;
        if (packet.stream_index == streamIndex)
        {
            lastIdx = pStream->nb_index_entries - 1;
            if (lastIdx >= 0 &&
              (pStream->index_entries [lastIdx].flags & AVINDEX_KEYFRAME) &&
              pStream->index_entries [lastIdx].timestamp >= timestamp)
            {
                p_av_free_packet (&packet);
                break;
            }
        }
        p_av_free_packet (&packet);
    }
}

Note this code in the above just after the while statement:

        if (pOggStream)
            pOggStream->keyframe_seek = 1;

This just speeds up the check_index_entries function a bit, but still there are going to faster ways to create the index_entries for ogg. I am not sure this is 100 percent accurate and I may remove this. av_read_frame will set this back to 0 via ogg_read_packet.

I mentioned I had some other odd behavior with some ogg files. For my app this showed up as frames not displaying for some time after a seek. In other apps you will see other odd behavior like incomplete frames, audio out of sync, or just weirdness.

Here's the ogg_read_seek function as it stands in ffmpeg:

static int ogg_read_seek(AVFormatContext *s, int stream_index,
                         int64_t timestamp, int flags)
{
    struct ogg *ogg = s->priv_data;
    struct ogg_stream *os = ogg->streams + stream_index;
    int ret;

    // Try seeking to a keyframe first. If this fails (very possible),
    // av_seek_frame will fall back to ignoring keyframes
    if (s->streams[stream_index]->codec->codec_type == AVMEDIA_TYPE_VIDEO
        && !(flags & AVSEEK_FLAG_ANY))
        os->keyframe_seek = 1;

    ret = ff_seek_frame_binary(s, stream_index, timestamp, flags);
    os = ogg->streams + stream_index;
    if (ret < 0)
        os->keyframe_seek = 0;
    return ret;
}

Note this code at the bottom of the ogg_read_seek function:

    if (ret < 0)
        os->keyframe_seek = 0;

os->keyframe_seek needs to be set back to 0 regardless of the return value. After I changed this all the remaining problems went away and I can now seek perfect on all the ogg files I have.

In summary 3 things need to be fixed for ogg_read_seek:

1) create the index entries prior to seeking - without this seeking will be bad - hopefully someone will come up with a more efficient solution then the solution I am using.

2) make sure os->keyframe_seek is set back to 0 always and don't depend on any return value. This fixed all the remaining weirdness.

3) call ogg_reset after the seek - this is not perfect and there is something more with this but without this - the first packet read will contain stale information as mentioned above. You will still get AV_NOPTS_VALUE on the first read which is not right either. I think calling ogg_reset and setting the lastpts and lastdts in the ogg_stream may do it.

[DonMoir]

Note: I removed the code that was setting pOggStream->key_frame to 1 from the check_index_entries function above. I noticed a little strange behavior with this and it works fine without it, but check_index_entries should be optimized for ogg.

[reimar]

the index_entries table which have not been created but should be in the ogg_read_seek function

This is nonsense, there is no need for any index entries at that point, and your method of creating them has O(n) performance.
What happens is that FFmpeg falls back to seeking via read_timestamp, which can seek without index with O(log(n)) performance.
The problem however is that read_timestamp randomly fails due to a bug:

[ogg @ 0003B000] read_timestamp() failed in the middle

This can cause all kinds of issues like seeking completely failing or degrading to O(n*n) performance.
Worse, that bug can even for a single byte of corruption cause the Ogg demuxer to suddenly start discarding all following data from a stream for no good reason at all.
Patch to fix this bug is on the ffmpeg-devel list ([PATCH] Fix potential infinite discard loop.).

[reimar]

Fixed by d7b542ae.

[DonMoir]

reimar, not nonsense when you have to have something that works. I had to prevent it from ever getting to read_timestamp and creating the index entries in the read_seek function follows the model of the matroska decoder. I mentioned that my method was not optimal and hopefully someone would fix it for real, but it works and thats what I needed to do to get a demo out that worked.

Hopefully, items 2) and 3) in summary for comment #5 above will be addressed as well because those are just as important epecially item 2)

It would be nonsense if I tried to demo something that did not work.

[reimar]

I said nonsense to one specific statement, and that was claiming that index entries should be created in that function.
That is just not correct and will just cause confusion and misunderstanding about what the intention of the code is.
As to 2) and 3) they are separate issues and would be better to be reported as such.
However as far as I can tell for 2) the current code does exactly what it should and you did not say what issue you had with that.
For 3) as far as I call tell ogg_reset is always called at least when there is no index, though there might be one missing for the case where the index we look for is already in the index.

[DonMoir]

No problem reimar. I was just following the model of matroskadec.c matroska_read_seek which does create it's seek index prior to continuing. Since I am not as familiar with the code as you are, I just have to do what I can, but digging into these problems is definitely getting me more familiar :) matroska_read_seek creates it's index efficiently as far as I can tell though.

[reimar]

matroska_read_seek creates it's index efficiently as far as I can tell though.

It just reads the index from the file. For real-world cases that is the most efficient way (though from a pure theory standpoint it is still O(n) unless the writing application automatically culls the index which has its own issues and is generally not allowed).
I saw that your files use Ogg skeleton v4, which means that they probably contain an index, too, so you should be able to implement the same approach - however it would be a good amount of work I expect.