fix(sampler_core): bounded-blend declick (no overshoot), soundingNote() gates cap + legato
Replaces frozen-difference declick with bounded blend (out*(1-w)+ref*w, bounded by max(|ref|,|out|)). soundingNote() gates activePreserveVoices and mono-Legato retune away from ramp-only past-end voices. Magnitude-bound test asserts |v|<=1 across the ramp.
This commit is contained in:
+52
-33
@@ -424,19 +424,27 @@ double Voice::tickAmplitude() {
|
||||
}
|
||||
|
||||
void Voice::seedDeclick(double newOutL, double newOutR) {
|
||||
// First frame after a takeover restart: the compensation is the ACTUAL discontinuity —
|
||||
// (pre-cut reference − the new voice's raw output this frame). Added ungated in the
|
||||
// epilogue, so this frame's output is newOut + (ref − newOut) == ref: zero step at the
|
||||
// boundary by construction. Clamped to ±2.0 (both operands are bounded near full scale).
|
||||
// First frame after a takeover restart: ARM the bounded blend. The weight starts at 1.0
|
||||
// so this frame's output is `out*(1-1) + ref*1 == ref` — exact boundary identity whatever
|
||||
// the new envelope's first value. Each subsequent frame adds `w*(ref − outCurrent)` then
|
||||
// decays w, so output is provably bounded by max(|ref|, |outCurrent|) — mid-ramp overshoot
|
||||
// is impossible even if outCurrent rises while the weight is still significant.
|
||||
// [Rev 1 stored the frozen difference (ref − x₀); if outₙ rose while that residue was
|
||||
// still large the sum could exceed full scale. The ±2.0 clamp there was the only guard
|
||||
// and it silently broke the boundary identity when |x₀| > 1. The bounded blend removes
|
||||
// both the overshoot hole and the need for a clamp on the stored value.]
|
||||
// newOutL/R are used only to decide whether an active ramp exists (the seed is purely
|
||||
// the weight 1.0; ref was clamped to ±1 at start()). The ±2 clamp on the difference is
|
||||
// gone: the blend formula keeps every output within max(|ref|,|outₙ|) by construction.
|
||||
(void)newOutL; (void)newOutR; // consumed only for the floor guard below
|
||||
declickPending_ = false;
|
||||
double dl = declickRefL_ - newOutL;
|
||||
double dr = declickRefR_ - newOutR;
|
||||
dl = (dl > 2.0) ? 2.0 : (dl < -2.0) ? -2.0 : dl;
|
||||
dr = (dr > 2.0) ? 2.0 : (dr < -2.0) ? -2.0 : dr;
|
||||
declickL_ = dl;
|
||||
declickR_ = dr;
|
||||
declickActive_ = (dl > kDeclickFloor || dl < -kDeclickFloor ||
|
||||
dr > kDeclickFloor || dr < -kDeclickFloor);
|
||||
declickL_ = 1.0;
|
||||
declickR_ = 1.0;
|
||||
// The reference is already clamped to ±1.0 at start() (lines in start(): the ±1 clamp
|
||||
// on lastOutL_/R_ before storing into declickRefL_/R_). No secondary clamp needed here.
|
||||
// Activate only when the ref itself is above the floor — if ref ≈ 0 there is nothing to blend.
|
||||
declickActive_ = (declickRefL_ > kDeclickFloor || declickRefL_ < -kDeclickFloor ||
|
||||
declickRefR_ > kDeclickFloor || declickRefR_ < -kDeclickFloor);
|
||||
}
|
||||
|
||||
AudioSample Voice::advanceFrame(bool stereo, AudioSample& outR) {
|
||||
@@ -484,12 +492,13 @@ AudioSample Voice::advanceFrame(bool stereo, AudioSample& outR) {
|
||||
if (triggerRanOff || readPos_ >= static_cast<double>(frameCount)) {
|
||||
if (declickPending_) seedDeclick(0.0, 0.0); // the new output here is silence
|
||||
if (declickActive_) {
|
||||
const double l = declickL_;
|
||||
const double r = declickR_;
|
||||
// Bounded blend at silence: outCurrent == 0, so the blend is w*(ref − 0) == w*ref.
|
||||
// The weight decays by kDeclickDecay each frame, floor-checked on the weight itself.
|
||||
const double l = declickL_ * declickRefL_;
|
||||
const double r = declickL_ * declickRefR_; // same weight for both channels
|
||||
declickL_ *= kDeclickDecay;
|
||||
declickR_ *= kDeclickDecay;
|
||||
if (declickL_ < kDeclickFloor && declickL_ > -kDeclickFloor &&
|
||||
declickR_ < kDeclickFloor && declickR_ > -kDeclickFloor) {
|
||||
if (declickL_ < kDeclickFloor && declickL_ > -kDeclickFloor) {
|
||||
declickActive_ = false;
|
||||
active_ = false;
|
||||
}
|
||||
@@ -571,24 +580,24 @@ AudioSample Voice::advanceFrame(bool stereo, AudioSample& outR) {
|
||||
ratio_ = baseRatio_ * envFactor;
|
||||
}
|
||||
|
||||
// Takeover declick (Phase S GA fix, rev 2): on the FIRST frame after a takeover/steal
|
||||
// restart, seed the compensation from the ACTUAL discontinuity — the pre-cut output
|
||||
// minus this frame's raw new output — then add the decaying compensation UNGATED. The
|
||||
// boundary frame therefore carries the old level exactly (out + (ref − out) == ref) and
|
||||
// every later frame moves by the new signal's own slope plus a ≤5%-of-seed decay step.
|
||||
// Engine- and play-mode-agnostic: rev 1's (1 − amp) gate zeroed the ramp whenever the
|
||||
// new envelope opened instantly at ~1 (Trigger's no-fade-in onset, zero-attack Gate) —
|
||||
// exactly where the DAW still clicked. Because the seed is a DIFFERENCE, matching old
|
||||
// and new levels seed ~0 and add nothing — no +6 dB blip without any gate. Inactive
|
||||
// (the common case) costs one branch.
|
||||
// Takeover declick (Phase S GA fix, rev 2, bounded-blend revision): on the FIRST frame
|
||||
// after a takeover/steal restart, seed the blend weight at 1.0 so this frame's output is
|
||||
// outₙ*(1−w) + ref*w = out*(1−1) + ref*1 = ref (exact boundary identity).
|
||||
// Each subsequent frame the blend add is `w*(ref − outCurrent)` and then w decays by
|
||||
// kDeclickDecay. The output is therefore bounded by max(|ref|, |outCurrent|) in every
|
||||
// frame — mid-ramp overshoot from a rising outCurrent is structurally impossible.
|
||||
// [Rev 1 added the frozen difference (ref − x₀) ungated; if outₙ rose while the residue
|
||||
// was still large the sum could exceed ±1 by up to ~+3.8 dB on an extreme retrig.]
|
||||
// Inactive (the common case) costs one branch; the blend itself costs one extra subtract.
|
||||
if (declickPending_) seedDeclick(outL, stereo ? outRlocal : outL);
|
||||
if (declickActive_) {
|
||||
outL += declickL_;
|
||||
if (stereo) outRlocal += declickR_;
|
||||
const double addL = declickL_ * (declickRefL_ - outL);
|
||||
const double addR = declickL_ * (declickRefR_ - (stereo ? outRlocal : outL));
|
||||
outL += addL;
|
||||
if (stereo) outRlocal += addR;
|
||||
declickL_ *= kDeclickDecay;
|
||||
declickR_ *= kDeclickDecay;
|
||||
if (declickL_ < kDeclickFloor && declickL_ > -kDeclickFloor &&
|
||||
declickR_ < kDeclickFloor && declickR_ > -kDeclickFloor) {
|
||||
declickR_ *= kDeclickDecay; // kept in sync (mirrors L — both channels share one weight)
|
||||
if (declickL_ < kDeclickFloor && declickL_ > -kDeclickFloor) {
|
||||
declickActive_ = false;
|
||||
}
|
||||
}
|
||||
@@ -655,9 +664,13 @@ VoiceEngine::VoiceEngine(std::size_t maxVoices, const Keymap& keymap,
|
||||
}
|
||||
|
||||
std::size_t VoiceEngine::activePreserveVoices() const {
|
||||
// Count only voices that are SOUNDING A NOTE (playable span still running), not voices
|
||||
// that have finished their note but are still ringing out a declick tail. A ramp-only
|
||||
// past-end voice must not consume a cap slot — that would cause a new Preserve note-on to
|
||||
// be dropped (kNoVoice return at :797-800) during the narrow ~4 ms window the ramp lives.
|
||||
std::size_t n = 0;
|
||||
for (const Voice& v : voices_) {
|
||||
if (v.active() && v.pitchEngine() == PitchEngine::Preserve) ++n;
|
||||
if (v.soundingNote() && v.pitchEngine() == PitchEngine::Preserve) ++n;
|
||||
}
|
||||
return n;
|
||||
}
|
||||
@@ -729,7 +742,13 @@ std::size_t VoiceEngine::monoNoteOn(int note, int velocity) {
|
||||
// re-attacked. NOTE: a one-held-note same-note re-press (heldCount_ becomes 1 after the
|
||||
// removeHeld/re-push above — so heldCount_ < 2) re-attacks rather than retuning, which is
|
||||
// the correct fresh-phrase behavior for that edge case.) Same-sample requirement unchanged.
|
||||
if (v.active() && heldCount_ >= 2 && monoTrigger_ == MonoTrigger::Legato &&
|
||||
//
|
||||
// soundingNote() (not just active()): a voice whose note has run to its play-end but is
|
||||
// still ringing a declick tail must NOT be retuned — that would move the pitch of a dying
|
||||
// ramp rather than restarting the new note, producing a silent note on the common
|
||||
// "hammer same key while a past-end ring-out is active" path. The tail should keep fading;
|
||||
// the new note-on restarts the voice normally (monoNoteOn falls through to start() below).
|
||||
if (v.soundingNote() && heldCount_ >= 2 && monoTrigger_ == MonoTrigger::Legato &&
|
||||
v.playingSample() == &sample) {
|
||||
v.retune(note, zone.rootNote, zone.keyTrack);
|
||||
return 0;
|
||||
|
||||
+21
-8
@@ -464,8 +464,16 @@ public:
|
||||
// instantly (which release() cannot do). RT-safe: no allocation, no lock.
|
||||
void hardStop();
|
||||
|
||||
// True while this voice is producing (or about to produce) sound.
|
||||
// True while this voice is producing (or about to produce) sound (including any
|
||||
// declick ring-out tail past the note's playable span).
|
||||
bool active() const { return active_; }
|
||||
// True while this voice is sounding a PLAYABLE NOTE — active AND the amplitude
|
||||
// envelope has not yet finished. A voice whose note has run to its end but is still
|
||||
// ringing out a declick tail is active() but NOT soundingNote(). Use this to
|
||||
// distinguish "note is alive" (active) from "note occupies a voice slot" (soundingNote)
|
||||
// for the Preserve-cap count and the mono-Legato takeover predicate — both must ignore
|
||||
// a ramp-only past-end voice or a new note-on can be dropped / silently muted.
|
||||
bool soundingNote() const { return active_ && !amplitudeDone_; }
|
||||
// The note this voice was started on (for note-off routing). Meaningless if idle.
|
||||
int note() const { return note_; }
|
||||
// Monotonic age counter — higher = started earlier relative to others. The voice
|
||||
@@ -557,17 +565,22 @@ private:
|
||||
|
||||
// Takeover declick state (see kDeclickDecay above). lastOut{L,R}_ track the voice's most
|
||||
// recent rendered output (post-gain, incl. any running declick). A takeover/steal start()
|
||||
// records them as declickRef{L,R}_ (the pre-cut reference) and sets declickPending_; the
|
||||
// first frame rendered after the restart calls seedDeclick to derive declick{L,R}_ from
|
||||
// (reference − that frame's raw new output), and declickActive_ then gates the per-frame
|
||||
// ungated add + decay. lastOut is NOT zeroed by start() — a second same-block takeover
|
||||
// (no frame rendered between) must record the same pre-cut reference, not a phantom 0.
|
||||
// records them as declickRef{L,R}_ (the clamped pre-cut reference) and sets declickPending_;
|
||||
// the first frame rendered after the restart calls seedDeclick to arm the BOUNDED BLEND:
|
||||
// outₙ = outₙ*(1−w) + ref*w where w = declickL_/R_ starts at 1.0 and decays by
|
||||
// kDeclickDecay each frame. This is algebraically `outₙ + w*(ref − outₙ)`, so the
|
||||
// boundary frame (w=1) is exactly `ref` and every subsequent output is bounded by
|
||||
// max(|ref|, |outₙ|) — mid-ramp overshoot is impossible regardless of outₙ rising.
|
||||
// [Rev 1 stored the frozen difference (ref − x₀); when outₙ rose while that residue
|
||||
// was still large the sum could exceed full scale by up to ~+3.8 dB.]
|
||||
// lastOut is NOT zeroed by start() — a second same-block takeover (no frame rendered
|
||||
// between) must record the same pre-cut reference, not a phantom 0.
|
||||
// The whole declick state is cleared on a fresh (non-takeover) start.
|
||||
bool declickPending_ = false;
|
||||
bool declickActive_ = false;
|
||||
double declickRefL_ = 0.0;
|
||||
double declickRefL_ = 0.0; // clamped pre-cut reference (bounded blend target)
|
||||
double declickRefR_ = 0.0;
|
||||
double declickL_ = 0.0;
|
||||
double declickL_ = 0.0; // blend weight w; 1.0 on seed, decays by kDeclickDecay/frame
|
||||
double declickR_ = 0.0;
|
||||
double lastOutL_ = 0.0;
|
||||
double lastOutR_ = 0.0;
|
||||
|
||||
@@ -2335,6 +2335,50 @@ static void testPreviewCardReplaceStaleOffAndOutOfZone() {
|
||||
CHECK(!card.active());
|
||||
}
|
||||
|
||||
// GA2 — bounded-blend overshoot regression: mid-ramp output must stay within full scale.
|
||||
//
|
||||
// Construction of the worst case (§1 reviewer finding): retrig a sine at a point where the
|
||||
// pre-cut level is ~1.0 (old ref ≈ 1). The new voice starts at sin(0) == 0, so the OLD
|
||||
// frozen-seed declick adds (ref − x₀) ≈ 1.0 to the compensation. The new sine has a short
|
||||
// period (8 frames) so outₙ reaches ~1.0 again within just 2 frames; at that moment the
|
||||
// frozen seed is still ~0.9 → outₙ + seed ≈ 1.9, roughly +3.8 dB over full scale.
|
||||
//
|
||||
// The bounded blend keeps every frame within max(|ref|, |outCurrent|) ≤ 1.0 + tol — this
|
||||
// test must FAIL against the rev-2 frozen-seed code and PASS with the bounded blend.
|
||||
static void testDeclickBoundedBlendNoOvershoot() {
|
||||
// A sine with 8-frame period so it peaks within the declick ramp window (~80 frames).
|
||||
// 48000 frames, 6000 cycles -> period = 8 frames; quarter period = 2 frames = the peak.
|
||||
const std::size_t kFrames = 48000;
|
||||
const double kCycles = 6000.0; // period = 8 frames
|
||||
SampleData s = sineSample(kFrames, kCycles, 60);
|
||||
s.play.playMode = PlayMode::Trigger; // no fade-in -> amp 1 on frame 0 (worst case)
|
||||
Keymap km = Keymap::singleSampleChromatic(std::move(s));
|
||||
VoiceEngine eng(1, km, 0, 0, VoiceMode::Mono, MonoTrigger::Retrigger,
|
||||
/*takeoverDeclick=*/true);
|
||||
|
||||
// Start a voice and render to a quarter period so the sine is near its positive peak.
|
||||
// Period = 48000/6000 = 8 frames. Frame index 2 = sin(2π*6000*2/48000) = sin(π/2) = 1.0.
|
||||
// We render 3 frames (indices 0,1,2 are visited: readPos 0→1→2→3) so pre[2] reads
|
||||
// frame index 2 at the sine peak.
|
||||
eng.noteOn(60, 127);
|
||||
std::vector<AudioSample> pre;
|
||||
eng.render(pre, 3); // frame index 2 (read on third render): sin(pi/2) ≈ 1.0
|
||||
CHECK(pre.back() > 0.99f); // at peak: ref ≈ 1.0 when we cut
|
||||
|
||||
// Retrigger: hard restart at sin(0) == 0, ref == ~1.0. The frozen-seed approach would
|
||||
// add ~0.9 to a new output of ~1.0 two frames later → ~1.9. The bounded blend must not.
|
||||
eng.noteOn(60, 127);
|
||||
const double tol = 1e-3;
|
||||
std::vector<AudioSample> post;
|
||||
eng.render(post, 200); // 200 frames covers the full ramp (~80 frames at kDeclickDecay=0.95)
|
||||
for (std::size_t i = 0; i < post.size(); ++i) {
|
||||
const double v = static_cast<double>(post[i]);
|
||||
CHECK(v <= 1.0 + tol && v >= -1.0 - tol);
|
||||
}
|
||||
// Boundary identity: first frame must reproduce the pre-cut level (±small tol).
|
||||
CHECK(std::fabs(static_cast<double>(post[0]) - static_cast<double>(pre.back())) < 0.01);
|
||||
}
|
||||
|
||||
int main() {
|
||||
testChromaticSingleRoot();
|
||||
testZonedRangesBoundaries();
|
||||
@@ -2439,6 +2483,7 @@ int main() {
|
||||
testZeroAttackGateRetrigNoStep();
|
||||
testPreviewRetriggerDeclicksRestart();
|
||||
testPreviewDefaultOffKeepsHardCutBaseline();
|
||||
testDeclickBoundedBlendNoOvershoot();
|
||||
testPreviewCardIsolatedFromPool();
|
||||
testPreviewCardReplaceStaleOffAndOutOfZone();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user