diff mbox series

[FFmpeg-devel,18/25] avfilter/af_headphone: Avoid intermediate buffer III

Message ID 20200908211856.16290-18-andreas.rheinhardt@gmail.com
State Accepted
Commit abe0a5dd0a87aa8f509a366910ed6e3ccc2bd281
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 
The headphone filter allocates a pair of buffers to be used as
intermediate buffers lateron: Before every use they are zeroed, then
some elements of the buffer are set and lateron the complete buffers are
copied into another, bigger buffer. These intermediate buffers are
unnecessary as the data can be directly written into the bigger buffer.
Furthermore, the whole buffer has been zeroed initially and because no
piece of this buffer is set twice (due to the fact that duplicate
channel map entries are skipped), it is unnecessary to rezero the part
of the big buffer that is about to be written to.

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

Comments

Paul B Mahol Sept. 9, 2020, 1:27 a.m. UTC | #1 
On Tue, Sep 08, 2020 at 11:18:49PM +0200, Andreas Rheinhardt wrote:
> The headphone filter allocates a pair of buffers to be used as
> intermediate buffers lateron: Before every use they are zeroed, then
> some elements of the buffer are set and lateron the complete buffers are
> copied into another, bigger buffer. These intermediate buffers are
> unnecessary as the data can be directly written into the bigger buffer.
> Furthermore, the whole buffer has been zeroed initially and because no
> piece of this buffer is set twice (due to the fact that duplicate
> channel map entries are skipped), it is unnecessary to rezero the part
> of the big buffer that is about to be written to.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavfilter/af_headphone.c | 28 +++++-----------------------
>  1 file changed, 5 insertions(+), 23 deletions(-)
> 

should be fine if properly tested.
diff mbox series

Patch

diff --git a/libavfilter/af_headphone.c b/libavfilter/af_headphone.c
index b5c2bd6121..d57ecb1f50 100644
--- a/libavfilter/af_headphone.c
+++ b/libavfilter/af_headphone.c
@@ -367,9 +367,7 @@  static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
     const int ir_len = s->ir_len;
     int nb_input_channels = ctx->inputs[0]->channels;
     float gain_lin = expf((s->gain - 3 * nb_input_channels) / 20 * M_LN10);
-    FFTComplex *fft_in_l = NULL;
-    FFTComplex *fft_in_r = NULL;
-    int offset = 0, ret = 0;
+    int ret = 0;
     int n_fft;
     int i, j, k;
 
@@ -381,13 +379,6 @@  static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
     s->n_fft = n_fft = 1 << (32 - ff_clz(ir_len + s->size));
 
     if (s->type == FREQUENCY_DOMAIN) {
-        fft_in_l = av_calloc(n_fft, sizeof(*fft_in_l));
-        fft_in_r = av_calloc(n_fft, sizeof(*fft_in_r));
-        if (!fft_in_l || !fft_in_r) {
-            ret = AVERROR(ENOMEM);
-            goto fail;
-        }
-
         s->fft[0] = av_fft_init(av_log2(s->n_fft), 0);
         s->fft[1] = av_fft_init(av_log2(s->n_fft), 0);
         s->ifft[0] = av_fft_init(av_log2(s->n_fft), 1);
@@ -464,10 +455,9 @@  static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
                     data_ir_r[j] = ptr[len * 2 - j * 2 - 1] * gain_lin;
                 }
             } else {
-                memset(fft_in_l, 0, n_fft * sizeof(*fft_in_l));
-                memset(fft_in_r, 0, n_fft * sizeof(*fft_in_r));
+                FFTComplex *fft_in_l = s->data_hrtf[0] + idx * n_fft;
+                FFTComplex *fft_in_r = s->data_hrtf[1] + idx * n_fft;
 
-                offset = idx * n_fft;
                 for (j = 0; j < len; j++) {
                     fft_in_l[j].re = ptr[j * 2    ] * gain_lin;
                     fft_in_r[j].re = ptr[j * 2 + 1] * gain_lin;
@@ -475,10 +465,8 @@  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(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(s->data_hrtf[1] + offset, fft_in_r, n_fft * sizeof(*fft_in_r));
             }
         } else {
             int I, N = ctx->inputs[1]->channels;
@@ -499,10 +487,9 @@  static int convert_coeffs(AVFilterContext *ctx, AVFilterLink *inlink)
                         data_ir_r[j] = ptr[len * N - j * N - N + I + 1] * gain_lin;
                     }
                 } else {
-                    memset(fft_in_l, 0, n_fft * sizeof(*fft_in_l));
-                    memset(fft_in_r, 0, n_fft * sizeof(*fft_in_r));
+                    FFTComplex *fft_in_l = s->data_hrtf[0] + idx * n_fft;
+                    FFTComplex *fft_in_r = s->data_hrtf[1] + idx * n_fft;
 
-                    offset = idx * n_fft;
                     for (j = 0; j < len; j++) {
                         fft_in_l[j].re = ptr[j * N + I    ] * gain_lin;
                         fft_in_r[j].re = ptr[j * N + I + 1] * gain_lin;
@@ -510,10 +497,8 @@  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(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(s->data_hrtf[1] + offset, fft_in_r, n_fft * sizeof(*fft_in_r));
                 }
             }
         }
@@ -528,9 +513,6 @@  fail:
     for (i = 0; i < s->nb_inputs - 1; i++)
         av_frame_free(&s->in[i + 1].frame);
 
-    av_freep(&fft_in_l);
-    av_freep(&fft_in_r);
-
     return ret;
 }