fix: resolve review findings (audio-thread safety, robustness)#19
Merged
Conversation
juce::dsp::Limiter::process() (and each of its two internal Compressors) short-circuits to a plain copyFrom() as its first statement when context.isBypassed, skipping the BallisticsFilter envelope update entirely. Toggling that flag to represent "disabled" therefore froze the limiter's gain-reduction state instead of keeping it continuous, so re-enabling could resume from a stale pre-disable envelope rather than one consistent with the current input - the opposite of the documented no-pop-on-re-enable guarantee. BandCompressor now always runs the limiter at full strength into a preallocated scratch buffer and only splices the limited result back into the output when enabled, so its ballistics genuinely track the live signal at all times. Fixes #12 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… tail TriptychEngine::process() clamped numSamples to the per-band buffer capacity established in prepare() and only ever touched that first capacity-sized region of the host's buffer. Samples beyond it were never read or written at all - silently passing raw dry input through untouched, including bypassing the master Output trim, whenever a host called process() with more samples than it declared via prepareToPlay()'s maximumExpectedSamplesPerBlock (offline bounce/render passes commonly do this). process() now loops over the requested block in <= capacity-sized chunks, running each through the full signal chain via a new processChunk() (crossovers, band compressors, Mute/Solo gate, output trim) - previously the whole body of process() itself. Channel-count mismatches are still defensively clamped rather than chunked, since a host violating its own negotiated bus layout is not a realistic scenario the way an oversized block is. Fixes #14 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TriptychEngine::process() resolved Mute/Solo to a bare block-rate 0.0f/1.0f multiplier applied uniformly across the whole block, so toggling mid-playback produced a hard step discontinuity at whatever sample happened to land on the block boundary - audible as a click. This was independent of (and in addition to) each band's own compressor/limiter envelopes already staying continuous underneath: the gain step itself was never smoothed. The resolved gain is now ramped through a juce::SmoothedValue per band (lowGainSmoothed/midGainSmoothed/highGainSmoothed, linear, the same smoothingTimeSeconds used for every other real-time-varying scalar in this engine), filled into a per-sample ramp buffer once per chunk and consumed by the summing loop instead of a bare per-block constant. Fixes #13 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Three review findings from the 2026-07-15 suite-wide review sweep, each with its own regression test and its own commit:
limiterEnabled == false, because togglingjuce::dsp::Limiter's owncontext.isBypassedshort-circuits to a copy-through before the envelope update in both JUCE 8.0.14Limiter::process()and its two internalCompressor::process()calls — contradicting the documented "no pop on re-enable" guarantee. The limiter now always runs at full strength into a preallocated scratch buffer, splicing the limited result back into the output only when enabled, so re-enabling resumes from a state consistent with the live input.juce::SmoothedValueper band, matching every other real-time-varying scalar in this engine.prepareToPlay()left the excess samples as unprocessed dry passthrough (bypassing every stage, including the master Output trim) instead of being fully processed.TriptychEngine::process()now chunks any oversized block into<=-capacity pieces, each run through the full signal chain via a newprocessChunk().Each fix has a dedicated Catch2 regression test (verified to fail against the pre-fix code and pass against the fix):
tests/BandCompressorTests.cpp(#12),tests/MuteSoloAndLimiterTests.cpp(#13),tests/EngineTests.cpp(#14). Suite is 42/42 green locally (39 existing + 3 new).docs/architecture.md,CLAUDE.md, andCHANGELOG.md(## [Unreleased]→### Fixed) updated accordingly.JUCE 8.0.14
Limiter/Compressor/Gaininternals were read directly from~/.cache/CPM/juce/c074/modules/juce_dsp/widgets/to confirm thecontext.isBypassedshort-circuit behaviour before implementing the #12 fix.Test plan
cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug && cmake --build build --target Tests Triptych_Standalone --parallel 3— builds cleanctest --test-dir build --output-on-failure— 42/42 passed🤖 Generated with Claude Code