From 81c46587117250b57d50658654fc3d87980e0f20 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 28 Jul 2026 10:30:51 -0400 Subject: [PATCH] Preview rides the real note path: mailbox drains into VoiceEngine noteOn/noteOff at the root note; PreviewCard retired; first-poll reopen heal reloads a silent-but-should-be-loaded instrument --- src/vst/reasampler_processor.cpp | 89 +++++++-------- src/vst/reasampler_processor.h | 64 ++++++----- src/vst/sampler_core.cpp | 89 +-------------- src/vst/sampler_core.h | 92 +++------------ tests/test_sampler_core.cpp | 187 +++++++------------------------ 5 files changed, 136 insertions(+), 385 deletions(-) diff --git a/src/vst/reasampler_processor.cpp b/src/vst/reasampler_processor.cpp index d443d4a..af4e91c 100644 --- a/src/vst/reasampler_processor.cpp +++ b/src/vst/reasampler_processor.cpp @@ -579,7 +579,7 @@ void ReaSamplerProcessor::publishBuiltLocked(std::unique_ptr b void ReaSamplerProcessor::rebuildVoiceEngine() { // OFF THE AUDIO THREAD (the editor's voice-deck click handlers). See the header contract: - // a voice-param change touches NO audio data, so this rebuilds the engine + preview card + // a voice-param change touches NO audio data, so this rebuilds the engine // around a COPY of the live instrument's already-decoded keymap — no bridge, no disk — // and publishes through the same drain-slot swap, so ringing tails survive. std::lock_guard lock(reloadMutex_); @@ -718,9 +718,24 @@ ReaSamplerProcessor::pollBankSync(bool isFocusedTarget) { !firstPoll && bankGenerationChanged(lastSeenBankGeneration_, currentGen); lastSeenBankGeneration_ = currentGen; - if (genChanged || result.applied) { + // REOPEN HEAL: the baseline-without-reload assumption above fails when the setState-time + // reload ran BEFORE the extension's PROJEXTSTATE was parseable during project load — the + // bridge read came back empty, so live_ installed SILENCE while the restored state (a + // selection or zones) says something SHOULD be loaded. Without this, the swallowed + // baseline left the instrument (preview AND host MIDI) dead until some param change + // forced a reload. Detect the mismatch on the first poll and reload; a deliberately + // empty instance (no selection, no zones) never churns, and a load that legitimately + // failed (missing WAV) costs one redundant, harmless reload on editor open. + bool healReload = false; + if (firstPoll && live_.load(std::memory_order_acquire) == nullptr) { + healReload = !selectedSampleId().empty() || !performanceMap().empty(); + } + + if (genChanged || result.applied || healReload) { reloadFromBank(); // atomic pointer-swap handoff — glitch-free mid-play (S4 graveyard) - result.reloaded = genChanged; // report S9 vs S8 distinctly for the editor's reaction + // Report the reload (S9 change or reopen heal) distinctly from an S8 apply so the + // editor re-snapshots its bank view. + result.reloaded = genChanged || healReload; } return result; } @@ -758,7 +773,7 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { processGeneration_.store(heldGen, std::memory_order_release); // Phase S drain retirement: publish whether the drain snapshot is FULLY idle (every engine - // voice AND its preview card silent) by naming its OWN installedAt (0 = no drain / still + // voice silent) by naming its OWN installedAt (0 = no drain / still // sounding). Evaluated at block START — idleness is monotone for a drain (it receives no // note-ons), so a snapshot observed idle here stays idle; a tail that dies mid-block simply // publishes one block later. Bounded scan (<= maxVoices), relaxed store — RT-safe. @@ -797,28 +812,17 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { // CC 120 (All Sounds Off): hard-stop semantics — immediate silence regardless // of play mode, including Trigger one-shots that ignore CC 123. This is the // true "panic" for a ringing one-shot (e.g. a full-length capture). - // Both clear the mono held stack. Both apply to live AND drain, engine AND preview. - // allNotesOff / allSoundsOff / releaseAll / hardStop are RT-safe (no allocation, - // bounded scans). + // Both clear the mono held stack. Both apply to live AND drain. A ringing + // preview note is a real engine voice since the PreviewCard retirement, so + // the panics cover it with no separate routing. allNotesOff / allSoundsOff + // are RT-safe (no allocation, bounded scans). const auto cc = static_cast(e.midiCCOut.controlNumber); if (cc == kCtrlAllSoundsOff) { - if (inst) { - inst->engine.allSoundsOff(); - inst->preview.hardStop(); - } - if (drain) { - drain->engine.allSoundsOff(); - drain->preview.hardStop(); - } + if (inst) inst->engine.allSoundsOff(); + if (drain) drain->engine.allSoundsOff(); } else if (cc == kCtrlAllNotesOff) { - if (inst) { - inst->engine.allNotesOff(); - inst->preview.releaseAll(); - } - if (drain) { - drain->engine.allNotesOff(); - drain->preview.releaseAll(); - } + if (inst) inst->engine.allNotesOff(); + if (drain) drain->engine.allNotesOff(); } } } @@ -827,10 +831,11 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { // S-VIEW-4 preview mailbox: drain the off-thread preview-trigger requests (a single relaxed // atomic load each — RT-safe). A request is NEW when its packed sequence differs from the last // one we consumed; fire it once, then latch the sequence so the same request never re-fires. - // Phase S: preview note-on/off drive the dedicated PREVIEW CARD — a single voice structurally - // OUTSIDE the MIDI pool, so a full pool can never drop a preview and a preview can never - // steal a playing MIDI voice (the FA1-review isolation fix). Host MIDI routes ONLY to the - // engine (above); the card is summed alongside it in the render below. + // Preview redesign: the drained requests drive the MAIN VoiceEngine — the exact + // noteOn/noteOff calls the host MIDI marshal above makes — so a preview note is a real + // voice: it counts against the voice count, can steal / be stolen, and respects + // Poly/Mono + Retrigger/Legato (deliberate reversal of the retired PreviewCard's + // isolation). The editor posts the root note, so it plays at unity. // Consume (advance the sequence) even when inst is null so a note-on posted while no instrument // is loaded does not re-fire stale on the next instrument load. { @@ -841,7 +846,7 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { if (inst) { const int vel = static_cast((on >> 8) & 0xFF); const int note = static_cast(on & 0xFF); - if (vel > 0) inst->preview.noteOn(note, vel); + if (vel > 0) inst->engine.noteOn(note, vel); } } } @@ -850,11 +855,11 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { const std::uint16_t offSeq = static_cast(off >> 16); if (offSeq != 0 && offSeq != previewOffConsumed_) { previewOffConsumed_ = offSeq; - // Route the preview note-off to BOTH cards (mirror of the host note-off): a + // Route the preview note-off to BOTH engines (mirror of the host note-off): a // preview held across a reload — e.g. a curve edit committed mid-press — must - // release the old-snapshot card now draining, not just the (fresh) live one. - if (inst) inst->preview.noteOff(static_cast(off & 0xFF)); - if (drain) drain->preview.noteOff(static_cast(off & 0xFF)); + // release the old-snapshot voice now draining, not just the (fresh) live one. + if (inst) inst->engine.noteOff(static_cast(off & 0xFF)); + if (drain) drain->engine.noteOff(static_cast(off & 0xFF)); } } @@ -892,14 +897,8 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { // path (both channels equal), so a mono capture in stereo mode is centered, not silent. // The DRAIN engine's ringing tails ADD on top (render mixes into the cleared buffer). for (int32 i = 0; i < frames; ++i) { ch0[i] = 0.f; ch1[i] = 0.f; } - if (inst) { - inst->engine.render(ch0, ch1, static_cast(frames)); - inst->preview.render(ch0, ch1, static_cast(frames)); - } - if (drain) { - drain->engine.render(ch0, ch1, static_cast(frames)); - drain->preview.render(ch0, ch1, static_cast(frames)); - } + if (inst) inst->engine.render(ch0, ch1, static_cast(frames)); + if (drain) drain->engine.render(ch0, ch1, static_cast(frames)); // FB1 post-mixer master gain: ramp gainCurrent_ toward the atomic target per-sample so // continuous knob drags produce no zipper noise and the true-zero bottom causes no click. // Applied AFTER the voice sum and BEFORE the extra-channel mirror + peak so both see the @@ -943,14 +942,8 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { // Mono: render into channel 0, replicate to any extra channels (mono bus is 1 channel; // the replicate is defensive for a host that still hands >1 channel on a mono bus). for (int32 i = 0; i < frames; ++i) ch0[i] = 0.f; - if (inst) { - inst->engine.render(ch0, static_cast(frames)); - inst->preview.render(ch0, static_cast(frames)); - } - if (drain) { - drain->engine.render(ch0, static_cast(frames)); - drain->preview.render(ch0, static_cast(frames)); - } + if (inst) inst->engine.render(ch0, static_cast(frames)); + if (drain) drain->engine.render(ch0, static_cast(frames)); // FB1 post-mixer master gain (mono path) — same ramp contract as the stereo branch: // post-sum, pre-peak/replicate, per-sample gainCurrent_ ramp toward target, RT-safe. { diff --git a/src/vst/reasampler_processor.h b/src/vst/reasampler_processor.h index fe4dd98..2cdba8e 100644 --- a/src/vst/reasampler_processor.h +++ b/src/vst/reasampler_processor.h @@ -35,11 +35,10 @@ namespace reasampler::vst { class ReaSamplerEmbed; // S6 embedded TCP/MCP UI shell (owned below; see queryInterface) -// One fully-built, ready-to-play instrument snapshot: the decoded keymap, the voice -// engine that plays it, and the isolated PREVIEW CARD (Phase S) summed alongside it. -// Engine and card both hold references into the keymap, so the three MUST live and die -// together at a STABLE address — hence this is heap-allocated and neither copyable nor -// movable. The audio thread only ever reads it through an atomic pointer; it is built +// One fully-built, ready-to-play instrument snapshot: the decoded keymap and the voice +// engine that plays it. The engine holds references into the keymap, so the two MUST live +// and die together at a STABLE address — hence this is heap-allocated and neither copyable +// nor movable. The audio thread only ever reads it through an atomic pointer; it is built // and destroyed off the audio thread. // // installedAt: the reloadGeneration_ value at which this instrument was atomically @@ -49,14 +48,13 @@ class ReaSamplerEmbed; // S6 embedded TCP/MCP UI shell (owned below; see queryI struct LoadedInstrument { Keymap keymap; VoiceEngine engine; - PreviewCard preview; // Phase S: the isolated preview voice — never part of the pool std::uint64_t installedAt = 0; // reload generation at which this was installed // The takeover declick (GA fix, rev 2) is opted IN here — the PRODUCT default: any // restart of a sounding voice (mono Retrigger takeover/fallback, cross-sample legato - // restart, POLY at-cap steal, AND the preview card's replace-restart) smooths the cut - // via the difference-seeded ramp instead of clicking. The pure core defaults it off - // (regression baseline) — same layering as the kDefaultPitchEngine product default. + // restart, POLY at-cap steal — the preview note included, now that it is a real pool + // voice) smooths the cut via the difference-seeded ramp instead of clicking. The pure + // core defaults it off (regression baseline) — same layering as kDefaultPitchEngine. LoadedInstrument(Keymap km, std::size_t maxVoices, std::uint64_t gen, std::size_t preserveVoiceCap = 0, std::int64_t preserveWindowFrames = 0, @@ -65,13 +63,12 @@ struct LoadedInstrument { : keymap(std::move(km)), engine(maxVoices, keymap, preserveVoiceCap, preserveWindowFrames, voiceMode, monoTrigger, /*takeoverDeclick=*/true), - preview(keymap, preserveWindowFrames, /*takeoverDeclick=*/true), installedAt(gen) {} - // True when nothing in this snapshot is sounding — engine voices AND the preview card. - // process() publishes this for the drain slot so the off-thread retirer can park an - // idle drain in the graveyard early (FA1-review Major #2). Bounded scan (<= maxVoices). - bool fullyIdle() const { return engine.activeVoiceCount() == 0 && !preview.active(); } + // True when nothing in this snapshot is sounding. process() publishes this for the + // drain slot so the off-thread retirer can park an idle drain in the graveyard early + // (FA1-review Major #2). Bounded scan (<= maxVoices). + bool fullyIdle() const { return engine.activeVoiceCount() == 0; } LoadedInstrument(const LoadedInstrument&) = delete; LoadedInstrument& operator=(const LoadedInstrument&) = delete; @@ -228,21 +225,24 @@ public: } void setMasterGainLinear(double linear); // clamped to [0, masterGainMaxLinear()] - // Fire a one-shot PREVIEW note-on / note-off through the live instrument's PREVIEW CARD - // (S-VIEW-4; Phase S isolation) — a dedicated single voice structurally OUTSIDE the MIDI - // pool, so a full pool never drops a preview and a preview never steals a playing voice. - // OFF the audio thread (the editor's preview-trigger button drives these on the UI thread). - // The request is handed to process() via a lock-free single-slot mailbox drained at block - // start — no allocation, no lock on the audio thread. previewNoteOn plays `note` at the - // current previewVelocity(); previewNoteOff releases it (Gate) — Trigger zones ignore - // note-off and play through. A momentary button (down = on, up = off) reads as a natural - // key press. This is PLAYBACK ONLY: it never captures, never inserts a timeline item. + // Fire a one-shot PREVIEW note-on / note-off through the live instrument's MAIN + // VoiceEngine — the SAME noteOn/noteOff calls host MIDI takes, so a preview is a REAL + // voice: it counts against the voice count, can steal / be stolen, and respects + // Poly/Mono + Retrigger/Legato (deliberate reversal of the retired PreviewCard's + // isolation — preview must obey voicing). The editor posts the loaded capture's / + // selected zone's ROOT note (plays at unity); previewNoteOn plays it at the current + // previewVelocity() (the velocity curve applies); previewNoteOff releases it (Gate) — + // Trigger zones ignore note-off and play through. OFF the audio thread (the editor's + // preview-trigger button, UI thread); the request is handed to process() via a + // lock-free single-slot mailbox drained at block start — no allocation, no lock on the + // audio thread. A momentary button (down = on, up = off) reads as a natural key press. + // This is PLAYBACK ONLY: it never captures, never inserts a timeline item. void previewNoteOn(int note); void previewNoteOff(int note); private: // Phase S drain retirement (FA1-review Major #2): if process() has published that the - // CURRENT drain instrument is fully idle (every engine voice + the preview card silent), + // CURRENT drain instrument is fully idle (every engine voice silent), // move it out of the drain slot into the graveyard and prune — so an edited-away snapshot // stops costing resident memory as soon as its tails die, instead of squatting in the slot // until the NEXT reload. Off the audio thread only (takes reloadMutex_); driven from @@ -254,8 +254,8 @@ private: // proof (see below) covers the free. void retireIdleDrain(); - // Phase S voice-param LIGHT rebuild (voice-review Major #3): rebuild the engine + preview - // card around a COPY of the LIVE instrument's already-decoded Keymap — no bridge read, no + // Phase S voice-param LIGHT rebuild (voice-review Major #3): rebuild the engine + // around a COPY of the LIVE instrument's already-decoded Keymap — no bridge read, no // filesystem, no WAV re-decode — and publish through the same tail-preserving drain-slot // swap as a full reload. A polyphony/mode/trigger change touches no audio data, so the // full reloadFromBank (which re-decodes every zone WAV from disk on the UI thread) was @@ -310,7 +310,7 @@ private: std::atomic reloadGeneration_{0}; // incremented by each reload (off-thread, under reloadMutex_; read atomically by process) std::atomic processGeneration_{0}; // min installedAt held by process (written on audio thread, read off-thread) // Phase S: the installedAt of the drain instrument process() last observed FULLY IDLE - // (0 = none / the current drain still sounds). Written relaxed on the audio thread each + // (every engine voice silent; 0 = none / the current drain still sounds). Written relaxed on the audio thread each // block; read by retireIdleDrain() off-thread. Naming the generation (not a bool) closes // the swap race: a publication about an old drain can never retire its successor. std::atomic drainIdleGeneration_{0}; @@ -352,7 +352,11 @@ private: // -1 SENTINEL (no real generation can be negative — parseBankGeneration yields >= 0) so the // FIRST poll after an editor open BASELINES the seen value without a redundant reload (setState // already loaded the current bank); a subsequent generation CHANGE then drives the reload. - // NOT read on the audio thread. + // REOPEN HEAL exception: when that setState-time load LEFT NOTHING LIVE despite restored + // intent (a selection or zones) — the project-load ordering can run setState before the + // extension's PROJEXTSTATE block is parseable, so the bridge read came back empty — the + // first poll reloads instead of silently baselining, or the instrument would stay silent + // until some param change forced a reload. NOT read on the audio thread. std::int64_t lastSeenBankGeneration_ = -1; // S-VIEW-4 preview-trigger velocity (MIDI 1..127). Persisted in component state (v6) so the @@ -384,8 +388,8 @@ private: // --- S-VIEW-4 preview-trigger mailbox (off-thread -> audio thread, lock-free) --------- // The editor's preview-trigger button posts a note-on/off request from the UI thread; process() - // drains it at block start and drives the live instrument's PREVIEW CARD (Phase S — never the - // MIDI pool). ONE slot per direction, each a packed + // drains it at block start and drives the live instrument's MAIN VoiceEngine — the same + // noteOn/noteOff host MIDI takes, so the preview obeys voicing. ONE slot per direction, each a packed // request whose high bits are a monotonically-incrementing sequence so process() detects a NEW // request by comparing against the last sequence it consumed (never re-firing a stale one). The // low 8 bits carry the note (on) / note (off); the on request also carries the velocity in the diff --git a/src/vst/sampler_core.cpp b/src/vst/sampler_core.cpp index 9c3bba2..2b8e24f 100644 --- a/src/vst/sampler_core.cpp +++ b/src/vst/sampler_core.cpp @@ -271,7 +271,7 @@ bool Voice::sustainLoopUsable() const { void Voice::start(int note, int velocity, const SampleData& sample, int rootNote, double keyTrack, const vst::VelocityCurve& velocityCurve, - bool unityVarispeedBypass, bool declickTakeover) { + bool declickTakeover) { // Takeover declick (Phase S GA fix, rev 2): BEFORE any state reset, record the PRE-CUT // REFERENCE — the last rendered output — and mark the compensation PENDING iff this // start is a takeover/steal of a SOUNDING voice and the caller opted in. The ramp itself @@ -319,20 +319,6 @@ void Voice::start(int note, int velocity, const SampleData& sample, int rootNote playMode_ = p.playMode; pitchEngine_ = p.pitchEngine; - // 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. 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; - } - // Initial read position honors the sample's start-point offset (S11), in BOTH modes. Clamp // into [0, frames): a start at or past the end degrades to 0 (play from the top) rather than // starting a voice already off the end. A negative start (shouldn't occur) is pinned to 0. @@ -816,7 +802,7 @@ std::size_t VoiceEngine::monoNoteOn(int note, int velocity) { // The declick opt-in rides every mono restart: start() self-gates it on the voice being // ACTIVE, so a first-note fresh start never ramps — only a hard cut of a sounding tone. v.start(note, velocity, sample, zone.rootNote, zone.keyTrack, zone.velocityCurve, - /*unityVarispeedBypass=*/false, /*declickTakeover=*/takeoverDeclick_); + /*declickTakeover=*/takeoverDeclick_); v.setStartOrder(nextStartOrder_++); return 0; } @@ -852,7 +838,7 @@ void VoiceEngine::monoNoteOff(int note) { // original velocity. Peer restart site of monoNoteOn's takeover — same declick opt-in // (the fallback also hard-cuts the sounding tone). v.start(fb.note, fb.velocity, sample, zone.rootNote, zone.keyTrack, zone.velocityCurve, - /*unityVarispeedBypass=*/false, /*declickTakeover=*/takeoverDeclick_); + /*declickTakeover=*/takeoverDeclick_); v.setStartOrder(nextStartOrder_++); } @@ -884,7 +870,7 @@ std::size_t VoiceEngine::noteOn(int note, int velocity) { // the same hard cut of a sounding tone as the mono retrig takeover. const std::size_t v = allocateVoice(); voices_[v].start(note, velocity, sample, zone.rootNote, zone.keyTrack, zone.velocityCurve, - /*unityVarispeedBypass=*/false, /*declickTakeover=*/takeoverDeclick_); + /*declickTakeover=*/takeoverDeclick_); voices_[v].setStartOrder(nextStartOrder_++); return v; } @@ -979,71 +965,4 @@ std::size_t VoiceEngine::activeVoiceCount() const { return n; } -// --------------------------------------------------------------------------- -// PreviewCard (Phase S) — the isolated preview voice. See the header contract. -// --------------------------------------------------------------------------- - -PreviewCard::PreviewCard(const Keymap& keymap, std::int64_t preserveWindowFrames, - bool takeoverDeclick) - : keymap_(keymap), takeoverDeclick_(takeoverDeclick) { - // Construction is off the audio thread (the shell builds the card inside its - // LoadedInstrument) — the one allocation point for the preview voice's shifter rings, - // mirroring VoiceEngine's constructor. <= 1 leaves them pass-through. - if (preserveWindowFrames > 1) voice_.presizePreserveShifters(preserveWindowFrames); -} - -void PreviewCard::noteOn(int note, int velocity) { - const ZoneResolution res = keymap_.resolve(note, velocity); - if (!res.matched) return; // out-of-zone: defined no-play - const KeyZone& zone = keymap_.zones[res.zoneIndex]; - if (zone.sampleIndex >= keymap_.samples.size()) return; - const SampleData& sample = keymap_.samples[zone.sampleIndex]; - // One voice, one finger: a new preview replaces the ringing one. The unity-Varispeed - // bypass is OPTED IN here (and only here) — the preview fires at the effective root, so - // this is its zero-added-latency path (the FA1 fix, re-scoped off the MIDI engine). No - // Preserve cap and no stealing interplay: the card is structurally outside the pool. - // The takeover declick (GA fix rev 2) rides the REPLACE-restart: start() self-gates on - // the voice being active, so a fresh preview (idle voice) never ramps — only the hard - // cut of a still-ringing preview, the same physics as the mono retrig takeover. - voice_.start(note, velocity, sample, zone.rootNote, zone.keyTrack, zone.velocityCurve, - /*unityVarispeedBypass=*/true, /*declickTakeover=*/takeoverDeclick_); -} - -void PreviewCard::noteOff(int note) { - // Release only the note that is actually sounding — an off for a replaced (stale) preview - // note must not cut the new one. - if (voice_.active() && voice_.note() == note) voice_.release(); -} - -void PreviewCard::releaseAll() { - // CC 123 peer of VoiceEngine::allNotesOff: unconditional release, whatever note rings. - // Gate enters release; Trigger plays through (bounded, cannot be stuck). RT-safe. - if (voice_.active()) voice_.release(); -} - -void PreviewCard::hardStop() { - // CC 120 peer of VoiceEngine::allSoundsOff: immediate silence, no release ramp. - // Silences Trigger one-shots that releaseAll() cannot stop. RT-safe. - voice_.hardStop(); -} - -void PreviewCard::render(AudioSample* out, std::size_t frameCount) { - if (out == nullptr || frameCount == 0 || !voice_.active()) return; - for (std::size_t f = 0; f < frameCount; ++f) { - if (!voice_.active()) break; - out[f] += voice_.renderFrame(); - } -} - -void PreviewCard::render(AudioSample* left, AudioSample* right, std::size_t frameCount) { - if (left == nullptr || right == nullptr || frameCount == 0 || !voice_.active()) return; - for (std::size_t f = 0; f < frameCount; ++f) { - if (!voice_.active()) break; - AudioSample l = 0.0f, r = 0.0f; - voice_.renderFrameStereo(l, r); - left[f] += l; - right[f] += r; - } -} - } // namespace reasampler diff --git a/src/vst/sampler_core.h b/src/vst/sampler_core.h index 93df898..90dc92a 100644 --- a/src/vst/sampler_core.h +++ b/src/vst/sampler_core.h @@ -387,9 +387,10 @@ private: // restarted). A takeover restart HARD-CUTS the sounding tone: the read head and envelope // restart in one frame, a step discontinuity that clicks. This is the same physics on EVERY // restart-of-a-sounding-voice path — the MONO Retrigger takeover/fallback, the mono -// cross-sample legato restart, the POLY at-cap voice steal, AND the preview card's -// replace-restart. When the caller opts in (start()'s declickTakeover; the engine and the -// preview card pass it on all of those restart paths when constructed with takeoverDeclick), +// cross-sample legato restart, and the POLY at-cap voice steal (the editor's preview is a +// plain engine noteOn since the PreviewCard retirement, so a preview re-fire at cap is just +// an at-cap steal). When the caller opts in (start()'s declickTakeover; the engine passes +// it on all of those restart paths when constructed with takeoverDeclick), // the restart smooths the ACTUAL output discontinuity: start() records the last rendered // output as the pre-cut reference, and the FIRST frame rendered after the restart seeds a // compensation equal to (reference − that frame's raw new output). The compensation is @@ -404,7 +405,7 @@ private: // and new levels already match, the seed is ~0 and nothing is added, so the +6 dB sum the // gate defended against is structurally impossible.] OFF by default so the bare core stays // byte-identical to the pre-fix engine (the regression baseline); the processor shell opts -// in for both the engine and the preview card, mirroring the kDefaultPitchEngine layering. +// in for the engine, mirroring the kDefaultPitchEngine layering. inline constexpr double kDeclickDecay = 0.95; // per-frame decay of the compensation inline constexpr double kDeclickFloor = 1e-4; // below this the ramp is done (~ -80 dB) @@ -431,13 +432,6 @@ public: // 1.0 (the default) is standard 12-tone-ET, bit-identical to the pre-S-VIEW-6 baseRatio_. // `velocityCurve` (S-VIEW-9) maps the note-on velocity to the voice's amp gain, evaluated ONCE // here (off the per-frame path); defaults to flat y=1 (R10-F1) — every velocity plays at unity. - // `unityVarispeedBypass` (Phase S, re-scoping FA1): when TRUE, a Preserve voice started at - // UNITY shift (baseRatio_ == 1.0, pitch env off) is demoted to the Varispeed read path — at - // unity the two engines are byte-identical except the OLA shifter's structural half-window - // onset delay, which buys nothing when there is no shift to preserve duration against. The - // PREVIEW card opts in (it fires at the root, so this is its zero-added-latency path); the - // MIDI VoiceEngine does NOT (default false) — a chromatic line must not step ~25 ms faster - // at the root note than one semitone away (the FA1-review timing-step finding). // `declickTakeover` (Phase S GA fix): when TRUE and this voice is currently ACTIVE (a // takeover/steal restart, not a fresh start), smooth the restart's output discontinuity — // the pre-cut output is recorded here and the difference-seeded compensation is armed on @@ -446,7 +440,6 @@ public: void start(int note, int velocity, const SampleData& sample, int rootNote, double keyTrack = 1.0, const vst::VelocityCurve& velocityCurve = vst::VelocityCurve::flat(), - bool unityVarispeedBypass = false, bool declickTakeover = false); // MONO LEGATO takeover (Phase S): re-pitch this ACTIVE voice to `note` without touching the @@ -484,11 +477,10 @@ public: void setStartOrder(std::uint64_t order) { startOrder_ = order; } bool releasing() const { return releasing_; } // The S16 pitch engine this voice is running (for the engine's Preserve-voice tally). Only - // 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. 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. + // meaningful while active(). NOTE: the FA1 unity-shift demotion to Varispeed is GONE — + // it was scoped to the retired PreviewCard, and since GA2 the primed shifter speaks on + // frame 0 at every ratio, so a Preserve voice keeps its shifter at every note (one code + // path, uniform onset across the keyboard). 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 @@ -758,63 +750,13 @@ private: std::size_t heldCount_ = 0; }; -// --------------------------------------------------------------------------- -// PREVIEW VOICE CARD (Phase S). A dedicated single voice ENTIRELY ISOLATED from the MIDI -// VoiceEngine pool: the editor's preview trigger routes ONLY here, host MIDI ONLY to the -// engine, and the two are summed by the shell — neither can steal from, drop, or cap the -// other (the FA1 "preview dropped when voices are full" bug is structurally impossible). -// -// The card plays the loaded zone through its REAL params (the same Keymap resolution and -// Voice machinery — you hear the actual sound), but with the FA1 unity-Varispeed bypass -// OPTED IN: the preview fires at the effective root (unity shift), where the Preserve -// shifter's half-window onset delay buys nothing, so the preview speaks on frame one. A -// transposed preview (if ever fired off-root) keeps the genuine shifter path — the -// shifters are pre-sized at construction (off-thread), so noteOn stays RT-safe. -// -// PLAYBACK ONLY (load-bearing principle): the card never captures, never writes the bank, -// never inserts a timeline item. Pure — no VST3/REAPER types; the shell owns the mailbox -// cadence and the output summation. -// --------------------------------------------------------------------------- - -class PreviewCard { -public: - // `keymap` must outlive the card (same lifetime contract as VoiceEngine — both live on - // the shell's LoadedInstrument next to the Keymap they read). `preserveWindowFrames` - // pre-sizes the voice's Preserve shifters off-thread (0/1 = pass-through), mirroring the - // engine's constructor, so an off-root Preserve preview never allocates at note-on. - // `takeoverDeclick` (Phase S GA fix, rev 2) mirrors VoiceEngine's flag: a preview fired - // over a RINGING preview REPLACES the single voice — the same hard cut as a mono - // takeover — and with the opt-in that replace-restart runs the same difference-seeded - // declick ramp (start() self-gates on the voice being active, so a fresh preview never - // ramps). Default FALSE keeps the bare core byte-identical (regression baseline); the - // processor shell opts in. - explicit PreviewCard(const Keymap& keymap, std::int64_t preserveWindowFrames = 0, - bool takeoverDeclick = false); - - // Fire the preview note (RT-safe: no allocation). A new preview replaces the ringing one - // (single voice — the card is one finger, not a pool). Out-of-zone is a defined no-play. - void noteOn(int note, int velocity); - // Release the preview IF `note` is the one sounding (a stale off for a replaced note is a - // no-op). Gate zones enter release; Trigger zones ignore note-off and play through. - void noteOff(int note); - // CC 123 peer of VoiceEngine::allNotesOff: release the ringing preview UNCONDITIONALLY, - // whatever note it is on. Gate enters release; Trigger plays through (bounded). RT-safe. - void releaseAll(); - // CC 120 peer of VoiceEngine::allSoundsOff: HARD-STOP the preview immediately (no release - // ramp, silences Trigger one-shots too). RT-safe. - void hardStop(); - - bool active() const { return voice_.active(); } - - // Sum the card's contribution into the caller's buffer(s), ADDING (mirror of the engine's - // RT render contract — no allocation, no lock; null/zero-count is a no-op). - void render(AudioSample* out, std::size_t frameCount); - void render(AudioSample* left, AudioSample* right, std::size_t frameCount); - -private: - Voice voice_; - const Keymap& keymap_; - bool takeoverDeclick_ = false; // GA fix rev 2: declick the replace-restart of a ringing preview -}; +// NOTE (preview redesign): the Phase S PreviewCard — a dedicated preview voice isolated +// from the MIDI pool — is RETIRED. The editor's preview trigger is now a synthetic note-on +// at the loaded capture's root note through the SAME VoiceEngine host MIDI drives, so a +// preview is a real voice: it counts against the voice count, can steal / be stolen, and +// respects Poly/Mono + Retrigger/Legato (a deliberate reversal of the earlier isolation +// decision). The FA1 unity-Varispeed demotion in Voice::start went with it — since the GA2 +// prime fix the shifter speaks on frame 0 at every ratio, so the demotion bought nothing +// but a second code path. } // namespace reasampler diff --git a/tests/test_sampler_core.cpp b/tests/test_sampler_core.cpp index dcb1eb4..15e13b2 100644 --- a/tests/test_sampler_core.cpp +++ b/tests/test_sampler_core.cpp @@ -1331,8 +1331,8 @@ static void testPreserveGateStereoLoopComposes() { } // --- Preserve voice cap: a Preserve note-on past the cap is dropped; Varispeed unaffected. --- -// Since the Phase S re-scope EVERY engine Preserve voice (root included) runs the shifter and -// counts toward the cap — the unity demotion is preview-card-only (see the Phase S section). +// EVERY engine Preserve voice (root included) runs the shifter and counts toward the cap — +// the FA1 unity demotion is gone (it was scoped to the retired PreviewCard). static void testPreserveVoiceCap() { SampleData s = dcSample(2000, 60); s.play.pitchEngine = PitchEngine::Preserve; // held (Gate, no loop -> runs long enough) @@ -1346,11 +1346,10 @@ 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. 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. +// FA1 postscript — the Preserve unity-Varispeed bypass is REMOVED with the PreviewCard +// (preview is now a plain engine noteOn at the root). The engine keeps the shifter at +// EVERY Preserve note; GA2's primed ring speaks on frame 0 at every ratio, so onset is +// uniformly IMMEDIATE across the keyboard with no demotion path at all. // --------------------------------------------------------------------------- // The ENGINE'S root-note Preserve voice keeps the OLA path — and since the GA2 prime fix the @@ -1385,54 +1384,6 @@ static void testPreserveUnityEngineVoiceSpeaksImmediately() { 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. -static void testPreviewCardUnitySpeaksImmediately() { - SampleData s = dcSample(2000, 60); - s.play.pitchEngine = PitchEngine::Preserve; - s.play.adsr = flatAdsr(); - Keymap km = Keymap::singleSampleChromatic(std::move(s)); - PreviewCard card(km, /*preserveWindowFrames=*/512); - card.noteOn(60, 127); // at root: unity shift -> demoted inside the card, zero onset delay - std::vector buf(4, 0.0f); - card.render(buf.data(), buf.size()); - CHECK(approx(buf[0], 1.0, 1e-6)); // the DC sample, on the very first frame -} - -// keyTrack 0 collapses EVERY note to unity — an off-root preview also demotes, speaks at once. -static void testPreviewCardKeyTrackZeroAlsoSpeaksImmediately() { - SampleData s = dcSample(2000, 60); - s.play.pitchEngine = PitchEngine::Preserve; - s.play.adsr = flatAdsr(); - Keymap km = Keymap::singleSampleChromatic(std::move(s)); - km.zones[0].keyTrack = 0.0; // no tracking: all keys play root pitch (unity) - PreviewCard card(km, /*preserveWindowFrames=*/512); - card.noteOn(67, 127); - std::vector buf(4, 0.0f); - card.render(buf.data(), buf.size()); - CHECK(approx(buf[0], 1.0, 1e-6)); -} - -// 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 = preserveTriggerSample(1000, 1.0); - Keymap km = Keymap::singleSampleChromatic(std::move(s)); - PreviewCard card(km, /*preserveWindowFrames=*/512); - 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(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 — 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 @@ -1829,19 +1780,6 @@ static void testAllNotesOffClearsMonoHeldStack() { CHECK(eng.activeVoiceCount() == 0); } -// MAJOR-2 companion: the preview card's unconditional releaseAll (the panic peer). -static void testPreviewCardReleaseAll() { - Keymap km = twoLevelKeymap(); - PreviewCard card(km); - card.noteOn(70, 127); - CHECK(card.active()); - card.releaseAll(); // no note argument: quiets whatever rings - std::vector buf(1, 0.0f); - card.render(buf.data(), buf.size()); - CHECK(approx(buf[0], 0.0, 1e-9)); - CHECK(!card.active()); -} - // CC 120 (allSoundsOff) hard-stops a ringing TRIGGER one-shot that would otherwise play to // its bounded playEnd (minutes on a full-length capture). This is the primary repro: allNotesOff // (CC 123) is a NO-OP on a Trigger voice — only allSoundsOff provides the actual hard stop. @@ -2211,46 +2149,30 @@ static void testZeroAttackGateRetrigNoStep() { CHECK(maxDeltaAcross(static_cast(pre.back()), post) < 0.08); } -// GA2 — the PREVIEW click: a preview fired over a RINGING preview replaces the card's single -// voice — the same hard cut as a mono takeover, previously entirely un-declicked (the card -// never passed the opt-in). With takeoverDeclick opted in at construction, the -// replace-restart runs the same difference-seeded ramp: boundary continuity + bounded slope. -static void testPreviewRetriggerDeclicksRestart() { +// PREVIEW re-audition declick (GA2, re-homed on the engine): the preview is now a plain +// engine noteOn at the root, so re-auditioning while the first preview still rings is a +// POLY AT-CAP STEAL when the pool is saturated — with takeoverDeclick opted in (the shell's +// product default), the restart runs the same difference-seeded ramp: boundary continuity +// + bounded slope. Same physics testPolyStealDeclicksRestart pins; this pins it at the +// preview's exact shape (same note, root, full pool of 1). +static void testPreviewReauditionDeclicksViaEngineSteal() { SampleData s = sineSample(48000, 100.0, 60); // default ADSR: instant unity (worst case) Keymap km = Keymap::singleSampleChromatic(std::move(s)); - PreviewCard card(km, 0, /*takeoverDeclick=*/true); + VoiceEngine eng(1, km, 0, 0, VoiceMode::Poly, MonoTrigger::Retrigger, + /*takeoverDeclick=*/true); - card.noteOn(60, 127); - std::vector pre(120, 0.0f); - card.render(pre.data(), pre.size()); // ringing at ~ the sine peak + eng.noteOn(60, 127); // preview: root note through the pool + std::vector pre; + eng.render(pre, 120); // ringing at ~ the sine peak CHECK(pre.back() > 0.99f); - card.noteOn(60, 127); // audition again: replaces the ringing preview - std::vector post(400, 0.0f); - card.render(post.data(), post.size()); + eng.noteOn(60, 127); // audition again: at-cap steal restart + std::vector post; + eng.render(post, 400); CHECK(std::fabs(static_cast(post[0]) - static_cast(pre.back())) < 0.01); CHECK(maxDeltaAcross(static_cast(pre.back()), post) < 0.08); } -// The preview declick is OPT-IN: a default-constructed card keeps the pre-fix hard cut -// byte-identical — the replace-restart's first frame is the new voice's raw onset (sin(0) -// == 0 here), pinning the pure-core regression baseline the shell layers the opt-in above. -static void testPreviewDefaultOffKeepsHardCutBaseline() { - SampleData s = sineSample(48000, 100.0, 60); - Keymap km = Keymap::singleSampleChromatic(std::move(s)); - PreviewCard card(km); // declick NOT opted in - - card.noteOn(60, 127); - std::vector pre(120, 0.0f); - card.render(pre.data(), pre.size()); - CHECK(pre.back() > 0.99f); - - card.noteOn(60, 127); - std::vector post(1, 0.0f); - card.render(post.data(), post.size()); - CHECK(approx(post[0], 0.0, 1e-4)); // the raw hard cut: new onset, no ramp -} - // GA-VoiceSteal repro (DAW bug): voiceCount 3, a triad note-on'd at the SAME sample time // (three note-ons in one block, no render between), then a 4th note. The steal must take // EXACTLY ONE voice (the oldest, none releasing) and leave the other two RINGING — the DAW @@ -2309,50 +2231,26 @@ static void testOverCapChordStealsExactlyOne() { CHECK(eng.activeVoiceCount() == 0); // the stolen-into 4th note releases last } -// PREVIEW-CARD ISOLATION: the card never consumes a pool voice, a FULL pool never drops a -// preview, and pool stealing never touches the ringing preview. The two sum independently. -static void testPreviewCardIsolatedFromPool() { +// PREVIEW OBEYS VOICING (the PreviewCard-isolation reversal): a preview is a plain engine +// noteOn, so it is a REAL pool voice — a full pool STEALS for it (never a parallel voice on +// top), it counts toward activeVoiceCount, and its note-off releases through the normal +// path. This is the processor's mailbox-drain contract, pinned in the pure core. +static void testPreviewNoteObeysVoicing() { SampleData s = dcLevelSample(200000, 1.0f, 60); Keymap km = Keymap::singleSampleChromatic(std::move(s)); VoiceEngine eng(2, km); - PreviewCard card(km); eng.noteOn(60, 127); eng.noteOn(62, 127); // the pool is now FULL - card.noteOn(64, 127); // preview fires anyway — its own voice - CHECK(eng.activeVoiceCount() == 2); // no pool voice consumed - CHECK(card.active()); - std::vector buf(4, 0.0f); - eng.render(buf.data(), buf.size()); // engine sums 2 voices... - card.render(buf.data(), buf.size()); // ...card ADDS its own on top - CHECK(approx(buf[0], 3.0, 1e-6)); - eng.noteOn(64, 127); // pool steals INTERNALLY... + eng.noteOn(64, 127); // the preview note: steals — no third voice CHECK(eng.activeVoiceCount() == 2); - CHECK(card.active()); // ...the preview is untouched - card.noteOff(64); // flat release: card gates off instantly + std::vector buf(4, 0.0f); + eng.render(buf.data(), buf.size()); + CHECK(approx(buf[0], 2.0, 1e-6)); // TWO voices sum — never 3 (no side-car) + eng.noteOff(64); // flat release: the preview gates off std::vector buf2(1, 0.0f); - card.render(buf2.data(), buf2.size()); - CHECK(approx(buf2[0], 0.0, 1e-9)); - CHECK(eng.activeVoiceCount() == 2); // and the pool never noticed -} - -// The card is ONE voice: a new preview replaces the ringing one, a STALE note-off (for the -// replaced note) is a no-op, and an out-of-zone preview is a defined no-play. -static void testPreviewCardReplaceStaleOffAndOutOfZone() { - Keymap km = twoLevelKeymap(); // zones [40,59] + [60,80] - PreviewCard card(km); - card.noteOn(50, 127); - card.noteOn(70, 127); // replaces the first preview - std::vector buf(1, 0.0f); - card.render(buf.data(), buf.size()); - CHECK(approx(buf[0], 0.75, 1e-6)); // zone B is what rings - card.noteOff(50); // STALE off for the replaced note: no-op - CHECK(card.active()); - card.noteOff(70); // the sounding note's off gates it (release 0) - std::vector buf2(1, 0.0f); - card.render(buf2.data(), buf2.size()); - CHECK(approx(buf2[0], 0.0, 1e-9)); - card.noteOn(20, 127); // out of every zone: defined no-play - CHECK(!card.active()); + eng.render(buf2.data(), buf2.size()); + CHECK(eng.activeVoiceCount() == 1); // the surviving MIDI note still rings + CHECK(approx(buf2[0], 1.0, 1e-6)); } // GA2 — bounded-blend overshoot regression: mid-ramp output must stay within full scale. @@ -2591,12 +2489,9 @@ int main() { testPreserveGateStereoLoopComposes(); testPreserveVoiceCap(); - // FA1 (re-scoped by Phase S) — the unity bypass is preview-card-only; the engine keeps a - // uniform Preserve onset. Velocity under Preserve unchanged. + // FA1 postscript — the unity demotion is gone with the PreviewCard; the engine keeps a + // uniform Preserve onset at every note. Velocity under Preserve unchanged. testPreserveUnityEngineVoiceSpeaksImmediately(); - testPreviewCardUnitySpeaksImmediately(); - testPreviewCardKeyTrackZeroAlsoSpeaksImmediately(); - testPreviewCardTransposedKeepsShifter(); testPreserveTransposedVoiceSpeaksImmediately(); testPreserveUnityVoiceCountsTowardCap(); testVelocityCurveAppliesUnderPreserve(); @@ -2605,7 +2500,8 @@ int main() { testPerZoneAdsrReachesVoiceEnvelope(); testZeroAdsrIsInstantSustain(); - // Phase S — voice count, MONO mode (held stack + Retrigger/Legato), preview card. + // Phase S — voice count, MONO mode (held stack + Retrigger/Legato); preview as a real + // pool voice (PreviewCard retired — preview routes through the engine). testMonoLastNotePriorityAndFallback(); testMonoReleaseOfLowerHeldNoteIsInaudible(); testMonoRepressHeldNoteMovesToTop(); @@ -2621,7 +2517,6 @@ int main() { testMonoLegatoTriggerHeldKeyStillRetunes(); testAllNotesOffReleasesPolyVoices(); testAllNotesOffClearsMonoHeldStack(); - testPreviewCardReleaseAll(); testAllSoundsOffStopsTriggerOneShot(); testAllNotesOffStillReleasesGateVoices(); testMonoLegatoSameNoteRepressReattacks(); @@ -2636,11 +2531,9 @@ int main() { testZeroAttackTakeoverNeverExceedsFullScale(); testMonoRetrigTriggerZoneDeclicksRestart(); testZeroAttackGateRetrigNoStep(); - testPreviewRetriggerDeclicksRestart(); - testPreviewDefaultOffKeepsHardCutBaseline(); + testPreviewReauditionDeclicksViaEngineSteal(); testDeclickBoundedBlendNoOvershoot(); - testPreviewCardIsolatedFromPool(); - testPreviewCardReplaceStaleOffAndOutOfZone(); + testPreviewNoteObeysVoicing(); // GA3 — Preserve tail wind-down (writer freeze at source exhaustion). testPreserveTailFinalWindowGapFree();