From 2ef3514bc7960565c61673fe229de549afc038b7 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 28 Jul 2026 08:49:05 -0400 Subject: [PATCH 1/2] =?UTF-8?q?fix(preserve):=20prime=20SOLA=20rings=20wit?= =?UTF-8?q?h=20real=20source=20(zero-latency,=20gap-free=20onset)=20+=20su?= =?UTF-8?q?b-sample=20splice=20alignment=20=E2=80=94=20clean=20repitch=20C?= =?UTF-8?q?1..C8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/vst/pitch_shift.cpp | 124 +++++++++++++++++---- src/vst/pitch_shift.h | 51 +++++++-- src/vst/sampler_core.cpp | 149 +++++++++++++++---------- src/vst/sampler_core.h | 30 ++++- tests/test_pitch_shift.cpp | 213 +++++++++++++++++++++--------------- tests/test_sampler_core.cpp | 118 +++++++++++--------- 6 files changed, 447 insertions(+), 238 deletions(-) diff --git a/src/vst/pitch_shift.cpp b/src/vst/pitch_shift.cpp index ed6cdf7..8885585 100644 --- a/src/vst/pitch_shift.cpp +++ b/src/vst/pitch_shift.cpp @@ -8,13 +8,17 @@ // `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. +// down-shifts) — CLAMPED to the filled span so it can never land in unwritten silence (the +// GA2 onset fix) — refined by a cross-correlation search over +/- maxLag PLUS a parabolic +// peak interpolation for a SUB-SAMPLE lag, so the relocated read point is waveform-aligned +// to a fraction of a sample (integer-lag splices left +/-0.5-sample errors: a -59 dB +// sideband comb at the splice cadence on a repitched pure sine — the GA2 "alias lines" on +// the spectrogram). 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 + fraction) 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: a primed shifter passes the +// stream through with ZERO added latency; a silence-warmed one is a clean window delay. #include "pitch_shift.h" @@ -41,6 +45,7 @@ void PitchShifter::configure(std::int64_t windowFrames) { fading_ = false; fadePos_ = 0; fadeFrames_ = fadeLen_ = maxLag_ = corrFrames_ = dLow_ = dHigh_ = 0; + filled_ = 0; ratio_ = 1.0; return; } @@ -70,11 +75,13 @@ void PitchShifter::configure(std::int64_t windowFrames) { void PitchShifter::reset() { if (window_ > 1) { - // 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. + // Zero the ring and seed the active tap one window behind the writer — the exact + // middle of the safe band [dLow, dHigh] = [w/4, 2w - w/4], so unity holds it there + // forever and either shift direction has maximal drift room. No history is declared + // (filled_ = 0): follow with prime() or warm() before streaming. std::fill(ring_.begin(), ring_.end(), 0.0f); writePos_ = 0; - posA_ = static_cast(ringLen_ - window_ / 2); + posA_ = static_cast(ringLen_ - window_); posB_ = posA_; fading_ = false; fadePos_ = 0; @@ -86,13 +93,42 @@ void PitchShifter::reset() { fadePos_ = 0; fadeLen_ = 0; } + filled_ = 0; ratio_ = 1.0; } +void PitchShifter::prime(const AudioSample* src, std::int64_t count) { + if (window_ <= 1) return; // pass-through needs no priming + // Clamp to one window: the intended call primes exactly window() frames, and delay == + // count must stay inside the safe band so the seed does not itself trigger a splice. + if (count < 0) count = 0; + if (count > window_) count = window_; + std::fill(ring_.begin(), ring_.end(), 0.0f); + for (std::int64_t i = 0; i < count; ++i) ring_[static_cast(i)] = src[i]; + // Writer continues after the primed span; the tap parks ON src[0] (delay == count), so + // the very first process() output is src[0] — zero structural latency at every ratio. + writePos_ = count % ringLen_; + posA_ = posB_ = 0.0; + fading_ = false; + fadePos_ = 0; + fadeLen_ = 0; + filled_ = count; + // ratio_ deliberately untouched: the voice sets it per frame around the prime. +} + void PitchShifter::warm() { if (window_ <= 1) return; // pass-through needs no warm-up - // 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); + // A prime() with one window of silence: same geometry (tap parked mid-band one window + // behind the writer), the zeros declared as valid history. At unity this is a bit-exact + // window() delay; an up-shift plays ~a window of silence before speaking (the pre-GA2 + // onset) — stream callers with access to the upcoming source should prime() instead. + std::fill(ring_.begin(), ring_.end(), 0.0f); + writePos_ = window_ % ringLen_; + posA_ = posB_ = 0.0; + fading_ = false; + fadePos_ = 0; + fadeLen_ = 0; + filled_ = window_; } void PitchShifter::setShiftRatio(double ratio) { @@ -114,20 +150,41 @@ double PitchShifter::readTap(double pos) const { return s0 + (s1 - s0) * frac; } -void PitchShifter::splice(std::int64_t nominalJump) { +void PitchShifter::splice(std::int64_t nominalJump, double delay) { // 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. + // The search is coarse (step 4 over +/- maxLag_) then fine (+/- 3 around the coarse best, + // then a parabolic sub-sample peak): a bounded burst of ~ (maxLag_/2 + 9) * corrFrames_ + // multiply-adds, once per splice. + const std::int64_t d = static_cast(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 + // delay d + jump + maxLag + 1, so cap the jump at filled_ - d - maxLag_ - 1. In steady + // state (filled_ == ringLen_) this is > 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, never out-of-range. + std::int64_t jump = nominalJump; + if (jump > 0) { + const std::int64_t maxJump = filled_ - d - maxLag_ - 1; + if (jump > maxJump) jump = maxJump; + if (jump < 1) jump = 1; + } + // The correlation reference reads FORWARD from the tap; keep it strictly behind the + // writer even when the trigger undershot dLow_ by a large per-frame drift (extreme + // up-ratios): d - corr must stay >= 0. + const std::int64_t corr = std::max(1, std::min(corrFrames_, d - 1)); + 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_; + std::int64_t ic = ((iA - jump + lag) % ringLen_ + ringLen_) % ringLen_; double s = 0.0, ec = 0.0; - for (std::int64_t k = 0; k < corrFrames_; ++k) { + for (std::int64_t k = 0; k < corr; ++k) { const double a = static_cast(ring_[static_cast(ia)]); const double c = static_cast(ring_[static_cast(ic)]); s += a * c; @@ -163,11 +220,28 @@ void PitchShifter::splice(std::int64_t nominalJump) { } } - // 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. + // SUB-SAMPLE peak (GA2 alias fix): the integer-lag best leaves a residual misalignment of + // up to half a sample; at the splice cadence that residual phase-modulates a pure tone + // into a ~-59 dB sideband comb (the DAW spectrogram "alias lines"). A parabola through + // the scores at bestLag-1/bestLag/bestLag+1 locates the correlation peak to a fraction of + // a sample; readTap()'s linear interpolation realizes the fractional tap position. The + // denominator is negative at a genuine peak — anything else (flat correlation: DC or + // silence) keeps the integer lag, which is already benign there. + double frac = 0.0; + { + const double sM = scoreAt(bestLag - 1); + const double sP = scoreAt(bestLag + 1); + const double den = sM - 2.0 * bestScore + sP; + if (den < 0.0) { + frac = 0.5 * (sM - sP) / den; + if (frac > 0.5) frac = 0.5; + if (frac < -0.5) frac = -0.5; + } + } + + // Hand the current position to the outgoing tap and relocate the active one. posB_ = posA_; - double p = posA_ - static_cast(nominalJump) + static_cast(bestLag); + double p = posA_ - static_cast(jump) + static_cast(bestLag) + frac; const double len = static_cast(ringLen_); while (p < 0.0) p += len; while (p >= len) p -= len; @@ -200,8 +274,10 @@ void PitchShifter::splice(std::int64_t nominalJump) { AudioSample PitchShifter::process(AudioSample in) { if (window_ <= 1) return in; // pass-through (unconfigured / degenerate) - // 1. Write the incoming sample at the write head (source rate). + // 1. Write the incoming sample at the write head (source rate). One more slot of the + // ring now holds valid history (capped at the ring length once it has wrapped). ring_[static_cast(writePos_)] = in; + if (filled_ < ringLen_) ++filled_; // 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 @@ -222,9 +298,9 @@ AudioSample PitchShifter::process(AudioSample in) { while (d < 0.0) d += len; while (d >= len) d -= len; if (d <= static_cast(dLow_)) { - splice(+window_); + splice(+window_, d); } else if (d >= static_cast(dHigh_)) { - splice(-window_); + splice(-window_, d); } } diff --git a/src/vst/pitch_shift.h b/src/vst/pitch_shift.h index 0d3f1d2..f368496 100644 --- a/src/vst/pitch_shift.h +++ b/src/vst/pitch_shift.h @@ -29,13 +29,24 @@ // PitchEngine::Preserve contract behind the seam — if WDL is ever preferred it swaps in at // the SHELL, never in the pure core. // +// WHY PRIME WITH REAL CONTENT (GA2-Preserve onset fix, 2026-07). Splices RELOCATE the tap +// into ring HISTORY — at note onset a silence-warmed ring has none, so every early splice +// jumped into zeros: a burst/gap/burst stutter for the first ~2 windows of every off-root +// note (the DAW "zero-sample gaps in the first few ms"; at +48 st the ~300 Hz gap cadence +// reads as a square-ish buzz). But this engine is NOT a streaming context: the caller owns +// the whole decoded sample, so the FUTURE of the stream is known at note-on. `prime()` +// pre-fills the ring with the actual first window of upcoming source and parks the tap on +// its oldest frame — output frame 0 IS source frame 0 (zero structural latency at every +// ratio), and `splice()` clamps its jump to the really-filled span so no splice can ever +// land in unwritten silence. +// // PURE MODULE: NO VST3, NO REAPER, NO SWELL, NO vendor/ includes. Standard library only. // Shares the `AudioSample` float alias from peaks (the one house precedent — sampler_core / // wav_trim do the same). // // 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 +// thread, at voice allocation). `prime()` / `warm()` only copy into the pre-sized ring +// (bounded, allocation-free — safe on the audio thread at note-on). `process()` does // 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. @@ -61,14 +72,25 @@ public: // 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. + // state. A larger window = fewer splices and a deeper alignment search; a PRIMED shifter + // has no added latency regardless (see prime()); 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 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). + // Pre-fill the ring with the first `count` frames of the UPCOMING source stream and park + // the tap on src[0] (delay == count, mid safe band at count == window()). The caller then + // feeds process() the stream CONTINUING at src[count]. Output frame 0 is src[0]: ZERO + // structural latency at every ratio, and splices always have `count` frames of real + // history to land in — the GA2 onset-gap fix. `count` is clamped to [0, window()]; pass + // the full window (pad the tail with silence yourself if the source is shorter — trailing + // silence IS the true stream there). RT-safe: bounded copy into the pre-sized ring, no + // allocation. No-op when unconfigured. The current shift ratio is left untouched. + void prime(const AudioSample* src, std::int64_t count); + + // prime()-with-silence: zero the ring, park the tap one window behind the writer, and + // declare that window of silence as valid history. Kept for callers with no access to the + // upcoming stream (a silence-primed up-shift plays ~a window of silence before speaking — + // the pre-GA2 onset; the Voice path uses prime() instead). At unity a warmed shifter is a + // bit-exact window() delay. No-op when unconfigured. void warm(); // The pitch shift ratio: 2^((note - root)/12) plus any per-frame pitch-envelope bias. @@ -85,8 +107,10 @@ public: // 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) + // Reset running state to silence (ring zeroed, heads re-seeded mid-band, fill count zeroed) // WITHOUT reallocating — for voice reuse without a re-configure. Keeps the current window. + // Follow with prime() (or warm()) before streaming: a bare reset has no declared history, + // so an immediate up-shift would starve its splices. void reset(); // True once configure() sized a real ring (window > 1). A pass-through shifter is false. @@ -96,7 +120,10 @@ public: private: double readTap(double pos) const; // fractional ring read, linear interp - void splice(std::int64_t nominalJump); // relocate the active tap, start the fade + // Relocate the active tap by ~`nominalJump` frames of added delay (clamped to the filled + // span for up-jumps) and start the crossfade. `delay` is the tap's current delay behind + // the writer (the caller just computed it for the trigger test). + void splice(std::int64_t nominalJump, double delay); std::vector ring_; // delay line, length `ringLen_` == 2 * window_ std::int64_t window_ = 0; // nominal splice jump in frames; <= 1 = pass-through @@ -116,6 +143,10 @@ private: // 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) + std::int64_t filled_ = 0; // frames of VALID history behind the writer (prime count + // + frames streamed, capped at ringLen_). splice() clamps + // its up-jump to this so no splice lands in unwritten + // silence — the GA2 onset-gap fix. double ratio_ = 1.0; // current shift ratio (>0) }; diff --git a/src/vst/sampler_core.cpp b/src/vst/sampler_core.cpp index 1132f39..c7b8de7 100644 --- a/src/vst/sampler_core.cpp +++ b/src/vst/sampler_core.cpp @@ -254,9 +254,19 @@ double PitchEnvelope::tick() { void Voice::presizePreserveShifters(std::int64_t windowFrames) { // OFF the audio thread (allocates). Both channels are sized so a stereo Preserve voice needs - // no allocation at note-on; a mono Preserve voice simply never process()es shiftR_. + // no allocation at note-on; a mono Preserve voice simply never process()es shiftR_. The + // prime scratch (one window, reused per channel) is sized here for the same reason: start() + // assembles the first window of the upcoming source stream into it with zero allocation. shiftL_.configure(windowFrames); shiftR_.configure(windowFrames); + primeBuf_.assign(windowFrames > 1 ? static_cast(windowFrames) : 0, 0.0f); +} + +bool Voice::sustainLoopUsable() const { + if (sample_ == nullptr || playMode_ != PlayMode::Gate) return false; + const SampleLoop& loop = sample_->loop; + return loop.hasLoop && loop.end > loop.start && loop.start >= 0 && + loop.end <= static_cast(sample_->frames.size()); } void Voice::start(int note, int velocity, const SampleData& sample, int rootNote, @@ -305,13 +315,13 @@ 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 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 - // passes false and keeps one uniform onset across the keyboard. + // off — is demoted to the Varispeed read path ONLY when the caller opted in. Since the + // GA2 prime fix the shifter has ZERO structural onset latency at every ratio (the ring + // is primed with the first window of source), so the original FA1 timing concern (a + // ~25 ms root-vs-neighbor onset step) no longer exists in either direction: onset is + // uniform across the keyboard with or without the bypass. The demotion survives purely + // as a work-skip — a unity voice pays no per-frame shifter cost — still scoped to the + // PREVIEW card (true); the MIDI VoiceEngine passes false, keeping one code path per line. if (unityVarispeedBypass && pitchEngine_ == PitchEngine::Preserve && baseRatio_ == 1.0 && !p.pitchEnv.enabled) { pitchEngine_ = PitchEngine::Varispeed; @@ -359,18 +369,38 @@ void Voice::start(int note, int velocity, const SampleData& sample, int rootNote pitchEnv_.configure(p.pitchEnv); pitchEnv_.noteOn(); - // --- Preserve engine (S16): reset + pre-warm the ALREADY-SIZED per-channel shifters. The - // rings were allocated off-thread by presizePreserveShifters (the engine calls it at - // construction), so this RT-safe path only zeroes state (reset) and runs a silence pass - // (warm) to settle the OLA taps before the first output frame — NO allocation here. - // Varispeed voices never touch the shifters (advanceFrame checks configured()), so a - // Varispeed instrument is byte-identical to pre-S16 and pays no per-frame shifter cost. --- + // --- Preserve engine (S16, GA2 onset fix): PRIME the ALREADY-SIZED per-channel shifters + // with the first window of the ACTUAL upcoming source stream (loop-unrolled under the + // sustain-loop wrap rule, silence past the sample end — that silence IS the true + // stream there). The tap parks on source frame `start`, so the voice speaks on output + // frame 0 at EVERY ratio (no ring-fill silence), and every splice has a full window + // of real history to land in — the fix for the DAW onset zero-gaps (a silence-warmed + // ring made every early splice jump into zeros). The rings and the prime scratch were + // allocated off-thread by presizePreserveShifters (the engine calls it at + // construction); this path is a bounded copy — NO allocation here. Varispeed voices + // never touch the shifters (advanceFrame checks configured()), so a Varispeed + // instrument is byte-identical to pre-S16 and pays no per-frame shifter cost. --- if (pitchEngine_ == PitchEngine::Preserve && shiftL_.configured()) { - shiftL_.reset(); - shiftL_.warm(); - if (sample.channelCount() == 2 && shiftR_.configured()) { - shiftR_.reset(); - shiftR_.warm(); + const std::int64_t w = shiftL_.window(); + const bool loopWrap = sustainLoopUsable(); + const SampleLoop& loop = sample.loop; + const std::int64_t loopLen = loopWrap ? (loop.end - loop.start) : 0; + const bool stereoSample = sample.channelCount() == 2 && shiftR_.configured(); + for (int ch = 0; ch < (stereoSample ? 2 : 1); ++ch) { + const std::vector& pcmCh = ch == 0 ? sample.frames : sample.framesR; + std::int64_t p = start; + for (std::int64_t i = 0; i < w; ++i) { + if (loopWrap) { + while (p >= loop.end) p -= loopLen; + } + primeBuf_[static_cast(i)] = + (p < frameCount) ? pcmCh[static_cast(p)] : 0.0f; + ++p; + } + (ch == 0 ? shiftL_ : shiftR_).prime(primeBuf_.data(), w); + // Both channels walk identical positions; the per-frame feed continues at `p`, + // exactly one window ahead of the output anchor readPos_. + feedPos_ = p; } } ratio_ = baseRatio_; // seeded; advanceFrame recomputes per frame under the active engine. @@ -440,8 +470,7 @@ AudioSample Voice::advanceFrame(bool stereo, AudioSample& outR) { // it back into [start, end). A zero-length loop is treated as "no loop". Under Preserve the // loop is over the SOURCE read (loop the source, shift the output — S15×S16 contract). const SampleLoop& loop = sample_->loop; - const bool loopUsable = playMode_ == PlayMode::Gate && loop.hasLoop && - loop.end > loop.start && loop.start >= 0 && loop.end <= frameCount; + const bool loopUsable = sustainLoopUsable(); if (loopUsable) { const double loopLen = static_cast(loop.end - loop.start); while (readPos_ >= static_cast(loop.end)) { @@ -460,34 +489,11 @@ AudioSample Voice::advanceFrame(bool stereo, AudioSample& outR) { return 0.0f; } - // Linear interpolation between the two bracketing SOURCE frames. For the loop case, the - // second point wraps to loopStart so the seam is continuous. - const std::int64_t i0 = static_cast(readPos_); - const double frac = readPos_ - static_cast(i0); - std::int64_t i1 = i0 + 1; - if (loopUsable && i1 >= loop.end) { - i1 = loop.start; // seamless wrap for the interpolation partner. - } - const bool i0ok = (i0 >= 0 && i0 < frameCount); - const bool i1ok = (i1 >= 0 && i1 < frameCount); - // Envelopes tick once per output frame. Pitch envelope biases pitch under EITHER engine. const double amp = tickAmplitude(); const double gain = amp * velocityGain_; const double pitchEnvSemis = pitchEnv_.tick(); - // Raw interpolated source values (pre-shift). These are the SOURCE stream both engines read; - // Varispeed applies pitch by the read RATE, Preserve applies it by the shifter. - const double srcL = (i0ok ? static_cast(pcm[i0]) : 0.0) + - ((i1ok ? static_cast(pcm[i1]) : 0.0) - - (i0ok ? static_cast(pcm[i0]) : 0.0)) * frac; - double srcR = 0.0; - if (stereo) { - srcR = (i0ok ? static_cast(pcmR[i0]) : 0.0) + - ((i1ok ? static_cast(pcmR[i1]) : 0.0) - - (i0ok ? static_cast(pcmR[i0]) : 0.0)) * frac; - } - // The pitch-envelope bias factor 2^(semis/12). When the envelope is off (semis exactly 0) // this is 1.0 and we skip the pow entirely — the Varispeed-off path stays a bare ratio read // (no per-frame transcendental), byte-identical to pre-S16. @@ -495,36 +501,69 @@ AudioSample Voice::advanceFrame(bool stereo, AudioSample& outR) { double outL, outRlocal = 0.0; if (pitchEngine_ == PitchEngine::Preserve && shiftL_.configured()) { - // PRESERVE: read the source at unity rate (duration held) and TRANSPOSE the output by - // 2^((note-root + pitchEnvSemis)/12). Pitch envelope adds to the shift amount, not the - // read rate — pitch bends, duration unchanged (S16 contract). + // PRESERVE: feed the shifters the SOURCE stream at unity rate (duration held) and + // TRANSPOSE the output by 2^((note-root + pitchEnvSemis)/12). Pitch envelope adds to + // the shift amount, not the read rate — pitch bends, duration unchanged (S16 + // contract). The feed runs one window AHEAD of readPos_ (the rings were primed with + // that window at start()), under the SAME sustain-loop wrap rule as the anchor, and + // reads integer source frames (readPos_ advances by exactly 1.0 under Preserve, so + // there is nothing to interpolate). Past the sample end the stream is silence — the + // shifter keeps transposing the real tail it already holds. + if (loopUsable) { + const std::int64_t loopLen = loop.end - loop.start; + while (feedPos_ >= loop.end) feedPos_ -= loopLen; + } + const bool feedOk = (feedPos_ >= 0 && feedPos_ < frameCount); + const AudioSample feedL = feedOk ? pcm[static_cast(feedPos_)] : 0.0f; const double shift = baseRatio_ * envFactor; shiftL_.setShiftRatio(shift); - const double shiftedL = static_cast(shiftL_.process(static_cast(srcL))); + const double shiftedL = static_cast(shiftL_.process(feedL)); outL = shiftedL * gain; if (stereo) { - if (shiftR_.configured()) { + if (haveR && shiftR_.configured()) { // Genuine stereo: an independent shifter transposes channel 1. Each shifter is // process()'d EXACTLY ONCE per output frame (never twice — that would advance its - // heads twice and corrupt the OLA state). + // 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(feedPos_)] : 0.0f; shiftR_.setShiftRatio(shift); - outRlocal = - static_cast(shiftR_.process(static_cast(srcR))) * gain; + outRlocal = static_cast(shiftR_.process(feedR)) * gain; } else { // Mono sample in stereo mode (dual-mono): shiftL_ already produced the shifted - // value from srcL (== srcR since pcmR aliases pcm); mirror it to R. Do NOT call - // shiftL_.process again this frame. + // value from the mono feed; mirror it to R. Do NOT call shiftL_.process again + // this frame. outRlocal = shiftedL * gain; } } + ++feedPos_; // Preserve advances the read head at the SOURCE rate (duration preserved). ratio_ = 1.0; } else { // VARISPEED: pitch and duration coupled. The read rate carries the repitch; the pitch // envelope multiplies the ratio for the read-rate bias (unchanged pre-S16 idiom when the // envelope is off -> pitchEnvSemis == 0 -> factor 1.0 -> byte-identical). + // + // Linear interpolation between the two bracketing SOURCE frames at the read head. For + // the loop case, the second point wraps to loopStart so the seam is continuous. + const std::int64_t i0 = static_cast(readPos_); + const double frac = readPos_ - static_cast(i0); + std::int64_t i1 = i0 + 1; + if (loopUsable && i1 >= loop.end) { + i1 = loop.start; // seamless wrap for the interpolation partner. + } + const bool i0ok = (i0 >= 0 && i0 < frameCount); + const bool i1ok = (i1 >= 0 && i1 < frameCount); + const double srcL = (i0ok ? static_cast(pcm[i0]) : 0.0) + + ((i1ok ? static_cast(pcm[i1]) : 0.0) - + (i0ok ? static_cast(pcm[i0]) : 0.0)) * frac; outL = srcL * gain; - if (stereo) outRlocal = srcR * gain; + if (stereo) { + const double srcR = (i0ok ? static_cast(pcmR[i0]) : 0.0) + + ((i1ok ? static_cast(pcmR[i1]) : 0.0) - + (i0ok ? static_cast(pcmR[i0]) : 0.0)) * frac; + outRlocal = srcR * gain; + } ratio_ = baseRatio_ * envFactor; } diff --git a/src/vst/sampler_core.h b/src/vst/sampler_core.h index 722f03f..3f8d8bd 100644 --- a/src/vst/sampler_core.h +++ b/src/vst/sampler_core.h @@ -466,7 +466,8 @@ public: // meaningful while active(). NOTE (Phase S re-scope of FA1): the unity-shift demotion to // Varispeed is now OPT-IN via start()'s unityVarispeedBypass — only the preview card takes // it; a MIDI Preserve voice keeps its shifter at every note so a chromatic line has one - // uniform onset (no ~25 ms step at the root). + // uniform onset. GA2 update: the primed shifter speaks on frame 0 at every ratio, so the + // demotion is purely a per-frame work-skip — onset timing is uniform either way. PitchEngine pitchEngine() const { return pitchEngine_; } // The SampleData this voice is playing (nullptr when never started). The engine's mono // legato path compares it against the new note's resolved sample — a same-sample takeover @@ -474,11 +475,12 @@ public: const SampleData* playingSample() const { return sample_; } // Pre-SIZE this voice's Preserve pitch shifters (both channels) to `windowFrames`, OFF the - // audio thread (this allocates). The engine calls it once at construction so start() — which - // runs on the audio thread inside process() — never allocates: start() only reset()s + warm()s - // the already-sized rings. `windowFrames` <= 1 leaves the shifters as pass-through (Varispeed - // instruments pay no ring cost). Idempotent: a re-presize to the same window is a cheap no-op - // in the underlying vector. + // audio thread (this allocates; also sizes the prime scratch buffer). The engine calls it + // once at construction so start() — which runs on the audio thread inside process() — never + // allocates: start() only prime()s the already-sized rings with the first window of source + // (a bounded copy). `windowFrames` <= 1 leaves the shifters as pass-through (Varispeed + // instruments pay no ring cost). Idempotent: a re-presize to the same window is a cheap + // no-op in the underlying vector. void presizePreserveShifters(std::int64_t windowFrames); // Renders one frame's contribution, advancing the read head and envelope by one @@ -511,6 +513,12 @@ private: // finishes (Gate: release complete; Trigger: play length reached) so advanceFrame frees the voice. double tickAmplitude(); + // True when the sustain loop applies to this voice: GATE mode with a valid, non-empty loop + // inside the sample (S15 — Trigger one-shots never loop). The single source of truth for + // the wrap rule shared by the output anchor (readPos_), the Preserve feed (feedPos_), and + // the start()-time ring prime. + bool sustainLoopUsable() const; + bool active_ = false; bool releasing_ = false; int note_ = 0; @@ -533,10 +541,20 @@ private: // S16 pitch engine + pitch envelope. pitchEngine_ selects Varispeed (ratio bias) vs Preserve // (source-rate read + shifter). shiftL_/shiftR_ transpose the Preserve output per channel // (one read head, per-channel shift — S7 compose). pitchEnv_ rides EITHER engine. + // + // GA2 onset fix: the shifter rings are PRIMED at start() with the first window of the + // actual upcoming source (loop-unrolled, silence past the end) — output frame 0 is source + // frame `start`, no ring-fill silence, and splices always land in real history. feedPos_ + // is the integer SOURCE frame the shifters are fed next; it runs exactly one window AHEAD + // of readPos_ (the wall-clock output anchor) under the same sustain-loop wrap rule. + // primeBuf_ is the presized scratch the prime stream is assembled into (never touched + // outside start()). PitchEngine pitchEngine_ = PitchEngine::Varispeed; PitchEnvelope pitchEnv_; PitchShifter shiftL_; PitchShifter shiftR_; + std::int64_t feedPos_ = 0; + std::vector primeBuf_; // 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 diff --git a/tests/test_pitch_shift.cpp b/tests/test_pitch_shift.cpp index 73f441a..4cdcad5 100644 --- a/tests/test_pitch_shift.cpp +++ b/tests/test_pitch_shift.cpp @@ -12,15 +12,17 @@ // 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). 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. +// 5. spectral purity + onset integrity (GA / GA2 regressions) — a PRIMED repitched PURE +// SINE must come out as a SINGLE tone at the shifted frequency FROM THE VERY FIRST +// MILLISECOND: no zero-gaps anywhere (the GA2 DAW report: silence-warmed rings made +// every early splice jump into zeros — burst/gap/burst stutter in the first few ms), +// and a per-block least-squares residual floor that catches harmonics, splice-cadence +// sideband combs, and crossfade cancellation alike. Ratios cover the FULL playable +// range the DAW report exercised: +2/-3 st, +/-1 octave, +24 st, +48 st (C8 from C4, +// ratio 16) and -36 st (C1 from C4, ratio 1/8). +// 6. unity + latency contract — asserted bit-exactly: a warm()ed shifter at ratio 1.0 IS a +// clean window delay; a prime()d one has ZERO added latency (out[i] == src[i] to the +// bit) — the GA2 immediate-onset claim. #include "../src/vst/pitch_shift.h" @@ -181,8 +183,9 @@ static void testRtDisciplineAndPassthrough() { } } -// --- 5. Spectral purity: a repitched pure sine stays a SINGLE shifted tone. --- -static void testRepitchSpectralPurity() { +// --- 5. Spectral purity + onset integrity: a PRIMED repitched pure sine is a SINGLE shifted +// tone from the very first millisecond. --- +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 @@ -196,106 +199,138 @@ static void testRepitchSpectralPurity() { // 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. + // + // 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 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 (D from C) std::pow(2.0, -3.0 / 12.0), // -3 semitones (down-shift path) - 2.0, // octave up (nominal-fade boundary) + 2.0, // octave up (the GA2 report: C5) 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) + std::pow(2.0, 48.0 / 12.0), // +48 st: ratio 16 — C8 from C4 (the + // GA2 "awful at C8" report; fast + // splice cadence, short fades) + std::pow(2.0, -12.0 / 12.0), // octave down (full down-shift path) + std::pow(2.0, -36.0 / 12.0)}; // -36 st: ratio 1/8 — C1 from C4 + // (the GA2 down-shift report) for (double r : ratios) { PitchShifter ps; ps.configure(w); - ps.warm(); - ps.setShiftRatio(r); const std::size_t n = 120000; + std::vector src(n + static_cast(w)); + for (std::size_t i = 0; i < src.size(); ++i) { + src[i] = static_cast( + std::sin(2.0 * kPi * f0 * static_cast(i))); + } + ps.prime(src.data(), w); // the Voice's note-on path: real content, not warm zeros + ps.setShiftRatio(r); 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))); + out[i] = static_cast(ps.process(src[i + static_cast(w)])); } - // 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); + // (a) ONSET/GAP integrity over the ENTIRE run, frame 0 included: no near-zero run + // longer than 32 frames (~0.7 ms). A unit-amplitude shifted sine dwells below 1e-3 + // for well under one frame per zero crossing even at the lowest ratio here, while the + // pre-fix onset gaps were hundreds to thousands of frames of literal silence. + std::size_t worstGap = 0, run = 0; + for (std::size_t i = 0; i < n; ++i) { + if (std::fabs(out[i]) < 1e-3) { + ++run; + if (run > worstGap) worstGap = run; + } else { + run = 0; + } + } + CHECK(worstGap < 32); + + // (b) PER-BLOCK least-squares fit of a*sin + b*cos at the SHIFTED frequency, from the + // FIRST block. Fitting phase per block deliberately tolerates the slow (pitch-true, + // inaudible) SOLA phase wander across seconds while catching everything audible: + // harmonics ("square-ish"), splice-cadence sideband combs (the spectrogram alias + // lines), crossfade cancellation, and onset gaps all land in the residual or collapse + // the in-block fit amplitude. Solve the exact 2x2 normal equations per block. 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 std::size_t block = 4096; + for (std::size_t b0 = 0; b0 + block <= n; b0 += block) { + double sss = 0.0, scc = 0.0, ssc = 0.0, sys = 0.0, syc = 0.0; + for (std::size_t i = b0; i < b0 + block; ++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 = b0; i < b0 + block; ++i) { + const double ph = 2.0 * kPi * f1 * static_cast(i); + const double fit = a * std::sin(ph) + b * std::cos(ph); + residSq += (out[i] - fit) * (out[i] - fit); + fitSq += fit * fit; + } + const double fitRms = std::sqrt(fitSq / static_cast(block)); + const double residRms = std::sqrt(residSq / static_cast(block)); + // The shifted tone is there at full amplitude (unit sine RMS ~0.707) in EVERY + // 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. + // (Post-fix the engine measures ~-75 dB and better; the old integer-lag splices + // sat near -59 dB sidebands and the warm-zero onset failed outright.) + CHECK(residRms < 0.0316 * fitRms); } - 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). - // 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 = 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; - 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 } } -// --- 6. Unity contract: bit-exact window/2 delay == the latency claim. --- +// --- 6. Unity + latency contract: warm = bit-exact window delay; primed = bit-exact ZERO +// latency. --- 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); + // A configured shifter at ratio 1.0 parks the tap mid-band (no splice ever fires) at an + // integral delay (no interpolation error). After warm() that delay is exactly one window + // of declared silence, so out[i] == in[i - w] to the bit. After prime() with the first + // window of source the tap sits ON src[0] — out[i] == src[i] to the bit from the very + // first frame: the GA2 zero-structural-latency (immediate onset) claim. + const std::int64_t w = 2205; // the product window 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 + const std::vector in = sine(n + static_cast(w), 37.0); + // warm(): a clean, bit-exact one-window delay of the streamed input. + { + PitchShifter ps; + ps.configure(w); + ps.warm(); + ps.setShiftRatio(1.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(w); ++i) { + if (out[i] != 0.0f) ++badSilence; // pre-latency region: declared silence, exact + } + for (std::size_t i = static_cast(w); i < n; ++i) { + if (out[i] != in[i - static_cast(w)]) ++badDelay; // bit-exact delay + } + CHECK(badSilence == 0); + CHECK(badDelay == 0); } - for (std::size_t i = static_cast(lat); i < n; ++i) { - if (out[i] != in[i - static_cast(lat)]) ++badDelay; // bit-exact delay + // prime(): zero added latency — the output IS the source from frame 0, bit-exact. + { + PitchShifter ps; + ps.configure(w); + ps.prime(in.data(), w); + ps.setShiftRatio(1.0); + std::size_t badZeroLat = 0; + for (std::size_t i = 0; i < n; ++i) { + if (ps.process(in[i + static_cast(w)]) != in[i]) ++badZeroLat; + } + CHECK(badZeroLat == 0); } - CHECK(badSilence == 0); - CHECK(badDelay == 0); } int main() { @@ -303,7 +338,7 @@ int main() { testUnityRoughlyReproduces(); testTransposeDirection(); testRtDisciplineAndPassthrough(); - testRepitchSpectralPurity(); + testRepitchSpectralPurityAndOnset(); testUnityBitExactAndLatency(); if (g_fail == 0) { diff --git a/tests/test_sampler_core.cpp b/tests/test_sampler_core.cpp index 070c681..e8105cd 100644 --- a/tests/test_sampler_core.cpp +++ b/tests/test_sampler_core.cpp @@ -1337,33 +1337,42 @@ static void testPreserveVoiceCap() { // --------------------------------------------------------------------------- // FA1 (re-scoped by Phase S) — the Preserve unity-Varispeed bypass now belongs to the PREVIEW -// CARD ONLY. The MIDI engine keeps the shifter at EVERY Preserve note so a chromatic line has -// one uniform onset (the FA1-review ~25 ms root-note timing-step finding); the card — always -// fired at the effective root, latency-critical, with no line to be uneven against — opts in -// and speaks on frame one. +// CARD ONLY. The MIDI engine keeps the shifter at EVERY Preserve note. GA2 update: the primed +// shifter speaks on frame 0 at every ratio (the ring holds the first window of real source), +// so onset is uniformly IMMEDIATE across the keyboard and the bypass survives purely as a +// per-frame work-skip for the card. // --------------------------------------------------------------------------- -// The ENGINE'S root-note Preserve voice now keeps the OLA path: frame 0 is the shifter's fill -// (near-silent), full level once the ring fills — the SAME onset as its transposed neighbors. -// Pre-re-scope this voice was demoted and spoke at 1.0 on frame 0. -static void testPreserveUnityEngineVoiceKeepsUniformOnset() { - SampleData s = dcSample(4000, 60); - s.play.pitchEngine = PitchEngine::Preserve; - Keymap km = Keymap::singleSampleChromatic(std::move(s)); - VoiceEngine eng(1, km, /*preserveCap=*/0, /*window=*/512); - eng.noteOn(60, 127); // at root: unity shift — NO demotion in the MIDI engine - std::vector out; - eng.render(out, 1500); - double early = 0.0; - for (std::size_t i = 0; i < 8; ++i) { - early = (std::max)(early, static_cast(std::fabs(out[i]))); - } - CHECK(early < 0.1); // shifter onset, exactly like a transposed note - double late = 0.0; - for (std::size_t i = 600; i < 1500; ++i) { - late = (std::max)(late, static_cast(std::fabs(out[i]))); - } - CHECK(late > 0.9); // and the ring fills to full level +// The ENGINE'S root-note Preserve voice keeps the OLA path — and since the GA2 prime fix the +// primed shifter speaks on frame 0 at EVERY ratio (the ring holds the first window of real +// source, not warm-up zeros). Uniform onset across the keyboard now means uniformly IMMEDIATE: +// the root and a transposed neighbor both open at full level on the very first frames. +static void testPreserveUnityEngineVoiceSpeaksImmediately() { + auto earlyAndLate = [](int note, double& early, double& late) { + SampleData s = dcSample(4000, 60); + s.play.pitchEngine = PitchEngine::Preserve; + s.play.adsr = flatAdsr(); // isolate the shifter onset from the amp attack + Keymap km = Keymap::singleSampleChromatic(std::move(s)); + VoiceEngine eng(1, km, /*preserveCap=*/0, /*window=*/512); + eng.noteOn(note, 127); + std::vector out; + eng.render(out, 1500); + early = 1e9; + for (std::size_t i = 0; i < 8; ++i) { + early = (std::min)(early, static_cast(std::fabs(out[i]))); + } + late = 0.0; + for (std::size_t i = 600; i < 1500; ++i) { + late = (std::max)(late, static_cast(std::fabs(out[i]))); + } + }; + double earlyRoot = 0.0, lateRoot = 0.0, earlyUp = 0.0, lateUp = 0.0; + earlyAndLate(60, earlyRoot, lateRoot); // at root: unity shift — NO demotion in the engine + earlyAndLate(62, earlyUp, lateUp); // +2 st: a real shift, same immediate onset + CHECK(earlyRoot > 0.9); // primed ring: full level from frame 0 (no fill silence) + CHECK(earlyUp > 0.9); // ...uniformly across the keyboard (the GA2 onset-gap fix) + CHECK(lateRoot > 0.9); + CHECK(lateUp > 0.9); // and no gaps later either (splices land in real history) } // The PREVIEW CARD at unity speaks on frame ONE — the FA1 latency fix, now scoped to the card. @@ -1393,47 +1402,48 @@ static void testPreviewCardKeyTrackZeroAlsoSpeaksImmediately() { CHECK(approx(buf[0], 1.0, 1e-6)); } -// A TRANSPOSED preview keeps the genuine OLA path — the card's demotion is unity-ONLY. +// A TRANSPOSED preview keeps the genuine OLA path — the card's demotion is unity-ONLY. Since +// the GA2 prime fix onset silence can no longer distinguish the paths (both speak on frame 0), +// so prove it by DURATION: a Preserve Trigger at 100% of a 1000-frame sample holds ~1000 +// output frames at +12 st, where a Varispeed demotion would run off in ~500. static void testPreviewCardTransposedKeepsShifter() { - SampleData s = dcSample(4000, 60); - s.play.pitchEngine = PitchEngine::Preserve; - s.play.adsr = flatAdsr(); + SampleData s = preserveTriggerSample(1000, 1.0); Keymap km = Keymap::singleSampleChromatic(std::move(s)); PreviewCard card(km, /*preserveWindowFrames=*/512); - card.noteOn(62, 127); // +2 semitones: a real shift, NOT demoted - std::vector buf(8, 0.0f); - card.render(buf.data(), buf.size()); - double early = 0.0; - for (std::size_t i = 0; i < 8; ++i) { - early = (std::max)(early, static_cast(std::fabs(buf[i]))); + card.noteOn(72, 127); // +12 semitones: a real shift, NOT demoted + std::vector buf(1, 0.0f); + std::size_t len = 0; + for (std::size_t f = 0; f < 2000; ++f) { + buf[0] = 0.0f; + card.render(buf.data(), 1); + if (card.active()) len = f + 1; + else break; } - CHECK(early < 0.1); // shifter fill — duration preservation kept for off-root previews + CHECK(len > 700); // duration held (Preserve) — a Varispeed demotion would stop near 500 + CHECK(len < 1300); // ...and not doubled either (sanity) } -// A TRANSPOSED Preserve note keeps the genuine OLA path: onset is shifter-delayed (the inherent -// half-window cost of preserving duration) and the voice reaches full level once the ring fills. -// Also proves the demotion is unity-ONLY — the shifter still transposes off-root notes. -static void testPreserveTransposedVoiceKeepsOlaPath() { +// A TRANSPOSED Preserve note keeps the genuine OLA path — and since the GA2 prime fix that +// path has NO onset cost: the ring is primed with the first window of real source, so a +// transposed voice opens at full level on frame 0 (the DAW "zero-sample gaps in the first few +// ms" regression) and NEVER dips while the source sustains (splices land in real history, not +// warm-up zeros). +static void testPreserveTransposedVoiceSpeaksImmediately() { SampleData s = dcSample(4000, 60); s.play.pitchEngine = PitchEngine::Preserve; + s.play.adsr = flatAdsr(); // isolate the shifter onset from the amp attack Keymap km = Keymap::singleSampleChromatic(std::move(s)); VoiceEngine eng(1, km, /*preserveCap=*/0, /*window=*/512); eng.noteOn(62, 127); // +2 semitones: a real shift, NOT demoted std::vector out; eng.render(out, 1500); - // Early frames are the shifter's fill (near-silent) — the structural OLA onset. - double early = 0.0; - for (std::size_t i = 0; i < 8; ++i) { - early = (std::max)(early, static_cast(std::fabs(out[i]))); + // Full level from the very first frame (a DC source through complementary crossfades and + // aligned splices holds 1.0 throughout) — pre-fix the first ~window was fill silence. + double lo = 1e9; + for (std::size_t i = 0; i < 1500; ++i) { + lo = (std::min)(lo, static_cast(std::fabs(out[i]))); } - CHECK(early < 0.1); - // Once the ring is full of the DC source (>= window frames in), output reaches the sample - // level (Hann taps partition unity, so DC passes at gain 1). - double late = 0.0; - for (std::size_t i = 600; i < 1500; ++i) { - late = (std::max)(late, static_cast(std::fabs(out[i]))); - } - CHECK(late > 0.9); + CHECK(lo > 0.9); } // Phase S re-scope consequence: a ROOT-note engine Preserve voice keeps its shifter, so it @@ -2284,11 +2294,11 @@ int main() { // FA1 (re-scoped by Phase S) — the unity bypass is preview-card-only; the engine keeps a // uniform Preserve onset. Velocity under Preserve unchanged. - testPreserveUnityEngineVoiceKeepsUniformOnset(); + testPreserveUnityEngineVoiceSpeaksImmediately(); testPreviewCardUnitySpeaksImmediately(); testPreviewCardKeyTrackZeroAlsoSpeaksImmediately(); testPreviewCardTransposedKeepsShifter(); - testPreserveTransposedVoiceKeepsOlaPath(); + testPreserveTransposedVoiceSpeaksImmediately(); testPreserveUnityVoiceCountsTowardCap(); testVelocityCurveAppliesUnderPreserve(); From f7dfaa9f419d174c49310ceba637bef475fe681d Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 28 Jul 2026 09:10:03 -0400 Subject: [PATCH 2/2] 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 | 14 +++++++------ src/vst/sampler_core.cpp | 38 +++++++++++++++++++++++++---------- src/vst/sampler_core.h | 4 +++- tests/test_pitch_shift.cpp | 41 +++++++++++++++++++++++--------------- 4 files changed, 64 insertions(+), 33 deletions(-) diff --git a/src/vst/pitch_shift.cpp b/src/vst/pitch_shift.cpp index 8885585..72cd27b 100644 --- a/src/vst/pitch_shift.cpp +++ b/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(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 - // delay d + jump + maxLag + 1, so cap the jump at filled_ - d - maxLag_ - 1. In steady - // state (filled_ == ringLen_) this is > 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, never out-of-range. + // search (and the +/-1-lag parabolic refinement calls at bestLag ± 1, and the interpolator's + // read-ahead) can touch is delay d + jump + maxLag + 2 (maxLag from the coarse/fine search, + // +1 for the parabola's outer ± 1 probe, +1 for the interpolator's i1 = i0+1 read-ahead), + // so the tight cap is filled_ - d - maxLag_ - 2. The code uses - 1 here — one sample of + // conservative margin, never out-of-range. In steady state (filled_ == ringLen_) this is + // > 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; diff --git a/src/vst/sampler_core.cpp b/src/vst/sampler_core.cpp index c7b8de7..c48b4b5 100644 --- a/src/vst/sampler_core.cpp +++ b/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& 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(i)] = - (p < frameCount) ? pcmCh[static_cast(p)] : 0.0f; - ++p; + (q < frameCount) ? pcmCh[static_cast(q)] : 0.0f; + ++q; } (ch == 0 ? shiftL_ : shiftR_).prime(primeBuf_.data(), w); - // Both channels walk identical positions; the per-frame feed continues at `p`, - // exactly one window ahead of the output anchor readPos_. - feedPos_ = p; + if (ch == 0) p = q; // capture the end position once from channel 0's walk } + // 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); - const AudioSample feedL = feedOk ? pcm[static_cast(feedPos_)] : 0.0f; + // validThrough_ tail clamp (symmetric with the onset filled_ clamp in pitch_shift). + // 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(clampedFeedPos)] : 0.0f; const double shift = baseRatio_ * envFactor; shiftL_.setShiftRatio(shift); const double shiftedL = static_cast(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(feedPos_)] : 0.0f; + const AudioSample feedR = feedOk ? pcmR[static_cast(clampedFeedPos)] : 0.0f; shiftR_.setShiftRatio(shift); outRlocal = static_cast(shiftR_.process(feedR)) * gain; } else { diff --git a/src/vst/sampler_core.h b/src/vst/sampler_core.h index 3f8d8bd..de6c956 100644 --- a/src/vst/sampler_core.h +++ b/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 diff --git a/tests/test_pitch_shift.cpp b/tests/test_pitch_shift.cpp index 4cdcad5..371ac6b 100644 --- a/tests/test_pitch_shift.cpp +++ b/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 - // 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. + // (a) f0*(w/2) ≈ (2205/2)/196.37 ≈ 5.609 cycles (frac ≈ 0.609) — 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.37 ≈ 22.46 — near the half-integer alignment that makes the + // pre-fix fade-headroom artifact visible at ratio 4 (+24 st). Additionally, period + // 196.37 is NON-INTEGER, so the correlation peak is NOT on the integer lag grid; the + // sub-sample parabolic refinement is LOAD-BEARING to stay at the -84 dB floor — the + // old integer f0=1/196 put the optimum on the grid and the parabola contributed + // nothing, making the -30 dB floor reachable without it. // // 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. - // (Post-fix the engine measures ~-75 dB and better; the old integer-lag splices - // sat near -59 dB sidebands and the warm-zero onset failed outright.) - CHECK(residRms < 0.0316 * fitRms); + // ...and it is the ONLY thing there: residual at least 84 dB under the tone. + // With the non-integer f0=1/196.37, the sub-sample parabolic refinement is + // LOAD-BEARING: reverting it raises the floor to ~-50 dB (ratio 1/8), failing here. + // 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 } } }