diff mbox series

[FFmpeg-devel,1/3] avformat/aviobuf: read till the very end of the IO buffer

Message ID 20200920085253.7107-1-cus@passwd.hu
State New
Headers show
Series [FFmpeg-devel,1/3] avformat/aviobuf: read till the very end of the IO buffer
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Marton Balint Sept. 20, 2020, 8:52 a.m. UTC
There was an off-by-one error when checking if the IO buffer still has enough
space till the end.

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

Comments

Paul B Mahol Sept. 20, 2020, 10:20 a.m. UTC | #1
On Sun, Sep 20, 2020 at 10:52:51AM +0200, Marton Balint wrote:
> There was an off-by-one error when checking if the IO buffer still has enough
> space till the end.

How to reproduce such error(s)?

> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/aviobuf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> index a77517d712..9675425349 100644
> --- a/libavformat/aviobuf.c
> +++ b/libavformat/aviobuf.c
> @@ -540,7 +540,7 @@ static void fill_buffer(AVIOContext *s)
>  {
>      int max_buffer_size = s->max_packet_size ?
>                            s->max_packet_size : IO_BUFFER_SIZE;
> -    uint8_t *dst        = s->buf_end - s->buffer + max_buffer_size < s->buffer_size ?
> +    uint8_t *dst        = s->buf_end - s->buffer + max_buffer_size <= s->buffer_size ?
>                            s->buf_end : s->buffer;
>      int len             = s->buffer_size - (dst - s->buffer);
>  
> -- 
> 2.26.2
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Marton Balint Sept. 20, 2020, 10:59 a.m. UTC | #2
On Sun, 20 Sep 2020, Paul B Mahol wrote:

> On Sun, Sep 20, 2020 at 10:52:51AM +0200, Marton Balint wrote:
>> There was an off-by-one error when checking if the IO buffer still has enough
>> space till the end.
>
> How to reproduce such error(s)?

It is not something you will notice, only the buffer is flushed when it
does not necessarily need to be.

But I guess you can reproduce it by using an buffer size of 66852 and try
receiving an MPEGTS UDP stream. Without the patch fill_buffer always
flushes, with the patch fill_buffer only flushes in every second
call, as it needs to, because a single UDP TS packet (1316 bytes) and
the potential maximum sized UDP packet (65536 bytes) simultaneously fit in
the buffer.

Regards,
Marton


>
>> 
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/aviobuf.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
>> index a77517d712..9675425349 100644
>> --- a/libavformat/aviobuf.c
>> +++ b/libavformat/aviobuf.c
>> @@ -540,7 +540,7 @@ static void fill_buffer(AVIOContext *s)
>>  {
>>      int max_buffer_size = s->max_packet_size ?
>>                            s->max_packet_size : IO_BUFFER_SIZE;
>> -    uint8_t *dst        = s->buf_end - s->buffer + max_buffer_size < s->buffer_size ?
>> +    uint8_t *dst        = s->buf_end - s->buffer + max_buffer_size <= s->buffer_size ?
>>                            s->buf_end : s->buffer;
>>      int len             = s->buffer_size - (dst - s->buffer);
>> 
>> -- 
>> 2.26.2
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Marton Balint Sept. 26, 2020, 9:16 a.m. UTC | #3
On Sun, 20 Sep 2020, Marton Balint wrote:

>
>
> On Sun, 20 Sep 2020, Paul B Mahol wrote:
>
>> On Sun, Sep 20, 2020 at 10:52:51AM +0200, Marton Balint wrote:
>>> There was an off-by-one error when checking if the IO buffer still has 
> enough
>>> space till the end.
>>
>> How to reproduce such error(s)?
>
> It is not something you will notice, only the buffer is flushed when it
> does not necessarily need to be.
>
> But I guess you can reproduce it by using an buffer size of 66852 and try
> receiving an MPEGTS UDP stream. Without the patch fill_buffer always
> flushes, with the patch fill_buffer only flushes in every second
> call, as it needs to, because a single UDP TS packet (1316 bytes) and
> the potential maximum sized UDP packet (65536 bytes) simultaneously fit in
> the buffer.

Ping.

Thanks,
Marton

>
> Regards,
> Marton
>
>
>>
>>> 
>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>> ---
>>>  libavformat/aviobuf.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
>>> index a77517d712..9675425349 100644
>>> --- a/libavformat/aviobuf.c
>>> +++ b/libavformat/aviobuf.c
>>> @@ -540,7 +540,7 @@ static void fill_buffer(AVIOContext *s)
>>>  {
>>>      int max_buffer_size = s->max_packet_size ?
>>>                            s->max_packet_size : IO_BUFFER_SIZE;
>>> -    uint8_t *dst        = s->buf_end - s->buffer + max_buffer_size < 
> s->buffer_size ?
>>> +    uint8_t *dst        = s->buf_end - s->buffer + max_buffer_size <= 
> s->buffer_size ?
>>>                            s->buf_end : s->buffer;
>>>      int len             = s->buffer_size - (dst - s->buffer);
>>> 
>>> -- 
>>> 2.26.2
>>> 
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index a77517d712..9675425349 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -540,7 +540,7 @@  static void fill_buffer(AVIOContext *s)
 {
     int max_buffer_size = s->max_packet_size ?
                           s->max_packet_size : IO_BUFFER_SIZE;
-    uint8_t *dst        = s->buf_end - s->buffer + max_buffer_size < s->buffer_size ?
+    uint8_t *dst        = s->buf_end - s->buffer + max_buffer_size <= s->buffer_size ?
                           s->buf_end : s->buffer;
     int len             = s->buffer_size - (dst - s->buffer);