diff mbox series

[FFmpeg-devel] mov: Compare frag times in correct time base when seeking a stream without a corresponding sidx

Message ID 20220517203956.866831-1-derek.buitenhuis@gmail.com
State New
Headers show
Series [FFmpeg-devel] mov: Compare frag times in correct time base when seeking a stream without a corresponding sidx | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Derek Buitenhuis May 17, 2022, 8:39 p.m. UTC
Some muxers, such as GPAC, create files with only one sidx, but two streams
muxed into the same fragments pointed to by this sidx.

Prevously, in such a case, when we seeked in such files, we fell back
to, for example, using the sidx associated with the video stream, to
seek the audio stream, leaving the seekhead in the wrong place.

We can still do this, but we need to take care to compare timestamps
in the same time base.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
 libavformat/mov.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

zhilizhao(赵志立) May 18, 2022, 3:38 a.m. UTC | #1
> On May 18, 2022, at 4:39 AM, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> 
> Some muxers, such as GPAC, create files with only one sidx, but two streams
> muxed into the same fragments pointed to by this sidx.
> 
> Prevously, in such a case, when we seeked in such files, we fell back
> to, for example, using the sidx associated with the video stream, to
> seek the audio stream, leaving the seekhead in the wrong place.
> 
> We can still do this, but we need to take care to compare timestamps
> in the same time base.
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
> libavformat/mov.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index d7be593a86..fe3b2b15ab 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -1270,12 +1270,12 @@ static int64_t get_stream_info_time(MOVFragmentStreamInfo * frag_stream_info)
>     return frag_stream_info->tfdt_dts;
> }
> 
> -static int64_t get_frag_time(MOVFragmentIndex *frag_index,
> -                             int index, int track_id)
> +static int64_t get_frag_time(AVFormatContext *s, AVStream *dst_st,
> +                             MOVFragmentIndex *frag_index, int index, int track_id)
> {

I have an idea which needs more refactor:

1. Remove track_id argument, the `if (track_id >= 0)` code path, and the related
part in search_frag_timestamp().

Setting track_id in search_frag_timestamp() is defect. sc->has_sidx is checked
there, while tfra can be used here.

track_id means to set a higher priority on a stream, while now we have dst_st,
which has more information than track_id.


2. Give dst_st a higher priority, and search all streams.


>     MOVFragmentStreamInfo * frag_stream_info;
>     int64_t timestamp;
> -    int i;
> +    int i, j;
> 
>     if (track_id >= 0) {
>         frag_stream_info = get_frag_stream_info(frag_index, index, track_id);
> @@ -1287,15 +1287,23 @@ static int64_t get_frag_time(MOVFragmentIndex *frag_index,
>     }
> 
>     for (i = 0; i < frag_index->item[index].nb_stream_info; i++) {
> +        AVStream *frag_stream = NULL;
>         frag_stream_info = &frag_index->item[index].stream_info[i];
> +        for (j = 0; j < s->nb_streams; j++)
> +            if (s->streams[j]->id == frag_stream_info->id)
> +                frag_stream = s->streams[j];
>         timestamp = get_stream_info_time(frag_stream_info);
> -        if (timestamp != AV_NOPTS_VALUE)
> -            return timestamp;
> +        if (timestamp != AV_NOPTS_VALUE) {
> +            if (frag_stream)
> +                return av_rescale_q(timestamp, frag_stream->time_base, dst_st->time_base);
> +            else
> +                return timestamp;
> +        }
>     }
>     return AV_NOPTS_VALUE;
> }
> 
> -static int search_frag_timestamp(MOVFragmentIndex *frag_index,
> +static int search_frag_timestamp(AVFormatContext *s, MOVFragmentIndex *frag_index,
>                                  AVStream *st, int64_t timestamp)
> {
>     int a, b, m, m0;
> @@ -1317,7 +1325,7 @@ static int search_frag_timestamp(MOVFragmentIndex *frag_index,
>         m0 = m = (a + b) >> 1;
> 
>         while (m < b &&
> -               (frag_time = get_frag_time(frag_index, m, id)) == AV_NOPTS_VALUE)
> +               (frag_time = get_frag_time(s, st, frag_index, m, id)) == AV_NOPTS_VALUE)
>             m++;
> 
>         if (m < b && frag_time <= timestamp)
> @@ -8782,7 +8790,7 @@ static int mov_seek_fragment(AVFormatContext *s, AVStream *st, int64_t timestamp
>     if (!mov->frag_index.complete)
>         return 0;
> 
> -    index = search_frag_timestamp(&mov->frag_index, st, timestamp);
> +    index = search_frag_timestamp(s, &mov->frag_index, st, timestamp);
>     if (index < 0)
>         index = 0;
>     if (!mov->frag_index.item[index].headers_read)
> -- 
> 2.36.1
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavformat/mov.c b/libavformat/mov.c
index d7be593a86..fe3b2b15ab 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1270,12 +1270,12 @@  static int64_t get_stream_info_time(MOVFragmentStreamInfo * frag_stream_info)
     return frag_stream_info->tfdt_dts;
 }
 
-static int64_t get_frag_time(MOVFragmentIndex *frag_index,
-                             int index, int track_id)
+static int64_t get_frag_time(AVFormatContext *s, AVStream *dst_st,
+                             MOVFragmentIndex *frag_index, int index, int track_id)
 {
     MOVFragmentStreamInfo * frag_stream_info;
     int64_t timestamp;
-    int i;
+    int i, j;
 
     if (track_id >= 0) {
         frag_stream_info = get_frag_stream_info(frag_index, index, track_id);
@@ -1287,15 +1287,23 @@  static int64_t get_frag_time(MOVFragmentIndex *frag_index,
     }
 
     for (i = 0; i < frag_index->item[index].nb_stream_info; i++) {
+        AVStream *frag_stream = NULL;
         frag_stream_info = &frag_index->item[index].stream_info[i];
+        for (j = 0; j < s->nb_streams; j++)
+            if (s->streams[j]->id == frag_stream_info->id)
+                frag_stream = s->streams[j];
         timestamp = get_stream_info_time(frag_stream_info);
-        if (timestamp != AV_NOPTS_VALUE)
-            return timestamp;
+        if (timestamp != AV_NOPTS_VALUE) {
+            if (frag_stream)
+                return av_rescale_q(timestamp, frag_stream->time_base, dst_st->time_base);
+            else
+                return timestamp;
+        }
     }
     return AV_NOPTS_VALUE;
 }
 
-static int search_frag_timestamp(MOVFragmentIndex *frag_index,
+static int search_frag_timestamp(AVFormatContext *s, MOVFragmentIndex *frag_index,
                                  AVStream *st, int64_t timestamp)
 {
     int a, b, m, m0;
@@ -1317,7 +1325,7 @@  static int search_frag_timestamp(MOVFragmentIndex *frag_index,
         m0 = m = (a + b) >> 1;
 
         while (m < b &&
-               (frag_time = get_frag_time(frag_index, m, id)) == AV_NOPTS_VALUE)
+               (frag_time = get_frag_time(s, st, frag_index, m, id)) == AV_NOPTS_VALUE)
             m++;
 
         if (m < b && frag_time <= timestamp)
@@ -8782,7 +8790,7 @@  static int mov_seek_fragment(AVFormatContext *s, AVStream *st, int64_t timestamp
     if (!mov->frag_index.complete)
         return 0;
 
-    index = search_frag_timestamp(&mov->frag_index, st, timestamp);
+    index = search_frag_timestamp(s, &mov->frag_index, st, timestamp);
     if (index < 0)
         index = 0;
     if (!mov->frag_index.item[index].headers_read)