From 3c8dd62ecbebd96cc95d99b0a37ff6c49659268a Mon Sep 17 00:00:00 2001 From: OmikronApex Date: Mon, 13 Jul 2026 23:28:34 +0200 Subject: [PATCH] fix(audio): sanitize input/chain/output/backing gains at the engine setters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NaN/Inf from any JS caller (audio:setGain does no validation) previously reached the gain atomics raw; a NaN master gain multiplies the whole device output to NaN and poisons the peak meters (TLC deep-read §2). Clamp at the four setters — the single choke point covering the legacy facade, the source-indexed API, and the audio-effects executor. Bounds 0..32 match the executor's clampGain (Phase 0.b compat pin); stream/ renderer-bus keep their historical 0..8 via the same JUCE-free helper, now testable in the new tests/engine_units target (Phase 0.c harness). Co-Authored-By: Claude Fable 5 --- src/audio/AudioEngine.h | 10 +++-- src/audio/GainSanitize.h | 36 +++++++++++++++++ src/audio/SourceChain.h | 7 +++- tests/CMakeLists.txt | 1 + tests/engine_units/CMakeLists.txt | 7 ++++ tests/engine_units/gain_sanitize_test.cpp | 48 +++++++++++++++++++++++ 6 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 src/audio/GainSanitize.h create mode 100644 tests/engine_units/CMakeLists.txt create mode 100644 tests/engine_units/gain_sanitize_test.cpp diff --git a/src/audio/AudioEngine.h b/src/audio/AudioEngine.h index a384ae5..7aa975a 100644 --- a/src/audio/AudioEngine.h +++ b/src/audio/AudioEngine.h @@ -1,5 +1,6 @@ #pragma once #include "SourceChain.h" +#include "GainSanitize.h" #include "BackingLeveler.h" #include "signalsmith-stretch.h" #include @@ -154,7 +155,10 @@ public: // Gain controls. Input + chain-output gain are per-source (sources[0]); // output gain is the post-mix master and stays engine-global. void setInputGain(float gain) { source0().setInputGain(gain); } - void setOutputGain(float gain) { outputGain.store(gain); } + // Sanitized (see GainSanitize.h): a NaN/Inf master gain from JS would + // multiply the whole device output to NaN downstream of the per-source + // scrub — clamp at the store so every caller is covered. + void setOutputGain(float gain) { outputGain.store(slopsmith::sanitizeMasterGain(gain)); } float getInputGain() const { return source0().getInputGain(); } float getOutputGain() const { return outputGain.load(); } @@ -216,7 +220,7 @@ public: void setTonePolishEnabled(bool enabled) { source0().setTonePolishEnabled(enabled); } // Backing track - void setBackingVolume(float vol) { backingVolume.store(vol); } + void setBackingVolume(float vol) { backingVolume.store(slopsmith::sanitizeMasterGain(vol)); } bool loadBackingTrack(const juce::File& file); void setBackingPosition(double seconds); void startBacking(); @@ -792,7 +796,7 @@ private: // Clamp a requested stream gain to a finite, sane range so a NaN/Inf (or a // wild value) from the JS bridge can never be packed into the stream ring. - static float sanitizeStreamGain(float g) { return std::isfinite(g) ? juce::jlimit(0.0f, 8.0f, g) : 0.0f; } + static float sanitizeStreamGain(float g) { return slopsmith::sanitizeStreamGain(g); } void streamSinkCallback(float* const* outputData, int numOutputChannels, int numSamples); void streamSinkAboutToStart(juce::AudioIODevice* device); diff --git a/src/audio/GainSanitize.h b/src/audio/GainSanitize.h new file mode 100644 index 0000000..9f19310 --- /dev/null +++ b/src/audio/GainSanitize.h @@ -0,0 +1,36 @@ +#pragma once + +// Gain-argument containment (audio-engine TLC, deep-read §2). +// +// N-API's Number coercion lets NaN/Infinity from JS reach the engine's gain +// atomics raw — a NaN master gain multiplies the whole device output to NaN +// (buffer.applyGain) and poisons the peak meters, and nothing downstream +// scrubs it (the per-source NaN scrub runs before the master gain). Clamping +// at the engine setters is the single choke point that fixes every caller: +// audio:setGain, the source-indexed API, and the audio-effects executor. +// +// Bounds: 0..32 for input/chain/output/backing — matching the executor's +// JS-side clampGain so a legit high rig gain is never under-shot (compat pin, +// docs/audio-engine-tlc.md Phase 0.b). The stream/renderer-bus gains keep +// their tighter historical 0..8 (previously sanitizeStreamGain). +// +// JUCE-free on purpose, like AudioSanitize.h, so tests/engine_units can test +// it without a device. + +#include + +namespace slopsmith { + +// Non-finite → 0 (silence beats a poisoned mix); otherwise clamp to [0, max]. +inline float sanitizeGain(float g, float maxGain) noexcept +{ + if (!std::isfinite(g)) return 0.0f; + if (g < 0.0f) return 0.0f; + if (g > maxGain) return maxGain; + return g; +} + +inline float sanitizeMasterGain(float g) noexcept { return sanitizeGain(g, 32.0f); } +inline float sanitizeStreamGain(float g) noexcept { return sanitizeGain(g, 8.0f); } + +} // namespace slopsmith diff --git a/src/audio/SourceChain.h b/src/audio/SourceChain.h index 47c86a7..5f8fee6 100644 --- a/src/audio/SourceChain.h +++ b/src/audio/SourceChain.h @@ -3,6 +3,7 @@ #include "NoiseGate.h" #include "TonePolish.h" #include "SignalChain.h" +#include "GainSanitize.h" #include "PitchDetector.h" #include "ChordScorer.h" #include "MlNoteDetector.h" @@ -125,9 +126,11 @@ public: } void setTonePolishEnabled(bool enabled) { tonePolish.setEnabled(enabled); } - void setInputGain(float gain) { inputGain.store(gain); } + // Sanitized (see GainSanitize.h) so NaN/Inf from the JS bridge can't + // reach the audio thread via either the legacy or the indexed API. + void setInputGain(float gain) { inputGain.store(slopsmith::sanitizeMasterGain(gain)); } float getInputGain() const { return inputGain.load(); } - void setChainOutputGain(float gain) { chainOutputGain.store(gain); } + void setChainOutputGain(float gain) { chainOutputGain.store(slopsmith::sanitizeMasterGain(gain)); } float getChainOutputGain() const { return chainOutputGain.load(); } void setInputChannel(int channel) { selectedInputChannel.store(channel); } int getInputChannel() const { return selectedInputChannel.load(); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9d5cddb..3083ec0 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -13,6 +13,7 @@ endif() # Pure-helper tests (no JUCE / no platform deps) build everywhere. add_subdirectory(audio_sanitize) +add_subdirectory(engine_units) # Note: enable_testing() lives in the top-level CMakeLists.txt — calling it # only here would register tests in build/tests/CTestTestfile.cmake but diff --git a/tests/engine_units/CMakeLists.txt b/tests/engine_units/CMakeLists.txt new file mode 100644 index 0000000..71e8632 --- /dev/null +++ b/tests/engine_units/CMakeLists.txt @@ -0,0 +1,7 @@ +# engine_units — home for the per-unit tests of the audio-engine decomposition +# (docs/audio-engine-tlc.md Part IV §4/0.c). JUCE-free targets only: units are +# extracted so they can be tested against a state struct + fake ring, without a +# real device. One executable per unit keeps failures attributable. +add_executable(gain_sanitize_test gain_sanitize_test.cpp) +target_compile_features(gain_sanitize_test PRIVATE cxx_std_17) +add_test(NAME gain_sanitize COMMAND gain_sanitize_test) diff --git a/tests/engine_units/gain_sanitize_test.cpp b/tests/engine_units/gain_sanitize_test.cpp new file mode 100644 index 0000000..6747bd4 --- /dev/null +++ b/tests/engine_units/gain_sanitize_test.cpp @@ -0,0 +1,48 @@ +// Pins the Phase 0.b compat decision (docs/audio-engine-tlc.md §4): native +// gain clamp bounds are 0..32 — matching the audio-effects executor's JS-side +// clampGain so a legit high rig gain is never under-shot — with NaN/Inf +// rejected universally; stream/renderer-bus gains keep the tighter 0..8. + +#include "../../src/audio/GainSanitize.h" + +#include +#include +#include +#include + +int main() +{ + using slopsmith::sanitizeMasterGain; + using slopsmith::sanitizeStreamGain; + + const float nan = std::numeric_limits::quiet_NaN(); + const float inf = std::numeric_limits::infinity(); + + struct Case { float in, master, stream; }; + const Case cases[] = { + { 0.0f, 0.0f, 0.0f }, + { 1.0f, 1.0f, 1.0f }, + { 8.0f, 8.0f, 8.0f }, + { 8.5f, 8.5f, 8.0f }, // executor range beyond the stream clamp + { 32.0f, 32.0f, 8.0f }, // upper compat bound must not under-shoot + { 33.0f, 32.0f, 8.0f }, + { 1e9f, 32.0f, 8.0f }, + { -1.0f, 0.0f, 0.0f }, + { -0.0f, 0.0f, 0.0f }, + { nan, 0.0f, 0.0f }, // non-finite → silence, never a poisoned mix + { inf, 0.0f, 0.0f }, + { -inf, 0.0f, 0.0f }, + }; + + for (const auto& c : cases) + { + const float m = sanitizeMasterGain(c.in); + const float s = sanitizeStreamGain(c.in); + assert(std::isfinite(m) && std::isfinite(s)); + assert(m == c.master); + assert(s == c.stream); + } + + std::puts("gain_sanitize: all cases passed"); + return 0; +}