| Message ID | 20201102114219.459524-1-matthieu.bouron@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/mov: adjust skip_samples according to seek timestamp | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On Mon, Nov 02, 2020 at 12:42:19PM +0100, Matthieu Bouron wrote: > Currently skip_samples is set to start_pad if sample_time is lesser or > equal to 0. This can cause issues if the stream starts with packets that > have negative pts. Calling avformat_seek_file() with ts set to 0 on such > streams makes the mov demuxer return the right corresponding packets > (near the 0 timestamp) but set skip_samples to start_pad which is > incorrect as the audio decoder will discard the returned samples > according to skip_samples from the first packet it receives (which has > its timestamp near 0). > > For example, considering the following audio stream with start_pad=1344: > > [PKT pts=-1344] [PKT pts=-320] [PKT pts=704] [PKT pts=1728] [...] > > Calling avformat_seek_file() with ts=0 makes the next call to > av_read_frame() return the packet with pts=-320 and a skip samples > side data set to 1344 (start_pad). This makes the audio decoder > incorrectly discard (1344 - 320) samples. > > This commit makes the move demuxer adjust skip_samples according to the > stream start_pad, seek timestamp and first sample timestamp. > > The above example will now result in av_read_frame() still returning the > packet with pts=-320 but with a skip samples side data set to 320 > (src_pad - (seek_timestamp - first_timestamp)). This makes the audio > decoder only discard 320 samples (from pts=-320 to pts=0). > --- > libavformat/mov.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index dd0db6bca79..f99cb0df25a 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -8098,20 +8098,34 @@ static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti > return sample; > > if (mc->seek_individually) { > + MOVStreamContext *sc = s->streams[stream_index]->priv_data; > + > /* adjust seek timestamp to found sample timestamp */ > + int64_t first_timestamp = st->internal->index_entries[0].timestamp; > int64_t seek_timestamp = st->internal->index_entries[sample].timestamp; > > + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ > + int64_t skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); > + st->internal->skip_samples = skip_samples; > + > for (i = 0; i < s->nb_streams; i++) { > int64_t timestamp; > MOVStreamContext *sc = s->streams[i]->priv_data; > st = s->streams[i]; > - st->internal->skip_samples = (sample_time <= 0) ? sc->start_pad : 0; > > if (stream_index == i) > continue; > > timestamp = av_rescale_q(seek_timestamp, s->streams[stream_index]->time_base, st->time_base); > - mov_seek_stream(s, st, timestamp, flags); > + sample = mov_seek_stream(s, st, timestamp, flags); > + if (sample >= 0) { > + first_timestamp = st->internal->index_entries[0].timestamp; > + seek_timestamp = st->internal->index_entries[sample].timestamp; > + > + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ > + skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); > + st->internal->skip_samples = skip_samples; > + } > } > } else { > for (i = 0; i < s->nb_streams; i++) { > -- > 2.29.2 > Ping.
On Mon, Nov 09, 2020 at 06:26:46PM +0100, Matthieu Bouron wrote: > On Mon, Nov 02, 2020 at 12:42:19PM +0100, Matthieu Bouron wrote: > > Currently skip_samples is set to start_pad if sample_time is lesser or > > equal to 0. This can cause issues if the stream starts with packets that > > have negative pts. Calling avformat_seek_file() with ts set to 0 on such > > streams makes the mov demuxer return the right corresponding packets > > (near the 0 timestamp) but set skip_samples to start_pad which is > > incorrect as the audio decoder will discard the returned samples > > according to skip_samples from the first packet it receives (which has > > its timestamp near 0). > > > > For example, considering the following audio stream with start_pad=1344: > > > > [PKT pts=-1344] [PKT pts=-320] [PKT pts=704] [PKT pts=1728] [...] > > > > Calling avformat_seek_file() with ts=0 makes the next call to > > av_read_frame() return the packet with pts=-320 and a skip samples > > side data set to 1344 (start_pad). This makes the audio decoder > > incorrectly discard (1344 - 320) samples. > > > > This commit makes the move demuxer adjust skip_samples according to the > > stream start_pad, seek timestamp and first sample timestamp. > > > > The above example will now result in av_read_frame() still returning the > > packet with pts=-320 but with a skip samples side data set to 320 > > (src_pad - (seek_timestamp - first_timestamp)). This makes the audio > > decoder only discard 320 samples (from pts=-320 to pts=0). > > --- > > libavformat/mov.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index dd0db6bca79..f99cb0df25a 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -8098,20 +8098,34 @@ static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti > > return sample; > > > > if (mc->seek_individually) { > > + MOVStreamContext *sc = s->streams[stream_index]->priv_data; > > + > > /* adjust seek timestamp to found sample timestamp */ > > + int64_t first_timestamp = st->internal->index_entries[0].timestamp; > > int64_t seek_timestamp = st->internal->index_entries[sample].timestamp; > > > > + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ > > + int64_t skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); > > + st->internal->skip_samples = skip_samples; > > + > > for (i = 0; i < s->nb_streams; i++) { > > int64_t timestamp; > > MOVStreamContext *sc = s->streams[i]->priv_data; > > st = s->streams[i]; > > - st->internal->skip_samples = (sample_time <= 0) ? sc->start_pad : 0; > > > > if (stream_index == i) > > continue; > > > > timestamp = av_rescale_q(seek_timestamp, s->streams[stream_index]->time_base, st->time_base); > > - mov_seek_stream(s, st, timestamp, flags); > > + sample = mov_seek_stream(s, st, timestamp, flags); > > + if (sample >= 0) { > > + first_timestamp = st->internal->index_entries[0].timestamp; > > + seek_timestamp = st->internal->index_entries[sample].timestamp; > > + > > + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ > > + skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); > > + st->internal->skip_samples = skip_samples; > > + } > > } > > } else { > > for (i = 0; i < s->nb_streams; i++) { > > -- > > 2.29.2 > > > > Ping. > Ping.
On Tue, Nov 17, 2020 at 02:18:46PM +0100, Matthieu Bouron wrote: > On Mon, Nov 09, 2020 at 06:26:46PM +0100, Matthieu Bouron wrote: > > On Mon, Nov 02, 2020 at 12:42:19PM +0100, Matthieu Bouron wrote: > > > Currently skip_samples is set to start_pad if sample_time is lesser or > > > equal to 0. This can cause issues if the stream starts with packets that > > > have negative pts. Calling avformat_seek_file() with ts set to 0 on such > > > streams makes the mov demuxer return the right corresponding packets > > > (near the 0 timestamp) but set skip_samples to start_pad which is > > > incorrect as the audio decoder will discard the returned samples > > > according to skip_samples from the first packet it receives (which has > > > its timestamp near 0). > > > > > > For example, considering the following audio stream with start_pad=1344: > > > > > > [PKT pts=-1344] [PKT pts=-320] [PKT pts=704] [PKT pts=1728] [...] > > > > > > Calling avformat_seek_file() with ts=0 makes the next call to > > > av_read_frame() return the packet with pts=-320 and a skip samples > > > side data set to 1344 (start_pad). This makes the audio decoder > > > incorrectly discard (1344 - 320) samples. > > > > > > This commit makes the move demuxer adjust skip_samples according to the > > > stream start_pad, seek timestamp and first sample timestamp. > > > > > > The above example will now result in av_read_frame() still returning the > > > packet with pts=-320 but with a skip samples side data set to 320 > > > (src_pad - (seek_timestamp - first_timestamp)). This makes the audio > > > decoder only discard 320 samples (from pts=-320 to pts=0). > > > --- > > > libavformat/mov.c | 18 ++++++++++++++++-- > > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > > index dd0db6bca79..f99cb0df25a 100644 > > > --- a/libavformat/mov.c > > > +++ b/libavformat/mov.c > > > @@ -8098,20 +8098,34 @@ static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti > > > return sample; > > > > > > if (mc->seek_individually) { > > > + MOVStreamContext *sc = s->streams[stream_index]->priv_data; > > > + > > > /* adjust seek timestamp to found sample timestamp */ > > > + int64_t first_timestamp = st->internal->index_entries[0].timestamp; > > > int64_t seek_timestamp = st->internal->index_entries[sample].timestamp; > > > > > > + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ > > > + int64_t skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); > > > + st->internal->skip_samples = skip_samples; > > > + > > > for (i = 0; i < s->nb_streams; i++) { > > > int64_t timestamp; > > > MOVStreamContext *sc = s->streams[i]->priv_data; > > > st = s->streams[i]; > > > - st->internal->skip_samples = (sample_time <= 0) ? sc->start_pad : 0; > > > > > > if (stream_index == i) > > > continue; > > > > > > timestamp = av_rescale_q(seek_timestamp, s->streams[stream_index]->time_base, st->time_base); > > > - mov_seek_stream(s, st, timestamp, flags); > > > + sample = mov_seek_stream(s, st, timestamp, flags); > > > + if (sample >= 0) { > > > + first_timestamp = st->internal->index_entries[0].timestamp; > > > + seek_timestamp = st->internal->index_entries[sample].timestamp; > > > + > > > + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ > > > + skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); > > > + st->internal->skip_samples = skip_samples; > > > + } > > > } > > > } else { > > > for (i = 0; i < s->nb_streams; i++) { > > > -- > > > 2.29.2 > > > > > > > Ping. > > > > Ping. > Ping.
On Mon, Nov 23, 2020 at 05:43:06PM +0100, Matthieu Bouron wrote: > On Tue, Nov 17, 2020 at 02:18:46PM +0100, Matthieu Bouron wrote: > > On Mon, Nov 09, 2020 at 06:26:46PM +0100, Matthieu Bouron wrote: > > > On Mon, Nov 02, 2020 at 12:42:19PM +0100, Matthieu Bouron wrote: > > > > Currently skip_samples is set to start_pad if sample_time is lesser or > > > > equal to 0. This can cause issues if the stream starts with packets that > > > > have negative pts. Calling avformat_seek_file() with ts set to 0 on such > > > > streams makes the mov demuxer return the right corresponding packets > > > > (near the 0 timestamp) but set skip_samples to start_pad which is > > > > incorrect as the audio decoder will discard the returned samples > > > > according to skip_samples from the first packet it receives (which has > > > > its timestamp near 0). > > > > > > > > For example, considering the following audio stream with start_pad=1344: > > > > > > > > [PKT pts=-1344] [PKT pts=-320] [PKT pts=704] [PKT pts=1728] [...] > > > > > > > > Calling avformat_seek_file() with ts=0 makes the next call to > > > > av_read_frame() return the packet with pts=-320 and a skip samples > > > > side data set to 1344 (start_pad). This makes the audio decoder > > > > incorrectly discard (1344 - 320) samples. > > > > > > > > This commit makes the move demuxer adjust skip_samples according to the > > > > stream start_pad, seek timestamp and first sample timestamp. > > > > > > > > The above example will now result in av_read_frame() still returning the > > > > packet with pts=-320 but with a skip samples side data set to 320 > > > > (src_pad - (seek_timestamp - first_timestamp)). This makes the audio > > > > decoder only discard 320 samples (from pts=-320 to pts=0). > > > > --- > > > > libavformat/mov.c | 18 ++++++++++++++++-- > > > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > > > index dd0db6bca79..f99cb0df25a 100644 > > > > --- a/libavformat/mov.c > > > > +++ b/libavformat/mov.c > > > > @@ -8098,20 +8098,34 @@ static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti > > > > return sample; > > > > > > > > if (mc->seek_individually) { > > > > + MOVStreamContext *sc = s->streams[stream_index]->priv_data; > > > > + > > > > /* adjust seek timestamp to found sample timestamp */ > > > > + int64_t first_timestamp = st->internal->index_entries[0].timestamp; > > > > int64_t seek_timestamp = st->internal->index_entries[sample].timestamp; > > > > > > > > + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ > > > > + int64_t skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); > > > > + st->internal->skip_samples = skip_samples; > > > > + > > > > for (i = 0; i < s->nb_streams; i++) { > > > > int64_t timestamp; > > > > MOVStreamContext *sc = s->streams[i]->priv_data; > > > > st = s->streams[i]; > > > > - st->internal->skip_samples = (sample_time <= 0) ? sc->start_pad : 0; > > > > > > > > if (stream_index == i) > > > > continue; > > > > > > > > timestamp = av_rescale_q(seek_timestamp, s->streams[stream_index]->time_base, st->time_base); > > > > - mov_seek_stream(s, st, timestamp, flags); > > > > + sample = mov_seek_stream(s, st, timestamp, flags); > > > > + if (sample >= 0) { > > > > + first_timestamp = st->internal->index_entries[0].timestamp; > > > > + seek_timestamp = st->internal->index_entries[sample].timestamp; > > > > + > > > > + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ > > > > + skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); > > > > + st->internal->skip_samples = skip_samples; > > > > + } > > > > } > > > > } else { > > > > for (i = 0; i < s->nb_streams; i++) { > > > > -- > > > > 2.29.2 > > > > > > > > > > Ping. > > > > > > > Ping. > > > > Ping. > Ping.
On Mon, 2 Nov 2020, Matthieu Bouron wrote: > Currently skip_samples is set to start_pad if sample_time is lesser or > equal to 0. This can cause issues if the stream starts with packets that > have negative pts. Calling avformat_seek_file() with ts set to 0 on such > streams makes the mov demuxer return the right corresponding packets > (near the 0 timestamp) but set skip_samples to start_pad which is > incorrect as the audio decoder will discard the returned samples > according to skip_samples from the first packet it receives (which has > its timestamp near 0). > > For example, considering the following audio stream with start_pad=1344: > > [PKT pts=-1344] [PKT pts=-320] [PKT pts=704] [PKT pts=1728] [...] > > Calling avformat_seek_file() with ts=0 makes the next call to > av_read_frame() return the packet with pts=-320 and a skip samples > side data set to 1344 (start_pad). This makes the audio decoder > incorrectly discard (1344 - 320) samples. > > This commit makes the move demuxer adjust skip_samples according to the > stream start_pad, seek timestamp and first sample timestamp. > > The above example will now result in av_read_frame() still returning the > packet with pts=-320 but with a skip samples side data set to 320 > (src_pad - (seek_timestamp - first_timestamp)). This makes the audio > decoder only discard 320 samples (from pts=-320 to pts=0). > --- > libavformat/mov.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index dd0db6bca79..f99cb0df25a 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -8098,20 +8098,34 @@ static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti > return sample; > > if (mc->seek_individually) { > + MOVStreamContext *sc = s->streams[stream_index]->priv_data; > + > /* adjust seek timestamp to found sample timestamp */ > + int64_t first_timestamp = st->internal->index_entries[0].timestamp; > int64_t seek_timestamp = st->internal->index_entries[sample].timestamp; > > + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ > + int64_t skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); > + st->internal->skip_samples = skip_samples; > + Could you factorize this to a function? It seems you are doing exactly the same thing here and below. Thanks, Marton > for (i = 0; i < s->nb_streams; i++) { > int64_t timestamp; > MOVStreamContext *sc = s->streams[i]->priv_data; > st = s->streams[i]; > - st->internal->skip_samples = (sample_time <= 0) ? sc->start_pad : 0; > > if (stream_index == i) > continue; > > timestamp = av_rescale_q(seek_timestamp, s->streams[stream_index]->time_base, st->time_base); > - mov_seek_stream(s, st, timestamp, flags); > + sample = mov_seek_stream(s, st, timestamp, flags); > + if (sample >= 0) { > + first_timestamp = st->internal->index_entries[0].timestamp; > + seek_timestamp = st->internal->index_entries[sample].timestamp; > + > + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ > + skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); > + st->internal->skip_samples = skip_samples; > + } > } > } else { > for (i = 0; i < s->nb_streams; i++) { > -- > 2.29.2 > > _______________________________________________ > 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".
On Mon, Jan 04, 2021 at 09:04:05PM +0100, Marton Balint wrote: > > > On Mon, 2 Nov 2020, Matthieu Bouron wrote: > > > Currently skip_samples is set to start_pad if sample_time is lesser or > > equal to 0. This can cause issues if the stream starts with packets that > > have negative pts. Calling avformat_seek_file() with ts set to 0 on such > > streams makes the mov demuxer return the right corresponding packets > > (near the 0 timestamp) but set skip_samples to start_pad which is > > incorrect as the audio decoder will discard the returned samples > > according to skip_samples from the first packet it receives (which has > > its timestamp near 0). > > > > For example, considering the following audio stream with start_pad=1344: > > > > [PKT pts=-1344] [PKT pts=-320] [PKT pts=704] [PKT pts=1728] [...] > > > > Calling avformat_seek_file() with ts=0 makes the next call to > > av_read_frame() return the packet with pts=-320 and a skip samples > > side data set to 1344 (start_pad). This makes the audio decoder > > incorrectly discard (1344 - 320) samples. > > > > This commit makes the move demuxer adjust skip_samples according to the > > stream start_pad, seek timestamp and first sample timestamp. > > > > The above example will now result in av_read_frame() still returning the > > packet with pts=-320 but with a skip samples side data set to 320 > > (src_pad - (seek_timestamp - first_timestamp)). This makes the audio > > decoder only discard 320 samples (from pts=-320 to pts=0). > > --- > > libavformat/mov.c | 18 ++++++++++++++++-- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index dd0db6bca79..f99cb0df25a 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -8098,20 +8098,34 @@ static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti > > return sample; > > > > if (mc->seek_individually) { > > + MOVStreamContext *sc = s->streams[stream_index]->priv_data; > > + > > /* adjust seek timestamp to found sample timestamp */ > > + int64_t first_timestamp = st->internal->index_entries[0].timestamp; > > int64_t seek_timestamp = st->internal->index_entries[sample].timestamp; > > > > + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ > > + int64_t skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); > > + st->internal->skip_samples = skip_samples; > > + > > Could you factorize this to a function? It seems you are doing exactly the > same thing here and below. New patch attached. [...] Thanks,
On Tue, 5 Jan 2021, Matthieu Bouron wrote: >> Could you factorize this to a function? It seems you are doing exactly the >> same thing here and below. > > New patch attached. [...] > diff --git a/libavformat/mov.c b/libavformat/mov.c > index c6a2d9c388d..e6d0de38c6f 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -8122,6 +8122,15 @@ static int mov_seek_stream(AVFormatContext *s, AVStream *st, int64_t timestamp, > return sample; > } > > +static int64_t mov_get_skip_samples(AVStream *st, int64_t seek_ts) > +{ Maybe cleaner to make "sample" a parameter instead of seek_ts. > + MOVStreamContext *sc = st->priv_data; > + int64_t first_ts = st->internal->index_entries[0].timestamp; > + > + /* compute skip samples according to stream start_pad, seek ts and first ts */ > + return FFMAX(sc->start_pad - (seek_ts - first_ts), 0); Are you sure you don't need any time base correction here? Difference of timestamps is in stream time base, but skip_samples is the number of samples is in 1/sample_rate time base... Also you are setting skip_samples for all streams, shouldn't you restrict it to audio only, so return 0 for non-audio streams? > +} > + > static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_time, int flags) > { > MOVContext *mc = s->priv_data; > @@ -8140,18 +8149,21 @@ static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti > if (mc->seek_individually) { > /* adjust seek timestamp to found sample timestamp */ > int64_t seek_timestamp = st->internal->index_entries[sample].timestamp; > + st->internal->skip_samples = mov_get_skip_samples(st, seek_timestamp); > > for (i = 0; i < s->nb_streams; i++) { > int64_t timestamp; > - MOVStreamContext *sc = s->streams[i]->priv_data; > st = s->streams[i]; > - st->internal->skip_samples = (sample_time <= 0) ? sc->start_pad : 0; > > if (stream_index == i) > continue; > > timestamp = av_rescale_q(seek_timestamp, s->streams[stream_index]->time_base, st->time_base); > - mov_seek_stream(s, st, timestamp, flags); > + sample = mov_seek_stream(s, st, timestamp, flags); > + if (sample >= 0) { > + seek_timestamp = st->internal->index_entries[sample].timestamp; This is destorying seek_timestamp, which seems unintended. > + st->internal->skip_samples = mov_get_skip_samples(st, seek_timestamp); > + } Regards, Marton
On Tue, Jan 05, 2021 at 09:56:39PM +0100, Marton Balint wrote: > > On Tue, 5 Jan 2021, Matthieu Bouron wrote: > > > > Could you factorize this to a function? It seems you are doing exactly the > > > same thing here and below. > > > > New patch attached. > > [...] > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index c6a2d9c388d..e6d0de38c6f 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -8122,6 +8122,15 @@ static int mov_seek_stream(AVFormatContext *s, AVStream *st, int64_t timestamp, > > return sample; > > } > > > > +static int64_t mov_get_skip_samples(AVStream *st, int64_t seek_ts) > > +{ > > Maybe cleaner to make "sample" a parameter instead of seek_ts. Done. > > > + MOVStreamContext *sc = st->priv_data; > > + int64_t first_ts = st->internal->index_entries[0].timestamp; > > + > > + /* compute skip samples according to stream start_pad, seek ts and first ts */ > > + return FFMAX(sc->start_pad - (seek_ts - first_ts), 0); > > Are you sure you don't need any time base correction here? Difference of > timestamps is in stream time base, but skip_samples is the number of samples > is in 1/sample_rate time base... Done. > > Also you are setting skip_samples for all streams, shouldn't you restrict it > to audio only, so return 0 for non-audio streams? I agree, it should only makes sense for audio streams. Updated. > > > +} > > + > > static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_time, int flags) > > { > > MOVContext *mc = s->priv_data; > > @@ -8140,18 +8149,21 @@ static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti > > if (mc->seek_individually) { > > /* adjust seek timestamp to found sample timestamp */ > > int64_t seek_timestamp = st->internal->index_entries[sample].timestamp; > > + st->internal->skip_samples = mov_get_skip_samples(st, seek_timestamp); > > > > for (i = 0; i < s->nb_streams; i++) { > > int64_t timestamp; > > - MOVStreamContext *sc = s->streams[i]->priv_data; > > st = s->streams[i]; > > - st->internal->skip_samples = (sample_time <= 0) ? sc->start_pad : 0; > > > > if (stream_index == i) > > continue; > > > > timestamp = av_rescale_q(seek_timestamp, s->streams[stream_index]->time_base, st->time_base); > > - mov_seek_stream(s, st, timestamp, flags); > > + sample = mov_seek_stream(s, st, timestamp, flags); > > + if (sample >= 0) { > > + seek_timestamp = st->internal->index_entries[sample].timestamp; > > This is destorying seek_timestamp, which seems unintended. It was unintended. Fixed. > > > + st->internal->skip_samples = mov_get_skip_samples(st, seek_timestamp); > > + } New patch attached. [...]
On Wed, 6 Jan 2021, Matthieu Bouron wrote: > On Tue, Jan 05, 2021 at 09:56:39PM +0100, Marton Balint wrote: >> >> On Tue, 5 Jan 2021, Matthieu Bouron wrote: >> >>>> Could you factorize this to a function? It seems you are doing exactly the >>>> same thing here and below. >>> >>> New patch attached. >> >> [...] >> >>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>> index c6a2d9c388d..e6d0de38c6f 100644 >>> --- a/libavformat/mov.c >>> +++ b/libavformat/mov.c >>> @@ -8122,6 +8122,15 @@ static int mov_seek_stream(AVFormatContext *s, AVStream *st, int64_t timestamp, >>> return sample; >>> } >>> >>> +static int64_t mov_get_skip_samples(AVStream *st, int64_t seek_ts) >>> +{ >> >> Maybe cleaner to make "sample" a parameter instead of seek_ts. > > Done. > >> >>> + MOVStreamContext *sc = st->priv_data; >>> + int64_t first_ts = st->internal->index_entries[0].timestamp; >>> + >>> + /* compute skip samples according to stream start_pad, seek ts and first ts */ >>> + return FFMAX(sc->start_pad - (seek_ts - first_ts), 0); >> >> Are you sure you don't need any time base correction here? Difference of >> timestamps is in stream time base, but skip_samples is the number of samples >> is in 1/sample_rate time base... > > Done. > >> >> Also you are setting skip_samples for all streams, shouldn't you restrict it >> to audio only, so return 0 for non-audio streams? > > I agree, it should only makes sense for audio streams. Updated. > >> >>> +} >>> + >>> static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_time, int flags) >>> { >>> MOVContext *mc = s->priv_data; >>> @@ -8140,18 +8149,21 @@ static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti >>> if (mc->seek_individually) { >>> /* adjust seek timestamp to found sample timestamp */ >>> int64_t seek_timestamp = st->internal->index_entries[sample].timestamp; >>> + st->internal->skip_samples = mov_get_skip_samples(st, seek_timestamp); >>> >>> for (i = 0; i < s->nb_streams; i++) { >>> int64_t timestamp; >>> - MOVStreamContext *sc = s->streams[i]->priv_data; >>> st = s->streams[i]; >>> - st->internal->skip_samples = (sample_time <= 0) ? sc->start_pad : 0; >>> >>> if (stream_index == i) >>> continue; >>> >>> timestamp = av_rescale_q(seek_timestamp, s->streams[stream_index]->time_base, st->time_base); >>> - mov_seek_stream(s, st, timestamp, flags); >>> + sample = mov_seek_stream(s, st, timestamp, flags); >>> + if (sample >= 0) { >>> + seek_timestamp = st->internal->index_entries[sample].timestamp; >> >> This is destorying seek_timestamp, which seems unintended. > > It was unintended. Fixed. > >> >>> + st->internal->skip_samples = mov_get_skip_samples(st, seek_timestamp); >>> + } > > New patch attached. Thanks, applied. Regards, Marton
diff --git a/libavformat/mov.c b/libavformat/mov.c index dd0db6bca79..f99cb0df25a 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -8098,20 +8098,34 @@ static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti return sample; if (mc->seek_individually) { + MOVStreamContext *sc = s->streams[stream_index]->priv_data; + /* adjust seek timestamp to found sample timestamp */ + int64_t first_timestamp = st->internal->index_entries[0].timestamp; int64_t seek_timestamp = st->internal->index_entries[sample].timestamp; + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ + int64_t skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); + st->internal->skip_samples = skip_samples; + for (i = 0; i < s->nb_streams; i++) { int64_t timestamp; MOVStreamContext *sc = s->streams[i]->priv_data; st = s->streams[i]; - st->internal->skip_samples = (sample_time <= 0) ? sc->start_pad : 0; if (stream_index == i) continue; timestamp = av_rescale_q(seek_timestamp, s->streams[stream_index]->time_base, st->time_base); - mov_seek_stream(s, st, timestamp, flags); + sample = mov_seek_stream(s, st, timestamp, flags); + if (sample >= 0) { + first_timestamp = st->internal->index_entries[0].timestamp; + seek_timestamp = st->internal->index_entries[sample].timestamp; + + /* adjust skip samples according to stream start_pad, seek timestamp and first timestamp */ + skip_samples = FFMAX(sc->start_pad - (seek_timestamp - first_timestamp), 0); + st->internal->skip_samples = skip_samples; + } } } else { for (i = 0; i < s->nb_streams; i++) {