From 080a7be8cac940e412d78d493154a30073e55f0c Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Mon, 27 Jul 2026 22:08:56 -0400 Subject: [PATCH] fix(voice): CC 120 hard-stops voices (incl. Trigger); Mono sizes voices_ to 1; comments corrected CC 120 -> allSoundsOff/hardStop (immediate silence, stops Trigger one-shots); CC 123 -> allNotesOff/releaseAll (release, unchanged). Mono VoiceEngine sizes voices_ to 1 structurally. Legato same-note re-press edge documented. Three new tests. --- src/vst/reasampler_processor.cpp | 30 ++++++++++----- src/vst/sampler_core.cpp | 64 +++++++++++++++++++++----------- src/vst/sampler_core.h | 27 ++++++++++---- tests/test_sampler_core.cpp | 62 +++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 37 deletions(-) diff --git a/src/vst/reasampler_processor.cpp b/src/vst/reasampler_processor.cpp index a9c546a..c81832d 100644 --- a/src/vst/reasampler_processor.cpp +++ b/src/vst/reasampler_processor.cpp @@ -766,16 +766,28 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { if (inst) inst->engine.noteOff(e.noteOff.pitch); if (drain) drain->engine.noteOff(e.noteOff.pitch); } else if (e.type == Event::kLegacyMIDICCOutEvent) { - // PANIC (Phase S voice-review Major #2): CC 123 (All Notes Off) / CC 120 (All - // Sound Off) reset the engine — clear the mono held stack and release every - // voice, live AND drain, engine AND preview card — so a phantom held-stack - // entry left by a lost note-off can never be resurrected by the mono fallback - // and sustain forever. REAPER delivers raw input MIDI CC to a VST3 instrument - // as kLegacyMIDICCOut events on the INPUT event list (a REAPER-ism — the type - // is nominally an output event; DAW-verify, see handoff). allNotesOff / - // releaseAll are RT-safe (no allocation, bounded scans). + // PANIC (Phase S voice-review Major #2): REAPER delivers raw input MIDI CC to a + // VST3 instrument as kLegacyMIDICCOut events on the INPUT event list (a REAPER-ism + // — the type is nominally an output event; DAW-verify, see handoff). + // CC 123 (All Notes Off): release semantics — Gate voices enter their AHDSR + // release tail; Trigger one-shots play through their bounded play length. + // CC 120 (All Sounds Off): hard-stop semantics — immediate silence regardless + // of play mode, including Trigger one-shots that ignore CC 123. This is the + // true "panic" for a ringing one-shot (e.g. a full-length capture). + // Both clear the mono held stack. Both apply to live AND drain, engine AND preview. + // allNotesOff / allSoundsOff / releaseAll / hardStop are RT-safe (no allocation, + // bounded scans). const auto cc = static_cast(e.midiCCOut.controlNumber); - if (cc == kCtrlAllNotesOff || cc == kCtrlAllSoundsOff) { + if (cc == kCtrlAllSoundsOff) { + if (inst) { + inst->engine.allSoundsOff(); + inst->preview.hardStop(); + } + if (drain) { + drain->engine.allSoundsOff(); + drain->preview.hardStop(); + } + } else if (cc == kCtrlAllNotesOff) { if (inst) { inst->engine.allNotesOff(); inst->preview.releaseAll(); diff --git a/src/vst/sampler_core.cpp b/src/vst/sampler_core.cpp index f885c9d..c48b546 100644 --- a/src/vst/sampler_core.cpp +++ b/src/vst/sampler_core.cpp @@ -373,6 +373,12 @@ void Voice::release() { env_.noteOff(); } +void Voice::hardStop() { + // CC 120 (All Sounds Off): immediate silence regardless of play mode. Stops Trigger one-shots + // that ignore release(), and short-circuits Gate release tails. RT-safe: no allocation. + active_ = false; +} + double Voice::tickAmplitude() { double amp; if (playMode_ == PlayMode::Gate) { @@ -528,23 +534,22 @@ VoiceEngine::VoiceEngine(std::size_t maxVoices, const Keymap& keymap, std::size_t preserveVoiceCap, std::int64_t preserveWindowFrames, VoiceMode voiceMode, MonoTrigger monoTrigger) - : voices_(maxVoices == 0 ? 1 : maxVoices), keymap_(keymap), + // MONO always uses voices_[0] only (last-note priority, single voice); size to 1 so + // the "only voices_[0] is ever driven" invariant is structurally enforced — no latent + // RT-discipline risk if a future mono path touched voices_[1..]. maxVoices == 0 clamps + // to 1 (documented degenerate: at least one voice so a note-on is always serviceable). + : voices_(voiceMode == VoiceMode::Mono ? 1 + : (maxVoices == 0 ? 1 : maxVoices)), + keymap_(keymap), preserveVoiceCap_(preserveVoiceCap), voiceMode_(voiceMode), monoTrigger_(monoTrigger) { - // maxVoices == 0 would mean "no polyphony at all", which cannot service a note-on; - // clamp to a single voice so the engine is always usable (documented degenerate). - // // Pre-size every voice's Preserve shifters HERE (construction is off the audio thread), so // note-on never allocates. A 0 window leaves them pass-through (no ring). This is the one // allocation point for the shifter rings across the engine's lifetime. + // MONO: voices_.size() == 1, so the loop below sizes exactly one voice regardless of + // maxVoices — the Poly path sizes the whole pool as before. if (preserveWindowFrames > 1) { - // MONO drives voices_[0] only, and the mode is immutable per engine (a change - // rebuilds via the processor's drain-slot reload), so sizing the other voices' - // shifter rings would waste up to (maxVoices-1) windows on voices that can - // never start. Poly sizes the whole pool as before. - const std::size_t ringCount = - voiceMode_ == VoiceMode::Mono ? 1 : voices_.size(); - for (std::size_t i = 0; i < ringCount; ++i) { + for (std::size_t i = 0; i < voices_.size(); ++i) { voices_[i].presizePreserveShifters(preserveWindowFrames); } } @@ -622,8 +627,9 @@ std::size_t VoiceEngine::monoNoteOn(int note, int velocity) { // predicate. (The previous guard, `active && !releasing`, broke for TRIGGER zones: // Voice::release() is a no-op in Trigger, so releasing_ never latches, and a one-shot // still ringing after the last key-up was silently RETUNED in place instead of - // re-attacked. Equivalent in every Gate case — a gated, non-releasing voice always has - // its note on the stack; a released note has left it.) Same-sample requirement unchanged. + // re-attacked. NOTE: a one-held-note same-note re-press (heldCount_ becomes 1 after the + // removeHeld/re-push above — so heldCount_ < 2) re-attacks rather than retuning, which is + // the correct fresh-phrase behavior for that edge case.) Same-sample requirement unchanged. if (v.active() && heldCount_ >= 2 && monoTrigger_ == MonoTrigger::Legato && v.playingSample() == &sample) { v.retune(note, zone.rootNote, zone.keyTrack); @@ -641,8 +647,8 @@ void VoiceEngine::monoNoteOff(int note) { if (note < 0 || note > 127) return; removeHeld(note); Voice& v = voices_[0]; - // Releasing a note that is not the sounding one (a lower held note, an already-released - // note, or a note the stack overflowed past) changes nothing audible. + // Releasing a note that is not the sounding one (a lower held note or an already-released + // note) changes nothing audible. if (!v.active() || v.releasing() || v.note() != note) return; if (heldCount_ == 0) { @@ -717,17 +723,27 @@ void VoiceEngine::noteOff(int note) { } void VoiceEngine::allNotesOff() { - // PANIC / CC 123. Clear the mono held stack FIRST so no fallback can resurrect a phantom - // note (the stuck-note scenario: a lost note-off leaves an entry that monoNoteOff's - // fallback restarts and sustains forever), then gate off every active voice. Gate voices - // enter their release tail; Trigger one-shots ignore release by design and play through - // their bounded play length. RT-safe: no allocation, bounded by the pool size. + // CC 123. Clear the mono held stack so no fallback can resurrect a phantom note (the + // stuck-note scenario: a lost note-off leaves an entry that monoNoteOff's fallback + // restarts and sustains forever with no key held), then gate off every active voice. + // Gate voices enter their release tail; Trigger one-shots ignore release by design and + // play through their bounded play length. RT-safe: no allocation, bounded by the pool size. heldCount_ = 0; for (Voice& v : voices_) { if (v.active()) v.release(); } } +void VoiceEngine::allSoundsOff() { + // CC 120. Hard-stop EVERY voice immediately (no release ramp — silences Trigger one-shots + // that allNotesOff() cannot stop) and clear the mono held stack. RT-safe: no allocation, + // bounded by the pool size. + heldCount_ = 0; + for (Voice& v : voices_) { + v.hardStop(); + } +} + void VoiceEngine::render(AudioSample* out, std::size_t frameCount) { // Real-time safe: no allocation, no resize — mix straight into the caller's buffer. // The VST3 process callback hands us the host's output channel buffer here, so the @@ -810,11 +826,17 @@ void PreviewCard::noteOff(int note) { } void PreviewCard::releaseAll() { - // Panic peer of VoiceEngine::allNotesOff: unconditional release, whatever note rings. + // CC 123 peer of VoiceEngine::allNotesOff: unconditional release, whatever note rings. // Gate enters release; Trigger plays through (bounded, cannot be stuck). RT-safe. if (voice_.active()) voice_.release(); } +void PreviewCard::hardStop() { + // CC 120 peer of VoiceEngine::allSoundsOff: immediate silence, no release ramp. + // Silences Trigger one-shots that releaseAll() cannot stop. RT-safe. + voice_.hardStop(); +} + void PreviewCard::render(AudioSample* out, std::size_t frameCount) { if (out == nullptr || frameCount == 0 || !voice_.active()) return; for (std::size_t f = 0; f < frameCount; ++f) { diff --git a/src/vst/sampler_core.h b/src/vst/sampler_core.h index e8c06f0..2207a84 100644 --- a/src/vst/sampler_core.h +++ b/src/vst/sampler_core.h @@ -428,6 +428,11 @@ public: // TRIGGER mode it is a NO-OP (Trigger ignores note-off and plays through to its play length). void release(); + // HARD STOP — CC 120 (All Sounds Off) semantics. Immediately silences this voice regardless + // of play mode: sets active_ = false with no release ramp. Stops a ringing Trigger one-shot + // instantly (which release() cannot do). RT-safe: no allocation, no lock. + void hardStop(); + // True while this voice is producing (or about to produce) sound. bool active() const { return active_; } // The note this voice was started on (for note-off routing). Meaningless if idle. @@ -568,14 +573,19 @@ public: // the older tail to ring — matches hardware behavior). No-op if none match. void noteOff(int note); - // PANIC / MIDI All-Notes-Off (CC 123). Clears the MONO held stack and releases EVERY - // active voice: Gate voices enter their release tail; Trigger one-shots ignore release - // by design and play through their bounded play length (they can never be stuck). This - // is the mono stack's ONLY reset path — a phantom entry left by a lost note-off would - // otherwise be resurrected by the next fallback and sustain forever with no key held. - // RT-safe (no allocation, bounded by maxVoices); callable from the audio thread. + // CC 123 — MIDI All-Notes-Off: clears the MONO held stack and RELEASES every active voice + // (Gate voices enter their AHDSR release tail; Trigger one-shots ignore release and play + // through their bounded play length). This is the mono stack's ONLY reset path — a phantom + // entry left by a lost note-off would otherwise be resurrected by the fallback and sustain + // forever with no key held. RT-safe (no allocation, bounded by maxVoices). void allNotesOff(); + // CC 120 — MIDI All-Sounds-Off: hard-stops EVERY voice immediately (active_ = false, no + // release ramp), clears the MONO held stack, and silences even Trigger one-shots that would + // ignore a release. Use for panic; CC 123 for the softer "let gates release" behavior. + // RT-safe (no allocation, bounded by maxVoices); callable from the audio thread. + void allSoundsOff(); + // REAL-TIME render (S4): sums all active voices into the caller-provided buffer // `out[0..frameCount)`, ADDING to whatever is there (the caller clears or mixes — // this never touches memory it does not own and NEVER allocates). This is the @@ -680,9 +690,12 @@ public: // Release the preview IF `note` is the one sounding (a stale off for a replaced note is a // no-op). Gate zones enter release; Trigger zones ignore note-off and play through. void noteOff(int note); - // PANIC peer of VoiceEngine::allNotesOff: release the ringing preview UNCONDITIONALLY, + // CC 123 peer of VoiceEngine::allNotesOff: release the ringing preview UNCONDITIONALLY, // whatever note it is on. Gate enters release; Trigger plays through (bounded). RT-safe. void releaseAll(); + // CC 120 peer of VoiceEngine::allSoundsOff: HARD-STOP the preview immediately (no release + // ramp, silences Trigger one-shots too). RT-safe. + void hardStop(); bool active() const { return voice_.active(); } diff --git a/tests/test_sampler_core.cpp b/tests/test_sampler_core.cpp index e638361..2ea67c7 100644 --- a/tests/test_sampler_core.cpp +++ b/tests/test_sampler_core.cpp @@ -1793,6 +1793,65 @@ static void testPreviewCardReleaseAll() { CHECK(!card.active()); } +// CC 120 (allSoundsOff) hard-stops a ringing TRIGGER one-shot that would otherwise play to +// its bounded playEnd (minutes on a full-length capture). This is the primary repro: allNotesOff +// (CC 123) is a NO-OP on a Trigger voice — only allSoundsOff provides the actual hard stop. +static void testAllSoundsOffStopsTriggerOneShot() { + // A Trigger sample with a very long play length (all-1 DC, flat velocity). After noteOn the + // voice is active and ringing; allSoundsOff must silence it immediately. + SampleData s = dcLevelSample(200000, 1.0f, 60); + s.play.playMode = PlayMode::Trigger; + s.play.trigger.lengthFraction = 1.0; // full length — would ring for 200000 frames + Keymap km = Keymap::singleSampleChromatic(std::move(s)); + VoiceEngine eng(1, km); + eng.noteOn(60, 127); + CHECK(eng.activeVoiceCount() == 1); + // CC 123 (release) must be a NO-OP on a Trigger voice — the one-shot plays through. + eng.allNotesOff(); + CHECK(eng.activeVoiceCount() == 1); // still ringing (Trigger ignores release) + std::vector out; + eng.render(out, 1); + CHECK(out[0] > 0.5f); // still sounding + // CC 120 (hard-stop) must silence it instantly. + eng.allSoundsOff(); + CHECK(eng.activeVoiceCount() == 0); // immediately idle + out.clear(); + eng.render(out, 1); + CHECK(approx(out[0], 0.0, 1e-9)); // silent +} + +// CC 123 (allNotesOff) still releases Gate voices — the existing release behavior is unchanged. +static void testAllNotesOffStillReleasesGateVoices() { + SampleData s = dcLevelSample(200000, 1.0f, 60); + // Default Gate mode, instant release (releaseFrames 0). + Keymap km = Keymap::singleSampleChromatic(std::move(s)); + VoiceEngine eng(4, km); + eng.noteOn(60, 127); + eng.noteOn(62, 127); + CHECK(eng.activeVoiceCount() == 2); + eng.allNotesOff(); + std::vector out; + eng.render(out, 1); + CHECK(approx(out[0], 0.0, 1e-9)); // Gate with 0-release: instant silence + CHECK(eng.activeVoiceCount() == 0); +} + +// MONO LEGATO same-note re-press (one-held-note edge case): with only that note on the +// stack, heldCount_ after the re-push is 1 (not >= 2), so it falls through to re-attack +// rather than retune. This is the correct fresh-phrase behavior documented in the comment. +static void testMonoLegatoSameNoteRepressReattacks() { + SampleData s = rampSample(200000, 60); + Keymap km = Keymap::singleSampleChromatic(std::move(s)); + VoiceEngine eng(2, km, 0, 0, VoiceMode::Mono, MonoTrigger::Legato); + eng.noteOn(60, 127); // first press; read starts at 0 + std::vector out; + eng.render(out, 10); // advance the read head to ~10 + // Re-press the SAME note while it is the only held note: heldCount_ after removeHeld+push = 1 + // -> does NOT satisfy heldCount_ >= 2 -> re-attack (not a legato retune). + eng.noteOn(60, 127); + CHECK(approx(probeFrame(eng), 0.0, 1e-4)); // read RESTARTED at 0 (re-attack, not retune) +} + // GREEN: out-of-range notes are rejected at BOTH mono entry points. The held stack stores // uint8, so an unguarded off for note 256 (== 0 mod 256) would alias-evict held note 0 — // losing its fallback. Note-ons out of [0,127] are a defined no-play. @@ -1970,6 +2029,9 @@ int main() { testAllNotesOffReleasesPolyVoices(); testAllNotesOffClearsMonoHeldStack(); testPreviewCardReleaseAll(); + testAllSoundsOffStopsTriggerOneShot(); + testAllNotesOffStillReleasesGateVoices(); + testMonoLegatoSameNoteRepressReattacks(); testMonoOutOfRangeNotesRejected(); testVoiceCountBoundsPolyphony(); testPreviewCardIsolatedFromPool();