[FFmpeg-devel] [PATCH] avformat/mov: adjust skip_samples according to seek timestamp

[Marton Balint]

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