[FFmpeg-devel,2/9] avformat/aviobuf: check if requested seekback buffer is already read

Existing code did not check if the requested seekback buffer is
already read entirely. In this case, nothing has to be done to guarantee
seekback.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/aviobuf.c | 3 +++
 1 file changed, 3 insertions(+)

On Tue, Sep 29, 2020 at 11:10:14PM +0200, Marton Balint wrote:
> Existing code did not check if the requested seekback buffer is
> already read entirely. In this case, nothing has to be done to guarantee
> seekback.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/aviobuf.c | 3 +++
>  1 file changed, 3 insertions(+)

Its interresting this needs a seperate check but

LGTM

thx

[...]

Hi Marton,

On Tue, 29. Sep 23:10, Marton Balint wrote:
> Existing code did not check if the requested seekback buffer is
> already read entirely. In this case, nothing has to be done to guarantee
> seekback.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/aviobuf.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> index 9675425349..d94be478ac 100644
> --- a/libavformat/aviobuf.c
> +++ b/libavformat/aviobuf.c
> @@ -999,6 +999,9 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
>      int filled = s->buf_end - s->buffer;
>      ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - s->buffer : -1;
>  
> +    if (buf_size <= s->buf_end - s->buf_ptr)
> +        return 0;
> +
>      buf_size += s->buf_ptr - s->buffer + max_buffer_size;
>  
>      if (buf_size < filled || s->seekable || !s->read_packet)

I was testing this set, and saw commit 53c25ee0736. It discards buffered data by
resetting s->{buf_ptr,buf_end} when we need to read more data. 

Maybe I missed something, but wouldn't this mean that seekback no longer works if
fill_buffer() is called in avio_read_partial().. so defeating the purpose of
ffio_ensure_seekback()?

Thanks,

On Sun, 4 Oct 2020, Andriy Gelman wrote:

> Hi Marton,
>
> On Tue, 29. Sep 23:10, Marton Balint wrote:
>> Existing code did not check if the requested seekback buffer is
>> already read entirely. In this case, nothing has to be done to guarantee
>> seekback.
>> 
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/aviobuf.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
>> index 9675425349..d94be478ac 100644
>> --- a/libavformat/aviobuf.c
>> +++ b/libavformat/aviobuf.c
>> @@ -999,6 +999,9 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
>>      int filled = s->buf_end - s->buffer;
>>      ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - s->buffer : -1;
>> 
>> +    if (buf_size <= s->buf_end - s->buf_ptr)
>> +        return 0;
>> +
>>      buf_size += s->buf_ptr - s->buffer + max_buffer_size;
>>
>>      if (buf_size < filled || s->seekable || !s->read_packet)
>
> I was testing this set, and saw commit 53c25ee0736. It discards buffered data by
> resetting s->{buf_ptr,buf_end} when we need to read more data. 
>
> Maybe I missed something, but wouldn't this mean that seekback no longer works if
> fill_buffer() is called in avio_read_partial().. so defeating the purpose of
> ffio_ensure_seekback()?

I think you are right. It looks like that commit is not needed after
2ca48e466675a8a3630061cd2c15325eab8eda97, so probably it should be 
reverted now to unbreak ffio_ensure_seekback().

Regards,
Marton