fix: post-rename findings (reset(), midFilter reset, tail length, chunking coverage)#63
Merged
Merged
Conversation
…stop/loop
CryptaAudioProcessor never overrode juce::AudioProcessor::reset(), so a
host transport-stop/loop/rewind (which calls reset(), not prepareToPlay())
never flushed the LR4 crossover's filter memory, gate/compressor
envelopes, the high-band voicing's oversampling/mid/tone filter state,
the low-band latency-compensation delay line, EQ biquad history, or the
IR convolution engine - all of which already exposed their own
real-time-safe reset() but were never wired into a processor-level
override.
Added tests/ResetTests.cpp ("reset(): clears DSP stage state so silence
right after a loud signal is actually silent"), confirmed red before the
fix (rms 0.46 with the base-class no-op reset() vs a margin of 1e-4),
green after wiring every stage's reset() into the new override.
Fixes #56.
Voicing::setVoicing() recomputed midFilter's coefficients on every voicing change but, unlike preHighPass's explicit Razor-switch handling, never reset its state - a stale biquad delay-line combined with a freshly-swapped coefficient set (e.g. Wool's -6dB scoop jumping to Razor's +5dB hump) rings a spurious transient that a plain "silence right after the switch" magnitude check can't isolate: the oversampling stage's own FIR flush tail and the boost/cut gain difference between voicings both legitimately leave residual regardless of the fix. Added tests/VoicingTests.cpp's on-resonance/off-resonance ratio test, which isolates midFilter's own stale-state contribution from those confounds by comparing the post-switch residual when the excite signal sits on Wool's own mid-filter frequency (500Hz, maximises midFilter's stored state) against off-resonance (20Hz, minimises it, leaving only the shared FIR/gain confound). Confirmed red before the fix (ratio 2.459 against a 2.35 threshold), green after wiring in midFilter.reset(). Fixes #57.
CryptaAudioProcessor::getTailLengthSeconds() unconditionally returned 0.0, correct only while IRLoader has its single-sample identity IR installed. loadImpulseResponse() is a live, callable seam into a real juce::dsp::Convolution with no length cap, so once a real IR is loaded the plugin's actual output tail grows to that IR's length while getTailLengthSeconds() kept reporting 0.0 - a host trusting that value for bounce/freeze/render-tail decisions could truncate the convolution tail. IRLoader now tracks the loaded IR's own duration (numSamples / irSampleRate at load time - invariant under Convolution's internal resampling to the session rate), exposed via IRLoader::getTailLengthSeconds() and wired into the processor's override. Added tests/TailLengthTests.cpp, confirmed red before the fix (both the "reflects loaded IR length" and "independent of session sample rate" cases failed with the hardcoded 0.0), green after wiring the tracked duration through. Fixes #58.
processBlock()'s chunking logic (splitting a host block into sub-blocks of at most prepareToPlay()'s samplesPerBlock, to avoid overrunning lowBandBuffer/highBandBuffer and Voicing's fixed-size Oversampling/ DryWetMixer buffers) was never exercised by any test: every existing test only ever calls processBlock() with a buffer sized at or below what prepareToPlay() promised, so the multi-iteration chunking branch never ran under CI. Added tests/ChunkingTests.cpp, comparing a single oversized (and non-multiple-of-block-size) processBlock() call against the same signal fed through in properly host-sized sub-blocks. This test passed immediately (no red state): the chunking loop in PluginProcessor.cpp was already correct, just untested - this closes that coverage gap rather than fixing a functional bug, and is a Release-safe path (no jassert-only branching in the code under test). Fixes #59.
CLAUDE.md and docs/manual.md still carried the pre-suite-bible "symphonic-metal" framing; reworded to "heavy music" (the suite bible's production/mix-context term) to match the rest of the documentation.
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
CryptaAudioProcessornever overrodereset(), so a host transport-stop/loop/rewind never flushed per-stage DSP state (crossover filter memory, gate/compressor envelopes, voicing oversampling/mid/tone filter state, the low-band latency-compensation delay line, EQ history, IR convolution engine).Voicing::setVoicing()recomputedmidFilter's coefficients on every voicing change but never reset its state, unlikepreHighPass's existing Razor-switch handling.getTailLengthSeconds()unconditionally returned0.0, ignoringIRLoader's actually-loaded impulse response length.processBlock()'s chunking path when a host block exceedsprepareToPlay()'ssamplesPerBlock(no functional bug found - this closes a coverage gap).CLAUDE.mdanddocs/manual.mdto "heavy-music", matching the suite bible.Test plan
tests/ResetTests.cpp(new) - confirmed red before the CryptaAudioProcessor does not override reset(): DSP stage state persists across host transport-stop/loop #56 fix (rms 0.46 vs a 1e-4 margin), green after.tests/VoicingTests.cpp(new case) - confirmed red before the Voicing::setVoicing() should reset midFilter state on voicing change to match preHighPass's Razor-switch handling #57 fix (on/off-resonance ratio 2.459 vs a 2.35 threshold), green after.tests/TailLengthTests.cpp(new) - confirmed red before the getTailLengthSeconds() ignores IRLoader's loaded impulse response length #58 fix (both non-default-IR cases failed against the hardcoded0.0), green after.tests/ChunkingTests.cpp(new) - passed immediately (no functional bug in the existing chunking loop, just missing coverage).CPM_SOURCE_CACHE=~/.cache/CPM):Tests+Crypta_Standalonetargets build clean.ctest --test-dir build --output-on-failure: 53/53 passed.🤖 Generated with Claude Code