[FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

[chcunningham at chromium.org]

From: Chris Cunningham <chcunningham at chromium.org>

"Fast seek" uses linear interpolation to find the position of the
requested seek time. For CBR this is more direct than using the
mp3 TOC and bypassing the TOC avoids problems with TOC precision.
(see https://crbug.com/545914#c13)

For VBR, fast seek is not precise, so continue to prefer the TOC
when available (the lesser of two evils).

Also, some re-ordering of the logic in mp3_seek to simplify and
give usetoc=1 precedence over fastseek flag.
---
 libavformat/mp3dec.c | 45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 32ca00c..a1f21b7 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -115,7 +115,8 @@ static void read_xing_toc(AVFormatContext *s, int64_t filesize, int64_t duration
 {
     int i;
     MP3DecContext *mp3 = s->priv_data;
-    int fill_index = mp3->usetoc == 1 && duration > 0;
+    int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
+    int fill_index = (mp3->usetoc || fast_seek) && duration > 0;
 
     if (!filesize &&
         !(filesize = avio_size(s->pb))) {
@@ -344,9 +345,6 @@ static int mp3_read_header(AVFormatContext *s)
     int ret;
     int i;
 
-    if (mp3->usetoc < 0)
-        mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 2;
-
     st = avformat_new_stream(s, NULL);
     if (!st)
         return AVERROR(ENOMEM);
@@ -501,40 +499,43 @@ static int mp3_seek(AVFormatContext *s, int stream_index, int64_t timestamp,
     MP3DecContext *mp3 = s->priv_data;
     AVIndexEntry *ie, ie1;
     AVStream *st = s->streams[0];
-    int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
-    int64_t best_pos;
     int fast_seek = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 0;
     int64_t filesize = mp3->header_filesize;
 
-    if (mp3->usetoc == 2)
-        return -1; // generic index code
-
     if (filesize <= 0) {
         int64_t size = avio_size(s->pb);
         if (size > 0 && size > s->internal->data_offset)
             filesize = size - s->internal->data_offset;
     }
 
-    if (   (mp3->is_cbr || fast_seek)
-        && (mp3->usetoc == 0 || !mp3->xing_toc)
-        && st->duration > 0
-        && filesize > 0) {
-        ie = &ie1;
-        timestamp = av_clip64(timestamp, 0, st->duration);
-        ie->timestamp = timestamp;
-        ie->pos       = av_rescale(timestamp, filesize, st->duration) + s->internal->data_offset;
-    } else if (mp3->xing_toc) {
+    if (mp3->xing_toc && (mp3->usetoc || (fast_seek && !mp3->is_cbr))) {
+        av_log(s, AV_LOG_ERROR, "XING seeking. filesize = %"PRId64"\n", filesize);
+        // NOTE: The MP3 TOC is not a precise lookup table. The precision is
+        // worse closer to the end of the file, especially for large files.
+        // Seeking to 90% of duration in file of size 4M will be off by roughly 1 second.
+        if (filesize > 4000000)
+            av_log(s, AV_LOG_WARNING, "Using MP3 TOC to seek; may be imprecise.\n");
+
+        int64_t ret  = av_index_search_timestamp(st, timestamp, flags);
         if (ret < 0)
             return ret;
 
         ie = &st->index_entries[ret];
+    } else if (fast_seek && st->duration > 0 && filesize > 0) {
+        if (!mp3->is_cbr)
+            av_log(s, AV_LOG_WARNING, "Using scaling to seek VBR MP3; may be imprecise.\n");
+
+        ie = &ie1;
+        timestamp = av_clip64(timestamp, 0, st->duration);
+        ie->timestamp = timestamp;
+        ie->pos       = av_rescale(timestamp, filesize, st->duration) + s->internal->data_offset;
     } else {
-        return -1;
+        return -1; // generic index code
     }
 
-    best_pos = mp3_sync(s, ie->pos, flags);
+    int64_t best_pos = mp3_sync(s, ie->pos, flags);
     if (best_pos < 0)
-        return best_pos;
+      return best_pos;
 
     if (mp3->is_cbr && ie == &ie1 && mp3->frames) {
         int frame_duration = av_rescale(st->duration, 1, mp3->frames);
@@ -546,7 +547,7 @@ static int mp3_seek(AVFormatContext *s, int stream_index, int64_t timestamp,
 }
 
 static const AVOption options[] = {
-    { "usetoc", "use table of contents", offsetof(MP3DecContext, usetoc), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 2, AV_OPT_FLAG_DECODING_PARAM},
+    { "usetoc", "use table of contents", offsetof(MP3DecContext, usetoc), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM},
     { NULL },
 };
 
-- 
2.6.0.rc2.230.g3dd15c0