Audio: stft_process: Add conversion to polar format and back #10496
Conversation
singalsu
force-pushed
the
add_complex_polar_convert
branch
from
bce13a4 to
2f173e0
Compare
singalsu
requested review from
dbaluta,
kv2019i,
lbetlej,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull request overview
This PR adds support for converting complex numbers between rectangular (real, imaginary) and polar (magnitude, angle) formats in the SOF audio processing framework. The implementation includes new conversion functions for 32-bit complex numbers, comprehensive unit tests, and integration with the STFT processing component.
Changes:
- Added complex number conversion functions (
sofm_icomplex32_to_polarandsofm_ipolar32_to_complex) - Implemented 32-bit square root function (
sofm_sqrt_int32) to support magnitude calculations - Reorganized complex number type definitions into dedicated header files (
icomplex16.h,icomplex32.h)
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/math/complex.c |
New implementation of polar/complex conversion functions |
src/math/sqrt_int32.c |
New 32-bit square root function for magnitude calculations |
src/include/sof/math/icomplex32.h |
Extracted and expanded 32-bit complex number types and operations |
src/include/sof/math/icomplex16.h |
Extracted 16-bit complex number types and helper functions |
src/include/sof/math/sqrt.h |
Added sofm_sqrt_int32 declaration and updated function naming |
src/audio/stft_process/stft_process_common.c |
Added polar format conversion to STFT processing |
test/ztest/unit/math/basic/complex/test_complex_polar.c |
Comprehensive unit tests for conversion functions |
| Various FFT files | Updated includes to use new header organization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/ztest/unit/math/basic/complex/test_complex_polar_reference.m
Outdated
Show resolved
Hide resolved
test/ztest/unit/math/basic/complex/test_complex_polar_reference.m
Outdated
Show resolved
Hide resolved
test/ztest/unit/math/basic/complex/test_complex_polar_reference.m
Outdated
Show resolved
Hide resolved
singalsu
force-pushed
the
add_complex_polar_convert
branch
from
2f173e0 to
8d72029
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * @brief Test complex to polar conversion function | ||
| * | ||
| * This test validates the icomplex32_to_polar(() function against |
There was a problem hiding this comment.
Corrected typo - extra opening parenthesis in function name reference.
| * This test validates the icomplex32_to_polar(() function against | |
| * This test validates the icomplex32_to_polar() function against |
| * @brief Test complex to polar conversion function | ||
| * | ||
| * This test validates the icomplex32_to_polar(() function against | ||
| * Octave-generated reference values. The test includes 1000 |
There was a problem hiding this comment.
The comment ends mid-sentence with "The test includes 1000" but doesn't specify what 1000 refers to. Complete the sentence to indicate it tests 1000 data points or test cases.
| * Octave-generated reference values. The test includes 1000 | |
| * Octave-generated reference values. The test includes 1000 data points. |
src/include/sof/math/sqrt.h
Outdated
| uint16_t sqrt_int16(uint16_t u); | ||
| #endif | ||
| /** | ||
| * sqrt_int16() - Calculate 16-bit fractional square root function. |
There was a problem hiding this comment.
The documentation header still refers to the old function name "sqrt_int16()" but the actual function has been renamed to "sofm_sqrt_int16()". Update the documentation to match the new function name.
| * sqrt_int16() - Calculate 16-bit fractional square root function. | |
| * sofm_sqrt_int16() - Calculate 16-bit fractional square root function. |
The rename is done to avoid possible conflict with other math libraries. The change is done to prepare add of 32 bit square root function. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch adds a higher precision 32-bit fractional integer square root function to SOF math library. The algorithm uses a lookup table for initial value and two iterations with Newton-Raphson method to improve the accuracy. Both input and output format is Q2.30. The format was chosen to match complex to polar conversions numbers range for Q1.31 complex values. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch helps with more generic use of complex numbers not directly related to the FFTs domain. It prepares to add polar complex numbers format that is commonly used in frequency domain signal processing. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch adds functions sofm_icomplex32_to_polar() and sofm_ipolar32_to_complex(). In polar format the Q1.31 (real, imag) numbers pair is converted to (magnitude, angle). The magnitude is Q2.30 format and angle in -pi to +pi radians in Q3.29 format. The conversion to polar and back loses some quality so there currently is no support for icomplex16. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
The new ztest cases test_icomplex32_to_polar and ipolar32_to_complex test the conversion functions with a pre-calculated -1,+1 box saturated (re, im) spiral shape with 1000 points. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
This patch adds to stft_process component conversion to polar (magnitude, angle) format for first FFT half from DC to Nyquist frequency. The polar format is converted back to (real, imaginary) complex and upper FFT half symmetry is applied. The magnitude domain is commonly used for signal processing in frequency domain. This change when enabled in Kconfig change increases load in MTL platform from 72 MCPS to 202 MCPS with 1024 size FFT and hop of 256. Currently the build option STFT_PROCESS_MAGNITUDE_PHASE is not set. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
singalsu
force-pushed
the
add_complex_polar_convert
branch
from
8d72029 to
fe02408
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| #include <stdint.h> | ||
|
|
||
| uint16_t sqrt_int16(uint16_t u); |
There was a problem hiding this comment.
Wouldn't be better to let a conflict to manifest itself (multiple definition error) so that we are aware of the redundant functionality?
There was a problem hiding this comment.
no, we are already aware of some feature duplication in C/maths libraries.
The C/maths lib versions are non xtensa optimized and always max precision for type. i.e. we use a different name as we only need to perform maths effort to the quality desired for the audio format (as any extra effort would be wasted MCPS).
|
|
||
| #include <stdint.h> | ||
|
|
||
| uint16_t sqrt_int16(uint16_t u); |
There was a problem hiding this comment.
no, we are already aware of some feature duplication in C/maths libraries.
The C/maths lib versions are non xtensa optimized and always max precision for type. i.e. we use a different name as we only need to perform maths effort to the quality desired for the audio format (as any extra effort would be wasted MCPS).
4 participants
No description provided.