From: Maxime Even Date: Tue, 9 Jul 2024 13:20:25 +0200 Subject: spectrogram: fix FFT result scaling factor MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit This function is just a scaling operation that passes a value between 0 and FFT_SCALING_VALUE to a value between 0 and 2^16-1. It is therefore a power that must be applied here and not a xor. Moreover, the initial formula was quite wrong since it was assuming that the max_input value, which is a signed int 16, was 2^15 = 32768 where it is in fact 2^15-1=32767. Moreover, the initial formula wasn't taking into account that, since the output of the fft_perform is the sum of two variables which have for max value (FFT_BUFFER_SIZE / 2 * INT16_MAX)^2, then we need to multiply the final max value by 2. Corrects the following compiler warnings: visualization/glspectrum.c:528:43: warning: result of ‘2^16’ is 18; did you mean ‘1 << 16’ (65536)? [-Wxor-used-as-pow] visualization/visual/effects.c:216:40: warning: result of ‘2^16’ is 18; did you mean ‘1 << 16’ (65536)? [-Wxor-used-as-pow] Fixes #28506 Co-authored-by: Disha Baghel (cherry picked from commit 77a08a60f72bda081d144119e4d0a57a9c3c72f6) --- modules/visualization/glspectrum.c | 8 +++++--- modules/visualization/visual/effects.c | 5 ++++- modules/visualization/visual/fft.h | 4 ++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/modules/visualization/glspectrum.c b/modules/visualization/glspectrum.c index fdd0127..bf0ffd6 100644 --- a/modules/visualization/glspectrum.c +++ b/modules/visualization/glspectrum.c @@ -445,9 +445,11 @@ static void *Thread( void *p_data ) window_scale_in_place (p_buffer1, &wind_ctx); fft_perform (p_buffer1, p_output, p_state); - for (i = 0; i< FFT_BUFFER_SIZE; ++i) - p_dest[i] = p_output[i] * (2 ^ 16) - / ((FFT_BUFFER_SIZE / 2 * 32768) ^ 2); + for( i = 0; i< FFT_BUFFER_SIZE ; i++ ) + { + /* Scale the output between 0 and UINT16MAX */ + p_dest[i] = p_output[i] * UINT16_MAX / FFT_SCALING_VALUE; + } for (i = 0 ; i < NB_BANDS; i++) { diff --git a/modules/visualization/visual/effects.c b/modules/visualization/visual/effects.c index 8cb8c13..9b7b07a 100644 --- a/modules/visualization/visual/effects.c +++ b/modules/visualization/visual/effects.c @@ -214,7 +214,10 @@ static int spectrum_Run(visual_effect_t * p_effect, vlc_object_t *p_aout, window_scale_in_place( p_buffer1, &wind_ctx ); fft_perform( p_buffer1, p_output, p_state); for( i = 0; i< FFT_BUFFER_SIZE ; i++ ) - p_dest[i] = p_output[i] * ( 2 ^ 16 ) / ( ( FFT_BUFFER_SIZE / 2 * 32768 ) ^ 2 ); + { + /* Scale the output between 0 and UINT16MAX */ + p_dest[i] = p_output[i] * UINT16_MAX / FFT_SCALING_VALUE; + } /* Compute the horizontal position of the first band */ i_band_width = floor( p_effect->i_width / i_nb_bands); diff --git a/modules/visualization/visual/fft.h b/modules/visualization/visual/fft.h index f4c8a6b..fd3b24f 100644 --- a/modules/visualization/visual/fft.h +++ b/modules/visualization/visual/fft.h @@ -29,6 +29,10 @@ #define FFT_BUFFER_SIZE (1 << FFT_BUFFER_SIZE_LOG) +#define FFT_MAX_VALUE_OUTPUT ((uint64_t)(FFT_BUFFER_SIZE/2 * INT16_MAX)) + +#define FFT_SCALING_VALUE (FFT_MAX_VALUE_OUTPUT * FFT_MAX_VALUE_OUTPUT * 2) + /* sound sample - should be an signed 16 bit value */ typedef short int sound_sample;