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

[Martin Storsjö]

On Sat, 25 Mar 2023, Michael Niedermayer wrote:

> On Tue, Mar 21, 2023 at 02:37:28PM +0200, 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 analysis showed a need for even larger
>> buffer sizes (for a really badly interleaved file), over the
>> sanity limits, we previously didn't increase the buffer sizes
>> at all.
>>
>> Instead, if the file shows a need for really large buffers and
>> short_seek_threshold, just set them to the maximum sanity limit;
>> while it might not be enough for all cases in the file, it might
>> be enough for most of it.
>>
>> 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>
>> ---
>>  libavformat/seek.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> I think, if iam not too tired ATM that
> instead of taking the maximum it would be better to ignore the cases
> which are above the maximum, so the value would be optimal for the
> cases that it can help

Sure, I guess that could be reasonable too - I can update the patch with 
that.

For the tricky file in my case, this algorithm currently observes mostly 
deltas around 2-32 MB here, but with a few outliers around 2 GB. So the 2 
GB outliers clearly are irrelevant regardless of whether we keep the limit 
at the current 16 MB or raise it a little bit.

This heuristic algorithm currently does observe some differences over 32 
MB - but even with the buffer size set to the maximum of 16 MB, it's quite 
enough to avoid most of the seeking.

One thing I noted is that this function is called with time_tolerance set 
to 1 second - but shouldn't it be quite enough to just have time_tolerance 
set to 0? I.e. check exactly the next sample in the other tracks (since 
that's what's going to be read in most cases), instead of checking a 
sample 1 second in the future in other tracks?

// Martin