diff mbox series

[FFmpeg-devel,25/25] avfilter/af_headphone: Don't check for clipping in separate loop

Message ID 20200908211856.16290-25-andreas.rheinhardt@gmail.com
State Accepted
Commit 3e950f53496d53a45c9e1ba03aa83dfaa90b2ac0
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 
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
More patches will be coming later.

 libavfilter/af_headphone.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Comments

Paul B Mahol Sept. 9, 2020, 1:05 a.m. UTC | #1 
On Tue, Sep 08, 2020 at 11:18:56PM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> More patches will be coming later.
> 
>  libavfilter/af_headphone.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/libavfilter/af_headphone.c b/libavfilter/af_headphone.c
> index 03ef88c4a4..edf8e773d7 100644
> --- a/libavfilter/af_headphone.c
> +++ b/libavfilter/af_headphone.c
> @@ -281,6 +281,8 @@ static int headphone_fast_convolute(AVFilterContext *ctx, void *arg, int jobnr,
>  
>      for (j = 0; j < in->nb_samples; j++) {
>          dst[2 * j] += fft_acc[j].re * fft_scale;
> +        if (fabsf(dst[2 * j]) > 1)
> +            n_clippings[0]++;
>      }
>  
>      for (j = 0; j < ir_len - 1; j++) {
> @@ -289,14 +291,6 @@ static int headphone_fast_convolute(AVFilterContext *ctx, void *arg, int jobnr,
>          *(ringbuffer + write_pos) += fft_acc[in->nb_samples + j].re * fft_scale;
>      }
>  
> -    for (i = 0; i < out->nb_samples; i++) {
> -        if (fabsf(dst[0]) > 1) {
> -            n_clippings[0]++;
> -        }
> -
> -        dst += 2;
> -    }
> -
>      *write = wr;

This patch is wrong and uneeded.
See, this filter use slice threading, and thus if multiple threads are used in multichannel audio
it will report incorrect results.
Paul B Mahol Sept. 9, 2020, 1:43 a.m. UTC | #2 
On Tue, Sep 08, 2020 at 11:18:56PM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> More patches will be coming later.
> 
>  libavfilter/af_headphone.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 

Actually on second look, it does not hurt at all.
diff mbox series

Patch

diff --git a/libavfilter/af_headphone.c b/libavfilter/af_headphone.c
index 03ef88c4a4..edf8e773d7 100644
--- a/libavfilter/af_headphone.c
+++ b/libavfilter/af_headphone.c
@@ -281,6 +281,8 @@  static int headphone_fast_convolute(AVFilterContext *ctx, void *arg, int jobnr,
 
     for (j = 0; j < in->nb_samples; j++) {
         dst[2 * j] += fft_acc[j].re * fft_scale;
+        if (fabsf(dst[2 * j]) > 1)
+            n_clippings[0]++;
     }
 
     for (j = 0; j < ir_len - 1; j++) {
@@ -289,14 +291,6 @@  static int headphone_fast_convolute(AVFilterContext *ctx, void *arg, int jobnr,
         *(ringbuffer + write_pos) += fft_acc[in->nb_samples + j].re * fft_scale;
     }
 
-    for (i = 0; i < out->nb_samples; i++) {
-        if (fabsf(dst[0]) > 1) {
-            n_clippings[0]++;
-        }
-
-        dst += 2;
-    }
-
     *write = wr;
 
     return 0;