From 22d7893431a6942d9e8bb3aca83a782a15e71bd5 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 28 Jul 2026 06:19:26 -0400 Subject: [PATCH 1/3] =?UTF-8?q?fix(preserve):=20correlation-aligned=20spli?= =?UTF-8?q?ces=20replace=20dual-tap=20OLA=20=E2=80=94=20fixed=20w/2=20tap?= =?UTF-8?q?=20offset=20anti-phase-cancelled=20crossfades=20(beating/partia?= =?UTF-8?q?ls=20on=20repitched=20sines);=20spectral-purity=20test=20added?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/vst/pitch_shift.cpp | 218 +++++++++++++++++++++++++------------ src/vst/pitch_shift.h | 75 +++++++++---- src/vst/sampler_core.cpp | 4 +- tests/test_pitch_shift.cpp | 76 +++++++++++++ 4 files changed, 280 insertions(+), 93 deletions(-) diff --git a/src/vst/pitch_shift.cpp b/src/vst/pitch_shift.cpp index d957543..26b03d1 100644 --- a/src/vst/pitch_shift.cpp +++ b/src/vst/pitch_shift.cpp @@ -1,34 +1,32 @@ -// pitch_shift — pure implementation. See pitch_shift.h for the contract and the S16-F2 -// route-(b) rationale (WDL drags , so the Preserve DSP is house-native here). +// pitch_shift — pure implementation. See pitch_shift.h for the contract, the S16-F2 +// route-(b) rationale (WDL drags ), and the GA-Preserve root cause that replaced +// the naive dual-tap OLA with correlation-aligned splices. // NO VST3 / REAPER / SWELL / vendor includes; standard library only. // -// Algorithm: a single delay ring of `window_` frames. The write head advances one frame per -// input sample (source rate → duration preserved). TWO read taps chase the write head, offset -// by half a window; each advances by the shift `ratio_` per frame. A tap that would cross the -// write head wraps by a full window (so it stays a bounded delay behind the writer). The two -// taps are crossfaded by an equal-power window keyed to each tap's distance from the write -// head, so the wrap discontinuity of one tap is masked by the other mid-window — the classic -// two-grain time-domain pitch shifter, no FFT. +// Algorithm: a delay ring of 2*window frames. The write head advances one frame per input +// sample (source rate -> duration preserved). ONE active read tap advances by the shift +// `ratio_` per frame, so its delay behind the writer drifts at (1 - ratio) per frame. When +// that delay leaves the safe band [dLow, dHigh], the tap is RELOCATED by a nominal jump of +// one window (+window toward older content for up-shifts, -window toward the writer for +// down-shifts), refined by a cross-correlation search over +/- maxLag so the relocated read +// point is WAVEFORM-ALIGNED with what the outgoing tap was about to play. Old and new taps +// then crossfade over fadeFrames with a raised-cosine, amplitude-complementary pair (in-phase +// content sums to exactly unity gain). For a pure sine the correlation snaps the jump to an +// integer period count, so the output stays a single tone at the shifted frequency — the +// GA-Preserve acceptance bar. At unity ratio the delay is frozen mid-band and no splice ever +// fires: the shifter is a clean window/2 delay. #include "pitch_shift.h" #include #include +#include namespace reasampler { namespace { -// A Hann OLA window over a grain phase in [0,1): 0.5(1 - cos(2*pi*phase)). Zero at the grain -// ends (where a tap wraps — the discontinuity), unity mid-grain. Two grains offset by half a -// window PARTITION UNITY (w(p) + w(p+0.5) == 1 for all p), so the two crossfaded taps sum to a -// gain of exactly 1 everywhere — no amplitude ripple across the window, and each tap's wrap -// seam is masked because its window is 0 exactly there. -double hannWeight(double phase) { - while (phase < 0.0) phase += 1.0; - while (phase >= 1.0) phase -= 1.0; - return 0.5 * (1.0 - std::cos(2.0 * 3.14159265358979323846 * phase)); -} +constexpr double kPi = 3.14159265358979323846; } // namespace @@ -37,37 +35,129 @@ void PitchShifter::configure(std::int64_t windowFrames) { if (window_ <= 1) { // Pass-through: no ring, process() returns input unchanged. ring_.clear(); + ringLen_ = 0; writePos_ = 0; - readPos_ = 0.0; + posA_ = posB_ = 0.0; + fading_ = false; + fadePos_ = 0; + fadeFrames_ = maxLag_ = corrFrames_ = dLow_ = dHigh_ = 0; ratio_ = 1.0; return; } - ring_.assign(static_cast(window_), 0.0f); + // 2x-window ring: one window of splice-jump span plus search + fade headroom on each side. + ringLen_ = 2 * window_; + ring_.assign(static_cast(ringLen_), 0.0f); + // Geometry (all quarters of the window): + // - fadeFrames_: the splice crossfade — long enough to be smooth, short enough that the + // outgoing tap cannot cross the writer mid-fade for ratios up to ~2x/0.5x. + // - 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). + fadeFrames_ = std::max(window_ / 4, 1); + maxLag_ = window_ / 4; + corrFrames_ = std::min(window_ / 4, 512); + dLow_ = window_ / 4; + dHigh_ = ringLen_ - window_ / 4; reset(); } void PitchShifter::reset() { if (window_ > 1) { - // Zero the ring and seed the read head a half-window behind the writer so the two taps - // (readPos_ and readPos_ + window/2) straddle the writer from the first frame. + // Zero the ring and seed the active tap half a window behind the writer — mid safe + // band, so unity holds it there forever and either shift direction has drift room. std::fill(ring_.begin(), ring_.end(), 0.0f); writePos_ = 0; - readPos_ = static_cast(window_) / 2.0; + posA_ = static_cast(ringLen_ - window_ / 2); + posB_ = posA_; + fading_ = false; + fadePos_ = 0; } else { writePos_ = 0; - readPos_ = 0.0; + posA_ = posB_ = 0.0; + fading_ = false; + fadePos_ = 0; } ratio_ = 1.0; } void PitchShifter::warm() { if (window_ <= 1) return; // pass-through needs no warm-up - // Push one full window of silence so the taps reach steady state before real audio. + // Push one full window of silence so the tap reaches steady state before real audio. for (std::int64_t i = 0; i < window_; ++i) process(0.0f); } void PitchShifter::setShiftRatio(double ratio) { - if (ratio > 0.0) ratio_ = ratio; // ignore non-positive (never run taps backward/stall) + if (ratio > 0.0) ratio_ = ratio; // ignore non-positive (never run the tap backward/stall) +} + +double PitchShifter::readTap(double pos) const { + // Fractional linear interpolation with ring wrap. + double p = pos; + const double len = static_cast(ringLen_); + while (p < 0.0) p += len; + while (p >= len) p -= len; + const std::int64_t i0 = static_cast(p); + const double frac = p - static_cast(i0); + std::int64_t i1 = i0 + 1; + if (i1 >= ringLen_) i1 = 0; + const double s0 = static_cast(ring_[static_cast(i0)]); + const double s1 = static_cast(ring_[static_cast(i1)]); + return s0 + (s1 - s0) * frac; +} + +void PitchShifter::splice(std::int64_t nominalJump) { + // Relocate the active tap by `nominalJump` frames of ADDED delay (+window_ = jump toward + // older content, -window_ = jump toward the writer), refined by a correlation search so + // the relocated read point is waveform-aligned with the outgoing tap's upcoming content. + // The search is coarse (step 4 over +/- maxLag_) then fine (+/- 3 around the coarse best): + // a bounded burst of ~ (maxLag_/2 + 7) * corrFrames_ multiply-adds, once per splice. + const std::int64_t iA = + ((static_cast(posA_) % ringLen_) + ringLen_) % ringLen_; + + 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; + for (std::int64_t k = 0; k < corrFrames_; ++k) { + s += static_cast(ring_[static_cast(ia)]) * + static_cast(ring_[static_cast(ic)]); + if (++ia >= ringLen_) ia = 0; + if (++ic >= ringLen_) ic = 0; + } + return s; + }; + + std::int64_t bestLag = 0; + double bestScore = -std::numeric_limits::infinity(); + for (std::int64_t lag = -maxLag_; lag <= maxLag_; lag += 4) { + const double s = scoreAt(lag); + if (s > bestScore) { + bestScore = s; + bestLag = lag; + } + } + const std::int64_t coarse = bestLag; + for (std::int64_t lag = coarse - 3; lag <= coarse + 3; ++lag) { + if (lag == coarse || lag < -maxLag_ || lag > maxLag_) continue; + const double s = scoreAt(lag); + if (s > bestScore) { + bestScore = s; + bestLag = lag; + } + } + + // Hand the current position to the outgoing tap and relocate the active one. Integer lag + // on top of the nominal jump preserves posA_'s fractional part — sub-sample continuity + // between the two taps, so the residual phase error is bounded by half a sample. + posB_ = posA_; + double p = posA_ - static_cast(nominalJump) + static_cast(bestLag); + const double len = static_cast(ringLen_); + while (p < 0.0) p += len; + while (p >= len) p -= len; + posA_ = p; + fading_ = true; + fadePos_ = 0; } AudioSample PitchShifter::process(AudioSample in) { @@ -76,49 +166,41 @@ AudioSample PitchShifter::process(AudioSample in) { // 1. Write the incoming sample at the write head (source rate). ring_[static_cast(writePos_)] = in; - const double w = static_cast(window_); - const double half = w / 2.0; + // 2. Read the active tap; while a splice fade is live, crossfade against the outgoing tap. + // Raised-cosine COMPLEMENTARY gains (gNew + gOld == 1): correlation-aligned content is + // 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(fadePos_) / static_cast(fadeFrames_); + const double gNew = 0.5 * (1.0 - std::cos(kPi * t)); + out = gNew * out + (1.0 - gNew) * readTap(posB_); + if (++fadePos_ >= fadeFrames_) 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- + // shifts grow it toward the ring length -> jump one window TOWARD the writer. At + // unity the delay is frozen at window/2 and neither trigger ever fires. + double d = static_cast(writePos_) - posA_; + const double len = static_cast(ringLen_); + while (d < 0.0) d += len; + while (d >= len) d -= len; + if (d <= static_cast(dLow_)) { + splice(+window_); + } else if (d >= static_cast(dHigh_)) { + splice(-window_); + } + } - // 2. Read the two taps, each a bounded delay behind the writer. tap0 is `readPos_`; tap1 is - // a half-window ahead of it (mod window). Distance-from-writer drives the crossfade so a - // tap near the writer (about to wrap) is faded out while its partner (mid-window) is up. - auto readTap = [&](double pos) -> double { - // Fractional linear interpolation with ring wrap. - double p = pos; - while (p < 0.0) p += w; - while (p >= w) p -= w; - const std::int64_t i0 = static_cast(p); - const double frac = p - static_cast(i0); - std::int64_t i1 = i0 + 1; - if (i1 >= window_) i1 = 0; - const double s0 = static_cast(ring_[static_cast(i0)]); - const double s1 = static_cast(ring_[static_cast(i1)]); - return s0 + (s1 - s0) * frac; - }; - - const double tap0 = readTap(readPos_); - const double tap1 = readTap(readPos_ + half); - - // Distance of tap0 behind the write head, in [0, window). Its crossfade phase is that - // distance over the window; tap1 (half a window offset) gets the complementary phase. - double dist0 = static_cast(writePos_) - readPos_; - while (dist0 < 0.0) dist0 += w; - while (dist0 >= w) dist0 -= w; - const double phase0 = dist0 / w; - - // Hann windows offset by half a grain partition unity, so the two taps sum to gain 1 with - // each tap's wrap seam masked by its window zero. phase0 drives tap0; tap1 (half-window - // offset) is at phase0 + 0.5. - const double g0 = hannWeight(phase0); - const double g1 = hannWeight(phase0 + 0.5); - const double out = tap0 * g0 + tap1 * g1; - - // 3. Advance heads: write head one frame (source rate), read head by the shift ratio. + // 4. Advance heads: write head one frame (source rate), tap(s) by the shift ratio. ++writePos_; - if (writePos_ >= window_) writePos_ = 0; - readPos_ += ratio_; - while (readPos_ >= w) readPos_ -= w; - while (readPos_ < 0.0) readPos_ += w; + if (writePos_ >= ringLen_) writePos_ = 0; + const double len = static_cast(ringLen_); + posA_ += ratio_; + while (posA_ >= len) posA_ -= len; + if (fading_) { + posB_ += ratio_; + while (posB_ >= len) posB_ -= len; + } return static_cast(out); } diff --git a/src/vst/pitch_shift.h b/src/vst/pitch_shift.h index d96ed9c..dba5c67 100644 --- a/src/vst/pitch_shift.h +++ b/src/vst/pitch_shift.h @@ -1,9 +1,23 @@ #pragma once // pitch_shift — a PURE, per-voice, duration-preserving pitch shifter: the S16 "Preserve" -// engine's DSP core. Time-domain overlap-add (OLA) with two half-window-offset read taps -// crossfaded to hide the ring-wrap seam. Source is consumed 1:1 and output produced 1:1 -// (duration held); only the PITCH changes — an octave up plays the same wall-clock length -// as the root note, unlike the Varispeed `readPos_ += ratio_` resample path. +// engine's DSP core. Time-domain delay-line shifter with CORRELATION-ALIGNED SPLICES +// (SOLA-style): one active read tap chases the write head at the shift ratio; when it drifts +// out of its safe delay band it is relocated by a nominal window jump REFINED BY A +// CROSS-CORRELATION SEARCH so the new read point is waveform-aligned, then the old and new +// taps are crossfaded (raised-cosine, amplitude-complementary). Source is consumed 1:1 and +// output produced 1:1 (duration held); only the PITCH changes — an octave up plays the same +// wall-clock length as the root note, unlike the Varispeed `readPos_ += ratio_` resample path. +// +// WHY CORRELATED SPLICES (GA-Preserve fix, 2026-07). The first S16 implementation was the +// naive two-tap OLA: taps hard-locked half a window apart, Hann-crossfaded by write-head +// distance. Its taps read the same stream at delays differing by exactly w/2, so their outputs +// carried a FIXED relative phase of 2*pi*f_src*(w/2) — arbitrary and source-frequency- +// dependent. Near anti-phase (roughly half of all frequencies) every crossfade midpoint +// nearly CANCELLED: deep periodic AM + phase slew = strong sidebands. A repitched pure sine +// came out mangled ("multiple partials" on a spectrogram) while the root stayed clean (unity +// freezes the crossfade). The fix is structural: splices must be PHASE-ALIGNED, so each jump +// is snapped to the best waveform match within a bounded lag search — a pure sine's jump +// lands on an integer period count and the output stays a single shifted tone. // // WHY A HAND-ROLLED PURE MODULE, NOT WDL (S16-F2, decided at build). The spec's lean was // route (a) `WDL_SimplePitchShifter`. But its include chain @@ -22,8 +36,9 @@ // RT DISCIPLINE (S16 hard constraint). `configure()` sizes the ring ONCE (off the audio // thread, at voice allocation). `warm()` pre-fills the ring with silence so steady-state // latency is reached before the first real sample (no cold-start click). `process()` does -// NO allocation and NO locks — it reads/writes the pre-sized ring only. All state is plain -// value fields, so a voice owning one by value costs a fixed ring buffer per channel. +// NO allocation and NO locks — it reads/writes the pre-sized ring only. The splice-time +// correlation search is a bounded burst of multiply-adds (coarse+refine over a fixed lag +// range) that fires once per splice cadence (window / |ratio-1| frames), never per frame. #include #include @@ -33,39 +48,41 @@ namespace reasampler { -// A per-channel time-domain OLA pitch shifter. One instance transposes ONE channel; a stereo -// voice owns two (or a stereo-aware wrapper) — the algorithm is per-sample and channel-count -// agnostic, matching the S7 "one read head, per-channel value" idiom of the core. +// A per-channel time-domain splice-aligned pitch shifter. One instance transposes ONE channel; +// a stereo voice owns two — the algorithm is per-sample and channel-count agnostic, matching +// the S7 "one read head, per-channel value" idiom of the core. // // The default-constructed shifter is INERT: with no configure() it passes input through // unchanged (shift ratio 1.0, empty ring), so a Varispeed voice that never touches it is // byte-identical to the pre-S16 engine. class PitchShifter { public: - // Size the delay ring for `windowFrames` (the OLA grain length) and prepare the two - // read taps a half-window apart. `windowFrames` <= 1 degrades to pass-through (no ring), - // so a degenerate configure never divides by zero or wraps a zero span. Called OFF the - // audio thread (allocates). Resets all running state. A larger window = smoother on large - // transpositions but more latency; the shell picks it from the Preserve quality setting. + // Size the delay ring for `windowFrames` (the nominal splice-jump length; the ring is 2x + // that for splice/search headroom) and derive the fade/search geometry. `windowFrames` + // <= 1 degrades to pass-through (no ring), so a degenerate configure never divides by + // zero or wraps a zero span. Called OFF the audio thread (allocates). Resets all running + // state. A larger window = fewer splices and a deeper alignment search but more latency + // (steady-state latency stays window/2); the shell picks it from kPreserveWindowMs. void configure(std::int64_t windowFrames); - // Pre-fill the ring with silence (one full window of zero writes) so the read taps reach + // Pre-fill the ring with silence (one full window of zero writes) so the read tap reaches // steady state before the first real sample. Removes the cold-start seam (the S16 "onset // click absent" requirement) — call once at voice allocation after configure(). No-op when // unconfigured (pass-through needs no warm-up). void warm(); // The pitch shift ratio: 2^((note - root)/12) plus any per-frame pitch-envelope bias. - // 1.0 = no shift (pass-through-equivalent output). Set per frame is fine (cheap); the tap - // advance simply uses the current value. Values <= 0 are ignored (kept at the last valid - // ratio) so a bad input never runs the taps backward or stalls them. + // 1.0 = no shift (pass-through-equivalent output, no splices ever fire). Set per frame is + // fine (cheap); the tap advance simply uses the current value. Values <= 0 are ignored + // (kept at the last valid ratio) so a bad input never runs the tap backward or stalls it. void setShiftRatio(double ratio); // Transform ONE input frame into ONE output frame (duration-preserving: 1 in, 1 out). // RT-safe: reads/writes the pre-sized ring only, no allocation, no lock. When unconfigured // (window <= 1) returns `in` unchanged (pass-through). Otherwise writes `in` at the write - // head, reads the two half-window-offset taps advancing at the shift ratio, crossfades - // them by the write-head-relative distance (equal-power), and advances both heads by one. + // head, reads the active tap (crossfading against the outgoing tap while a splice fade is + // live), then advances the write head by one and the tap(s) by the shift ratio. When the + // active tap leaves its safe delay band, a correlation-aligned splice is scheduled. AudioSample process(AudioSample in); // Reset running state to a freshly-warmed-equivalent silence (ring zeroed, heads re-seeded) @@ -78,10 +95,22 @@ public: std::int64_t window() const { return window_; } private: - std::vector ring_; // delay line, length `window_` (channel-local) - std::int64_t window_ = 0; // OLA grain length in frames; <= 1 = pass-through + double readTap(double pos) const; // fractional ring read, linear interp + void splice(std::int64_t nominalJump); // relocate the active tap, start the fade + + std::vector ring_; // delay line, length `ringLen_` == 2 * window_ + std::int64_t window_ = 0; // nominal splice jump in frames; <= 1 = pass-through + std::int64_t ringLen_ = 0; // ring length (2 * window_): splice + search headroom std::int64_t writePos_ = 0; // integer write head into the ring (source rate) - double readPos_ = 0.0; // fractional read head (advances at shift ratio) + 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 fadeFrames_ = 0; // crossfade length (window_/4) + 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 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) }; diff --git a/src/vst/sampler_core.cpp b/src/vst/sampler_core.cpp index c48b546..0920910 100644 --- a/src/vst/sampler_core.cpp +++ b/src/vst/sampler_core.cpp @@ -284,8 +284,8 @@ void Voice::start(int note, int velocity, const SampleData& sample, int rootNote // FA1 unity bypass, RE-SCOPED (Phase S): a UNITY-SHIFT Preserve voice — note at the // effective root (baseRatio_ == 1.0, exact per keyTrackedRatio) with the pitch envelope // off — is demoted to the Varispeed read path ONLY when the caller opted in. At ratio 1.0 - // the two engines are byte-identical EXCEPT the OLA shifter's structural onset cost (a - // half-window delay + Hann fade-in), which buys nothing at unity — but skipping it makes + // the two engines are byte-identical EXCEPT the shifter's structural onset cost (a + // half-window ring-fill delay), which buys nothing at unity — but skipping it makes // the root note speak ~25 ms EARLIER than its neighbors, an audible timing step in a // chromatic MIDI line (the FA1-review Major). So: the PREVIEW card (always at root, // latency-critical, no line to be uneven against) passes true; the MIDI VoiceEngine diff --git a/tests/test_pitch_shift.cpp b/tests/test_pitch_shift.cpp index aca05d6..0c4d550 100644 --- a/tests/test_pitch_shift.cpp +++ b/tests/test_pitch_shift.cpp @@ -12,6 +12,11 @@ // 4. RT discipline surrogate — after configure()+warm() (the off-thread setup), a long // process() run never resizes the ring (checked via window() constancy) and never returns // NaN/inf; pass-through (unconfigured) returns input verbatim. +// 5. spectral purity (GA-Preserve regression) — a repitched PURE SINE must come out as a +// 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). #include "../src/vst/pitch_shift.h" @@ -172,11 +177,82 @@ static void testRtDisciplineAndPassthrough() { } } +// --- 5. Spectral purity: a repitched pure sine stays a SINGLE shifted tone. --- +static void testRepitchSpectralPurity() { + // Frequencies are in cycles/sample (rate-free). The source tone is chosen ADVERSARIALLY: + // f0 * (window/2) = 5.5125 cycles, i.e. a fractional part of ~0.51 — content half a window + // apart in the ring is near ANTI-PHASE. The old dual-tap design (taps hard-locked w/2 + // apart) cancelled almost completely at every crossfade midpoint for such tones — the DAW + // "severe beating / multiple partials from a pure sine" bug. A correct shifter keeps the + // 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) + std::pow(2.0, -3.0 / 12.0), // -3 semitones (down-shift path) + 2.0}; // octave up (fastest splice cadence) + for (double r : ratios) { + PitchShifter ps; + ps.configure(w); + ps.warm(); + ps.setShiftRatio(r); + const std::size_t n = 120000; + std::vector out(n); + for (std::size_t i = 0; i < n; ++i) { + const double x = std::sin(2.0 * kPi * f0 * static_cast(i)); + out[i] = static_cast(ps.process(static_cast(x))); + } + + // Least-squares fit of a*sin + b*cos at the SHIFTED frequency over the settled span + // (past 3 windows of onset/latency). Solve the exact 2x2 normal equations so a + // non-integer cycle count doesn't leak into the residual. + const std::size_t from = static_cast(3 * w); + const double f1 = r * f0; + double sss = 0.0, scc = 0.0, ssc = 0.0, sys = 0.0, syc = 0.0; + for (std::size_t i = from; i < n; ++i) { + const double ph = 2.0 * kPi * f1 * static_cast(i); + const double s = std::sin(ph), c = std::cos(ph); + sss += s * s; scc += c * c; ssc += s * c; + sys += out[i] * s; syc += out[i] * c; + } + const double det = sss * scc - ssc * ssc; + CHECK(det > 0.0); + const double a = (sys * scc - syc * ssc) / det; + const double b = (syc * sss - sys * ssc) / det; + double residSq = 0.0, fitSq = 0.0; + for (std::size_t i = from; i < n; ++i) { + const double ph = 2.0 * kPi * f1 * static_cast(i); + const double fit = a * std::sin(ph) + b * std::cos(ph); + const double resid = out[i] - fit; + residSq += resid * resid; + fitSq += fit * fit; + } + const std::size_t span = n - from; + const double fitRms = std::sqrt(fitSq / static_cast(span)); + const double residRms = std::sqrt(residSq / static_cast(span)); + CHECK(fitRms > 0.5); // the shifted tone is actually there (unit sine ~0.707) + 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; + double minRms = 1e9, maxRms = 0.0; + for (std::size_t s0 = from; s0 + win <= n; s0 += hop) { + double e = 0.0; + for (std::size_t i = s0; i < s0 + win; ++i) e += out[i] * out[i]; + const double rms = std::sqrt(e / static_cast(win)); + if (rms < minRms) minRms = rms; + if (rms > maxRms) maxRms = rms; + } + CHECK(maxRms > 0.0); + CHECK(minRms > 0.8 * maxRms); // steady amplitude — no crossfade cancellation + } +} + int main() { testDurationInvariance(); testUnityRoughlyReproduces(); testTransposeDirection(); testRtDisciplineAndPassthrough(); + testRepitchSpectralPurity(); if (g_fail == 0) { std::printf("all pitch_shift tests passed\n"); From 436a685984a74e6802b814a4629e587246bb34f9 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 28 Jul 2026 06:37:38 -0400 Subject: [PATCH 2/3] 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 | 56 ++++++++++++++++++++++++++++++-------- src/vst/pitch_shift.h | 11 ++++++-- tests/test_pitch_shift.cpp | 55 +++++++++++++++++++++++++++++++++---- 3 files changed, 103 insertions(+), 19 deletions(-) diff --git a/src/vst/pitch_shift.cpp b/src/vst/pitch_shift.cpp index 26b03d1..ec95ec0 100644 --- a/src/vst/pitch_shift.cpp +++ b/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(ringLen_), 0.0f); // Geometry (all quarters of the window): - // - fadeFrames_: the splice crossfade — long enough to be smooth, short enough that the - // outgoing tap cannot cross the writer mid-fade for ratios up to ~2x/0.5x. + // - fadeFrames_: the NOMINAL splice crossfade. This window/4 length is only safe when + // 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(window_ / 4, 1); maxLag_ = window_ / 4; - corrFrames_ = std::min(window_ / 4, 512); dLow_ = window_ / 4; dHigh_ = ringLen_ - window_ / 4; + corrFrames_ = std::max(1, std::min(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(ring_[static_cast(ia)]) * - static_cast(ring_[static_cast(ic)]); + const double a = static_cast(ring_[static_cast(ia)]); + const double c = static_cast(ring_[static_cast(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(dLow_) - (ratio_ - 1.0) - 2.0; + const std::int64_t safe = + headroom > 0.0 ? static_cast(headroom / (ratio_ - 1.0)) : 1; + fadeLen_ = std::max(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(fadePos_) / static_cast(fadeFrames_); + const double t = static_cast(fadePos_) / static_cast(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- diff --git a/src/vst/pitch_shift.h b/src/vst/pitch_shift.h index dba5c67..0d3f1d2 100644 --- a/src/vst/pitch_shift.h +++ b/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 fadeFrames_ = 0; // crossfade length (window_/4) + std::int64_t fadePos_ = 0; // crossfade progress, [0, fadeLen_) + 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) diff --git a/tests/test_pitch_shift.cpp b/tests/test_pitch_shift.cpp index 0c4d550..b909bf8 100644 --- a/tests/test_pitch_shift.cpp +++ b/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) - std::pow(2.0, -3.0 / 12.0), // -3 semitones (down-shift path) - 2.0}; // octave up (fastest splice cadence) + 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) + 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::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 in = sine(n, 37.0); + std::vector 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(lat); ++i) { + if (out[i] != 0.0f) ++badSilence; // pre-latency region: warm-up silence, exact + } + for (std::size_t i = static_cast(lat); i < n; ++i) { + if (out[i] != in[i - static_cast(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"); From 3d0406ef64c018ed853c0fbb2546fa3fe145a288 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 28 Jul 2026 06:59:04 -0400 Subject: [PATCH 3/3] test(pitch_shift): tighten purity f0 to 1/196 so revert-of-fadeLen_ fails; add hop guard; clamp fadeLen_ in double before int64 cast --- src/vst/pitch_shift.cpp | 9 ++++++--- tests/test_pitch_shift.cpp | 23 +++++++++++++++-------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/vst/pitch_shift.cpp b/src/vst/pitch_shift.cpp index ec95ec0..ed6cdf7 100644 --- a/src/vst/pitch_shift.cpp +++ b/src/vst/pitch_shift.cpp @@ -186,9 +186,12 @@ void PitchShifter::splice(std::int64_t nominalJump) { fadeLen_ = fadeFrames_; if (ratio_ > 1.0) { const double headroom = static_cast(dLow_) - (ratio_ - 1.0) - 2.0; - const std::int64_t safe = - headroom > 0.0 ? static_cast(headroom / (ratio_ - 1.0)) : 1; - fadeLen_ = std::max(1, std::min(fadeFrames_, safe)); + // Clamp in double before the int64 cast to avoid UB at pathological near-unity ratios + // at very high sample rates (where headroom/(ratio_-1.0) could overflow int64). + const double safeDbl = headroom > 0.0 + ? std::min(headroom / (ratio_ - 1.0), static_cast(fadeFrames_)) + : 1.0; + fadeLen_ = std::max(1, static_cast(safeDbl)); } fading_ = true; fadePos_ = 0; diff --git a/tests/test_pitch_shift.cpp b/tests/test_pitch_shift.cpp index b909bf8..73f441a 100644 --- a/tests/test_pitch_shift.cpp +++ b/tests/test_pitch_shift.cpp @@ -183,14 +183,21 @@ static void testRtDisciplineAndPassthrough() { // --- 5. Spectral purity: a repitched pure sine stays a SINGLE shifted tone. --- static void testRepitchSpectralPurity() { - // Frequencies are in cycles/sample (rate-free). The source tone is chosen ADVERSARIALLY: - // f0 * (window/2) = 5.5125 cycles, i.e. a fractional part of ~0.51 — content half a window - // apart in the ring is near ANTI-PHASE. The old dual-tap design (taps hard-locked w/2 - // apart) cancelled almost completely at every crossfade midpoint for such tones — the DAW - // "severe beating / multiple partials from a pure sine" bug. A correct shifter keeps the - // output a single sinusoid at ratio*f0 with a steady amplitude. + // 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 + // window apart in the ring is near ANTI-PHASE. The old dual-tap design cancelled + // almost completely at every crossfade midpoint for such tones — the DAW "severe + // beating / multiple partials from a pure sine" bug. + // (b) ringLen_*f0 = 4410/196 = 22.5 EXACTLY — at ratio 4 the write head advances 4 taps + // per output frame, so each splice-period the outgoing tap crosses the writer at the + // HALF-period point of the source waveform (sign flip), producing a visible null when + // gNew == gOld if fadeLen_ is not clamped to headroom. With f0=0.005 this product + // is 22.05 (frac ≈ 0.05), near a zero-crossing — the artifact is near-benign, so the + // +24 st purity case would pass even with the clamping reverted. f0=1/196 forces the + // half-integer alignment that makes the pre-fix artifact catastrophic. const std::int64_t w = 2205; // ~50 ms @ 44.1k (the product window) - const double f0 = 0.005; // source: period 200 samples + const double f0 = 1.0 / 196.0; // source: period 196 samples; see adversarial note above 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) 2.0, // octave up (nominal-fade boundary) @@ -248,7 +255,7 @@ static void testRepitchSpectralPurity() { // 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::lround(1.0 / f1)); - const std::size_t hop = win / 2; + const std::size_t hop = std::max(1, win / 2); double minRms = 1e9, maxRms = 0.0; for (std::size_t s0 = from; s0 + win <= n; s0 += hop) { double e = 0.0;