adafruit / Adafruit_ZeroFFT

simple FFT for cortex m0
BSD 3-Clause "New" or "Revised" License
42 stars 18 forks source link

Implementation of ZERO_FFT_MAX seems strange #11

Open t1mc0rd35 opened 1 year ago

t1mc0rd35 commented 1 year ago

While trying out this library, I found something strange with the behavior of the ZERO_FFT_MAX value.

While I am using this library with a Raspberry Pi Pico instead of an Arduino and have adapted it to work with the Pico-SDK, I only modified the library structure to work with cmake, exchanged arduino.h for pico.h and included stdlib.h because abs() was undefined. Everything else about the code has not been modified.

What I found is that the documentation for ZeroFFT() says that the parameter length has to be "less than or equal to ZERO_FFT_MAX":

/**************************************************************************/
/*!
    @brief  run an FFT on an int16_t array. Note that this is run in place.
    @param source the data to FFT
    @param length the length of the data. This must be a power of 2 and less
   than or equal to ZERO_FFT_MAX
    @return 0 on success, -1 on failure
    @note The FFT is run in place on the data. A hanning window is applied to
   the input data. The complex portion is discarded, and the real values are
   returned.
*/
/**************************************************************************/
extern int ZeroFFT(q15_t *source, uint16_t length);

However, when you actually set the length parameter to be equal to ZERO_FFT_MAX, the function will just return -1 without doing anything, because the code that should be applied for a given length is excluded by the preprocessor unless ZERO_FFT_MAX is twice as high.

For example, leaving ZERO_FFT_MAX at it's default value of 4096 and giving 4096 as value for length won't do anything because the section that handles length == 4096 is only included when ZERO_FFT_MAX equals 8192:

if ZERO_FFT_MAX == 8192

case 4096u: / Initializations of structure parameters for 4096 point FFT /

/*  Initialise the twiddle coef modifier value */
twidCoefModifier = 1u;
/*  Initialise the bit reversal table modifier */
bitRevFactor = 1u;
/*  Initialise the bit reversal table pointer */
pBitRevTable = (uint16_t *)armBitRevTable;

applyWindow(source, window_hanning_4096, 4096);

break;

endif



So I wonder: Is the source code wrong, is the Doxygen wrong or am I misunderstanding something?
t1mc0rd35 commented 11 months ago

I would fix it and submit a PR but first I would like to know if this is an actual issue that needs to be fixed or not.