[FFmpeg-devel] [PATCH 2/2] aviobuf: Avoid clearing the whole buffer in fill_buffer

[Anton Khirnov]

Quoting Martin Storsjö (2023-03-21 21:24:25)
> On Tue, 21 Mar 2023, Marton Balint wrote:
> 
> >
> >
> > On Tue, 21 Mar 2023, Martin Storsjö wrote:
> >
> >> Normally, fill_buffer reads in one max_packet_size/IO_BUFFER_SIZE
> >> worth of data into the buffer, slowly filling the buffer until it
> >> is full.
> >>
> >> Previously, when the buffer was full, fill_buffer would start over
> >> from the start, effectively discarding all the previously buffered
> >> data.
> >>
> >> For files that are read linearly, the previous behaviour was fine.
> >>
> >> For files that exhibit some amount of nonlinear read patterns,
> >> especially mov files (where ff_configure_buffers_for_index
> >> increases the buffer size to accomodate for the nonlinear reading!)
> >> we would mostly be able to seek within the buffer - but whenever
> >> we've hit the maximum buffer size, we'd discard most of the buffer
> >> and start over with a very small buffer, so the next seek backwards
> >> would end up outside of the buffer.
> >>
> >> Keep one fourth of the buffered data, moving it to the start of
> >> the buffer, freeing the rest to be refilled with future data.
> >>
> >> For mov files with nonlinear read patterns, this almost entirely
> >> avoids doing seeks on the lower IO level, where we previously would
> >> end up doing seeks occasionally.
> >
> > Maybe the demuxer should use ffio_ensure_seekback() instead if it knows
> > that a seekback will happen? Unconditional memmove of even fourth of all 
> > data does not seem like a good idea.
> 
> Right, it's probably not ideal to do this unconditionally.
> 
> However, it's not that the demuxer really knows that a seekback _will_ 
> happen - unless we make it inspect the next couple index entries. And I 
> don't think we should make the demuxer pre-analyze the next access 
> locations, but keep optimization like this on the separate layer. That 
> way, it works as expected as long as the seeks are short enough within the 
> expected tolerance, and falls back graciously on regular seeking for the 
> accesses that are weirder than that.

I suppose changing the buffer into a ring buffer so you don't have to
move the data is not feasible?

-- 
Anton Khirnov