[FFmpeg-devel] [PATCHv2 2/2] libavformat: Improve ff_configure_buffers_for_index for excessive deltas

[Martin Storsjö]

On Tue, 28 Mar 2023, Martin Storsjö wrote:

> Previously, the ff_configure_buffers_for_index function had
> upper sanity limits of 16 MB (1<<24) for buffer_size and
> 8 MB (1<<23) for short_seek_threshold.
>
> However, if the index contained entries with a much larger
> delta, setting pos_delta to a value larger than the sanity
> limit, we would end up not increasing the buffer size at all.
>
> Instead, ignore the individual deltas that are excessive, but
> increase the buffer size based on the deltas that are below the
> sanity limit.
>
> Only count deltas that are below 1<<23, 8 MB; pos_delta gets doubled
> before setting the buffer size - this matches the previous maximum
> buffer size of 1<<24, 16 MB.
>
> This can happen e.g. with a mov file with some tracks containing
> some samples that belong in the start of the file, at the end of
> the mdat, while the rest of the file is mostly reasonably interleaved;
> previously those samples caused the maximum pos_delta to skyrocket,
> skipping any buffer size enlargement.
>
> Signed-off-by: Martin Storsjö <martin at martin.st>
> ---
> v2: Ignore entries that are out of range instead of clipping to
> the maximum allowed.
> ---
> libavformat/seek.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/seek.c b/libavformat/seek.c
> index faa47f961f..386312cd3a 100644
> --- a/libavformat/seek.c
> +++ b/libavformat/seek.c
> @@ -204,7 +204,9 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance)
>                 const AVIndexEntry *const e1 = &sti1->index_entries[i1];
>                 int64_t e1_pts = av_rescale_q(e1->timestamp, st1->time_base, AV_TIME_BASE_Q);
>
> -                skip = FFMAX(skip, e1->size);
> +                if (e1->size < (1 << 23))
> +                    skip = FFMAX(skip, e1->size);
> +
>                 for (; i2 < sti2->nb_index_entries; i2++) {
>                     const AVIndexEntry *const e2 = &sti2->index_entries[i2];
>                     int64_t e2_pts = av_rescale_q(e2->timestamp, st2->time_base, AV_TIME_BASE_Q);
> @@ -212,7 +214,8 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance)
>                     if (e2_pts < e1_pts || e2_pts - (uint64_t)e1_pts < time_tolerance)
>                         continue;
>                     cur_delta = FFABS(e1->pos - e2->pos);
> -                    pos_delta = FFMAX(pos_delta, cur_delta);
> +                    if (cur_delta < (1 << 23))
> +                        pos_delta = FFMAX(pos_delta, cur_delta);
>                     break;
>                 }
>             }
> @@ -222,7 +225,7 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance)
>     pos_delta *= 2;
>     ctx = ffiocontext(s->pb);
>     /* XXX This could be adjusted depending on protocol*/
> -    if (s->pb->buffer_size < pos_delta && pos_delta < (1<<24)) {
> +    if (s->pb->buffer_size < pos_delta) {
>         av_log(s, AV_LOG_VERBOSE, "Reconfiguring buffers to size %"PRId64"\n", pos_delta);
>
>         /* realloc the buffer and the original data will be retained */
> @@ -234,9 +237,7 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance)
>         ctx->short_seek_threshold = FFMAX(ctx->short_seek_threshold, pos_delta/2);
>     }
>
> -    if (skip < (1<<23)) {
> -        ctx->short_seek_threshold = FFMAX(ctx->short_seek_threshold, skip);
> -    }
> +    ctx->short_seek_threshold = FFMAX(ctx->short_seek_threshold, skip);
> }
>
> int av_index_search_timestamp(AVStream *st, int64_t wanted_timestamp, int flags)
> -- 
> 2.37.1 (Apple Git-137.1)

Will push these two patches today, if there's no further input on it; the 
previous iteration was already accepted by some, and this version includes 
the other feedback that I got.

// Martin