[FFmpeg-devel] swr/resample: free existing ResampleContext on reinit

Submitted by James Almer on March 21, 2017, 1:33 a.m.

Details

Message ID 20170321013353.14128-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer March 21, 2017, 1:33 a.m.
Fixes memleak (See opus-testvector12 fate test).

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libswresample/resample.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

wm4 March 21, 2017, 4:47 a.m.
On Mon, 20 Mar 2017 22:33:53 -0300
James Almer <jamrial@gmail.com> wrote:

> Fixes memleak (See opus-testvector12 fate test).
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libswresample/resample.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/libswresample/resample.c b/libswresample/resample.c
> index 144b232462..ad6410142b 100644
> --- a/libswresample/resample.c
> +++ b/libswresample/resample.c
> @@ -276,6 +276,13 @@ fail:
>      return ret;
>  }
>  
> +static void resample_free(ResampleContext **c){
> +    if(!*c)
> +        return;
> +    av_freep(&(*c)->filter_bank);

I know you only moved that code, but that sure is an awkward way to
write av_free(c->filter_bank) (plus avoids a dead store).

> +    av_freep(c);
> +}
> +
>  static ResampleContext *resample_init(ResampleContext *c, int out_rate, int in_rate, int filter_size, int phase_shift, int linear,
>                                      double cutoff0, enum AVSampleFormat format, enum SwrFilterType filter_type, double kaiser_beta,
>                                      double precision, int cheby, int exact_rational)
> @@ -302,6 +309,7 @@ static ResampleContext *resample_init(ResampleContext *c, int out_rate, int in_r
>      if (!c || c->phase_count != phase_count || c->linear!=linear || c->factor != factor
>             || c->filter_length != filter_length || c->format != format
>             || c->filter_type != filter_type || c->kaiser_beta != kaiser_beta) {
> +        resample_free(&c);
>          c = av_mallocz(sizeof(*c));
>          if (!c)
>              return NULL;
> @@ -371,13 +379,6 @@ error:
>      return NULL;
>  }
>  
> -static void resample_free(ResampleContext **c){
> -    if(!*c)
> -        return;
> -    av_freep(&(*c)->filter_bank);
> -    av_freep(c);
> -}
> -
>  static int rebuild_filter_bank_with_compensation(ResampleContext *c)
>  {
>      uint8_t *new_filter_bank;

Seems OK.
Michael Niedermayer March 21, 2017, 2:39 p.m.
On Mon, Mar 20, 2017 at 10:33:53PM -0300, James Almer wrote:
> Fixes memleak (See opus-testvector12 fate test).
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libswresample/resample.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)

the move could be in a separete commit, if you want to split it

LGTM

thx

[...]
James Almer March 21, 2017, 3:07 p.m.
On 3/21/2017 11:39 AM, Michael Niedermayer wrote:
> On Mon, Mar 20, 2017 at 10:33:53PM -0300, James Almer wrote:
>> Fixes memleak (See opus-testvector12 fate test).
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libswresample/resample.c | 15 ++++++++-------
>>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> the move could be in a separete commit, if you want to split it
> 
> LGTM
> 
> thx

Split and pushed. Thanks.

Patch hide | download patch | download mbox

diff --git a/libswresample/resample.c b/libswresample/resample.c
index 144b232462..ad6410142b 100644
--- a/libswresample/resample.c
+++ b/libswresample/resample.c
@@ -276,6 +276,13 @@  fail:
     return ret;
 }
 
+static void resample_free(ResampleContext **c){
+    if(!*c)
+        return;
+    av_freep(&(*c)->filter_bank);
+    av_freep(c);
+}
+
 static ResampleContext *resample_init(ResampleContext *c, int out_rate, int in_rate, int filter_size, int phase_shift, int linear,
                                     double cutoff0, enum AVSampleFormat format, enum SwrFilterType filter_type, double kaiser_beta,
                                     double precision, int cheby, int exact_rational)
@@ -302,6 +309,7 @@  static ResampleContext *resample_init(ResampleContext *c, int out_rate, int in_r
     if (!c || c->phase_count != phase_count || c->linear!=linear || c->factor != factor
            || c->filter_length != filter_length || c->format != format
            || c->filter_type != filter_type || c->kaiser_beta != kaiser_beta) {
+        resample_free(&c);
         c = av_mallocz(sizeof(*c));
         if (!c)
             return NULL;
@@ -371,13 +379,6 @@  error:
     return NULL;
 }
 
-static void resample_free(ResampleContext **c){
-    if(!*c)
-        return;
-    av_freep(&(*c)->filter_bank);
-    av_freep(c);
-}
-
 static int rebuild_filter_bank_with_compensation(ResampleContext *c)
 {
     uint8_t *new_filter_bank;