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 } } }