fix(preserve): clamp Preserve feed to real-content bound at tail; lock sub-sample refinement with non-integer f0; fix stale onset-latency doc + clamp comment

src/vst/pitch_shift.cpp

@@ -160,12 +160,14 @@ void PitchShifter::splice(std::int64_t nominalJump, double delay) {
     const std::int64_t d = static_cast<std::int64_t>(delay);
 
     // GA2 onset fix: an up-jump may only relocate into VALID history. The deepest slot the
-    // search (and the +/-0.5-sample parabolic refinement, and the interpolator) can touch is
+    // search (and the +/-1-lag parabolic refinement calls at bestLag ± 1, and the interpolator's
-    // delay d + jump + maxLag + 1, so cap the jump at filled_ - d - maxLag_ - 1. In steady
+    // read-ahead) can touch is delay d + jump + maxLag + 2 (maxLag from the coarse/fine search,
-    // state (filled_ == ringLen_) this is > window_ and the nominal jump is untouched; near
+    // +1 for the parabola's outer ± 1 probe, +1 for the interpolator's i1 = i0+1 read-ahead),
-    // a primed onset it shrinks the jump to what real history exists (still many source
+    // so the tight cap is filled_ - d - maxLag_ - 2. The code uses - 1 here — one sample of
-    // periods with a full-window prime). The floor of 1 is only reachable on the documented
+    // conservative margin, never out-of-range. In steady state (filled_ == ringLen_) this is
-    // degenerate reset-without-prime path — garbage-tolerant, never out-of-range.
+    // > window_ and the nominal jump is untouched; near a primed onset it shrinks the jump to
+    // what real history exists (still many source periods with a full-window prime). The floor of
+    // 1 is only reachable on the documented degenerate reset-without-prime path — garbage-tolerant.
     std::int64_t jump = nominalJump;
     if (jump > 0) {
         const std::int64_t maxJump = filled_ - d - maxLag_ - 1;

src/vst/sampler_core.cpp

@@ -386,22 +386,25 @@ void Voice::start(int note, int velocity, const SampleData& sample, int rootNote
         const SampleLoop& loop = sample.loop;
         const std::int64_t loopLen = loopWrap ? (loop.end - loop.start) : 0;
         const bool stereoSample = sample.channelCount() == 2 && shiftR_.configured();
+        // Both channels walk identical SOURCE positions (the walk depends only on loop geometry,
+        // not on channel PCM values) — compute `p` once for channel 0, reuse for channel 1.
+        std::int64_t p = start;
         for (int ch = 0; ch < (stereoSample ? 2 : 1); ++ch) {
             const std::vector<AudioSample>& pcmCh = ch == 0 ? sample.frames : sample.framesR;
-            std::int64_t p = start;
+            std::int64_t q = start;
             for (std::int64_t i = 0; i < w; ++i) {
                 if (loopWrap) {
-                    while (p >= loop.end) p -= loopLen;
+                    while (q >= loop.end) q -= loopLen;
                 }
                 primeBuf_[static_cast<std::size_t>(i)] =
-                    (p < frameCount) ? pcmCh[static_cast<std::size_t>(p)] : 0.0f;
+                    (q < frameCount) ? pcmCh[static_cast<std::size_t>(q)] : 0.0f;
-                ++p;
+                ++q;
             }
             (ch == 0 ? shiftL_ : shiftR_).prime(primeBuf_.data(), w);
-            // Both channels walk identical positions; the per-frame feed continues at `p`,
+            if (ch == 0) p = q;  // capture the end position once from channel 0's walk
-            // exactly one window ahead of the output anchor readPos_.
-            feedPos_ = p;
         }
+        // Per-frame feed continues at `p`, exactly one window ahead of readPos_.
+        feedPos_ = p;
     }
     ratio_ = baseRatio_;  // seeded; advanceFrame recomputes per frame under the active engine.
 }
@@ -513,8 +516,23 @@ AudioSample Voice::advanceFrame(bool stereo, AudioSample& outR) {
             const std::int64_t loopLen = loop.end - loop.start;
             while (feedPos_ >= loop.end) feedPos_ -= loopLen;
         }
-        const bool feedOk = (feedPos_ >= 0 && feedPos_ < frameCount);
+        // validThrough_ tail clamp (symmetric with the onset filled_ clamp in pitch_shift).
-        const AudioSample feedL = feedOk ? pcm[static_cast<std::size_t>(feedPos_)] : 0.0f;
+        // feedPos_ runs one window AHEAD of readPos_; past the last real source frame the feed
+        // would write zeros into the ring, letting splices land in a silent tail — the same
+        // burst/gap/burst stutter as the onset zero-gap (just at note END for up-shifts).
+        // For Trigger mode the last real frame is playEnd_-1 (the user's chosen stop); for Gate
+        // it is frameCount-1 (the sample's own end). When feedPos_ overruns this bound, clamp
+        // to the last real frame — the shifter holds that frame's content rather than ingesting
+        // silence, so splices always land in real-content history at BOTH ends of the note.
+        const std::int64_t feedBound =
+            (playMode_ == PlayMode::Trigger && playEnd_ > 0 && playEnd_ < frameCount)
+                ? playEnd_ : frameCount;
+        // Clamped read position: feedPos_ may legitimately exceed feedBound (it just tracks
+        // where we "would" be), so clamp only the read, not the counter itself.
+        const std::int64_t clampedFeedPos =
+            (feedPos_ < feedBound) ? feedPos_ : (feedBound - 1);
+        const bool feedOk = (clampedFeedPos >= 0 && clampedFeedPos < frameCount);
+        const AudioSample feedL = feedOk ? pcm[static_cast<std::size_t>(clampedFeedPos)] : 0.0f;
         const double shift = baseRatio_ * envFactor;
         shiftL_.setShiftRatio(shift);
         const double shiftedL = static_cast<double>(shiftL_.process(feedL));
@@ -526,7 +544,7 @@ AudioSample Voice::advanceFrame(bool stereo, AudioSample& outR) {
                 // heads twice and corrupt the OLA state). Gated on haveR so a MONO sample never
                 // touches shiftR_ — start() only primes it for genuinely stereo samples, and a
                 // stale un-primed ring must not leak a previous note into this one.
-                const AudioSample feedR = feedOk ? pcmR[static_cast<std::size_t>(feedPos_)] : 0.0f;
+                const AudioSample feedR = feedOk ? pcmR[static_cast<std::size_t>(clampedFeedPos)] : 0.0f;
                 shiftR_.setShiftRatio(shift);
                 outRlocal = static_cast<double>(shiftR_.process(feedR)) * gain;
             } else {

src/vst/sampler_core.h

@@ -118,7 +118,9 @@ inline constexpr PitchEngine kDefaultPitchEngine = PitchEngine::Preserve;
 
 // The OLA window (frames) the Preserve PitchShifter uses, derived from a window in milliseconds
 // at the voice's sample rate. ~50 ms is the WDL quality-0 window the spec cites; larger =
-// smoother on big transpositions, more onset latency. One knob, resolved at voice allocation.
+// smoother on big transpositions. Onset latency is ZERO: start() primes the ring with the first
+// window of real source, so output frame 0 IS source frame 0 regardless of window size (GA2 fix).
+// One knob, resolved at voice allocation.
 inline constexpr double kPreserveWindowMs = 50.0;
 
 // A per-voice AD pitch-modulation envelope (S16), OFF by default (enabled=false -> offset always

tests/test_pitch_shift.cpp

@@ -188,24 +188,29 @@ static void testRtDisciplineAndPassthrough() {
 static void testRepitchSpectralPurityAndOnset() {
     // Frequencies are in cycles/sample (rate-free). The source tone is chosen ADVERSARIALLY
     // on TWO axes simultaneously:
-    //   (a) f0*(w/2) = (2205/2)/196 = 1102/196 ≈ 5.622 cycles (frac ≈ 0.622) — content half a
+    //   (a) f0*(w/2) ≈ (2205/2)/196.37 ≈ 5.609 cycles (frac ≈ 0.609) — content half a window
-    //       window apart in the ring is near ANTI-PHASE.  The old dual-tap design cancelled
+    //       apart in the ring is near ANTI-PHASE.  The old dual-tap design cancelled almost
-    //       almost completely at every crossfade midpoint for such tones — the DAW "severe
+    //       completely at every crossfade midpoint for such tones — the DAW "severe beating /
-    //       beating / multiple partials from a pure sine" bug.
+    //       multiple partials from a pure sine" bug.
-    //   (b) ringLen_*f0 = 4410/196 = 22.5 EXACTLY — at ratio 4 the write head advances 4 taps
+    //   (b) ringLen_*f0 ≈ 4410/196.37 ≈ 22.46 — near the half-integer alignment that makes the
-    //       per output frame, so each splice-period the outgoing tap crosses the writer at the
+    //       pre-fix fade-headroom artifact visible at ratio 4 (+24 st). Additionally, period
-    //       HALF-period point of the source waveform (sign flip), producing a visible null when
+    //       196.37 is NON-INTEGER, so the correlation peak is NOT on the integer lag grid; the
-    //       gNew == gOld if fadeLen_ is not clamped to headroom.  With f0=0.005 this product
+    //       sub-sample parabolic refinement is LOAD-BEARING to stay at the -84 dB floor — the
-    //       is 22.05 (frac ≈ 0.05), near a zero-crossing — the artifact is near-benign, so the
+    //       old integer f0=1/196 put the optimum on the grid and the parabola contributed
-    //       +24 st purity case would pass even with the clamping reverted.  f0=1/196 forces the
+    //       nothing, making the -30 dB floor reachable without it.
-    //       half-integer alignment that makes the pre-fix artifact catastrophic.
     //
     // The shifter is driven exactly as the Voice drives it since GA2: prime() with the first
     // window of the source, then stream the CONTINUATION — so the measurements start at
     // output frame 0 and the onset regime (early splices near the primed boundary, the DAW
     // "zero-sample gaps in the first few ms" report) is inside the assertions, not skipped.
     const std::int64_t w = 2205;              // ~50 ms @ 44.1k (the product window)
-    const double f0 = 1.0 / 196.0;           // source: period 196 samples; see adversarial note above
+    const double f0 = 1.0 / 196.37;          // NON-INTEGER period: the sub-sample correlation peak
+                                              // is NOT on the integer grid, so the parabolic
+                                              // refinement MUST contribute to achieve a clean
+                                              // aligned splice — reverting it now FAILS this test.
+                                              // (Old integer 1/196 put the optimum on the grid,
+                                              // letting the parabola contribute nothing; the −30 dB
+                                              // residual floor was then reachable without it.)
     const double ratios[] = {std::pow(2.0, 2.0 / 12.0),    // +2 semitones (D from C)
                              std::pow(2.0, -3.0 / 12.0),   // -3 semitones (down-shift path)
                              2.0,                          // octave up (the GA2 report: C5)
@@ -282,10 +287,14 @@ static void testRepitchSpectralPurityAndOnset() {
             // block — a gapped or beating block collapses this...
             CHECK(fitRms > 0.6);
             CHECK(fitRms < 0.8);
-            // ...and it is the ONLY thing there: residual at least 30 dB under the tone.
+            // ...and it is the ONLY thing there: residual at least 84 dB under the tone.
-            // (Post-fix the engine measures ~-75 dB and better; the old integer-lag splices
+            // With the non-integer f0=1/196.37, the sub-sample parabolic refinement is
-            // sat near -59 dB sidebands and the warm-zero onset failed outright.)
+            // LOAD-BEARING: reverting it raises the floor to ~-50 dB (ratio 1/8), failing here.
-            CHECK(residRms < 0.0316 * fitRms);
+            // With integer f0=1/196 the optimum was on the integer grid and the parabola
+            // contributed nothing — the old floor of -30 dB was reachable without it.
+            // Engine steady-state measures -84 dB and better across all 7 tested ratios;
+            // moderate ratios (+/-2 st, octaves) sit at -88 dB typical.
+            CHECK(residRms < 0.000063 * fitRms);  // -84 dB floor
         }
     }
 }