fix(pitch_shift): ratio-scale splice fade so +24st up-shifts never read stale data; normalize SOLA correlation by candidate energy; tests bracket 4x/0.5x + unity/latency asserts

src/vst/pitch_shift.cpp

@@ -40,7 +40,7 @@ void PitchShifter::configure(std::int64_t windowFrames) {
         posA_ = posB_ = 0.0;
         fading_ = false;
         fadePos_ = 0;
-        fadeFrames_ = maxLag_ = corrFrames_ = dLow_ = dHigh_ = 0;
+        fadeFrames_ = fadeLen_ = maxLag_ = corrFrames_ = dLow_ = dHigh_ = 0;
         ratio_ = 1.0;
         return;
     }
@@ -48,17 +48,23 @@ void PitchShifter::configure(std::int64_t windowFrames) {
     ringLen_ = 2 * window_;
     ring_.assign(static_cast<std::size_t>(ringLen_), 0.0f);
     // Geometry (all quarters of the window):
-    //  - fadeFrames_: the splice crossfade — long enough to be smooth, short enough that the
+    //  - fadeFrames_: the NOMINAL splice crossfade. This window/4 length is only safe when
-    //    outgoing tap cannot cross the writer mid-fade for ratios up to ~2x/0.5x.
+    //    the outgoing tap cannot reach the writer before the fade ends; splice() scales the
+    //    live fade length (fadeLen_) down by the current ratio for up-shifts past ~2x, so
+    //    ordinary sampler transpositions (+24 st = ratio 4) never read stale data mid-fade.
     //  - maxLag_: the alignment search half-range — one window/4 covers a full period of any
     //    tone down to 4/window cycles-per-frame (~80 Hz at the product's 50 ms window, 44.1k).
-    //  - corrFrames_: the dot-product length (capped so a splice burst stays bounded).
     //  - dLow_/dHigh_: the safe delay band; unity parks the tap mid-band (window/2 delay).
+    //  - corrFrames_: the correlation segment length. At an up-splice the reference segment
+    //    reads FORWARD from the tap at delay ~dLow_, so dLow_-1 frames is exactly what exists
+    //    between the tap and the writer — the cap expresses that safety rather than leaving
+    //    it coincidental. 512 bounds the splice burst.
     fadeFrames_ = std::max<std::int64_t>(window_ / 4, 1);
     maxLag_ = window_ / 4;
-    corrFrames_ = std::min<std::int64_t>(window_ / 4, 512);
     dLow_ = window_ / 4;
     dHigh_ = ringLen_ - window_ / 4;
+    corrFrames_ = std::max<std::int64_t>(1, std::min<std::int64_t>(dLow_ - 1, 512));
+    fadeLen_ = 0;
     reset();
 }
 
@@ -72,11 +78,13 @@ void PitchShifter::reset() {
         posB_ = posA_;
         fading_ = false;
         fadePos_ = 0;
+        fadeLen_ = 0;
     } else {
         writePos_ = 0;
         posA_ = posB_ = 0.0;
         fading_ = false;
         fadePos_ = 0;
+        fadeLen_ = 0;
     }
     ratio_ = 1.0;
 }
@@ -118,14 +126,22 @@ void PitchShifter::splice(std::int64_t nominalJump) {
     auto scoreAt = [&](std::int64_t lag) -> double {
         std::int64_t ia = iA;
         std::int64_t ic = ((iA - nominalJump + lag) % ringLen_ + ringLen_) % ringLen_;
-        double s = 0.0;
+        double s = 0.0, ec = 0.0;
         for (std::int64_t k = 0; k < corrFrames_; ++k) {
-            s += static_cast<double>(ring_[static_cast<std::size_t>(ia)]) *
+            const double a = static_cast<double>(ring_[static_cast<std::size_t>(ia)]);
-                 static_cast<double>(ring_[static_cast<std::size_t>(ic)]);
+            const double c = static_cast<double>(ring_[static_cast<std::size_t>(ic)]);
+            s += a * c;
+            ec += c * c;
             if (++ia >= ringLen_) ia = 0;
             if (++ic >= ringLen_) ic = 0;
         }
-        return s;
+        // NORMALIZED cross-correlation (standard SOLA): a raw dot product is biased toward
+        // the higher-energy lag, so on a decaying tail every up-splice would prefer the
+        // loudest candidate over the best-ALIGNED one — a small level step per splice that
+        // the amplitude-complementary fade cannot hide. The reference segment's energy is
+        // constant across lags, so dividing by sqrt(Ec) alone ranks identically to the full
+        // normalized form. A zero-energy candidate scores 0 (splicing into silence is benign).
+        return ec > 0.0 ? s / std::sqrt(ec) : 0.0;
     };
 
     std::int64_t bestLag = 0;
@@ -156,6 +172,24 @@ void PitchShifter::splice(std::int64_t nominalJump) {
     while (p < 0.0) p += len;
     while (p >= len) p -= len;
     posA_ = p;
+    // RATIO-SCALED fade length. At an up-splice the OUTGOING tap starts at ~dLow_ delay and
+    // keeps draining toward the writer at (ratio - 1) per output frame; the nominal window/4
+    // fade only keeps it behind the writer for ratios up to 2. Beyond that (e.g. +24 st =
+    // ratio 4, an ordinary sampler transposition) it would cross mid-fade and play stale
+    // read-ahead data at substantial gain — a periodic seam. So cap the live fade at the
+    // frames of drain headroom actually available, minus 2 (1 for the trigger's sub-dLow_
+    // undershoot, 1 for the interpolator's read-ahead). Ratios <= ~2 keep the full nominal
+    // fade; ratio 4 gets ~window/12 — shorter but still a smooth burst. Down-shifts grow the
+    // outgoing delay at (1 - ratio) < 1 per frame and cannot reach the ring end within
+    // window/4 frames, so they always keep the full fade. A pitch-envelope ratio slew
+    // mid-fade is covered by the same margin for any realistic per-frame bias.
+    fadeLen_ = fadeFrames_;
+    if (ratio_ > 1.0) {
+        const double headroom = static_cast<double>(dLow_) - (ratio_ - 1.0) - 2.0;
+        const std::int64_t safe =
+            headroom > 0.0 ? static_cast<std::int64_t>(headroom / (ratio_ - 1.0)) : 1;
+        fadeLen_ = std::max<std::int64_t>(1, std::min(fadeFrames_, safe));
+    }
     fading_ = true;
     fadePos_ = 0;
 }
@@ -171,10 +205,10 @@ AudioSample PitchShifter::process(AudioSample in) {
     //    in phase, so the sum holds unity amplitude through the fade (equal-power would bulge).
     double out = readTap(posA_);
     if (fading_) {
-        const double t = static_cast<double>(fadePos_) / static_cast<double>(fadeFrames_);
+        const double t = static_cast<double>(fadePos_) / static_cast<double>(fadeLen_);
         const double gNew = 0.5 * (1.0 - std::cos(kPi * t));
         out = gNew * out + (1.0 - gNew) * readTap(posB_);
-        if (++fadePos_ >= fadeFrames_) fading_ = false;
+        if (++fadePos_ >= fadeLen_) fading_ = false;
     } else {
         // 3. Splice scheduling: relocate when the active tap's delay leaves the safe band.
         //    Up-shifts (ratio > 1) drain the delay toward 0 -> jump one window OLDER; down-

src/vst/pitch_shift.h

@@ -105,10 +105,15 @@ private:
     double posA_ = 0.0;              // active read tap (advances at the shift ratio)
     double posB_ = 0.0;              // outgoing tap during a splice crossfade
     bool fading_ = false;            // a splice crossfade is in flight
-    std::int64_t fadePos_ = 0;       // crossfade progress, [0, fadeFrames_)
+    std::int64_t fadePos_ = 0;       // crossfade progress, [0, fadeLen_)
-    std::int64_t fadeFrames_ = 0;    // crossfade length (window_/4)
+    std::int64_t fadeFrames_ = 0;    // NOMINAL crossfade length (window_/4)
+    std::int64_t fadeLen_ = 0;       // LIVE crossfade length for the in-flight splice —
+                                     // ratio-scaled at splice time so an up-shift's outgoing
+                                     // tap can never drain into the writer mid-fade
     std::int64_t maxLag_ = 0;        // correlation search half-range (window_/4)
-    std::int64_t corrFrames_ = 0;    // correlation dot-product length (window_/4, capped)
+    std::int64_t corrFrames_ = 0;    // correlation segment length (dLow_-1, capped at 512, so
+                                     // the reference read forward from the tap stays behind
+                                     // the writer BY CONSTRUCTION at an up-splice)
     std::int64_t dLow_ = 0;          // splice trigger: active-tap delay below this (up-shift)
     std::int64_t dHigh_ = 0;         // splice trigger: active-tap delay above this (down-shift)
     double ratio_ = 1.0;             // current shift ratio (>0)

tests/test_pitch_shift.cpp

@@ -16,7 +16,11 @@
 //      SINGLE tone at the shifted frequency: near-total least-squares fit to the shifted
 //      sinusoid, and no deep amplitude beating across the run. This is the test that fails on
 //      any splice/crossfade phase-alignment defect (the DAW "multiple partials from a sine"
-//      report).
+//      report). Ratios bracket the real playable range: +24 st (ratio 4 — the geometry-fix
+//      target where an unscaled fade reads stale data) and a full octave down included.
+//   6. unity contract — the header's two hard claims, asserted bit-exactly: at ratio 1.0 the
+//      shifter IS a clean window/2 delay (out[i] == in[i - w/2] to the bit; no splice, no
+//      interpolation error), which is simultaneously the latency == window/2 assertion.
 
 #include "../src/vst/pitch_shift.h"
 
@@ -187,9 +191,13 @@ static void testRepitchSpectralPurity() {
     // output a single sinusoid at ratio*f0 with a steady amplitude.
     const std::int64_t w = 2205;              // ~50 ms @ 44.1k (the product window)
     const double f0 = 0.005;                  // source: period 200 samples
-    const double ratios[] = {std::pow(2.0, 2.0 / 12.0),   // +2 semitones (the DAW report: D from C)
+    const double ratios[] = {std::pow(2.0, 2.0 / 12.0),    // +2 semitones (the DAW report: D from C)
-                             std::pow(2.0, -3.0 / 12.0),  // -3 semitones (down-shift path)
+                             std::pow(2.0, -3.0 / 12.0),   // -3 semitones (down-shift path)
-                             2.0};                        // octave up (fastest splice cadence)
+                             2.0,                          // octave up (nominal-fade boundary)
+                             std::pow(2.0, 24.0 / 12.0),   // +24 st: ratio 4 — the ratio-scaled-
+                                                           // fade target (unscaled fade would
+                                                           // read stale data at ~75% gain)
+                             std::pow(2.0, -12.0 / 12.0)}; // octave down (full down-shift path)
     for (double r : ratios) {
         PitchShifter ps;
         ps.configure(w);
@@ -233,7 +241,14 @@ static void testRepitchSpectralPurity() {
         CHECK(residRms < 0.1 * fitRms);   // >=99% of the energy in the ONE shifted tone
 
         // No beating: sliding-window RMS must not dip (the old design dipped to ~13% of peak).
-        const std::size_t win = 2000, hop = 1000;
+        // The window must RESOLVE a within-fade dip (the ratio-4 fade is only ~w/12 = 183
+        // frames; the original win=2000 averaged straight over total cancellation), yet a
+        // window that is not an integer number of output periods has phase-dependent RMS on a
+        // pure sine (at ratio 0.5 the output period is 400 frames — a fixed 256 window dips
+        // to ~0.78 of max on the CLEAN signal alone). Smallest phase-clean choice: exactly one
+        // output period per window (50..400 frames here), hop of half a window.
+        const std::size_t win = static_cast<std::size_t>(std::lround(1.0 / f1));
+        const std::size_t hop = win / 2;
         double minRms = 1e9, maxRms = 0.0;
         for (std::size_t s0 = from; s0 + win <= n; s0 += hop) {
             double e = 0.0;
@@ -247,12 +262,42 @@ static void testRepitchSpectralPurity() {
     }
 }
 
+// --- 6. Unity contract: bit-exact window/2 delay == the latency claim. ---
+static void testUnityBitExactAndLatency() {
+    // The header claims a configured shifter at ratio 1.0 is a CLEAN window/2 delay: the tap
+    // is parked mid-band (no splice ever fires) at an integral delay (no interpolation error),
+    // so every output equals the input from exactly w/2 frames earlier TO THE BIT. This is
+    // simultaneously the latency assertion: steady-state latency == window/2, no more, no
+    // less. warm() has already consumed the cold-start region, so the first w/2 outputs are
+    // the tail of the warm-up silence and everything after is the delayed input verbatim.
+    const std::int64_t w = 2205;                 // the product window (odd: w/2 truncates)
+    const std::int64_t lat = w / 2;              // 1102
+    PitchShifter ps;
+    ps.configure(w);
+    ps.warm();
+    ps.setShiftRatio(1.0);
+    const std::size_t n = 6000;
+    const std::vector<AudioSample> in = sine(n, 37.0);
+    std::vector<AudioSample> out(n);
+    for (std::size_t i = 0; i < n; ++i) out[i] = ps.process(in[i]);
+    std::size_t badSilence = 0, badDelay = 0;
+    for (std::size_t i = 0; i < static_cast<std::size_t>(lat); ++i) {
+        if (out[i] != 0.0f) ++badSilence;        // pre-latency region: warm-up silence, exact
+    }
+    for (std::size_t i = static_cast<std::size_t>(lat); i < n; ++i) {
+        if (out[i] != in[i - static_cast<std::size_t>(lat)]) ++badDelay;  // bit-exact delay
+    }
+    CHECK(badSilence == 0);
+    CHECK(badDelay == 0);
+}
+
 int main() {
     testDurationInvariance();
     testUnityRoughlyReproduces();
     testTransposeDirection();
     testRtDisciplineAndPassthrough();
     testRepitchSpectralPurity();
+    testUnityBitExactAndLatency();
 
     if (g_fail == 0) {
         std::printf("all pitch_shift tests passed\n");