> -----Original Message-----
> From: ffmpeg-devel-boun...@ffmpeg.org <ffmpeg-devel-boun...@ffmpeg.org> On
> Behalf Of Derek Buitenhuis
> Sent: 2022年8月16日 22:49
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v3] mov: Compare frag times in correct time
> base when seeking a stream without a corresponding sidx
>
> 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.buitenh...@gmail.com>
> ---
> libavformat/mov.c | 38 ++++++++++++++++++++------------------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 31f3249ca6..1d8c5b2904 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -1271,15 +1271,18 @@ 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)
> {
> MOVFragmentStreamInfo * frag_stream_info;
> + MOVStreamContext *sc = dst_st->priv_data;
> int64_t timestamp;
> - int i;
> + int i, j;
>
> - if (track_id >= 0) {
> - frag_stream_info = get_frag_stream_info(frag_index, index, track_id);
> + // If the stream is referenced by any sidx, limit the search
> + // to fragments that referenced this stream in the sidx
> + if (sc->has_sidx) {
> + frag_stream_info = get_frag_stream_info(frag_index, index,
> dst_st->id);
> if (frag_stream_info->sidx_pts != AV_NOPTS_VALUE)
> return frag_stream_info->sidx_pts;
> if (frag_stream_info->first_tfra_pts != AV_NOPTS_VALUE)
get_frag_time() can be called with an mp4 file which has no sidx at all. In
that case,
dst_st should have a higher priority than other streams, even if sc->has_sidx
is false.
And first_tfra_pts might be used here, which makes the check of sc->has_sidx
unnatural.
So in my opinion, the check on sc->has_sidx should be removed.
Secondly, this part can be simplified with get_stream_info_time(), which is:
+ frag_stream_info = get_frag_stream_info(frag_index, index, dst_st->id);
+ timestamp = get_stream_info_time(frag_stream_info);
+ if (timestamp != AV_NOPTS_VALUE)
+ return timestamp;
Sorry I missed this part with patch v2, because I can't remember what did myself
mean in the replying to patch v1 😊
> @@ -1288,28 +1291,27 @@ 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];
> + if (!frag_stream) {
> + av_log(s, AV_LOG_WARNING, "No stream matching sidx ID found.\n");
Neither frag_stream_info->id nor s->streams[j]->id comes from sidx ID.
> + continue;
> + }
> timestamp = get_stream_info_time(frag_stream_info);
> if (timestamp != AV_NOPTS_VALUE)
> - return timestamp;
> + return av_rescale_q(timestamp, frag_stream->time_base,
> dst_st->time_base);
> }
> 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;
> int64_t frag_time;
> - int id = -1;
> -
> - if (st) {
> - // If the stream is referenced by any sidx, limit the search
> - // to fragments that referenced this stream in the sidx
> - MOVStreamContext *sc = st->priv_data;
> - if (sc->has_sidx)
> - id = st->id;
> - }
>
> a = -1;
> b = frag_index->nb_items;
> @@ -1318,7 +1320,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)) ==
> AV_NOPTS_VALUE)
> m++;
>
> if (m < b && frag_time <= timestamp)
> @@ -8862,7 +8864,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-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
