fix(voice): declick every takeover of a sounding voice — mono retrig/fallback and poly at-cap steal; same-block double-steal keeps its seed; over-cap repro proves the engine steals exactly one voice per note-on

src/vst/reasampler_processor.h

@@ -52,6 +52,11 @@ struct LoadedInstrument {
     PreviewCard preview;  // Phase S: the isolated preview voice — never part of the pool
     std::uint64_t installedAt = 0;  // reload generation at which this was installed
 
+    // The takeover declick (GA fix) is opted IN here — the PRODUCT default: any restart of a
+    // sounding voice (mono Retrigger takeover/fallback, cross-sample legato restart, POLY
+    // at-cap steal) fades the cut tone over a few ms instead of clicking. The pure core
+    // defaults it off (regression baseline) — same layering as the kDefaultPitchEngine
+    // product default.
     LoadedInstrument(Keymap km, std::size_t maxVoices,
                      std::uint64_t gen, std::size_t preserveVoiceCap = 0,
                      std::int64_t preserveWindowFrames = 0,
@@ -59,7 +64,7 @@ struct LoadedInstrument {
                      MonoTrigger monoTrigger = MonoTrigger::Retrigger)
         : keymap(std::move(km)),
           engine(maxVoices, keymap, preserveVoiceCap, preserveWindowFrames,
-                 voiceMode, monoTrigger),
+                 voiceMode, monoTrigger, /*takeoverDeclick=*/true),
           preview(keymap, preserveWindowFrames),
           installedAt(gen) {}
 

src/vst/sampler_core.cpp

@@ -261,7 +261,27 @@ void Voice::presizePreserveShifters(std::int64_t windowFrames) {
 
 void Voice::start(int note, int velocity, const SampleData& sample, int rootNote,
                   double keyTrack, const vst::VelocityCurve& velocityCurve,
-                  bool unityVarispeedBypass) {
+                  bool unityVarispeedBypass, bool declickTakeover) {
+    // Takeover declick (Phase S GA fix): BEFORE any state reset, seed the compensation
+    // from the last rendered output IFF this start is a takeover/steal of a SOUNDING voice
+    // and the caller opted in. The seed is exactly the value the hard cut removes, so the
+    // first new frame carries the old level and the step becomes a fast fade (see
+    // kDeclickDecay). A fresh start (idle voice) always clears the declick state — no
+    // phantom ramp. lastOut{L,R}_ are deliberately NOT zeroed here: a SECOND same-block
+    // takeover (two steals of this voice with no frame rendered between) must re-seed from
+    // the same pre-cut output level — zeroing would drop the pending ramp and bring the
+    // click back on that edge. The next rendered frame overwrites lastOut anyway.
+    if (declickTakeover && active_) {
+        declickL_ = lastOutL_;
+        declickR_ = lastOutR_;
+        declickActive_ = (declickL_ > kDeclickFloor || declickL_ < -kDeclickFloor ||
+                          declickR_ > kDeclickFloor || declickR_ < -kDeclickFloor);
+    } else {
+        declickL_ = 0.0;
+        declickR_ = 0.0;
+        declickActive_ = false;
+    }
+
     active_ = true;
     releasing_ = false;
     amplitudeDone_ = false;
@@ -506,8 +526,30 @@ AudioSample Voice::advanceFrame(bool stereo, AudioSample& outR) {
         ratio_ = baseRatio_ * envFactor;
     }
 
+    // Takeover declick (Phase S GA fix): sum the decaying compensation seeded by a
+    // takeover/steal start() so the restart's hard cut has no step. Engine-agnostic — applied
+    // after either pitch-engine branch, on the shared epilogue. Inactive (the common case)
+    // costs one branch.
+    if (declickActive_) {
+        outL += declickL_;
+        if (stereo) outRlocal += declickR_;
+        declickL_ *= kDeclickDecay;
+        declickR_ *= kDeclickDecay;
+        if (declickL_ < kDeclickFloor && declickL_ > -kDeclickFloor &&
+            declickR_ < kDeclickFloor && declickR_ > -kDeclickFloor) {
+            declickActive_ = false;
+        }
+    }
+
     if (stereo) outR = static_cast<AudioSample>(outRlocal);
 
+    // Track the value this voice actually contributed THIS frame (post-gain, incl. any running
+    // declick) — a future takeover restart seeds its declick from exactly this. In a mono
+    // render the R track mirrors L (dual-mono semantics, matching the stereo mirror of a mono
+    // sample), so a later stereo takeover still has a sane R seed.
+    lastOutL_ = outL;
+    lastOutR_ = stereo ? outRlocal : outL;
+
     readPos_ += ratio_;
 
     if (amplitudeDone_) {
@@ -533,7 +575,8 @@ void Voice::renderFrameStereo(AudioSample& l, AudioSample& r) {
 VoiceEngine::VoiceEngine(std::size_t maxVoices, const Keymap& keymap,
                          std::size_t preserveVoiceCap,
                          std::int64_t preserveWindowFrames,
-                         VoiceMode voiceMode, MonoTrigger monoTrigger)
+                         VoiceMode voiceMode, MonoTrigger monoTrigger,
+                         bool takeoverDeclick)
     // 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
@@ -542,7 +585,8 @@ VoiceEngine::VoiceEngine(std::size_t maxVoices, const Keymap& keymap,
                                            : (maxVoices == 0 ? 1 : maxVoices)),
       keymap_(keymap),
       preserveVoiceCap_(preserveVoiceCap),
-      voiceMode_(voiceMode), monoTrigger_(monoTrigger) {
+      voiceMode_(voiceMode), monoTrigger_(monoTrigger),
+      takeoverDeclick_(takeoverDeclick) {
     // 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.
@@ -636,7 +680,10 @@ std::size_t VoiceEngine::monoNoteOn(int note, int velocity) {
         return 0;
     }
     // RETRIGGER takeover / first note of a phrase / cross-sample legato: (re)start the voice.
-    v.start(note, velocity, sample, zone.rootNote, zone.keyTrack, zone.velocityCurve);
+    // The declick opt-in rides every mono restart: start() self-gates it on the voice being
+    // ACTIVE, so a first-note fresh start never ramps — only a hard cut of a sounding tone.
+    v.start(note, velocity, sample, zone.rootNote, zone.keyTrack, zone.velocityCurve,
+            /*unityVarispeedBypass=*/false, /*declickTakeover=*/takeoverDeclick_);
     v.setStartOrder(nextStartOrder_++);
     return 0;
 }
@@ -669,8 +716,10 @@ void VoiceEngine::monoNoteOff(int note) {
         return;
     }
     // Retrigger (or cross-sample) fallback: re-strike the fallen-back-to note at its own
-    // original velocity.
-    v.start(fb.note, fb.velocity, sample, zone.rootNote, zone.keyTrack, zone.velocityCurve);
+    // original velocity. Peer restart site of monoNoteOn's takeover — same declick opt-in
+    // (the fallback also hard-cuts the sounding tone).
+    v.start(fb.note, fb.velocity, sample, zone.rootNote, zone.keyTrack, zone.velocityCurve,
+            /*unityVarispeedBypass=*/false, /*declickTakeover=*/takeoverDeclick_);
     v.setStartOrder(nextStartOrder_++);
 }
 
@@ -697,8 +746,12 @@ std::size_t VoiceEngine::noteOn(int note, int velocity) {
 
     // The voice's Preserve shifters were pre-sized at engine construction (off-thread), so
     // start() only reset()s + warm()s them — no allocation on this audio-thread path.
+    // The takeover declick rides the STEAL restart too (GA fix): start() self-gates on the
+    // voice being active, so a free-voice start never ramps — only an at-cap steal, which is
+    // the same hard cut of a sounding tone as the mono retrig takeover.
     const std::size_t v = allocateVoice();
-    voices_[v].start(note, velocity, sample, zone.rootNote, zone.keyTrack, zone.velocityCurve);
+    voices_[v].start(note, velocity, sample, zone.rootNote, zone.keyTrack, zone.velocityCurve,
+                     /*unityVarispeedBypass=*/false, /*declickTakeover=*/takeoverDeclick_);
     voices_[v].setStartOrder(nextStartOrder_++);
     return v;
 }

src/vst/sampler_core.h

@@ -381,6 +381,22 @@ private:
     std::int64_t pos_ = 0;
 };
 
+// Takeover declick (Phase S GA fix — audible click when a sounding voice is restarted). A
+// takeover restart HARD-CUTS the sounding tone: the read head and envelope restart in one
+// frame, a step discontinuity that clicks. This is the same physics on EVERY restart-of-a-
+// sounding-voice path — the MONO Retrigger takeover/fallback, the mono cross-sample legato
+// restart, AND the POLY at-cap voice steal. When the caller opts in (start()'s
+// declickTakeover; the engine passes it on all of those restart paths when constructed with
+// takeoverDeclick), start() seeds a compensation from the voice's last rendered output;
+// each frame it is summed into the output and decays by kDeclickDecay, so the step becomes a
+// fast fade-out of the old tone under the new note's attack. The decay is a per-FRAME DSP
+// micro-ramp (~2-4 ms to the -80 dB floor across 44.1-96 kHz), not a stored wall-clock
+// quantity — no rate resolution needed. OFF by default so the bare core stays byte-identical
+// to the pre-fix engine (the regression baseline); the processor shell opts in, mirroring the
+// kDefaultPitchEngine layering.
+inline constexpr double kDeclickDecay = 0.95;  // per-frame decay of the compensation
+inline constexpr double kDeclickFloor = 1e-4;  // below this the ramp is done (~ -80 dB)
+
 // ---------------------------------------------------------------------------
 // A single voice: one active note playing one repitched, enveloped sample. Reads
 // the sample by fractional frame position with linear interpolation, advancing by
@@ -411,10 +427,14 @@ public:
     // PREVIEW card opts in (it fires at the root, so this is its zero-added-latency path); the
     // MIDI VoiceEngine does NOT (default false) — a chromatic line must not step ~25 ms faster
     // at the root note than one semitone away (the FA1-review timing-step finding).
+    // `declickTakeover` (Phase S GA fix): when TRUE and this voice is currently ACTIVE (a
+    // takeover/steal restart, not a fresh start), seed the takeover declick from the last
+    // rendered output — see kDeclickDecay above. A fresh start never declicks.
     void start(int note, int velocity, const SampleData& sample, int rootNote,
                double keyTrack = 1.0,
                const vst::VelocityCurve& velocityCurve = vst::VelocityCurve::flat(),
-               bool unityVarispeedBypass = false);
+               bool unityVarispeedBypass = false,
+               bool declickTakeover = false);
 
     // MONO LEGATO takeover (Phase S): re-pitch this ACTIVE voice to `note` without touching the
     // amplitude envelope, the read position, or the shifter state — pitch moves, no re-attack.
@@ -518,6 +538,18 @@ private:
     PitchShifter shiftL_;
     PitchShifter shiftR_;
 
+    // Takeover declick state (see kDeclickDecay above). lastOut{L,R}_ track the voice's
+    // most recent rendered output (post-gain, incl. any running declick) so a takeover/steal
+    // restart can seed declick{L,R}_ with the exact value the hard cut removed. lastOut is
+    // NOT zeroed by start() — a second same-block takeover (no frame rendered between) must
+    // re-seed from the same pre-cut level, not from a phantom 0. declickActive_ gates the
+    // per-frame add + decay; the declick trio is cleared on a fresh (non-takeover) start.
+    bool declickActive_ = false;
+    double declickL_ = 0.0;
+    double declickR_ = 0.0;
+    double lastOutL_ = 0.0;
+    double lastOutR_ = 0.0;
+
     std::uint64_t startOrder_ = 0;
 };
 
@@ -557,10 +589,19 @@ public:
     // takeover without a re-attack). Both default to today's behavior (Poly / Retrigger). The
     // engine's config is immutable — a mode/count change rebuilds the engine off-thread through
     // the processor's drain-slot reload, so ringing tails survive the swap.
+    //
+    // `takeoverDeclick` (Phase S GA fix): when TRUE, every RESTART of a SOUNDING voice —
+    // the MONO Retrigger takeover, the retrigger fallback on note-off, the cross-sample
+    // legato restart, and the POLY at-cap voice STEAL — seeds the per-voice declick ramp
+    // (see kDeclickDecay) so the hard cut of the old tone does not click. start() self-gates
+    // on the voice being active, so a fresh start (free voice) never ramps. Default FALSE
+    // keeps the bare core byte-identical to the pre-fix engine (regression baseline); the
+    // processor shell opts in — the same layering as the kDefaultPitchEngine product default.
     VoiceEngine(std::size_t maxVoices, const Keymap& keymap,
                 std::size_t preserveVoiceCap = 0, std::int64_t preserveWindowFrames = 0,
                 VoiceMode voiceMode = VoiceMode::Poly,
-                MonoTrigger monoTrigger = MonoTrigger::Retrigger);
+                MonoTrigger monoTrigger = MonoTrigger::Retrigger,
+                bool takeoverDeclick = false);
 
     // MIDI note-on. Resolves the note+velocity to a zone; if none matches (out of
     // zone) it is a defined no-op (no voice consumed). Otherwise allocates a free
@@ -654,6 +695,7 @@ private:
     std::uint64_t nextStartOrder_ = 1;  // monotonic; 0 reserved for "never started"
     VoiceMode voiceMode_ = VoiceMode::Poly;
     MonoTrigger monoTrigger_ = MonoTrigger::Retrigger;
+    bool takeoverDeclick_ = false;  // GA fix: declick every restart/steal of a sounding voice
     std::array<HeldNote, 128> heldStack_{};  // mono held notes, press order; top = heldCount_-1
     std::size_t heldCount_ = 0;
 };