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

2015-12-02 Thread wm4 
On Tue,  1 Dec 2015 10:54:38 -0800
chcunning...@chromium.org wrote:

> From: Chris Cunningham 
> 
> "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| 39 +++
>  libavformat/seek-test.c |  8 +---
>  tests/fate/seek.mak |  2 +-
>  3 files changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> index 32ca00c..04c9861 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,35 +499,36 @@ 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);

I moved this line above the av_log() to avoid
declaration-after-statement.

>  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);
> @@ -546,7 +545,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],

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

2015-12-02 Thread wm4 
On Wed, 2 Dec 2015 02:23:15 +0100
Michael Niedermayer  wrote:

> On Tue, Dec 01, 2015 at 10:47:53AM -0800, Chris Cunningham wrote:
> > Thanks wm4, next patch will fix that declaration.
> > 
> > Michael, any concerns?  
> 
> iam fine with the patch
> 
> 
> >
> > On Mon, Nov 30, 2015 at 2:51 PM, wm4  wrote:  
> [...]
> > > Other than that LGTM.  
> 
> wm4, your replies are missing from the mailing list

Must be my mail client again, which replied to the CC instead of the ML.

> (and i dont know what "Other" was meant for so dunno if i should apply
>  or wait)
> 
> [...]

There was a declaration after statement. I guess I can fix as I push,
later.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2015-12-01 Thread Michael Niedermayer 
On Tue, Dec 01, 2015 at 10:47:53AM -0800, Chris Cunningham wrote:
> Thanks wm4, next patch will fix that declaration.
> 
> Michael, any concerns?

iam fine with the patch


>
> On Mon, Nov 30, 2015 at 2:51 PM, wm4  wrote:
[...]
> > Other than that LGTM.

wm4, your replies are missing from the mailing list
(and i dont know what "Other" was meant for so dunno if i should apply
 or wait)

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2015-12-01 Thread chcunningham 
From: Chris Cunningham 

"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| 39 +++
 libavformat/seek-test.c |  8 +---
 tests/fate/seek.mak |  2 +-
 3 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 32ca00c..04c9861 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,35 +499,36 @@ 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);
@@ -546,7 +545,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/see

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

2015-12-01 Thread Chris Cunningham 
Thanks wm4, next patch will fix that declaration.

Michael, any concerns?

On Mon, Nov 30, 2015 at 2:51 PM, wm4  wrote:

> On Mon, 30 Nov 2015 14:29:50 -0800
> chcunning...@chromium.org wrote:
>
> > From: Chris Cunningham 
> >
> > "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_s

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

2015-11-30 Thread chcunningham 
From: Chris Cunningham 

"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);
 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_FL

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

2015-11-30 Thread Chris Cunningham 
Looks like I messed up the tab for "return best_pos;"... fix incoming.

On Mon, Nov 30, 2015 at 2:10 PM,  wrote:

> From: Chris Cunningham 
>
> "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| 44 +---
>  libavformat/seek-test.c |  8 +---
>  tests/fate/seek.mak |  2 +-
>  3 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> index 32ca00c..3c8c0ee 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,40 +499,40 @@ 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);
>  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 +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

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

2015-11-30 Thread chcunningham 
From: Chris Cunningham 

"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| 44 +---
 libavformat/seek-test.c |  8 +---
 tests/fate/seek.mak |  2 +-
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 32ca00c..3c8c0ee 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,40 +499,40 @@ 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);
 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 +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,

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

2015-11-30 Thread Chris Cunningham 
(Fixed CCs)

Thanks for review. Patch coming shortly.

On Thu, Nov 26, 2015 at 10:13 AM, wm4  wrote:

> On Tue, 24 Nov 2015 16:55:03 -0800
> chcunning...@chromium.org wrote:
>
> > From: Chris Cunningham 
> >
> > "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;
>
> Nit: the ?1:0 is not really needed.
>

Will fix.

>
> > +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 > 400)
> > +av_log(s, AV_LOG_WARNING, "Using MP3 TOC to seek; may be
> imprecise.\n");
>
> This seems like a rather questionable heuristic. And only to print a
> message? Maybe it would be better to drop it, or always print it
> regardless of file size.
>
> I'll make this always print. Hopefully this helps people avoid TOC.


> > +
> > +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[] = {
>

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

2015-11-24 Thread Chris Cunningham 
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,  wrote:

> From: Chris Cunningham 
>
> "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 > 400)
> +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

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

2015-11-24 Thread chcunningham 
From: Chris Cunningham 

"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 > 400)
+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

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

2015-11-23 Thread Chris Cunningham 
Adding everyone back on the cc list for a convo with wm4 (meant for
everyone). Expect a new patch shortly to go with this discussion.

Also, just a note for posterity and to make sure I haven't missed anything:
It seems the mp3 TOC is really not a precise tool for seeking, particularly
in large files. I break down what I know about the design in this bug:
https://code.google.com/p/chromium/issues/detail?id=545914#c13 - LMK if I'm
missing something.

On Wed, Nov 18, 2015 at 3:08 AM, wm4  wrote:

> On Tue, 17 Nov 2015 14:21:08 -0800
> Chris Cunningham  wrote:
>
> > On Tue, Nov 17, 2015 at 2:38 AM, wm4  wrote:
> >
> > > On Mon, 16 Nov 2015 16:58:57 -0800
> > > chcunning...@chromium.org wrote:
> > >
> > > > From: Chris Cunningham 
> > > >
> > > > "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 when the TOC is
> > > > corrupted (e.g. https://crbug.com/545914).
> > > >
> > > > For VBR, fast seek is not precise, so continue to prefer the TOC
> > > > when available.
> > > > ---
> > > >  libavformat/mp3dec.c | 6 --
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> > > > index 32ca00c..e12266c 100644
> > > > --- a/libavformat/mp3dec.c
> > > > +++ b/libavformat/mp3dec.c
> > > > @@ -515,8 +515,10 @@ static int mp3_seek(AVFormatContext *s, int
> > > stream_index, int64_t timestamp,
> > > >  filesize = size - s->internal->data_offset;
> > > >  }
> > > >
> > > > -if (   (mp3->is_cbr || fast_seek)
> > > > -&& (mp3->usetoc == 0 || !mp3->xing_toc)
> > > > +// When fast seeking, prefer to use the TOC when available for
> VBR
> > > files
> > > > +// since av_rescale may not be accurate for VBR. For CBR,
> rescaling
> > > is
> > > > +// always accurate and more direct than a TOC lookup.
> > > > +if (fast_seek && (mp3->is_cbr || !mp3->xing_toc)
> > > >  && st->duration > 0
> > > >  && filesize > 0) {
> > > >  ie = &ie1;
> > >
> > > Hm, it's still a bit weird IMHO. Here's a suggestion: if usetoc is
> > > unset (its value is -1), then always set it 0. Other uses of the usetoc
> > > or fast seek flag could be simplified in the code.
> > >
> > > I think this sounds good. Then the behavior becomes:
> > -1 - unset implies default to 0
> > 0 - generic indexing code
> > 1 - toc
> > 2 - goes away, this is now the behavior of 0
> >
> >
> >
> > > Ultimately, the fast seek flag just sets the default mode. Disabling
> > > the flag makes it always use the generic indexing code. Enabling it
> > > used to enable TOC seeking, but we really always want CBR seeking. We
> > > want TOC seeking only if the user explicitly enables it by setting
> > > usetoc. Is that what we want?
> > >
> > >
> > *"We want TOC seeking only if the user explicitly enables it by setting*
> > *usetoc. Is that what we want?"*
> >
> > I think this is the heart of the complexity.
> > Is it useful for users to be able to say things like "fastseek, but never
> > use toc (always use scaling, even for VBR)"?
> > Or alternatively, "fastseek, but always use toc when available (even for
> > CBR)"?
> >
> > I'm not sure. We can support those combinations, but its more complex and
> > harder to reason about.
> >
> > It may be simpler to simply say that fastseek *overrides* usetoc. It
> could
> > cause toc to be used for VBR, and scaling for CBR, with no regard for
> user
> > set values of usetoc (we might detect a user setting and warn that we're
> > ignoring it). I'm happy to take this approach if everyone is on board.
> What
> > do you think? (Also, not sure if you mean to reply just to me, but we'd
> > have to ask Michael too).
>
> No, this was accidental. My mail client selected the wrong reply
> address because the mail had multiple recipients.


I'll merge the threads.


>
> I think there are two things: sane defaults, which apply if usetoc is
> not used, and forcing the demuxer to use a specific method if the user
> passes usetoc.
>
> In my own opinion, there are 3 use-cases:
> - default: seeking is exact, no surprises, but may be slow
>   (what everyone wants)
> - fast: user doesn't care about precision
>   (possibly the API user wants to enable this for big/slow streams)
> - custom: user wants a very specific seeking method to be used
>   (used rarely, maybe only for debugging)
>
> So I think the "usetoc" would fit the "custom" use-case, and we don't
> have to think much about its interaction with the fast seek mode - it
> can simply override it.
>
> This might be what you had in mind anyway. I'm only arguing that it
> makes sense for usetoc to override the fast seek mode.
>

Sounds good to me, the next patch will follow this outline. I'm thinking
I'll do away with the -1 value as well. Seems we want 0 to be the default
and I don't think we have  a use case for telling default apart

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

2015-11-16 Thread Michael Niedermayer 
On Mon, Nov 16, 2015 at 05:28:58PM -0800, Chris Cunningham wrote:
> To test this new patch, again use testcount.mp3
>  (CBR,
> corrupt TOC).
> 
> Current ffmpeg (with none of my mp3 patches)
> 
> ./ffplay testcount.mp3 -ss 00:33:20 -usetoc 0
> 
> plays out "1395", which is correct
> 
> 
> ./ffplay testcount.mp3 -ss 00:33:20 -usetoc 1
> plays out "..378, 1389", which is incorrect due to the corrupted TOC.
> 
> ./ffplay testcount.mp3 -ss 00:33:20 -fflags fastseek
> also plays out "..378, 1389" because current fast_seek logic uses the TOC
> whenever available for CBR and VBR alike.
> 
> 
> After applying my *latest* patch:
> 
> ./ffplay testcount.mp3 -ss 00:33:20 -usetoc 0
> is unchanged: plays out "1395", which is correct
> 
> ./ffplay testcount.mp3 -ss 00:33:20 -usetoc 1
> is unchanged: still plays out "378, 1389". This is wrong, but it allows
> users to force using TOC should they prefer.
> 
> ./ffplay testcount.mp3 -ss 00:33:20 -fflags fastseek
> *is changed: *plays out "1395", which is correct. fast seek no longer uses
> the TOC for CBR files.
> 
> 
> 

> What do you think? I personally prefer this way so that usetoc is still
> meaningful for CBR files.

i tend to agree about usetocs meaningfullness but it seems breaking
fate tests:
--- ./tests/ref/seek/extra-mp3  2015-11-16 00:07:55.812389092 +0100
+++ tests/data/fate/seek-extra-mp3  2015-11-17 03:15:54.266446909 +0100
@@ -5,16 +5,14 @@
 ret: 0 st: 0 flags:1 dts: 1.880816 pts: 1.880816 pos:  31544 size:   
418
 ret: 0 st: 0 flags:0  ts: 0.788334
 ret: 0 st: 0 flags:1 dts: 0.809796 pts: 0.809796 pos:  14407 size:   
418
-ret: 0 st: 0 flags:1  ts:-0.317499
-ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos:   1451 size:   
440
+ret:-1 st: 0 flags:1  ts:-0.317499
 ret: 0 st:-1 flags:0  ts: 2.576668
 ret: 0 st: 0 flags:1 dts: 2.586122 pts: 2.586122 pos:  42828 size:   
418
 ret: 0 st:-1 flags:1  ts: 1.470835
 ret: 0 st: 0 flags:1 dts: 1.462857 pts: 1.462857 pos:  24856 size:   
418
 ret: 0 st: 0 flags:0  ts: 0.365002
 ret: 0 st: 0 flags:1 dts: 0.365714 pts: 0.365714 pos:   7302 size:   
418
-ret: 0 st: 0 flags:1  ts:-0.740831
-ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos:   1451 size:   
440
+ret:-1 st: 0 flags:1  ts:-0.740831
 ret: 0 st:-1 flags:0  ts: 2.153336
 ret: 0 st: 0 flags:1 dts: 2.168163 pts: 2.168163 pos:  36141 size:   
418
 ret: 0 st:-1 flags:1  ts: 1.047503
@@ -41,13 +39,11 @@
 ret: 0 st: 0 flags:1 dts: 1.985306 pts: 1.985306 pos:  33215 size:   
418
 ret: 0 st:-1 flags:0  ts: 0.883340
 ret: 0 st: 0 flags:1 dts: 0.888163 pts: 0.888163 pos:  15661 size:   
418
-ret: 0 st:-1 flags:1  ts:-0.222493
-ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos:   1451 size:   
440
+ret:-1 st:-1 flags:1  ts:-0.222493
 ret: 0 st: 0 flags:0  ts: 2.671674
 ret: 0 st: 0 flags:1 dts: 2.690612 pts: 2.690612 pos:  44500 size:   
418
 ret: 0 st: 0 flags:1  ts: 1.565841
-ret: 0 st: 0 flags:1 dts: 1.567347 pts: 1.567347 pos:  26528 size:   
418
+ret: 0 st: 0 flags:1 dts: 1.541224 pts: 1.541224 pos:  26110 size:   
418
 ret: 0 st:-1 flags:0  ts: 0.460008
 ret: 0 st: 0 flags:1 dts: 0.470204 pts: 0.470204 pos:   8974 size:   
418
-ret: 0 st:-1 flags:1  ts:-0.645825
-ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos:   1451 size:   
440
+ret:-1 st:-1 flags:1  ts:-0.645825
Test seek-extra-mp3 failed. Look at tests/data/fate/seek-extra-mp3.err for 
details.
make: *** [fate-seek-extra-mp3] Error 1
make: *** Waiting for unfinished jobs


> 
> Thanks,
> Chris
> 
> On Mon, Nov 16, 2015 at 4:58 PM,  wrote:
> 
> > From: Chris Cunningham 
> >
> > "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 when the TOC is
> > corrupted (e.g. https://crbug.com/545914).
> >
> > For VBR, fast seek is not precise, so continue to prefer the TOC
> > when available.
> > ---
> >  libavformat/mp3dec.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> > index 32ca00c..e12266c 100644
> > --- a/libavformat/mp3dec.c
> > +++ b/libavformat/mp3dec.c
> > @@ -515,8 +515,10 @@ static int mp3_seek(AVFormatContext *s, int
> > stream_index, int64_t timestamp,
> >  filesize = size - s->internal->data_offset;
> >  }
> >
> > -if (   (mp3->is_cbr || fast_seek)
> > -&& (mp3->usetoc == 0 || !mp3->xing_toc)
> > +// When fast seeking, prefer to use the TOC when available for VBR
> > files
> > +// since av_rescale may not be accurate for VBR. For CBR, rescaling is
> > +// always accurate and more direct than a TOC lookup

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

2015-11-16 Thread Chris Cunningham 
To test this new patch, again use testcount.mp3
 (CBR,
corrupt TOC).

Current ffmpeg (with none of my mp3 patches)

./ffplay testcount.mp3 -ss 00:33:20 -usetoc 0

plays out "1395", which is correct


./ffplay testcount.mp3 -ss 00:33:20 -usetoc 1
plays out "..378, 1389", which is incorrect due to the corrupted TOC.

./ffplay testcount.mp3 -ss 00:33:20 -fflags fastseek
also plays out "..378, 1389" because current fast_seek logic uses the TOC
whenever available for CBR and VBR alike.


After applying my *latest* patch:

./ffplay testcount.mp3 -ss 00:33:20 -usetoc 0
is unchanged: plays out "1395", which is correct

./ffplay testcount.mp3 -ss 00:33:20 -usetoc 1
is unchanged: still plays out "378, 1389". This is wrong, but it allows
users to force using TOC should they prefer.

./ffplay testcount.mp3 -ss 00:33:20 -fflags fastseek
*is changed: *plays out "1395", which is correct. fast seek no longer uses
the TOC for CBR files.



What do you think? I personally prefer this way so that usetoc is still
meaningful for CBR files.

Thanks,
Chris

On Mon, Nov 16, 2015 at 4:58 PM,  wrote:

> From: Chris Cunningham 
>
> "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 when the TOC is
> corrupted (e.g. https://crbug.com/545914).
>
> For VBR, fast seek is not precise, so continue to prefer the TOC
> when available.
> ---
>  libavformat/mp3dec.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
> index 32ca00c..e12266c 100644
> --- a/libavformat/mp3dec.c
> +++ b/libavformat/mp3dec.c
> @@ -515,8 +515,10 @@ static int mp3_seek(AVFormatContext *s, int
> stream_index, int64_t timestamp,
>  filesize = size - s->internal->data_offset;
>  }
>
> -if (   (mp3->is_cbr || fast_seek)
> -&& (mp3->usetoc == 0 || !mp3->xing_toc)
> +// When fast seeking, prefer to use the TOC when available for VBR
> files
> +// since av_rescale may not be accurate for VBR. For CBR, rescaling is
> +// always accurate and more direct than a TOC lookup.
> +if (fast_seek && (mp3->is_cbr || !mp3->xing_toc)
>  && st->duration > 0
>  && filesize > 0) {
>  ie = &ie1;
> --
> 2.6.0.rc2.230.g3dd15c0
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2015-11-16 Thread chcunningham 
From: Chris Cunningham 

"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 when the TOC is
corrupted (e.g. https://crbug.com/545914).

For VBR, fast seek is not precise, so continue to prefer the TOC
when available.
---
 libavformat/mp3dec.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 32ca00c..e12266c 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -515,8 +515,10 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 filesize = size - s->internal->data_offset;
 }
 
-if (   (mp3->is_cbr || fast_seek)
-&& (mp3->usetoc == 0 || !mp3->xing_toc)
+// When fast seeking, prefer to use the TOC when available for VBR files
+// since av_rescale may not be accurate for VBR. For CBR, rescaling is
+// always accurate and more direct than a TOC lookup.
+if (fast_seek && (mp3->is_cbr || !mp3->xing_toc)
 && st->duration > 0
 && filesize > 0) {
 ie = &ie1;
-- 
2.6.0.rc2.230.g3dd15c0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2015-11-16 Thread Chris Cunningham 
Hey, sorry for the slow reply.

Use this sample file. Its a CBR file with a corrupt TOC.
http://happyworm.com/jPlayerLab/halite/jumptest/testcount.mp3

Before applying my patch:

./ffplay -ss 00:33:20 testcount.mp3  -usetoc 0
plays out "1395", which is the correct audio for the given seek time

./ffplay -ss 00:33:20 testcount.mp3  -usetoc 1
plays out "...378, 1379", way off because it used the bad TOC


After applying my patch these commands both play out the correct time of
"1395" because usetoc=1 is ignored/overridden by virtue of this file being
CBR.

However, I've *just* thought of another (better) approach. My goal is
really to improve the behavior of AVFMT_FLAG_FAST_SEEK
,
which defaults to using the TOC whenever its available. Standby for a
second patch that will only change TOC behavior when the
AVFMT_FLAG_FAST_SEEK is being used.

Chris


On Mon, Nov 9, 2015 at 3:34 PM, Michael Niedermayer 
wrote:

> On Wed, Nov 04, 2015 at 05:21:57PM -0800, Chris Cunningham wrote:
> > "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 when the TOC is
> > corrupted (e.g. https://crbug.com/545914).
> >
> > For VBR, fast seek is not precise, so continue to prefer the TOC
> > when available.
> > ---
> >  libavformat/mp3dec.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
>
> how can this be reproduced with FFmpeg ?
> do you have a sample which you can share ?
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I have often repented speaking, but never of holding my tongue.
> -- Xenocrates
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2015-11-09 Thread Michael Niedermayer 
On Wed, Nov 04, 2015 at 05:21:57PM -0800, Chris Cunningham wrote:
> "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 when the TOC is
> corrupted (e.g. https://crbug.com/545914).
> 
> For VBR, fast seek is not precise, so continue to prefer the TOC
> when available.
> ---
>  libavformat/mp3dec.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

how can this be reproduced with FFmpeg ?
do you have a sample which you can share ?

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2015-11-04 Thread Chris Cunningham 
"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 when the TOC is
corrupted (e.g. https://crbug.com/545914).

For VBR, fast seek is not precise, so continue to prefer the TOC
when available.
---
 libavformat/mp3dec.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 32ca00c..7946261 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -515,8 +515,10 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 filesize = size - s->internal->data_offset;
 }
 
-if (   (mp3->is_cbr || fast_seek)
-&& (mp3->usetoc == 0 || !mp3->xing_toc)
+// Always use "fast seek" (linear interpolation) for CBR. For CBR this is
+// a precise method and more direct than using TOC. For VBR this is not
+// perfectly accurate, so prefer TOC if available.
+if ((mp3->is_cbr || (fast_seek && (mp3->usetoc == 0 || !mp3->xing_toc)))
 && st->duration > 0
 && filesize > 0) {
 ie = &ie1;
-- 
2.6.0.rc2.230.g3dd15c0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel