The new patch incorporates the "3 use-cases" feedback from wm4. 1. default: use generic (accurate but slow) seeking 2. fast: set -fflags fastseek to get fast accurate scaled seek for CBR, TOC seek for VBR falling back to scaling if no TOC exists 3. custom: set -usetoc 1 to always use TOC for VBR & CBR. Will override fastseek flag. This is very inaccurate for large files.
fastseek is really very close to generic seek for CBR files. for VBR its pretty rough. The fate-seek-extra-mp3 test is still failing. Its easy to fix, but I need some guidance on what the test is supposed to focus on. It was originally going through the av_rescale path of mp3_seek. With my patch it goes through the return -1 (slow generic index) path. IIUC, the generic index path is covered elsewhere ( https://github.com/FFmpeg/FFmpeg/blob/a62178be80dd6a643973f62002fc0ed42495c358/tests/fate-run.sh#L229). I think I can restore the behavior of this test to use the av_rescale path if I replace -usetoc 0 with -fflags fastseek (and update args parsing)... is that what we want? Thanks, Chris On Tue, Nov 24, 2015 at 4:55 PM, <chcunning...@chromium.org> wrote: > From: Chris Cunningham <chcunning...@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 > > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
