[FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.
Chris Cunningham
chcunningham at chromium.org
Tue Dec 1 19:47:53 CET 2015
Thanks wm4, next patch will fix that declaration.
Michael, any concerns?
On Mon, Nov 30, 2015 at 2:51 PM, wm4 <nfxjfg at googlemail.com> wrote:
> On Mon, 30 Nov 2015 14:29:50 -0800
> chcunningham at chromium.org wrote:
>
> > 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 | 42 ++++++++++++++++++++----------------------
> > libavformat/seek-test.c | 8 +++++---
> > tests/fate/seek.mak | 2 +-
> > 3 files changed, 26 insertions(+), 26 deletions(-)
> >
> > diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> > index 32ca00c..3dc2b5c 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;
> > + 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,38 +499,38 @@ 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;
> > + int fast_seek = s->flags & AVFMT_FLAG_FAST_SEEK;
> > 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))) {
> > + // NOTE: The MP3 TOC is not a precise lookup table. Accuracy is
> worse
> > + // for bigger files.
> > + 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);
>
> We don't like having declarations after statements.
>
> > if (best_pos < 0)
> > return best_pos;
> >
> > @@ -546,7 +544,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 },
> > };
> >
> > diff --git a/libavformat/seek-test.c b/libavformat/seek-test.c
> > index 1926f21..bfd06db 100644
> > --- a/libavformat/seek-test.c
> > +++ b/libavformat/seek-test.c
> > @@ -56,7 +56,7 @@ static void ts_str(char buffer[60], int64_t ts,
> AVRational base)
> > int main(int argc, char **argv)
> > {
> > const char *filename;
> > - AVFormatContext *ic = NULL;
> > + AVFormatContext *ic = avformat_alloc_context();
> > int i, ret, stream_id;
> > int j;
> > int64_t timestamp;
> > @@ -76,8 +76,10 @@ int main(int argc, char **argv)
> > frame_count = atoi(argv[i+1]);
> > } else if(!strcmp(argv[i], "-duration")){
> > duration = atoi(argv[i+1]);
> > - } else if(!strcmp(argv[i], "-usetoc")) {
> > - av_dict_set(&format_opts, "usetoc", argv[i+1], 0);
> > + } else if(!strcmp(argv[i], "-fastseek")) {
> > + if (atoi(argv[i+1])) {
> > + ic->flags |= AVFMT_FLAG_FAST_SEEK;
> > + }
> > } else {
> > argc = 1;
> > }
> > diff --git a/tests/fate/seek.mak b/tests/fate/seek.mak
> > index dfb2e84..a229e72 100644
> > --- a/tests/fate/seek.mak
> > +++ b/tests/fate/seek.mak
> > @@ -244,7 +244,7 @@ FATE_SEEK += $(FATE_SEEK_LAVF-yes:%=fate-seek-lavf-%)
> > # extra files
> >
> > FATE_SEEK_EXTRA-$(CONFIG_MP3_DEMUXER) += fate-seek-extra-mp3
> > -fate-seek-extra-mp3: CMD = run libavformat/seek-test$(EXESUF)
> $(TARGET_SAMPLES)/gapless/gapless.mp3 -usetoc 0
> > +fate-seek-extra-mp3: CMD = run libavformat/seek-test$(EXESUF)
> $(TARGET_SAMPLES)/gapless/gapless.mp3 -fastseek 1
> > FATE_SEEK_EXTRA += $(FATE_SEEK_EXTRA-yes)
> >
> >
>
> Other than that LGTM.
>
More information about the ffmpeg-devel
mailing list