[FFmpeg-devel] Fix unnecessary buffer reallocotion in ffio_ensure_seekback().

Submitted by Artyom Lebedev on Dec. 19, 2018, 12:33 p.m.

Details

Message ID 4b41ad6b-6ae9-8eac-af89-87d02edf1b2f@gmail.com
State New
Headers show

Commit Message

Artyom Lebedev Dec. 19, 2018, 12:33 p.m.
It was reallocated even if the exisiting buffer is larger than needed 
one, thus unnecessary shrinking it.

Comments

Michael Niedermayer Dec. 19, 2018, 6:49 p.m.
On Wed, Dec 19, 2018 at 02:33:49PM +0200, Artyom Lebedev wrote:
> It was reallocated even if the exisiting buffer is larger than needed one,
> thus unnecessary shrinking it.

>  aviobuf.c |    3 +++
>  1 file changed, 3 insertions(+)
> 17a6a27b38d8fc7336d7177338b915b507a69033  0001-Fix-unnecessary-buffer-reallocotion-in-ffio_ensure_s.patch
> From 2b8cea72a69abe6564367fb2149be936d2ffb916 Mon Sep 17 00:00:00 2001
> From: Artyom Lebedev <vagran.ast@gmail.com>
> Date: Wed, 19 Dec 2018 11:49:22 +0200
> Subject: [PATCH] Fix unnecessary buffer reallocotion in
>  ffio_ensure_seekback().
> To: ffmpeg-devel@ffmpeg.org
> 
> ---
>  libavformat/aviobuf.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> index 5a33f82..b867fdd 100644
> --- a/libavformat/aviobuf.c
> +++ b/libavformat/aviobuf.c
> @@ -1063,6 +1063,9 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
>          return 0;
>      av_assert0(!s->write_flag);
>  
> +    if (s->buffer_size >= buf_size)
> +        return 0;

Theres a check which returns out for unnecessary reallocation
a few lines above
if that is incorrect that should be corrected
also theres a typo in the commit message

thx

[...]
Artyom Lebedev Dec. 20, 2018, 9:11 a.m.
On 12/19/18 8:49 PM, Michael Niedermayer wrote:
> On Wed, Dec 19, 2018 at 02:33:49PM +0200, Artyom Lebedev wrote:
>> It was reallocated even if the exisiting buffer is larger than needed one,
>> thus unnecessary shrinking it.
>>   aviobuf.c |    3 +++
>>   1 file changed, 3 insertions(+)
>> 17a6a27b38d8fc7336d7177338b915b507a69033  0001-Fix-unnecessary-buffer-reallocotion-in-ffio_ensure_s.patch
>>  From 2b8cea72a69abe6564367fb2149be936d2ffb916 Mon Sep 17 00:00:00 2001
>> From: Artyom Lebedev <vagran.ast@gmail.com>
>> Date: Wed, 19 Dec 2018 11:49:22 +0200
>> Subject: [PATCH] Fix unnecessary buffer reallocotion in
>>   ffio_ensure_seekback().
>> To: ffmpeg-devel@ffmpeg.org
>>
>> ---
>>   libavformat/aviobuf.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
>> index 5a33f82..b867fdd 100644
>> --- a/libavformat/aviobuf.c
>> +++ b/libavformat/aviobuf.c
>> @@ -1063,6 +1063,9 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
>>           return 0;
>>       av_assert0(!s->write_flag);
>>   
>> +    if (s->buffer_size >= buf_size)
>> +        return 0;
> Theres a check which returns out for unnecessary reallocation
> a few lines above
> if that is incorrect that should be corrected
> also theres a typo in the commit message
>
> thx
>
> [...]
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Which check do you mean? "buf_size < filled"? This only checks if 
currently filled size is less than required (I do not know why, may be 
there was intended to do "buf_size < s->buffer_size" check)?
Michael Niedermayer Dec. 21, 2018, 11:47 p.m.
On Thu, Dec 20, 2018 at 11:11:45AM +0200, Artyom Lebedev wrote:
> On 12/19/18 8:49 PM, Michael Niedermayer wrote:
> >On Wed, Dec 19, 2018 at 02:33:49PM +0200, Artyom Lebedev wrote:
> >>It was reallocated even if the exisiting buffer is larger than needed one,
> >>thus unnecessary shrinking it.
> >>  aviobuf.c |    3 +++
> >>  1 file changed, 3 insertions(+)
> >>17a6a27b38d8fc7336d7177338b915b507a69033  0001-Fix-unnecessary-buffer-reallocotion-in-ffio_ensure_s.patch
> >> From 2b8cea72a69abe6564367fb2149be936d2ffb916 Mon Sep 17 00:00:00 2001
> >>From: Artyom Lebedev <vagran.ast@gmail.com>
> >>Date: Wed, 19 Dec 2018 11:49:22 +0200
> >>Subject: [PATCH] Fix unnecessary buffer reallocotion in
> >>  ffio_ensure_seekback().
> >>To: ffmpeg-devel@ffmpeg.org
> >>
> >>---
> >>  libavformat/aviobuf.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >>diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> >>index 5a33f82..b867fdd 100644
> >>--- a/libavformat/aviobuf.c
> >>+++ b/libavformat/aviobuf.c
> >>@@ -1063,6 +1063,9 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
> >>          return 0;
> >>      av_assert0(!s->write_flag);
> >>+    if (s->buffer_size >= buf_size)
> >>+        return 0;
> >Theres a check which returns out for unnecessary reallocation
> >a few lines above
> >if that is incorrect that should be corrected
> >also theres a typo in the commit message
> >
> >thx
> >
> >[...]
> >
> >
> >_______________________________________________
> >ffmpeg-devel mailing list
> >ffmpeg-devel@ffmpeg.org
> >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> Which check do you mean? "buf_size < filled"? This only checks if currently

yes


> filled size is less than required (I do not know why, may be there was
> intended to do "buf_size < s->buffer_size" check)?

maybe, that is the question. i do not remember why it was written this way
so it could infact be unintended

the point is adding this 2nd check on top is certainly wrong as the checks would
be redundant, or if its not redundant it really would need some explanation

thx

[...]

Patch hide | download patch | download mbox

From 2b8cea72a69abe6564367fb2149be936d2ffb916 Mon Sep 17 00:00:00 2001
From: Artyom Lebedev <vagran.ast@gmail.com>
Date: Wed, 19 Dec 2018 11:49:22 +0200
Subject: [PATCH] Fix unnecessary buffer reallocotion in
 ffio_ensure_seekback().
To: ffmpeg-devel@ffmpeg.org

---
 libavformat/aviobuf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 5a33f82..b867fdd 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -1063,6 +1063,9 @@  int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
         return 0;
     av_assert0(!s->write_flag);
 
+    if (s->buffer_size >= buf_size)
+        return 0;
+
     buffer = av_malloc(buf_size);
     if (!buffer)
         return AVERROR(ENOMEM);
-- 
2.7.4