fix: resolve review findings (audio-thread safety, robustness)#19
Merged
Conversation
Add a global operator new/delete counting harness (TestAlloc:: AllocationGuard) plus a processBlock()-level regression test exercising every DSP stage at once, since neither pluginval (--strictness-level 10) nor auval (-strict) do allocation-instrumented profiling and none of the existing 28 Catch2 tests had any allocation-counting mechanism. This test currently fails (64 allocations across 32 blocks), catching the two known audio-thread heap allocations fixed in the following commits. Fixes #14
…pute DeEsser::process() called Coefficients<float>::makeBandPass() every block regardless of amount/bypass state, which juce_IIRFilter.cpp implements as `return *new Coefficients(...)` - a heap alloc/dealloc pair on the audio thread every block, violating this repo's RT-safety invariant. Switch to ArrayCoefficients<float>::makeBandPass() (stack std::array, no allocation) and write its values into the already-allocated detectorCoefficients object in place via the new shared RealtimeCoefficients.h helper - the same pattern already proven in sibling plugin lancet's src/dsp/RealtimeCoefficients.h. Fixes #13
SeraphEngine::process() called Coefficients<float>::makeHighShelf() every block regardless of whether Air actually changed, which juce_IIRFilter.cpp implements as `return *new Coefficients(...)` - a heap alloc/dealloc pair on the audio thread every block, violating this repo's RT-safety invariant. Switch to ArrayCoefficients<float>::makeHighShelf() (stack std::array, no allocation) and write its values into the already-allocated airShelf.state object in place via RealtimeCoefficients.h, same as the DeEsser fix in the previous commit. With this, the full-chain regression test added for #14 goes green (0 allocations across 32 blocks, down from 64 before either fix). Fixes #12
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Fixes the two HIGH-severity audio-thread heap allocations found in the 2026-07-15 whole-repo review, plus adds the regression test that should have caught them.
Coefficients<float>::makeHighShelf()everyprocess()block; switched to the allocation-freeArrayCoefficients<float>::makeHighShelf()written in place into the existing filter state (sharedsrc/dsp/RealtimeCoefficients.hhelper, matching the proven pattern already used in sibling pluginlancet).Coefficients<float>::makeBandPass()every block, unconditionally (even atDeEssamount 0%); switched to the same allocation-freeArrayCoefficients<float>::makeBandPass()in-place pattern.tests/AllocationGuard.{h,cpp}(a globaloperator new/deletecounting harness, scoped by an RAII guard) plustests/AllocationTests.cppwith a permanent full-chainprocessBlock()regression test (every DSP stage engaged at once) and two issue-isolated tests (DeEsser::process,SeraphEngine::processwith Air active), so a future audio-thread allocation in any stage fails CI instead of passing pluginval/auval silently.Verification
Both fixes were verified against pinned JUCE 8.0.14 sources (
juce_dsp/processors/juce_IIRFilter.cpp/.h,juce_IIRFilter_Impl.h):Coefficients<NumericType>::makeHighShelf/makeBandPassarereturn *new Coefficients(...), whileArrayCoefficients<NumericType>::makeHighShelf/makeBandPassreturn a stackstd::array<float,6>with zero allocation.RealtimeCoefficients.h::applyBiquadCoefficientswrites that array's values into the existingCoefficientsobject's raw storage, normalising bya0the same wayCoefficients::assignImpldoes internally (juce_IIRFilter_Impl.h).Locally, on this branch:
→ 31/31 tests pass (28 pre-existing + 3 new allocation tests). The full-chain allocation test was confirmed to fail before either DSP fix (64 allocations/32 blocks with both bugs present, 32 with only the Air bug remaining after the DeEsser fix) and pass at 0 after both.
Test plan
ctest --test-dir build --output-on-failuregreen (31/31) with both fixes applied🤖 Generated with Claude Code