diff mbox series

[FFmpeg-devel,15/25] avfilter/af_headphone: Avoid intermediate buffers II

Message ID 20200908211856.16290-15-andreas.rheinhardt@gmail.com
State Accepted
Commit d883bca0f0f15bd4b86d57fb64c172b3c0c8850f
Headers show
Series [FFmpeg-devel,01/25] avfilter/af_headphone: Don't use uninitialized buffer in log message | expand

Checks

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

Commit Message

Andreas Rheinhardt Sept. 8, 2020, 9:18 p.m. UTC 
When the headphone filter is configured to perform its processing in the
frequency domain, it allocates (among other things) two pairs of
buffers, all of the same size. One pair is used to store data in it
during the initialization of the filter; the other pair is only
allocated lateron. It is zero-initialized and yet its data is
immediately overwritten by the content of the other pair of buffers
mentioned above; the latter pair is then freed.

This commit eliminates the pair of intermediate buffers.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavfilter/af_headphone.c | 33 +++++++--------------------------
 1 file changed, 7 insertions(+), 26 deletions(-)

Comments

Paul B Mahol Sept. 9, 2020, 1:22 a.m. UTC | #1 
On Tue, Sep 08, 2020 at 11:18:46PM +0200, Andreas Rheinhardt wrote:
> When the headphone filter is configured to perform its processing in the
> frequency domain, it allocates (among other things) two pairs of
> buffers, all of the same size. One pair is used to store data in it
> during the initialization of the filter; the other pair is only
> allocated lateron. It is zero-initialized and yet its data is
> immediately overwritten by the content of the other pair of buffers
> mentioned above; the latter pair is then freed.
> 
> This commit eliminates the pair of intermediate buffers.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavfilter/af_headphone.c | 33 +++++++--------------------------
>  1 file changed, 7 insertions(+), 26 deletions(-)
> 

should be fine if does not break usage.
diff mbox series

Patch

diff --git a/libavfilter/af_headphone.c b/libavfilter/af_headphone.c
index 359174d4b4..fb6af7a966 100644
--- a/libavfilter/af_headphone.c
+++ b/libavfilter/af_headphone.c
@@ -371,8 +371,6 @@  static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
     int nb_irs = s->nb_irs;
     int nb_input_channels = ctx->inputs[0]->channels;
     float gain_lin = expf((s->gain - 3 * nb_input_channels) / 20 * M_LN10);
-    FFTComplex *data_hrtf_l = NULL;
-    FFTComplex *data_hrtf_r = NULL;
     FFTComplex *fft_in_l = NULL;
     FFTComplex *fft_in_r = NULL;
     int offset = 0, ret = 0;
@@ -439,9 +437,9 @@  static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
             goto fail;
         }
     } else {
-        data_hrtf_l = av_calloc(n_fft, sizeof(*data_hrtf_l) * nb_irs);
-        data_hrtf_r = av_calloc(n_fft, sizeof(*data_hrtf_r) * nb_irs);
-        if (!data_hrtf_r || !data_hrtf_l) {
+        s->data_hrtf[0] = av_calloc(n_fft, sizeof(*s->data_hrtf[0]) * nb_irs);
+        s->data_hrtf[1] = av_calloc(n_fft, sizeof(*s->data_hrtf[1]) * nb_irs);
+        if (!s->data_hrtf[0] || !s->data_hrtf[1]) {
             ret = AVERROR(ENOMEM);
             goto fail;
         }
@@ -488,10 +486,10 @@  static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
 
                 av_fft_permute(s->fft[0], fft_in_l);
                 av_fft_calc(s->fft[0], fft_in_l);
-                memcpy(data_hrtf_l + offset, fft_in_l, n_fft * sizeof(*fft_in_l));
+                memcpy(s->data_hrtf[0] + offset, fft_in_l, n_fft * sizeof(*fft_in_l));
                 av_fft_permute(s->fft[0], fft_in_r);
                 av_fft_calc(s->fft[0], fft_in_r);
-                memcpy(data_hrtf_r + offset, fft_in_r, n_fft * sizeof(*fft_in_r));
+                memcpy(s->data_hrtf[1] + offset, fft_in_r, n_fft * sizeof(*fft_in_r));
             }
         } else {
             int I, N = ctx->inputs[1]->channels;
@@ -529,10 +527,10 @@  static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
 
                     av_fft_permute(s->fft[0], fft_in_l);
                     av_fft_calc(s->fft[0], fft_in_l);
-                    memcpy(data_hrtf_l + offset, fft_in_l, n_fft * sizeof(*fft_in_l));
+                    memcpy(s->data_hrtf[0] + offset, fft_in_l, n_fft * sizeof(*fft_in_l));
                     av_fft_permute(s->fft[0], fft_in_r);
                     av_fft_calc(s->fft[0], fft_in_r);
-                    memcpy(data_hrtf_r + offset, fft_in_r, n_fft * sizeof(*fft_in_r));
+                    memcpy(s->data_hrtf[1] + offset, fft_in_r, n_fft * sizeof(*fft_in_r));
                 }
             }
         }
@@ -540,20 +538,6 @@  static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
         av_frame_free(&s->in[i + 1].frame);
     }
 
-    if (s->type == FREQUENCY_DOMAIN) {
-        s->data_hrtf[0] = av_calloc(n_fft * s->nb_irs, sizeof(FFTComplex));
-        s->data_hrtf[1] = av_calloc(n_fft * s->nb_irs, sizeof(FFTComplex));
-        if (!s->data_hrtf[0] || !s->data_hrtf[1]) {
-            ret = AVERROR(ENOMEM);
-            goto fail;
-        }
-
-        memcpy(s->data_hrtf[0], data_hrtf_l,
-            sizeof(FFTComplex) * nb_irs * n_fft);
-        memcpy(s->data_hrtf[1], data_hrtf_r,
-            sizeof(FFTComplex) * nb_irs * n_fft);
-    }
-
     s->have_hrirs = 1;
 
 fail:
@@ -561,9 +545,6 @@  fail:
     for (i = 0; i < s->nb_inputs - 1; i++)
         av_frame_free(&s->in[i + 1].frame);
 
-    av_freep(&data_hrtf_l);
-    av_freep(&data_hrtf_r);
-
     av_freep(&fft_in_l);
     av_freep(&fft_in_r);