fix(voice): Trigger legato keys on held-stack depth, CC123 allNotesOff clears mono stack, voice-param edits rebuild from decoded PCM (no re-decode), mono sizes 1 shifter ring

src/vst/reasampler_processor.cpp

@@ -13,6 +13,7 @@
 #include "pluginterfaces/vst/ivstaudioprocessor.h"
 #include "pluginterfaces/vst/ivsteditcontroller.h"  // RestartFlags::kIoChanged (S7 re-negotiate)
 #include "pluginterfaces/vst/ivstevents.h"
+#include "pluginterfaces/vst/ivstmidicontrollers.h"  // kCtrlAllNotesOff / kCtrlAllSoundsOff (panic)
 #include "pluginterfaces/vst/vstspeaker.h"
 
 #include "public.sdk/source/vst/vstbus.h"  // Vst::AudioBus::setArrangement (S7 output arr)
@@ -314,10 +315,12 @@ void ReaSamplerProcessor::setVoiceCount(int count) {
         if (voiceCount_ == count) return;  // no-op: don't churn a rebuild
         voiceCount_ = count;
     }
-    // Rebuild the engine OFF-thread through the drain-slot swap (the FA1 machinery): the
-    // displaced instrument keeps rendering its ringing tails, so a polyphony change never
-    // cuts a sounding note. Same contract for the mode/trigger setters below.
-    reloadFromBank();
+    // LIGHT rebuild OFF-thread through the drain-slot swap: the engine is reconstructed from
+    // the already-decoded keymap (no bridge re-read, no WAV re-decode — a polyphony change
+    // touches no audio data) and the displaced instrument keeps rendering its ringing tails,
+    // so a voice-param change never cuts a sounding note NOR stalls the UI re-decoding every
+    // zone from disk. Same contract for the mode/trigger setters below.
+    rebuildVoiceEngine();
 }
 
 VoiceMode ReaSamplerProcessor::voiceMode() {
@@ -331,7 +334,7 @@ void ReaSamplerProcessor::setVoiceMode(VoiceMode mode) {
         if (voiceMode_ == mode) return;
         voiceMode_ = mode;
     }
-    reloadFromBank();
+    rebuildVoiceEngine();
 }
 
 MonoTrigger ReaSamplerProcessor::monoTrigger() {
@@ -345,7 +348,7 @@ void ReaSamplerProcessor::setMonoTrigger(MonoTrigger trigger) {
         if (monoTrigger_ == trigger) return;
         monoTrigger_ = trigger;
     }
-    reloadFromBank();
+    rebuildVoiceEngine();
 }
 
 void ReaSamplerProcessor::previewNoteOn(int note) {
@@ -525,12 +528,20 @@ std::string ReaSamplerProcessor::reloadFromBank() {
     //    `built` is heap-owned; release() hands ownership to the atomic; the drain-evicted
     //    pointer is re-owned by the graveyard.
     //
-    //    Bounded reclaim: free graveyard entries whose installedAt < seen, where seen is
-    //    the minimum installedAt process() published over the pointers it holds. Both
-    //    slots are monotone in installedAt, so seen is monotone and any future process()
-    //    load yields installedAt >= seen — an entry below seen is provably unreachable
-    //    (see the header proof). Remaining entries drain at setActive(false) / terminate()
-    //    when process is guaranteed stopped.
+    publishBuiltLocked(std::move(built));
+    return resolvedId;
+}
+
+void ReaSamplerProcessor::publishBuiltLocked(std::unique_ptr<LoadedInstrument> built) {
+    // REQUIRES reloadMutex_ held (single-writer over both slots + the graveyard). Shared by
+    // reloadFromBank and rebuildVoiceEngine — the one safety-critical swap dance.
+    //
+    // Bounded reclaim: free graveyard entries whose installedAt < seen, where seen is
+    // the minimum installedAt process() published over the pointers it holds. Both
+    // slots are monotone in installedAt, so seen is monotone and any future process()
+    // load yields installedAt >= seen — an entry below seen is provably unreachable
+    // (see the header proof). Remaining entries drain at setActive(false) / terminate()
+    // when process is guaranteed stopped.
     const std::uint64_t seen = processGeneration_.load(std::memory_order_acquire);
     graveyard_.erase(
         std::remove_if(graveyard_.begin(), graveyard_.end(),
@@ -541,7 +552,40 @@ std::string ReaSamplerProcessor::reloadFromBank() {
     LoadedInstrument* prev = live_.exchange(built.release());
     LoadedInstrument* evicted = draining_.exchange(prev);
     if (evicted) graveyard_.push_back(std::unique_ptr<LoadedInstrument>(evicted));
-    return resolvedId;
+}
+
+void ReaSamplerProcessor::rebuildVoiceEngine() {
+    // OFF THE AUDIO THREAD (the editor's voice-deck click handlers). See the header contract:
+    // a voice-param change touches NO audio data, so this rebuilds the engine + preview card
+    // around a COPY of the live instrument's already-decoded keymap — no bridge, no disk —
+    // and publishes through the same drain-slot swap, so ringing tails survive.
+    std::lock_guard<std::mutex> lock(reloadMutex_);
+    LoadedInstrument* cur = live_.load(std::memory_order_acquire);
+    if (!cur) return;  // nothing loaded: the new params bake into the next real reload.
+
+    int builtVoiceCount = kDefaultVoiceCount;
+    VoiceMode builtVoiceMode = VoiceMode::Poly;
+    MonoTrigger builtMonoTrigger = MonoTrigger::Retrigger;
+    {
+        std::lock_guard<std::mutex> vp(voiceParamsMutex_);
+        builtVoiceCount = voiceCount_;
+        builtVoiceMode = voiceMode_;
+        builtMonoTrigger = monoTrigger_;
+    }
+
+    const std::uint64_t gen = reloadGeneration_.fetch_add(1, std::memory_order_relaxed) + 1;
+    // Same Preserve-window derivation as reloadFromBank (kPreserveWindowMs at the host rate).
+    std::int64_t preserveWindow = static_cast<std::int64_t>(
+        kPreserveWindowMs * sampleRate_ / 1000.0 + 0.5);
+    if (preserveWindow < 2) preserveWindow = 2;
+
+    // Deep-copy the decoded PCM + zones. Safe to read concurrently with process(): the keymap
+    // is immutable after construction, and under reloadMutex_ nobody can free `cur`.
+    Keymap km = cur->keymap;
+    auto built = std::make_unique<LoadedInstrument>(
+        std::move(km), static_cast<std::size_t>(builtVoiceCount), gen,
+        kPreserveVoiceCap, preserveWindow, builtVoiceMode, builtMonoTrigger);
+    publishBuiltLocked(std::move(built));
 }
 
 void ReaSamplerProcessor::retireIdleDrain() {
@@ -721,6 +765,26 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) {
             } else if (e.type == Event::kNoteOffEvent) {
                 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).
+                const auto cc = static_cast<int>(e.midiCCOut.controlNumber);
+                if (cc == kCtrlAllNotesOff || cc == kCtrlAllSoundsOff) {
+                    if (inst) {
+                        inst->engine.allNotesOff();
+                        inst->preview.releaseAll();
+                    }
+                    if (drain) {
+                        drain->engine.allNotesOff();
+                        drain->preview.releaseAll();
+                    }
+                }
             }
         }
     }

src/vst/reasampler_processor.h

@@ -200,8 +200,9 @@ public:
     // --- Phase S voice-system parameters (per-instance, persisted in component state v7) ---
     // Read/written on the UI thread (the editor's voice deck) and by getState/setState; guarded
     // by voiceParamsMutex_. NOT read on the audio thread — each setter rebuilds the VoiceEngine
-    // OFF-thread through reloadFromBank's drain-slot swap, so changing polyphony / mode / the
-    // retrigger toggle never cuts a ringing tail (the same path FA1 added for curve edits).
+    // OFF-thread via rebuildVoiceEngine (a LIGHT rebuild around the already-decoded keymap; no
+    // bridge read, no WAV re-decode) published through the same tail-preserving drain-slot swap,
+    // so changing polyphony / mode / the retrigger toggle never cuts a ringing tail.
     int voiceCount();
     void setVoiceCount(int count);        // clamped to kMinVoiceCount..kMaxVoiceCount
     VoiceMode voiceMode();
@@ -240,6 +241,24 @@ private:
     // proof (see below) covers the free.
     void retireIdleDrain();
 
+    // Phase S voice-param LIGHT rebuild (voice-review Major #3): rebuild the engine + preview
+    // card around a COPY of the LIVE instrument's already-decoded Keymap — no bridge read, no
+    // filesystem, no WAV re-decode — and publish through the same tail-preserving drain-slot
+    // swap as a full reload. A polyphony/mode/trigger change touches no audio data, so the
+    // full reloadFromBank (which re-decodes every zone WAV from disk on the UI thread) was
+    // pure waste — a visible UI stall on a many-zone instrument. Copying the keymap is safe:
+    // it is immutable after construction and, under reloadMutex_, the live instrument can
+    // neither be swapped nor freed while we read it. When nothing is loaded this is a no-op —
+    // the new params bake into the next real reload. Off the audio thread only.
+    void rebuildVoiceEngine();
+
+    // Publish `built` (null = install silence) into live_: prune the graveyard by the last
+    // process()-published generation, swap `built` into live_, displace the previous live into
+    // the drain slot, and park the drain-evicted instrument in the graveyard. REQUIRES
+    // reloadMutex_ held — factored out so reloadFromBank and rebuildVoiceEngine share the ONE
+    // safety-critical swap dance (see the handoff proof below).
+    void publishBuiltLocked(std::unique_ptr<LoadedInstrument> built);
+
     ReaperBridge bridge_;
 
     // --- The audio-thread handoff (S4 real-time discipline, FA1 drain slot) --

src/vst/sampler_core.cpp

@@ -538,7 +538,15 @@ VoiceEngine::VoiceEngine(std::size_t maxVoices, const Keymap& keymap,
     // 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.
     if (preserveWindowFrames > 1) {
-        for (Voice& v : voices_) v.presizePreserveShifters(preserveWindowFrames);
+        // 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) {
+            voices_[i].presizePreserveShifters(preserveWindowFrames);
+        }
     }
 }
 
@@ -589,6 +597,10 @@ void VoiceEngine::removeHeld(int note) {
 }
 
 std::size_t VoiceEngine::monoNoteOn(int note, int velocity) {
+    // Reject out-of-range notes BEFORE touching the held stack: HeldNote stores the note as a
+    // uint8, so an unguarded value (e.g. 256, or a negative) would alias mod 256 onto a real
+    // held note and corrupt the stack. Mirrored in monoNoteOff.
+    if (note < 0 || note > 127) return kNoVoice;
     const ZoneResolution res = keymap_.resolve(note, velocity);
     if (!res.matched) return kNoVoice;  // out-of-zone: defined no-play, never joins the stack.
     const KeyZone& zone = keymap_.zones[res.zoneIndex];
@@ -605,10 +617,14 @@ std::size_t VoiceEngine::monoNoteOn(int note, int velocity) {
     }
 
     Voice& v = voices_[0];
-    // LEGATO takeover: another note is sounding (active + not releasing — a releasing voice's
-    // note has left the stack, so a fresh phrase after release always re-attacks) AND the new
-    // note resolves to the SAME sample. Retune in place: pitch moves, no re-attack.
-    if (v.active() && !v.releasing() && monoTrigger_ == MonoTrigger::Legato &&
+    // LEGATO takeover, keyed on the HELD-STACK DEPTH: after the push above, heldCount_ >= 2
+    // means another note was already physically held — the exact "takeover within a phrase"
+    // 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.
+    if (v.active() && heldCount_ >= 2 && monoTrigger_ == MonoTrigger::Legato &&
         v.playingSample() == &sample) {
         v.retune(note, zone.rootNote, zone.keyTrack);
         return 0;
@@ -620,6 +636,9 @@ std::size_t VoiceEngine::monoNoteOn(int note, int velocity) {
 }
 
 void VoiceEngine::monoNoteOff(int note) {
+    // Same range guard as monoNoteOn: removeHeld compares against the uint8-cast note, so an
+    // unguarded out-of-range off (e.g. 256 -> 0 mod 256) would evict a legitimately held 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
@@ -697,6 +716,18 @@ void VoiceEngine::noteOff(int note) {
     if (target != kNoVoice) voices_[target].release();
 }
 
+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.
+    heldCount_ = 0;
+    for (Voice& v : voices_) {
+        if (v.active()) v.release();
+    }
+}
+
 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
@@ -778,6 +809,12 @@ void PreviewCard::noteOff(int note) {
     if (voice_.active() && voice_.note() == note) voice_.release();
 }
 
+void PreviewCard::releaseAll() {
+    // Panic 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::render(AudioSample* out, std::size_t frameCount) {
     if (out == nullptr || frameCount == 0 || !voice_.active()) return;
     for (std::size_t f = 0; f < frameCount; ++f) {

src/vst/sampler_core.h

@@ -568,6 +568,14 @@ 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.
+    void allNotesOff();
+
     // 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
@@ -619,7 +627,8 @@ private:
     struct HeldNote { std::uint8_t note; std::uint8_t velocity; };
 
     // Mono note-on: push to the stack and take the voice over (legato retune on a same-sample
-    // takeover, else a fresh start). Returns 0 (the mono voice) or kNoVoice for out-of-zone.
+    // takeover, else a fresh start). Returns 0 (the mono voice) or kNoVoice for out-of-zone
+    // or out-of-range (note outside [0,127] — rejected BEFORE the stack, which stores uint8).
     // The S16 Preserve cap is NOT applied in mono — a single voice runs at most one shifter,
     // inherently within any cap; applying it would wrongly drop a Preserve->Preserve takeover.
     std::size_t monoNoteOn(int note, int velocity);
@@ -671,6 +680,9 @@ 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,
+    // whatever note it is on. Gate enters release; Trigger plays through (bounded). RT-safe.
+    void releaseAll();
 
     bool active() const { return voice_.active(); }