Thanks wm4. I updated the patch according to your comments.

>* @@ -489,19 +489,21 @@ static int mp3_seek(AVFormatContext *s, int
*>* stream_index, int64_t timestamp,
*>*      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 > 0 ? mp3->header_filesize
*>* +                            : avio_size(s->pb) - s->internal->data_offset;
*
> ok. But maybe the avio_size() return value should be checked. It can
> return an error.

Done. Please see the patch. Thanks for your comments.

> >* /*
*>* Many MP3 files in Podcast does not have VBRI, LAME, or XING tag, so
*>* header_filesize is not always available. (file size - data offset) could be
*>* a good estimation.
*>* */
*> > >*      if (mp3->usetoc == 2)
*>*          return -1; // generic index code
*> >* -    if (   mp3->is_cbr
*>* +    if (   (mp3->is_cbr || fast_seek)
*
> Not sure why you need this. If the file is VBR and has no TOC, then all
> odds are off. But I guess this is ok on the AVFMT_FLAG_FAST_SEEK code
> path, if you think it's more useful this way.

Yes, I really need the seek operation fast, and AVFMT_FLAG_FAST_SEEK
allows to sacrifice the accuracy in exchange for responsiveness. For
VBR files without TOC, like you said, it's not accurate. For CBR files
without INFO tag, it's fast and reasonable accurate. Since
AVFMT_FLAG_FAST_SEEK is set, we should make it fast instead of make it
safe.

>*          && (mp3->usetoc == 0 || !mp3->xing_toc)
*>*          && st->duration > 0
*>* -        && mp3->header_filesize > s->internal->data_offset
*>* -        && mp3->frames) {
*>* +        && filesize > 0) {
*>* /*
*>* Not sure why (mp3->header_filesize > s->internal->data_offset) has to be
*>* true. What if a MP3 file contains a short audio and a large picture?
*>* Again, mp3->frames is not always available. Do we really need it here? I
*>* move the check to below.
*>* */
*
> I can't think of any reason either. Maybe the original intention of
> this code was actually to check whether header_size is a sane value at
> all? There are lots of weird corner cases, like incomplete or
> concatenated mp3 files.

>*          ie = &ie1;
*>*          timestamp = av_clip64(timestamp, 0, st->duration);
*>*          ie->timestamp = timestamp;
*>* -        ie->pos       = av_rescale(timestamp, mp3->header_filesize,
*>* st->duration) + s->internal->data_offset;
*>* +        ie->pos       = av_rescale(timestamp, filesize, st->duration) +
*>* s->internal->data_offset;
*>*      } else if (mp3->xing_toc) {
*>*          if (ret < 0)
*>*              return ret;
*>* @@ -515,7 +517,7 @@ static int mp3_seek(AVFormatContext *s, int
*>* stream_index, int64_t timestamp,
*>*      if (best_pos < 0)
*>*          return best_pos;
*> >* -    if (mp3->is_cbr && ie == &ie1) {
*>* +    if (mp3->is_cbr && ie == &ie1 && mp3->frames) {
*
> Not sure what this does after all.

This tries to refine the timestamp. Because it may seek to middle of a
frame, mp3_sync() will align the position to a frame boundary. Thus,
the timestamp may be a little bit off.

Since it's the only place using mp3->frames in mp3_seek()*, *I think
we can check it right before using it.

>*          int frame_duration = av_rescale(st->duration, 1, mp3->frames);
*>*          ie1.timestamp = frame_duration * av_rescale(best_pos -
*>* s->internal->data_offset, mp3->frames, mp3->header_filesize);
*>*      }*
From 094c6efab6c5eec1fec274bf1bcace1987ae7d03 Mon Sep 17 00:00:00 2001
From: Andy Wu <tsungh...@chromium.org>
Date: Mon, 31 Aug 2015 17:08:30 -0700
Subject: [PATCH] avformat/mp3dec: Make MP3 seek fast

When AVFMT_FLAG_FAST_SEEK is specified, make MP3 seek operation as
fast as possible.

When no "-usetoc" is specified, the default operation is using TOC
if available; otherwise, uses linear interpolation. This is useful
when seeking a large MP3 file with no TOC available. One example is
Podcast, many MP3 files are large, but no CBR/VBR tags. Most of
them are actually CBR. Even in VBR cases, this option sacrifices the
accuracy of playback time in exchange for responsiveness.
---
 libavformat/mp3dec.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 007c6ea..d3080d7 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -342,7 +342,7 @@ static int mp3_read_header(AVFormatContext *s)
     int i;
 
     if (mp3->usetoc < 0)
-        mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 0 : 2;
+        mp3->usetoc = (s->flags & AVFMT_FLAG_FAST_SEEK) ? 1 : 2;
 
     st = avformat_new_stream(s, NULL);
     if (!st)
@@ -489,19 +489,26 @@ static int mp3_seek(AVFormatContext *s, int stream_index, int64_t timestamp,
     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 (   mp3->is_cbr
+    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
-        && mp3->header_filesize > s->internal->data_offset
-        && mp3->frames) {
+        && filesize > 0) {
         ie = &ie1;
         timestamp = av_clip64(timestamp, 0, st->duration);
         ie->timestamp = timestamp;
-        ie->pos       = av_rescale(timestamp, mp3->header_filesize, st->duration) + s->internal->data_offset;
+        ie->pos       = av_rescale(timestamp, filesize, st->duration) + s->internal->data_offset;
     } else if (mp3->xing_toc) {
         if (ret < 0)
             return ret;
@@ -515,7 +522,7 @@ static int mp3_seek(AVFormatContext *s, int stream_index, int64_t timestamp,
     if (best_pos < 0)
         return best_pos;
 
-    if (mp3->is_cbr && ie == &ie1) {
+    if (mp3->is_cbr && ie == &ie1 && mp3->frames) {
         int frame_duration = av_rescale(st->duration, 1, mp3->frames);
         ie1.timestamp = frame_duration * av_rescale(best_pos - s->internal->data_offset, mp3->frames, mp3->header_filesize);
     }
-- 
2.5.0.457.gab17608

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

Reply via email to