[FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

[Marton Balint]

On Sun, 20 Sep 2020, Paul B Mahol wrote:

> On Sat, Sep 19, 2020 at 11:46:50PM +0200, Marton Balint wrote:
>> 
>> 
>> On Sat, 19 Sep 2020, Paul B Mahol wrote:
>> 
>> > On Thu, Sep 17, 2020 at 12:31:06PM +0200, Paul B Mahol wrote:
>> > > This removes big CPU overhead for demuxing chained ogg streams.
>> > > 
>> > > Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> > > ---
>> > >  libavformat/aviobuf.c | 10 +++++-----
>> > >  1 file changed, 5 insertions(+), 5 deletions(-)
>> > > 
>> > 
>> > will apply soon.
>> 
>> Don't apply it, as I explained, it is not correct.
>
> I carefully examined your explanation and I already reported
> here on mailing list for every one to see that it does not work.
>
> So please reconsider your blocking statement and provide
> correct solution to problem.

Your solution will break the fundamentals of ffio_ensure_seekback, so it 
is not OK. Checking if (buf_size <= s->buffer_size) will not guarantee 
that the buffer will not be flushed when reading the next buf_size bytes 
because buf_ptr can be anywhere in the buffer. See how fill_buffer() 
works. It not only makes sure the buffer has space, it makes sure that the 
buffer has max_buffer_size space available. If not, then it gets flushed.

There are some issues with current code, I will send a patch to fix them. 
Unfortunately it will NOT fix your problem entirely. As I suggested, if 
you use 2*max_buffer_size buffers by default, that might help you, but the 
fundamental problem is that ffio_ensure_seekback simply cannot be 
implemented efficiently with the current design. The guarantees should be 
reduced (e.g. ffio_ensure_seekback can flush the buffer and invalidate 
previous ffio_ensure_seekback request)

Regards,
Marton