From 4f7e15390f220b9ff08ee3f75efccdc41f3a0230 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 28 Jul 2026 10:09:23 -0400 Subject: [PATCH] fix(pitch_shift): re-anchor fadePos_ on freeze so gNew is continuous at the freeze frame; guard left cast; add step-detector + loop-never-freezes regression --- src/vst/pitch_shift.cpp | 18 +++++++++++++--- tests/test_pitch_shift.cpp | 41 ++++++++++++++++++++++++++++++++++++- tests/test_sampler_core.cpp | 10 +++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/vst/pitch_shift.cpp b/src/vst/pitch_shift.cpp index 682752e..2fd34fc 100644 --- a/src/vst/pitch_shift.cpp +++ b/src/vst/pitch_shift.cpp @@ -105,15 +105,27 @@ void PitchShifter::freezeTail() { // An in-flight crossfade was sized for a RETREATING writer (outgoing tap drains at // ratio-1 per frame); frozen, the outgoing tap closes at the full ratio. Cap the live // fade so it completes before tap B reaches the parked writer and reads lapped (oldest- - // window) content mid-fade. `+1` keeps fadeLen_ > fadePos_, so t stays < 1 in process(). + // window) content mid-fade. fadePos_ is re-anchored to the same fractional t so gNew is + // continuous at the freeze frame (no gain step); see the re-anchor block below. if (fading_) { + // Preserve t = fadePos_/fadeLen_ across the shortening so gNew is continuous at the + // freeze frame (no gain step). Compute tOld BEFORE overwriting fadeLen_, then + // re-anchor fadePos_ to the same fractional position in the new (shorter) fade. + const double tOld = + static_cast(fadePos_) / static_cast(fadeLen_); double dB = static_cast(writePos_) - posB_; const double len = static_cast(ringLen_); while (dB < 0.0) dB += len; while (dB >= len) dB -= len; - const double left = (dB - 2.0) / ratio_; + // Clamp in double before the int64 cast (matches splice() pattern; guards against UB + // when dB/ratio_ is very large, e.g. near-unity ratio at a high sample rate). + double left = (dB - 2.0) / ratio_; + if (left > static_cast(fadeFrames_)) left = static_cast(fadeFrames_); const std::int64_t leftFrames = left > 1.0 ? static_cast(left) : 1; - fadeLen_ = std::min(fadeLen_, fadePos_ + leftFrames); + const std::int64_t newFadeLen = std::min(fadeLen_, fadePos_ + leftFrames); + // Re-anchor: tOld < 1 because we are mid-fade, so newFadePos < newFadeLen (still fading). + fadePos_ = static_cast(tOld * static_cast(newFadeLen)); + fadeLen_ = newFadeLen; } } diff --git a/tests/test_pitch_shift.cpp b/tests/test_pitch_shift.cpp index d036d4d..b913993 100644 --- a/tests/test_pitch_shift.cpp +++ b/tests/test_pitch_shift.cpp @@ -421,8 +421,9 @@ static void testFreezeTailContinuousTone() { ps.prime(src.data(), w); ps.setShiftRatio(2.0); const std::size_t preFreeze = static_cast(3 * w / 4 + w / 8); + double lastPre = 0.0; for (std::size_t i = 0; i < preFreeze; ++i) { - (void)ps.process(src[i + static_cast(w)]); + lastPre = static_cast(ps.process(src[i + static_cast(w)])); } ps.freezeTail(); std::size_t worstGap = 0, run = 0; @@ -450,6 +451,44 @@ static void testFreezeTailContinuousTone() { } CHECK(badZeroLat == 0); } + + // STEP-DETECTOR: freeze-transition continuity using a ramp source where tapA and tapB + // read values that differ by a predictable constant (≈ A * w / N), making the crossfade + // gain step directly visible in the output. With a ramp, the per-frame natural change is + // A/(N) * ratio ≈ 0.0002 per frame; the un-fixed gain step is ~0.247 * (w/N) ≈ 0.062 — + // roughly 300x the natural rate. A threshold of 0.02 clearly separates fixed from unfixed. + // + // The ramp also defeats correlation-alignment (all lags score equally on a linear ramp), + // so the splice jump of one window guarantees tapA - tapB = −A*w/N regardless of lag. + { + PitchShifter ps; + ps.configure(w); + // Ramp from 0.0 to 1.0 over 4*w frames (same buffer size as the mid-crossfade case). + const std::size_t rampLen = 4 * static_cast(w); + std::vector ramp(rampLen); + for (std::size_t i = 0; i < rampLen; ++i) { + ramp[i] = static_cast(static_cast(i) / + static_cast(rampLen - 1)); + } + ps.prime(ramp.data(), w); + ps.setShiftRatio(2.0); + // Drive to the deterministic mid-fade freeze point: same preFreeze offset as above. + const std::size_t preFreeze = static_cast(3 * w / 4 + w / 8); + double lastPre = 0.0; + for (std::size_t i = 0; i < preFreeze; ++i) { + lastPre = static_cast( + ps.process(ramp[i + static_cast(w)])); + } + ps.freezeTail(); + // First frozen frame — if gNew steps at the freeze boundary the output jumps by + // ~deltaGain * (tapA - tapB) ≈ 0.247 * 0.25 = 0.062. + const double firstFrozen = static_cast(ps.process(0.0f)); + CHECK(std::isfinite(firstFrozen)); + // Natural per-frame ramp advance at ratio 2 ≈ 2/(4*w - 1) ≈ 0.0002; the un-fixed + // step is ~0.062. Threshold 0.02 is 100x the natural rate but well below the step. + const double transitionStep = std::fabs(firstFrozen - lastPre); + CHECK(transitionStep < 0.02); + } } int main() { diff --git a/tests/test_sampler_core.cpp b/tests/test_sampler_core.cpp index d282912..dcb1eb4 100644 --- a/tests/test_sampler_core.cpp +++ b/tests/test_sampler_core.cpp @@ -1318,6 +1318,16 @@ static void testPreserveGateStereoLoopComposes() { } CHECK(maxL > 0.05); CHECK(maxR > 0.02); // R present (half amplitude), distinct from L -> stereo preserved + // Loop-never-freezes regression: the Preserve voice must NOT spuriously freeze when the + // read tap hits the sample end and the loop wraps it back. A spuriously frozen voice + // stops writing to the ring and the looped tail would go silent past the sample end. + // Check a block far past the sample end (sample = 400 frames; window = 512; well past + // any single-pass tail region) to catch any wrap-before-exhaustion ordering error. + double maxLFar = 0.0; + for (std::size_t i = 1800; i < 2000; ++i) { + if (std::fabs(left[i]) > maxLFar) maxLFar = std::fabs(left[i]); + } + CHECK(maxLFar > 0.05); // still alive at frame 1800 (4.5× the 400-frame sample length) } // --- Preserve voice cap: a Preserve note-on past the cap is dropped; Varispeed unaffected. ---