Thanks wm4, next patch will fix that declaration. Michael, any concerns?
On Mon, Nov 30, 2015 at 2:51 PM, wm4 <nfx...@googlemail.com> wrote: > On Mon, 30 Nov 2015 14:29:50 -0800 > 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 | 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. > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel