You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR fixes critical bugs in the dsps_fird_f32_aes3 optimized assembly implementation that were causing digital noise/corruption in the FIR filter output.
Issues Fixed:
Incorrect register usage in offset_3 branch: The final accumulation was overwriting f4 instead of using f6, causing partial loss of filter results.
Incorrect instruction ordering in non-offset_0 branches: The order of madd.s and EE.LDF.128.IP instructions in the tight FIR loops was leading to out-of-bounds data being added to FIR results..
Potential out-of-bounds memory access: The optimized implementation could read 4 floats beyond the delay line buffer, which could crash if the buffer was placed at a memory boundary.
Root Cause:
The optimized AES3 implementation handles different memory alignments (offset_0, offset_1, offset_2, offset_3) but contained copy-paste errors and subtle timing issues in the non-aligned cases. The ANSI implementation worked correctly, but the optimized version would intermittently produce digital noise when the delay line pointer wasn't 16-byte aligned.
Corrected instruction ordering in .offset_1, .offset_2, and .offset_3 loops for proper coefficient-data alignment
Added documentation comments explaining the complex alignment handling logic
Cleaned up redundant register loads and improved code clarity
Documentation Update Needed:
The requirement for delay line buffer to be N + 4 floats (not just N) should be documented for dsps_fird_f32_init, as it's currently only mentioned for dsps_fir_f32_init.
Testing
Hardware tested: ESP32-S3 with PDM microphone processing at 48kHz with 64-tap FIR decimation filter
Verification method: Direct comparison between ANSI and AES3 implementations - both now produce audibly equivalent results
Test cases:
Different delay line alignments (offset_0, offset_2 branches confirmed working)
Real audio input
The fix has been tested in a real-world PDM audio processing application where the digital noise was clearly audible before the fix and completely eliminated after.
Checklist
Before submitting a Pull Request, please ensure the following:
🚨 This PR does not introduce breaking changes.
All CI checks (GH Actions) pass.
Documentation is updated as needed.
Tests are updated or added as necessary.
Code is well-commented, especially in complex areas.
Git history is clean — commits are squashed to the minimum necessary.
@dmitry1945 Does this apply to dsps_fir_f32_aes3 ? I'm using it to process a high-pass filter, today I updated esp-dsp 1.7.0 as well as made hardware changes and now I'm getting distorted sound and I'm not sure where the problem is.
That's okay. As file contributor it is fine if you simply put Schuwi. Thanks!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes critical bugs in the
dsps_fird_f32_aes3optimized assembly implementation that were causing digital noise/corruption in the FIR filter output.Issues Fixed:
Incorrect register usage in offset_3 branch: The final accumulation was overwriting
f4instead of usingf6, causing partial loss of filter results.Incorrect instruction ordering in non-offset_0 branches: The order of
madd.sandEE.LDF.128.IPinstructions in the tight FIR loops was leading to out-of-bounds data being added to FIR results..Potential out-of-bounds memory access: The optimized implementation could read 4 floats beyond the delay line buffer, which could crash if the buffer was placed at a memory boundary.
Root Cause:
The optimized AES3 implementation handles different memory alignments (offset_0, offset_1, offset_2, offset_3) but contained copy-paste errors and subtle timing issues in the non-aligned cases. The ANSI implementation worked correctly, but the optimized version would intermittently produce digital noise when the delay line pointer wasn't 16-byte aligned.
Changes Made:
.offset_3section (line 204:madd.s f4, f3, f14→madd.s f6, f3, f14).offset_1,.offset_2, and.offset_3loops for proper coefficient-data alignmentDocumentation Update Needed:
The requirement for delay line buffer to be
N + 4floats (not justN) should be documented fordsps_fird_f32_init, as it's currently only mentioned fordsps_fir_f32_init.Testing
The fix has been tested in a real-world PDM audio processing application where the digital noise was clearly audible before the fix and completely eliminated after.
Checklist
Before submitting a Pull Request, please ensure the following: