From 81c46587117250b57d50658654fc3d87980e0f20 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 28 Jul 2026 10:30:51 -0400 Subject: [PATCH 1/4] 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(); From 93e28f4ea5398e79523e0c4cc825efde8feebe88 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 28 Jul 2026 10:56:40 -0400 Subject: [PATCH 2/4] =?UTF-8?q?fix(vst):=20reopen=20heal=20without=20the?= =?UTF-8?q?=20editor=20=E2=80=94=20bounded=20main-thread=20retry=20timer,?= =?UTF-8?q?=20re-armable=20first-poll=20heal,=20unconditional=20preview-of?= =?UTF-8?q?f=20consume=20+=20Mono/drain=20preview=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/vst/reasampler_processor.cpp | 142 +++++++++++++++++++++++++++++-- src/vst/reasampler_processor.h | 31 ++++++- tests/test_sampler_core.cpp | 44 ++++++++++ 3 files changed, 211 insertions(+), 6 deletions(-) diff --git a/src/vst/reasampler_processor.cpp b/src/vst/reasampler_processor.cpp index af4e91c..ed581f5 100644 --- a/src/vst/reasampler_processor.cpp +++ b/src/vst/reasampler_processor.cpp @@ -25,6 +25,13 @@ #include "sample_map.h" // selectSample, resolvePerformance, buildZonedKeymap, state (de)ser #include "wav_trim.h" // parseWavLayout, extractFloatFrames (shared WAV parse) +#ifdef _WIN32 +#ifndef NOMINMAX +#define NOMINMAX +#endif +#include // SetTimer/KillTimer (the reopen-heal retry's main-thread timer) +#endif + using namespace Steinberg; using namespace Steinberg::Vst; @@ -84,16 +91,99 @@ std::optional decodeRelative(const std::string& projectDir, return out; } +#ifdef _WIN32 +// --- Reopen-heal retry timer (the non-editor reload trigger; see the header) ----------- +// HWND-less Win32 thread timer: SetTimer(nullptr, ...) queues WM_TIMER on the CALLING +// thread's message queue and the TIMERPROC fires from its pump — REAPER's main thread, +// where every reload path already runs (setState, setActive, the editor, this timer). +// A TIMERPROC carries no user data, so a tiny id->instance registry maps a fired timer +// back to its processor. Set/kill/fire all happen on the pumping thread; the mutex is +// defensive against an exotic host threading setState from elsewhere (in which case +// SetTimer would not fire there anyway and we degrade to the pre-fix editor-open heal). +constexpr UINT kHealRetryIntervalMs = 250; // fast enough to catch the tail of project load +constexpr int kHealRetryMax = 40; // ~10 s, then stop churning (e.g. missing WAV) + +std::mutex g_healRegistryMutex; +std::vector> g_healRegistry; + +void CALLBACK healTimerProc(HWND, UINT, UINT_PTR id, DWORD) { + ReaSamplerProcessor* target = nullptr; + { + std::lock_guard lock(g_healRegistryMutex); + for (const auto& entry : g_healRegistry) { + if (entry.first == static_cast(id)) { + target = entry.second; + break; + } + } + } + if (!target) { + // Orphan fire (the processor disarmed/destroyed between queue and dispatch): + // stop the timer here — nobody else holds this id anymore. + KillTimer(nullptr, id); + return; + } + // The registry lock is RELEASED before the tick: healTick -> reloadFromBank takes + // reloadMutex_ then (via arm/disarm) the registry mutex — one consistent order. + target->healTick(); +} +#endif // _WIN32 + } // namespace +void ReaSamplerProcessor::armHealRetry() { +#ifdef _WIN32 + // Main thread. Idempotent: an already-armed timer keeps its running countdown (the + // retry ticks call reloadFromBank, which calls back here on every failed rebuild). + if (healTimerId_ != 0) return; + const UINT_PTR id = SetTimer(nullptr, 0, kHealRetryIntervalMs, &healTimerProc); + if (id == 0) return; // no message pump on this thread / OS refusal: editor-open heal remains + healTimerId_ = static_cast(id); + healRetriesLeft_ = kHealRetryMax; + std::lock_guard lock(g_healRegistryMutex); + g_healRegistry.emplace_back(healTimerId_, this); +#endif +} + +void ReaSamplerProcessor::disarmHealRetry() { +#ifdef _WIN32 + // Main thread. The common (already-disarmed) path costs one compare — this is called + // at the end of every successful reload. + if (healTimerId_ == 0) return; + KillTimer(nullptr, static_cast(healTimerId_)); + { + std::lock_guard lock(g_healRegistryMutex); + g_healRegistry.erase( + std::remove_if(g_healRegistry.begin(), g_healRegistry.end(), + [this](const auto& entry) { return entry.second == this; }), + g_healRegistry.end()); + } + healTimerId_ = 0; +#endif +} + +void ReaSamplerProcessor::healTick() { + // Main thread (the heal timer's TIMERPROC). One bounded retry: reloadFromBank re-reads + // the bank over the bridge and itself disarms this timer when it builds an instrument + // (or the restored intent is gone). If it stays armed, count the budget down and give + // up at zero — a genuinely-missing WAV must not poll ext-state forever. + if (healTimerId_ == 0) return; // raced a disarm between queue and dispatch + --healRetriesLeft_; + reloadFromBank(); + if (healTimerId_ != 0 && healRetriesLeft_ <= 0) disarmHealRetry(); +} + FUnknown* ReaSamplerProcessor::createInstance(void* /*context*/) { // The host owns the returned reference. Cast up to the combined interface the SDK // exposes (IAudioProcessor) so the FUnknown refcount is correctly rooted. return static_cast(new ReaSamplerProcessor()); } -// Out-of-line so unique_ptr sees the complete type here. -ReaSamplerProcessor::~ReaSamplerProcessor() = default; +// Out-of-line so unique_ptr sees the complete type here. The heal-retry +// disarm is defensive (terminate already disarms per the VST3 lifecycle): it removes this +// instance from the timer registry so a host that skips terminate can never leave a fired +// TIMERPROC holding a dangling pointer. +ReaSamplerProcessor::~ReaSamplerProcessor() { disarmHealRetry(); } tresult PLUGIN_API ReaSamplerProcessor::queryInterface(const TUID iid, void** obj) { // S6: expose REAPER's inline-embed interface. REAPER queries the IEditController for @@ -135,9 +225,11 @@ tresult PLUGIN_API ReaSamplerProcessor::initialize(FUnknown* context) { } tresult PLUGIN_API ReaSamplerProcessor::terminate() { - // process() is not running at terminate. Free the live + draining instruments and - // drain the graveyard. Take the pointers out of the atomics first so nothing else + // process() is not running at terminate. Stop the reopen-heal retry timer first (its + // tick would reload into a dying instance), then free the live + draining instruments + // and drain the graveyard. Take the pointers out of the atomics first so nothing else // races them. + disarmHealRetry(); std::lock_guard lock(reloadMutex_); delete live_.exchange(nullptr); delete draining_.exchange(nullptr); @@ -152,8 +244,15 @@ tresult PLUGIN_API ReaSamplerProcessor::setActive(TBool state) { // no reload could free while active). The build/drain are off the audio thread — // setActive is a main/UI-thread call. if (state) { + // NOTE: this is also the non-editor reload trigger's anchor — if this reload runs + // before the extension's PROJEXTSTATE is parseable, reloadFromBank arms the bounded + // heal-retry timer itself (see the header), so a restored instance played via host + // MIDI with the editor never opened still comes up sounding. reloadFromBank(); } else { + // An inactive instance has nothing to heal into — stop the retry; the reactivation + // reload above re-arms it if the bank is still not parseable then. + disarmHealRetry(); std::lock_guard lock(reloadMutex_); // process is guaranteed stopped: free EVERYTHING. The live instrument too — its // voices are frozen mid-flight, and if it survived deactivation the reactivate @@ -551,7 +650,22 @@ std::string ReaSamplerProcessor::reloadFromBank() { // `built` is heap-owned; release() hands ownership to the atomic; the drain-evicted // pointer is re-owned by the graveyard. // + const bool loaded = static_cast(built); publishBuiltLocked(std::move(built)); + + // Reopen-heal retry, the NON-editor trigger (see the header): this reload built NOTHING + // despite restored intent (a selection or zones) while the bridge is connected — during + // project load that almost always means the extension's PROJEXTSTATE block is not yet + // parseable — so arm the bounded main-thread retry timer. Every other outcome disarms: + // the timer only lives while there is something to heal. (Leaf-mutex order holds: + // reloadMutex_ -> registry mutex, matching the TIMERPROC which releases the registry + // before ticking into this function.) + const bool intent = !selectedSampleId().empty() || !performanceMap().empty(); + if (!loaded && intent && bridge_.isConnected()) { + armHealRetry(); + } else { + disarmHealRetry(); + } return resolvedId; } @@ -737,6 +851,18 @@ ReaSamplerProcessor::pollBankSync(bool isFocusedTarget) { // editor re-snapshots its bank view. result.reloaded = genChanged || healReload; } + + // Heal RETRY: if the heal reload above STILL left nothing live (the first poll was + // itself too early — the bank blob still absent/unparseable), reset the sentinel so + // the NEXT tick re-arms the first-poll heal instead of spending it one-shot. Bounded + // by the intent check inside the heal (a deliberately-empty instance never sets + // healReload, so never re-arms). This also heals a bank whose generation counter was + // never bumped: `bankGenerationChanged` is a plain != against a counter that stays 0 + // for such a bank (0 != 0 never fires), so the generation path alone could never + // recover it. + if (healReload && live_.load(std::memory_order_acquire) == nullptr) { + lastSeenBankGeneration_ = -1; + } return result; } @@ -850,14 +976,20 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { } } } - if (inst || drain) { + { const std::uint32_t off = previewOffRequest_.load(std::memory_order_acquire); const std::uint16_t offSeq = static_cast(off >> 16); if (offSeq != 0 && offSeq != previewOffConsumed_) { + // Consume UNCONDITIONALLY (mirror of the on path): a stale off left pending + // while nothing was loaded would otherwise survive until a (heal) reload lands + // and release the NEXT preview press in the same block. previewOffConsumed_ = offSeq; // 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 voice now draining, not just the (fresh) live one. + // NOTE: preview shares the host-MIDI note space — noteOff releases the newest + // voice at that pitch, so a preview release can release a host-held note at + // the same pitch (inherent to routing preview through the real note path). if (inst) inst->engine.noteOff(static_cast(off & 0xFF)); if (drain) drain->engine.noteOff(static_cast(off & 0xFF)); } diff --git a/src/vst/reasampler_processor.h b/src/vst/reasampler_processor.h index 2cdba8e..0c1e715 100644 --- a/src/vst/reasampler_processor.h +++ b/src/vst/reasampler_processor.h @@ -240,7 +240,32 @@ public: void previewNoteOn(int note); void previewNoteOff(int note); + // Reopen-heal retry tick — the target of the module-internal Win32 heal timer (see + // armHealRetry below), NOT host-facing. Public only because the file-static TIMERPROC + // in the .cpp must reach it. Main thread; re-runs reloadFromBank (which disarms the + // timer itself on success) and disarms when the bounded retry budget runs out. + void healTick(); + private: + // --- Reopen-heal retry (the NON-editor reload trigger) -------------------------- + // A project-restored instance with intent (a selection or zones) can come up SILENT: + // REAPER runs track-FX setState before the extension's PROJEXTSTATE block is parsed, + // so the setState-time reload reads an empty bank. The editor's WM_TIMER poll heals + // that — but only if the user opens the editor; an instance played via host MIDI with + // the editor never attached stayed silent indefinitely. Mechanism: reloadFromBank + // itself detects "built NOTHING despite restored intent, bridge connected" (off the + // audio thread — it just tried) and arms a BOUNDED, HWND-less Win32 retry timer + // (SetTimer + TIMERPROC: fires on the arming thread's message pump — REAPER's main + // thread, where every reload path already runs). Each tick re-runs reloadFromBank, + // which disarms on success or when the intent is gone; the bound stops the churn for + // an instance whose WAV is genuinely missing. A deliberately-empty instance never + // arms (no intent). process() is untouched — fully RT-safe. Main-thread only. + // No-ops on non-Windows builds (the VST target is Windows-only). + void armHealRetry(); + void disarmHealRetry(); + std::uintptr_t healTimerId_ = 0; // 0 = not armed (main thread only) + int healRetriesLeft_ = 0; // remaining timer-tick retries (main thread only) + // Phase S drain retirement (FA1-review Major #2): if process() has published that the // 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 @@ -356,7 +381,11 @@ private: // 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. + // until some param change forced a reload. If that heal reload STILL leaves nothing live, + // pollBankSync resets this back to the -1 sentinel so the next tick re-arms the heal — + // the retry is bounded by the intent check (a deliberately-empty instance never heals), + // and it also covers a bank whose generation counter was never bumped (0 != 0 can never + // fire the generation path). 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 diff --git a/tests/test_sampler_core.cpp b/tests/test_sampler_core.cpp index 15e13b2..e494d30 100644 --- a/tests/test_sampler_core.cpp +++ b/tests/test_sampler_core.cpp @@ -2253,6 +2253,48 @@ static void testPreviewNoteObeysVoicing() { CHECK(approx(buf2[0], 1.0, 1e-6)); } +// PREVIEW JOINS THE MONO HELD STACK: a preview routed through the real note path is a mono +// stack entry like any host note — it TAKES the single voice on press (last-note priority) +// and its release FALLS BACK to the still-held host note instead of cutting to silence. +// Pins the processor's mailbox-drain contract for Mono the way testPreviewNoteObeysVoicing +// pins it for Poly steal. +static void testPreviewNoteJoinsMonoHeldStack() { + Keymap km = twoLevelKeymap(); + VoiceEngine eng(4, km, 0, 0, VoiceMode::Mono, MonoTrigger::Retrigger); + CHECK(eng.noteOn(50, 127) == 0); // the host-MIDI note: zone A sounds + CHECK(approx(probeFrame(eng), 0.25, 1e-6)); + CHECK(eng.noteOn(70, 127) == 0); // the preview press: TAKES the voice + CHECK(eng.activeVoiceCount() == 1); // still mono — the preview is no side-car + CHECK(approx(probeFrame(eng), 0.75, 1e-6)); + eng.noteOff(70); // preview release: FALLBACK to the held note + CHECK(approx(probeFrame(eng), 0.25, 1e-6)); + eng.noteOff(50); // host note up: gate off (flat release = instant) + CHECK(approx(probeFrame(eng), 0.0, 1e-9)); + CHECK(eng.activeVoiceCount() == 0); +} + +// PREVIEW NOTE-OFF ROUTES TO THE DRAIN ENGINE: mirror of process()'s dual-engine off +// routing. A preview held across a reload leaves its ringing voice in the DISPLACED +// (draining) snapshot while the fresh live engine has no voice at that pitch. The off is +// sent to BOTH — exactly what the mailbox drain does: the fresh engine must safely no-op, +// the drain engine must release its voice (otherwise the old-snapshot preview would +// sustain until the next reload hard-cut it). +static void testPreviewNoteOffRoutesToDrainEngine() { + Keymap km = twoLevelKeymap(); + VoiceEngine drainEng(2, km); // was live when the preview fired + VoiceEngine liveEng(2, km); // the post-reload fresh snapshot: no voices + CHECK(drainEng.noteOn(70, 127) != VoiceEngine::kNoVoice); + CHECK(approx(probeFrame(drainEng), 0.75, 1e-6)); // the preview rings in the old snapshot + CHECK(liveEng.activeVoiceCount() == 0); + // The preview release, drained to BOTH engines like a host note-off: + liveEng.noteOff(70); + drainEng.noteOff(70); + CHECK(approx(probeFrame(liveEng), 0.0, 1e-9)); // fresh engine: safe no-op, stays silent + CHECK(liveEng.activeVoiceCount() == 0); + CHECK(approx(probeFrame(drainEng), 0.0, 1e-9)); // flat release: gates off NOW + CHECK(drainEng.activeVoiceCount() == 0); // the old-snapshot voice released +} + // 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 @@ -2534,6 +2576,8 @@ int main() { testPreviewReauditionDeclicksViaEngineSteal(); testDeclickBoundedBlendNoOvershoot(); testPreviewNoteObeysVoicing(); + testPreviewNoteJoinsMonoHeldStack(); + testPreviewNoteOffRoutesToDrainEngine(); // GA3 — Preserve tail wind-down (writer freeze at source exhaustion). testPreserveTailFinalWindowGapFree(); From 261a6affa51e41cf61817b74712499be6f429952 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 28 Jul 2026 11:45:10 -0400 Subject: [PATCH 3/4] =?UTF-8?q?self-contained=20playback=20=E2=80=94=20Com?= =?UTF-8?q?ponentState=20v10=20SampleRefs=20(path+intrinsics=20owned=20by?= =?UTF-8?q?=20the=20instance),=20reloadInstrument=20decodes=20bank-free,?= =?UTF-8?q?=20heal=20timer=20+=20poll-to-play=20removed;=20bank=20is=20a?= =?UTF-8?q?=20browser=20source?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/ext_keys.h | 2 +- src/ingest.cpp | 4 +- src/vst/bank_sync.h | 4 +- src/vst/editor_geometry.h | 2 +- src/vst/reasampler_editor.cpp | 38 ++- src/vst/reasampler_editor.h | 2 +- src/vst/reasampler_processor.cpp | 388 ++++++++++++------------------- src/vst/reasampler_processor.h | 122 +++++----- src/vst/sample_map.cpp | 165 +++++++++++-- src/vst/sample_map.h | 84 ++++++- tests/test_sample_map.cpp | 235 ++++++++++++++++++- 11 files changed, 697 insertions(+), 349 deletions(-) diff --git a/src/ext_keys.h b/src/ext_keys.h index 9f8595e..48eb03e 100644 --- a/src/ext_keys.h +++ b/src/ext_keys.h @@ -49,7 +49,7 @@ inline constexpr const char* kProjExtGuidKey = "project_guid"; // bumps on every bank-content mutation that changes what a live instance would PLAY (capture // add, re-capture-in-place, sample remove, move/copy affecting banks, ingest import). The VST3 // instrument READS it off the audio thread on a UI-timer cadence and, when the value differs -// from what it last saw, calls reloadFromBank() so a recapture/ingest refreshes playing +// from what it last saw, calls reloadInstrument() so a recapture/ingest refreshes playing // instances hands-free (the S9 change-detection trigger). WIRE-SHARED (instrument reads it); // the instrument never WRITES it (the extension owns it, same read-only-over-bank rule as the // assignment request). Additive to the persist blob — an absent stamp reads as generation 0 diff --git a/src/ingest.cpp b/src/ingest.cpp index b99f87e..9dd4395 100644 --- a/src/ingest.cpp +++ b/src/ingest.cpp @@ -432,7 +432,7 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) { // // LOAD-BEARING (CLAUDE.md): this adds ONE FX instance to the user's existing selected track. // It NEVER inserts a timeline item and NEVER creates a track. Persist ordering is critical — -// the fresh instance's setState -> reloadFromBank reads the bank from project ext-state, so +// the fresh instance's setState -> reloadInstrument reads the bank from project ext-state, so // the sample MUST be persisted (generation bumped when something new landed) BEFORE // loadInstrumentOntoTrack adds the FX, or the instance cannot resolve the sampleId. // Undo-wrapped: persist + FX-add + inject = one Ctrl-Z. @@ -500,7 +500,7 @@ void doImportFromMediaExplorer() { const std::vector preset = buildInstrumentDropPreset(r.sampleId); // One undo point for the whole gesture. Persist happens INSIDE the block and BEFORE the - // FX add so the new instance's setState -> reloadFromBank sees the just-persisted sample. + // FX add so the new instance's setState -> reloadInstrument sees the just-persisted sample. // The generation is bumped only when something NEW landed (a dedup collapse mutated nothing, // so it needs neither a bump nor a persist to resolve — the sample is already in ext-state). // If saveToActiveProject() no-ops (unsaved project), close with an empty label + zero flag so diff --git a/src/vst/bank_sync.h b/src/vst/bank_sync.h index acf49ad..cb1002d 100644 --- a/src/vst/bank_sync.h +++ b/src/vst/bank_sync.h @@ -13,7 +13,7 @@ // assignment request is NEW-and-resolvable-and-worth-applying — is pure and lives here. // // The processor shell owns the cadence (a UI-thread timer, NEVER process) and the side -// effects (reloadFromBank, setSelectedSampleId); this module owns only the yes/no maths so +// effects (reloadInstrument, setSelectedSampleId); this module owns only the yes/no maths so // the reader's rules are provable without a host. assignment_request.h owns the WIRE format // (encode/decode); this module owns the CONSUME decision layered over a decoded request. @@ -96,7 +96,7 @@ struct AssignConsumeDecision { // 4. A NEW request, target, and resolvable: APPLY (selection <- (bankId, sampleId)) and // advance the marker to the request's generation. // -// The shell then: if apply, setSelectedSampleId + reloadFromBank; always persist +// The shell then: if apply, setSelectedSampleId + reloadInstrument; always persist // consumedGeneration into component state when it advanced. AssignConsumeDecision consumeDecision(const std::optional& request, std::int64_t lastConsumed, bool resolves, diff --git a/src/vst/editor_geometry.h b/src/vst/editor_geometry.h index f9165d7..8ca0615 100644 --- a/src/vst/editor_geometry.h +++ b/src/vst/editor_geometry.h @@ -61,7 +61,7 @@ HitTarget hitTest(const EditorLayout& layout, int x, int y); // The Tier-0 editor lists the bank's samples as a vertical stack of fixed-height rows // below the title bar; clicking a row selects that sample. This is the pure geometry: // the row rectangles and the point->row hit-test, unit-tested outside the DAW while the -// shell draws the names and routes the click into the processor's reloadFromBank. +// shell draws the names and routes the click into the processor's reloadInstrument. // The fixed row height (px) for one sample entry. Exposed so the shell and tests agree. inline constexpr int kSampleRowHeight = 22; diff --git a/src/vst/reasampler_editor.cpp b/src/vst/reasampler_editor.cpp index 1afcb34..0a81bb1 100644 --- a/src/vst/reasampler_editor.cpp +++ b/src/vst/reasampler_editor.cpp @@ -243,11 +243,14 @@ void ReaSamplerEditor::onSyncTimer() { void ReaSamplerEditor::commitAndReload() { // UI thread only. Publish the edited selection + zones to the processor, then rebuild - // the instrument off the audio thread (reloadFromBank bakes them into the live Keymap). + // the instrument off the audio thread (reloadInstrument bakes them into the live Keymap). + // pS: the reload also COPIES the picked capture's file ref + intrinsics from the bank + // blob into the instance-owned refs table (refreshRefsFromBank) — a browser load is the + // moment the instance becomes self-contained for that sample. if (!processor_) return; processor_->setSelectedSampleId(selectedId_); processor_->setPerformanceMap(map_); - processor_->reloadFromBank(); + processor_->reloadInstrument(); // GA: the reload may have AUTO-DEFAULTED the channel mode from the loaded capture's // channel count (implicit mode only) — re-read so the Mono/Stereo toggle draws the mode // the engine actually decoded with. @@ -273,18 +276,22 @@ ReaSamplerEditor::SetupMarkers ReaSamplerEditor::pickedMarkers(std::int64_t fram SetupMarkers m; // Seed from the bank's S2 intrinsic loop (fact about the file), then let a per-zone override // for the picked id win (the instrument's performance choice, D-B). Read the loop intrinsic - // from the live bank blob (the same path selectSample uses); the override lives in map_. + // from the live bank blob (the same path selectSample uses); when that is not readable + // (extension absent / not yet parsed) the instance-OWNED ref carries the same intrinsics + // (pS fallback). The override lives in map_. if (processor_) { + std::optional sel; auto banksJson = processor_->bridge().readReasamplerExtState(reasampler::kProjExtBanksKey); - if (banksJson) { - if (auto sel = selectSample(*banksJson, selectedId_)) { - if (sel->loop.hasLoop) { - m.hasLoop = true; - m.loopStart = sel->loop.start; - m.loopEnd = sel->loop.end; - } - } + if (banksJson) sel = selectSample(*banksJson, selectedId_); + if (!sel) { + const SampleRefs refs = processor_->sampleRefs(); + if (const SelectedSample* r = findRef(refs, selectedId_)) sel = *r; + } + if (sel && sel->loop.hasLoop) { + m.hasLoop = true; + m.loopStart = sel->loop.start; + m.loopEnd = sel->loop.end; } } // The override (loop + start) on a zone for the picked id supersedes the intrinsic. @@ -719,6 +726,15 @@ const std::vector& ReaSamplerEditor::monoPcmFor(const std::string& if (banksJson) { if (auto sel = selectSample(*banksJson, sampleId)) relativePath = sel->relativePath; } + if (relativePath.empty()) { + // pS fallback: the bank blob is not readable (extension absent / not yet parsed) + // or the id went stale there — the instance-OWNED ref still carries the path, so + // a self-contained instance draws its loaded sound's waveform regardless. + const SampleRefs refs = processor_->sampleRefs(); + if (const SelectedSample* r = findRef(refs, sampleId)) { + relativePath = r->relativePath; + } + } if (!relativePath.empty()) { const std::string projectDir = processor_->bridge().activeProjectDir(); const std::string abs = resolveBankFile(projectDir, relativePath); diff --git a/src/vst/reasampler_editor.h b/src/vst/reasampler_editor.h index 9820d2d..238a2f8 100644 --- a/src/vst/reasampler_editor.h +++ b/src/vst/reasampler_editor.h @@ -16,7 +16,7 @@ // drag-state machine hit-testing via the pure resolvers). Peak thumbnails are computed // shell-side from the decoded WAV (bank_model's Sample carries no envelope) and cached — // the mirror of bank_panel::thumbnailFor. Every edit commits OFF the audio thread via the -// processor's reloadFromBank (RT path untouched). +// processor's reloadInstrument (RT path untouched). // // Subclasses CPluginView for the IPlugView boilerplate; overrides the attach/remove hooks // to create/destroy the child window and onSize to resize it. diff --git a/src/vst/reasampler_processor.cpp b/src/vst/reasampler_processor.cpp index ed581f5..2b0bc69 100644 --- a/src/vst/reasampler_processor.cpp +++ b/src/vst/reasampler_processor.cpp @@ -22,16 +22,9 @@ #include "master_gain.h" // masterGainMaxLinear (FB1 post-mixer gain clamp) #include "reasampler_editor.h" #include "reasampler_embed.h" // S6 embed shell + IReaperUIEmbedInterface (its iid DEF'd there) -#include "sample_map.h" // selectSample, resolvePerformance, buildZonedKeymap, state (de)ser +#include "sample_map.h" // refs resolve, buildZonedKeymap, state (de)ser (pS self-contained) #include "wav_trim.h" // parseWavLayout, extractFloatFrames (shared WAV parse) -#ifdef _WIN32 -#ifndef NOMINMAX -#define NOMINMAX -#endif -#include // SetTimer/KillTimer (the reopen-heal retry's main-thread timer) -#endif - using namespace Steinberg; using namespace Steinberg::Vst; @@ -91,99 +84,16 @@ std::optional decodeRelative(const std::string& projectDir, return out; } -#ifdef _WIN32 -// --- Reopen-heal retry timer (the non-editor reload trigger; see the header) ----------- -// HWND-less Win32 thread timer: SetTimer(nullptr, ...) queues WM_TIMER on the CALLING -// thread's message queue and the TIMERPROC fires from its pump — REAPER's main thread, -// where every reload path already runs (setState, setActive, the editor, this timer). -// A TIMERPROC carries no user data, so a tiny id->instance registry maps a fired timer -// back to its processor. Set/kill/fire all happen on the pumping thread; the mutex is -// defensive against an exotic host threading setState from elsewhere (in which case -// SetTimer would not fire there anyway and we degrade to the pre-fix editor-open heal). -constexpr UINT kHealRetryIntervalMs = 250; // fast enough to catch the tail of project load -constexpr int kHealRetryMax = 40; // ~10 s, then stop churning (e.g. missing WAV) - -std::mutex g_healRegistryMutex; -std::vector> g_healRegistry; - -void CALLBACK healTimerProc(HWND, UINT, UINT_PTR id, DWORD) { - ReaSamplerProcessor* target = nullptr; - { - std::lock_guard lock(g_healRegistryMutex); - for (const auto& entry : g_healRegistry) { - if (entry.first == static_cast(id)) { - target = entry.second; - break; - } - } - } - if (!target) { - // Orphan fire (the processor disarmed/destroyed between queue and dispatch): - // stop the timer here — nobody else holds this id anymore. - KillTimer(nullptr, id); - return; - } - // The registry lock is RELEASED before the tick: healTick -> reloadFromBank takes - // reloadMutex_ then (via arm/disarm) the registry mutex — one consistent order. - target->healTick(); -} -#endif // _WIN32 - } // namespace -void ReaSamplerProcessor::armHealRetry() { -#ifdef _WIN32 - // Main thread. Idempotent: an already-armed timer keeps its running countdown (the - // retry ticks call reloadFromBank, which calls back here on every failed rebuild). - if (healTimerId_ != 0) return; - const UINT_PTR id = SetTimer(nullptr, 0, kHealRetryIntervalMs, &healTimerProc); - if (id == 0) return; // no message pump on this thread / OS refusal: editor-open heal remains - healTimerId_ = static_cast(id); - healRetriesLeft_ = kHealRetryMax; - std::lock_guard lock(g_healRegistryMutex); - g_healRegistry.emplace_back(healTimerId_, this); -#endif -} - -void ReaSamplerProcessor::disarmHealRetry() { -#ifdef _WIN32 - // Main thread. The common (already-disarmed) path costs one compare — this is called - // at the end of every successful reload. - if (healTimerId_ == 0) return; - KillTimer(nullptr, static_cast(healTimerId_)); - { - std::lock_guard lock(g_healRegistryMutex); - g_healRegistry.erase( - std::remove_if(g_healRegistry.begin(), g_healRegistry.end(), - [this](const auto& entry) { return entry.second == this; }), - g_healRegistry.end()); - } - healTimerId_ = 0; -#endif -} - -void ReaSamplerProcessor::healTick() { - // Main thread (the heal timer's TIMERPROC). One bounded retry: reloadFromBank re-reads - // the bank over the bridge and itself disarms this timer when it builds an instrument - // (or the restored intent is gone). If it stays armed, count the budget down and give - // up at zero — a genuinely-missing WAV must not poll ext-state forever. - if (healTimerId_ == 0) return; // raced a disarm between queue and dispatch - --healRetriesLeft_; - reloadFromBank(); - if (healTimerId_ != 0 && healRetriesLeft_ <= 0) disarmHealRetry(); -} - FUnknown* ReaSamplerProcessor::createInstance(void* /*context*/) { // The host owns the returned reference. Cast up to the combined interface the SDK // exposes (IAudioProcessor) so the FUnknown refcount is correctly rooted. return static_cast(new ReaSamplerProcessor()); } -// Out-of-line so unique_ptr sees the complete type here. The heal-retry -// disarm is defensive (terminate already disarms per the VST3 lifecycle): it removes this -// instance from the timer registry so a host that skips terminate can never leave a fired -// TIMERPROC holding a dangling pointer. -ReaSamplerProcessor::~ReaSamplerProcessor() { disarmHealRetry(); } +// Out-of-line so unique_ptr sees the complete type here. +ReaSamplerProcessor::~ReaSamplerProcessor() = default; tresult PLUGIN_API ReaSamplerProcessor::queryInterface(const TUID iid, void** obj) { // S6: expose REAPER's inline-embed interface. REAPER queries the IEditController for @@ -225,11 +135,9 @@ tresult PLUGIN_API ReaSamplerProcessor::initialize(FUnknown* context) { } tresult PLUGIN_API ReaSamplerProcessor::terminate() { - // process() is not running at terminate. Stop the reopen-heal retry timer first (its - // tick would reload into a dying instance), then free the live + draining instruments - // and drain the graveyard. Take the pointers out of the atomics first so nothing else + // process() is not running at terminate: free the live + draining instruments and + // drain the graveyard. Take the pointers out of the atomics first so nothing else // races them. - disarmHealRetry(); std::lock_guard lock(reloadMutex_); delete live_.exchange(nullptr); delete draining_.exchange(nullptr); @@ -244,20 +152,16 @@ tresult PLUGIN_API ReaSamplerProcessor::setActive(TBool state) { // no reload could free while active). The build/drain are off the audio thread — // setActive is a main/UI-thread call. if (state) { - // NOTE: this is also the non-editor reload trigger's anchor — if this reload runs - // before the extension's PROJEXTSTATE is parseable, reloadFromBank arms the bounded - // heal-retry timer itself (see the header), so a restored instance played via host - // MIDI with the editor never opened still comes up sounding. - reloadFromBank(); + // Self-contained (pS): this rebuild resolves + decodes from the instance-OWNED + // sample refs — it needs no bank read, so it plays regardless of whether the + // extension's PROJEXTSTATE has parsed yet (or the extension exists at all). + reloadInstrument(); } else { - // An inactive instance has nothing to heal into — stop the retry; the reactivation - // reload above re-arms it if the bank is still not parseable then. - disarmHealRetry(); std::lock_guard lock(reloadMutex_); // process is guaranteed stopped: free EVERYTHING. The live instrument too — its // voices are frozen mid-flight, and if it survived deactivation the reactivate // reload would displace it into the DRAIN slot, resurrecting stale sustained - // voices as ghosts. Reactivation rebuilds from scratch (reloadFromBank above), + // voices as ghosts. Reactivation rebuilds from scratch (reloadInstrument above), // so nothing is lost by clearing here. delete live_.exchange(nullptr); delete draining_.exchange(nullptr); @@ -288,7 +192,7 @@ tresult PLUGIN_API ReaSamplerProcessor::setState(IBStream* state) { // from a lone zone). deserializeComponentState lifts older blobs cleanly: a v2 zones-only // blob restores {"", zones}; a v1 S4 single-selection blob restores {id, one-zone map} so // the old pick survives as both; an empty/unknown blob restores {"", no zones} — the S10 - // silent empty state (no first-sample fallback in reloadFromBank). + // silent empty state (no first-sample fallback in reloadInstrument). // Pass sampleRate_ as the project rate for legacy v3 blob conversion (frames -> seconds at // the v3 read boundary). sampleRate_ is set by setupProcessing; REAPER calls setupProcessing // before setState on project load, so sampleRate_ is the real host rate here. A v3 blob on a @@ -301,7 +205,7 @@ tresult PLUGIN_API ReaSamplerProcessor::setState(IBStream* state) { // editor shows is the sample the engine plays" for already-affected projects; authored // Zone-view maps (any narrow key range) pass through untouched. PerformanceMap restored = cs.map; - reconcileSingleCaptureZones(restored, cs.selectionId); // bool return ignored: setPerformanceMap + reloadFromBank run unconditionally on load + reconcileSingleCaptureZones(restored, cs.selectionId); // bool return ignored: setPerformanceMap + reloadInstrument run unconditionally on load setPerformanceMap(restored); // S8: restore the last-consumed assignment generation so a re-open does not re-apply a // stale assign_request (the user may have manually changed the selection after the assign). @@ -335,8 +239,17 @@ tresult PLUGIN_API ReaSamplerProcessor::setState(IBStream* state) { // deserializeComponentState — pre-FB1 output). One atomic store; the audio thread picks // it up at the next block start. setMasterGainLinear(cs.masterGainLinear); + // pS self-contained playback: restore the instance-OWNED sample refs (v10) BEFORE the + // reload so it decodes straight from them — no bank read required to play. A pre-v10 + // blob lifts to an EMPTY table; the reload then resolves nothing until the bank blob + // becomes readable (the reload's opportunistic refresh, or pollBankSync's legacy lift), + // after which the next save is self-contained. + { + std::lock_guard lock(refsMutex_); + sampleRefs_ = cs.sampleRefs; + } // Rebuild from the restored state (off-thread — setState is a load-time call). - reloadFromBank(); + reloadInstrument(); return kResultOk; } @@ -369,6 +282,13 @@ tresult PLUGIN_API ReaSamplerProcessor::getState(IBStream* state) { state_out.monoTrigger = monoTrigger_; } state_out.masterGainLinear = masterGainLinear(); // FB1: persist the post-mixer gain (v8) + // pS: persist the OWNED sample refs (v10) — the saved blob carries everything needed to + // decode + play with no extension present. Filtered (on the snapshot copy, the member is + // untouched) to exactly what the instance currently plays, so the table cannot grow with + // browsing history. + state_out.sampleRefs = sampleRefs(); + retainRefs(state_out.sampleRefs, + referencedSampleIds(state_out.selectionId, state_out.map)); const std::vector bytes = serializeComponentState(state_out); if (!bytes.empty()) { const tresult wr = state->write(const_cast(bytes.data()), @@ -398,6 +318,11 @@ void ReaSamplerProcessor::setPerformanceMap(const PerformanceMap& map) { performanceMap_ = map; } +SampleRefs ReaSamplerProcessor::sampleRefs() { + std::lock_guard lock(refsMutex_); + return sampleRefs_; +} + ChannelMode ReaSamplerProcessor::channelMode() { std::lock_guard lock(channelModeMutex_); return channelMode_; @@ -513,7 +438,7 @@ void ReaSamplerProcessor::setChannelMode(ChannelMode mode) { // The DECODE policy changed. The output bus is FIXED stereo (GA fix — no bus repoint, no // restartComponent): reloading re-decodes the loaded WAV(s) under the new mode off-thread // (mono = downmix, stereo = L/R split) and the RT path just keeps rendering. - reloadFromBank(); + reloadInstrument(); } tresult PLUGIN_API ReaSamplerProcessor::setBusArrangements( @@ -530,7 +455,7 @@ tresult PLUGIN_API ReaSamplerProcessor::setBusArrangements( return kResultFalse; } -std::string ReaSamplerProcessor::reloadFromBank() { +std::string ReaSamplerProcessor::reloadInstrument() { // OFF THE AUDIO THREAD. Serialize concurrent reloads (editor click + setState) so // the retired-slot free is single-writer. This mutex is NEVER taken on the audio // thread — process() only touches the atomic. @@ -540,10 +465,27 @@ std::string ReaSamplerProcessor::reloadFromBank() { // with it before publishing. Under reloadMutex_ no other reload races here. const std::uint64_t gen = reloadGeneration_.fetch_add(1, std::memory_order_relaxed) + 1; - // 1. Read the live bank + resolve the project dir over the bridge (allocates, - // calls REAPER — fine here, off-thread). - std::optional banksJson = - bridge_.readReasamplerExtState(kProjExtBanksKey); + // 1. SELF-CONTAINED RESOLUTION (pS). The instance-OWNED refs table is the source of + // truth for what to decode. The live bank blob, WHEN readable, is folded into the + // table first (refreshRefsFromBank) — that is the browser's copy-the-ref-in + // mechanism and the S9 recapture sync in one — but its absence changes NOTHING + // below: a project restored before the extension's PROJEXTSTATE parses (or with + // the extension absent entirely) resolves + plays from the persisted refs. The + // project dir comes from REAPER itself (EnumProjects), not from the extension. + const std::string selId = selectedSampleId(); + const PerformanceMap map = performanceMap(); + const std::vector ids = referencedSampleIds(selId, map); + SampleRefs refs; + { + std::optional banksJson = + bridge_.readReasamplerExtState(kProjExtBanksKey); + std::lock_guard rl(refsMutex_); + if (banksJson) refreshRefsFromBank(sampleRefs_, *banksJson, ids); + // Hygiene: the owned table tracks exactly what the instance currently plays, so a + // de-referenced sample's entry drops here (never grows with browsing history). + retainRefs(sampleRefs_, ids); + refs = sampleRefs_; // snapshot for the decode below (outside the refs lock) + } const std::string projectDir = bridge_.activeProjectDir(); // The active channel mode (S7) governs how each WAV decodes (mono downmix vs 2-channel). // Read once under its mutex, off the audio thread, before the decode loop. The single- @@ -563,115 +505,93 @@ std::string ReaSamplerProcessor::reloadFromBank() { std::string resolvedId; std::unique_ptr built; + Keymap km; + bool haveKeymap = false; - if (banksJson) { - // 2. Tier 1 first: if the instrument's performance map is non-empty, resolve its - // zones against the live bank (STALE ids drop cleanly), decode each zone's WAV - // off-thread, and build the ZONED keymap. Each surviving zone plays its bank - // sample repitched from its effective root note (override > bank intrinsic > C4). - // A zone whose WAV fails to decode is dropped (not the whole map). - const PerformanceMap map = performanceMap(); - Keymap km; - bool haveKeymap = false; - - if (!map.empty()) { - const ResolvedPerformance resolved = resolvePerformance(*banksJson, map); - if (!resolved.zones.empty()) { - std::vector decoded; - std::vector kept; - decoded.reserve(resolved.zones.size()); - kept.reserve(resolved.zones.size()); - for (const ResolvedZone& rz : resolved.zones) { - std::optional pcm = - decodeRelative(projectDir, rz.relativePath, mode); - if (!pcm) continue; // unreadable WAV -> drop this zone - kept.push_back(rz); - decoded.push_back(std::move(*pcm)); - } - km = buildZonedKeymap(kept, decoded); - haveKeymap = !km.zones.empty(); - } - } - - // 3. Single-capture fast path (S10): an empty performance map plays the ONE - // deliberately-selected capture chromatically across the whole keyboard. This is - // the default face — one picked capture, repitched from its root. NO first- - // sample fallback: an EMPTY selection (or a stale id) resolves to nullopt in - // selectSample, so an un-picked instrument stays SILENT (the editor shows its - // "pick a capture" empty state) rather than auto-playing sample #1 (S10 policy - // reversal of the S4 convenience default). - if (!haveKeymap) { - std::optional sel = - selectSample(*banksJson, selectedSampleId()); - if (sel) { - // GA auto-default: channelModeFor computes the mode from the loaded capture's - // REQUESTED channel count (always 2 for extension captures; mono only for - // ingest-imported mono files). An unknown count (0) or explicit user choice - // returns the current mode unchanged. Decode-only: the output bus is fixed - // stereo, so no bus work follows a flip. - { - std::lock_guard lock(channelModeMutex_); - channelMode_ = channelModeFor(sel->channelCount, channelMode_, - channelModeExplicit_); - mode = channelMode_; - } + // 2. Tier 1 first: if the instrument's performance map is non-empty, resolve its + // zones against the OWNED refs (an id with no ref drops cleanly), decode each + // zone's WAV off-thread, and build the ZONED keymap. Each surviving zone plays + // its sample repitched from its effective root note (override > ref intrinsic > + // C4). A zone whose WAV fails to decode — a MISSING FILE included — is dropped + // (not the whole map): the defined no-play, no crash, no retry loop. + if (!map.empty()) { + const ResolvedPerformance resolved = resolvePerformanceFromRefs(refs, map); + if (!resolved.zones.empty()) { + std::vector decoded; + std::vector kept; + decoded.reserve(resolved.zones.size()); + kept.reserve(resolved.zones.size()); + for (const ResolvedZone& rz : resolved.zones) { std::optional pcm = - decodeRelative(projectDir, sel->relativePath, mode); - if (pcm) { - km = buildTier0Keymap(std::move(pcm->monoFrames), pcm->sampleRate, - sel->rootNote, sel->loop, - std::move(pcm->framesR)); - haveKeymap = true; - resolvedId = selectedSampleId(); // the concrete pick that resolved - } + decodeRelative(projectDir, rz.relativePath, mode); + if (!pcm) continue; // unreadable/missing WAV -> drop this zone + kept.push_back(rz); + decoded.push_back(std::move(*pcm)); + } + km = buildZonedKeymap(kept, decoded); + haveKeymap = !km.zones.empty(); + } + } + + // 3. Single-capture fast path (S10): an empty performance map plays the ONE + // deliberately-selected capture chromatically across the whole keyboard, resolved + // against the OWNED refs. NO first-sample fallback: an EMPTY selection (or a + // selection with no ref) resolves to nothing, so an un-picked instrument stays + // SILENT (the editor shows its "pick a capture" empty state) rather than + // auto-playing sample #1 (S10 policy reversal of the S4 convenience default). + if (!haveKeymap) { + if (const SelectedSample* sel = findRef(refs, selId)) { + // GA auto-default: channelModeFor computes the mode from the loaded capture's + // REQUESTED channel count (always 2 for extension captures; mono only for + // ingest-imported mono files). An unknown count (0) or explicit user choice + // returns the current mode unchanged. Decode-only: the output bus is fixed + // stereo, so no bus work follows a flip. + { + std::lock_guard cm(channelModeMutex_); + channelMode_ = channelModeFor(sel->channelCount, channelMode_, + channelModeExplicit_); + mode = channelMode_; + } + std::optional pcm = + decodeRelative(projectDir, sel->relativePath, mode); + if (pcm) { + km = buildTier0Keymap(std::move(pcm->monoFrames), pcm->sampleRate, + sel->rootNote, sel->loop, + std::move(pcm->framesR)); + haveKeymap = true; + resolvedId = selId; // the concrete pick that resolved } } + } - if (haveKeymap) { - // Preserve OLA window in OUTPUT frames from the host sample rate (kPreserveWindowMs). - // Every voice's shifter is pre-sized to this off-thread here, so process()-time - // note-on never allocates. Floored at 2 so a valid window is always a real ring - // (which also covers a pathological host rate <= 0 — no rate literal needed). - std::int64_t preserveWindow = static_cast( - kPreserveWindowMs * sampleRate_ / 1000.0 + 0.5); - if (preserveWindow < 2) preserveWindow = 2; - built = std::make_unique( - std::move(km), static_cast(builtVoiceCount), gen, - kPreserveVoiceCap, preserveWindow, builtVoiceMode, builtMonoTrigger); - } + if (haveKeymap) { + // Preserve OLA window in OUTPUT frames from the host sample rate (kPreserveWindowMs). + // Every voice's shifter is pre-sized to this off-thread here, so process()-time + // note-on never allocates. Floored at 2 so a valid window is always a real ring + // (which also covers a pathological host rate <= 0 — no rate literal needed). + std::int64_t preserveWindow = static_cast( + kPreserveWindowMs * sampleRate_ / 1000.0 + 0.5); + if (preserveWindow < 2) preserveWindow = 2; + built = std::make_unique( + std::move(km), static_cast(builtVoiceCount), gen, + kPreserveVoiceCap, preserveWindow, builtVoiceMode, builtMonoTrigger); } // 4. Publish. Atomically install the new instrument; the DISPLACED one moves into the // DRAIN slot (FA1, bug 3b) where process() keeps rendering its ringing voices — // a reload never cuts a sounding note; the next note-on plays the new state. The // instrument evicted FROM the drain slot (two reloads old) goes to the graveyard - // (process may still be mid-block reading it). A null `built` (no bank / unreadable + // (process may still be mid-block reading it). A null `built` (no ref / unreadable // WAV) installs silence while the displaced tails still ring out via the drain. // `built` is heap-owned; release() hands ownership to the atomic; the drain-evicted // pointer is re-owned by the graveyard. - // - const bool loaded = static_cast(built); publishBuiltLocked(std::move(built)); - - // Reopen-heal retry, the NON-editor trigger (see the header): this reload built NOTHING - // despite restored intent (a selection or zones) while the bridge is connected — during - // project load that almost always means the extension's PROJEXTSTATE block is not yet - // parseable — so arm the bounded main-thread retry timer. Every other outcome disarms: - // the timer only lives while there is something to heal. (Leaf-mutex order holds: - // reloadMutex_ -> registry mutex, matching the TIMERPROC which releases the registry - // before ticking into this function.) - const bool intent = !selectedSampleId().empty() || !performanceMap().empty(); - if (!loaded && intent && bridge_.isConnected()) { - armHealRetry(); - } else { - disarmHealRetry(); - } return resolvedId; } void ReaSamplerProcessor::publishBuiltLocked(std::unique_ptr built) { // REQUIRES reloadMutex_ held (single-writer over both slots + the graveyard). Shared by - // reloadFromBank and rebuildVoiceEngine — the one safety-critical swap dance. + // reloadInstrument and rebuildVoiceEngine — the one safety-critical swap dance. // // Bounded reclaim: free graveyard entries whose installedAt < seen, where seen is // the minimum installedAt process() published over the pointers it holds. Both @@ -711,7 +631,7 @@ void ReaSamplerProcessor::rebuildVoiceEngine() { } const std::uint64_t gen = reloadGeneration_.fetch_add(1, std::memory_order_relaxed) + 1; - // Same Preserve-window derivation as reloadFromBank (kPreserveWindowMs at the host rate). + // Same Preserve-window derivation as reloadInstrument (kPreserveWindowMs at the host rate). std::int64_t preserveWindow = static_cast( kPreserveWindowMs * sampleRate_ / 1000.0 + 0.5); if (preserveWindow < 2) preserveWindow = 2; @@ -740,7 +660,7 @@ void ReaSamplerProcessor::retireIdleDrain() { draining_.store(nullptr, std::memory_order_release); graveyard_.push_back(std::unique_ptr(drain)); // Prune what is now provably unreachable — the same monotone-generation proof as the - // reload path's reclaim (see reloadFromBank): an entry with installedAt < seen cannot be + // reload path's reclaim (see reloadInstrument): an entry with installedAt < seen cannot be // held by process() now or ever again. The just-parked drain frees here immediately when // process() has already published past it; otherwise on the next reload/retire/deactivate. const std::uint64_t seen = processGeneration_.load(std::memory_order_acquire); @@ -804,7 +724,7 @@ ReaSamplerProcessor::pollBankSync(bool isFocusedTarget) { if (decision.apply) { // Apply the assignment as this instance's own selection (the same path a user card-pick - // takes) — the instrument updates its OWN state, never the bank. reloadFromBank below + // takes) — the instrument updates its OWN state, never the bank. reloadInstrument below // rebuilds against the new selection, so skip a redundant reload here. setSelectedSampleId(decision.sampleId); // Zone-bleed fix (3a), peer of the editor's Browse Load: a stale full-range zone @@ -819,8 +739,9 @@ ReaSamplerProcessor::pollBankSync(bool isFocusedTarget) { // --- S9: bank-generation change-detection ------------------------------------- // Read the generation stamp; parse (absent/malformed -> 0, the pre-S9 default). FIRST poll - // (lastSeenBankGeneration_ == -1 sentinel): BASELINE the seen value without a reload — setState - // already loaded the current bank, so a redundant reload on open would only churn. A later + // (lastSeenBankGeneration_ == -1 sentinel): BASELINE the seen value without a reload — + // setState already loaded the instrument from its OWNED refs (pS), so a redundant reload + // on open would only churn. A later // generation CHANGE (a recapture/ingest/remove, or an undo that lowers it) then drives the // reload. An assignment we just applied also needs a reload; fold both into ONE (coalesced). std::int64_t currentGen = kBankGenerationAbsent; @@ -832,36 +753,29 @@ ReaSamplerProcessor::pollBankSync(bool isFocusedTarget) { !firstPoll && bankGenerationChanged(lastSeenBankGeneration_, currentGen); lastSeenBankGeneration_ = currentGen; - // 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(); + // LEGACY LIFT (pre-v10 blob): the restored state carries intent (a selection or zones) + // but NO owned refs — a pre-pS blob had no path table, so the setState-time reload had + // nothing to decode unless the bank happened to be readable already. Reload on this + // editor tick until the lift lands: reloadInstrument folds the bank blob into the refs + // when readable, after which the table is non-empty and this never fires again (the + // next save is then self-contained). A deliberately-empty instance has no intent and + // never churns; a lift whose bank stays unreadable (or whose id went stale) retries a + // cheap null publish on the editor cadence only. This is a MIGRATION convenience for + // old projects, NOT a playback dependency — a v10 blob plays from its refs with no + // poll at all (pS). + bool legacyLift = false; + if (!genChanged && !result.applied && sampleRefs().empty()) { + legacyLift = !selectedSampleId().empty() || !performanceMap().empty(); } - if (genChanged || result.applied || healReload) { - reloadFromBank(); // atomic pointer-swap handoff — glitch-free mid-play (S4 graveyard) - // 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; - } - - // Heal RETRY: if the heal reload above STILL left nothing live (the first poll was - // itself too early — the bank blob still absent/unparseable), reset the sentinel so - // the NEXT tick re-arms the first-poll heal instead of spending it one-shot. Bounded - // by the intent check inside the heal (a deliberately-empty instance never sets - // healReload, so never re-arms). This also heals a bank whose generation counter was - // never bumped: `bankGenerationChanged` is a plain != against a counter that stays 0 - // for such a bank (0 != 0 never fires), so the generation path alone could never - // recover it. - if (healReload && live_.load(std::memory_order_acquire) == nullptr) { - lastSeenBankGeneration_ = -1; + if (genChanged || result.applied || legacyLift) { + reloadInstrument(); // atomic pointer-swap handoff — glitch-free mid-play (S4 graveyard) + // Report the reload distinctly from an S8 apply so the editor re-snapshots its bank + // view. A legacy lift counts only when it actually landed an instrument (otherwise + // every retry tick would churn the editor's caches for nothing). + result.reloaded = + genChanged || + (legacyLift && live_.load(std::memory_order_acquire) != nullptr); } return result; } diff --git a/src/vst/reasampler_processor.h b/src/vst/reasampler_processor.h index 0c1e715..9fe9b22 100644 --- a/src/vst/reasampler_processor.h +++ b/src/vst/reasampler_processor.h @@ -10,11 +10,19 @@ // state (the selected sample), and the IEditController seat so createView() can hand the // host our IPlugView LICE editor. // +// SELF-CONTAINED PLAYBACK (pS architecture correction). The instance OWNS its sample: the +// component state persists, per referenced bank sample, the project-relative WAV path + +// decode intrinsics (SampleRefs), and reloadInstrument decodes straight from that table. +// The extension's bank blob is a BROWSER SOURCE that opportunistically refreshes the refs +// when readable — NEVER a runtime requirement for playback. A project restored before the +// extension's PROJEXTSTATE parses (or with the extension absent) plays on load; the old +// reopen-heal timer + poll-to-play machinery that papered over the bank dependency is gone. +// // REAL-TIME DISCIPLINE (S4 hard constraint). The audio thread (process) does NO -// allocation, NO file I/O, NO bridge calls, NO locks. Sample loading — bridge ext-state -// read, WAV decode, path resolve, keymap build, VoiceEngine construction — all happens -// OFF the audio thread (reloadFromBank, driven from the main/UI thread) and is handed to -// process via a single atomic pointer swap. See the LoadedInstrument handoff below. +// allocation, NO file I/O, NO bridge calls, NO locks. Sample loading — ref resolve, WAV +// decode, keymap build, VoiceEngine construction — all happens OFF the audio thread +// (reloadInstrument, driven from the main/UI thread) and is handed to process via a +// single atomic pointer swap. See the LoadedInstrument handoff below. #pragma once @@ -131,27 +139,35 @@ public: } // Called by the editor (main/UI thread) when the user picks a sample, and internally - // on load. Reads the live bank over the bridge, resolves+decodes the selected WAV - // OFF the audio thread, and publishes the built instrument to process() via an - // atomic swap. Safe to call with no bridge / no bank (leaves silence). Returns the - // resolved selection id ("" if nothing was loaded) for the editor to reflect. - std::string reloadFromBank(); + // on load. SELF-CONTAINED (pS): resolves the selection/zones against the instance-OWNED + // SampleRefs table, decodes each WAV OFF the audio thread, and publishes the built + // instrument to process() via an atomic swap — NO bank read is required for playback. + // When the live bank blob IS readable it is first folded into the refs table + // (refreshRefsFromBank), which is both the browser's copy-the-ref-in mechanism and the + // S9 live-recapture sync. A missing/unreadable WAV is the defined no-play (silence, no + // retry). Returns the resolved selection id ("" if nothing was loaded) for the editor. + std::string reloadInstrument(); // The result of a bank-sync poll (S9/S8): what pollBankSync did this tick, so the editor // can react (repaint / re-snapshot its own view) only when something actually changed. struct BankSyncResult { - bool reloaded = false; // the bank generation changed -> reloadFromBank ran + // The bank generation changed (or a pre-v10 legacy lift landed an instrument) -> + // reloadInstrument ran and the editor should re-snapshot its bank view. + bool reloaded = false; bool applied = false; // a new assignment request was applied -> selection changed }; // Poll the S9 bank-generation counter and the S8 assignment request over the bridge, OFF - // THE AUDIO THREAD (the editor's UI timer drives this — NEVER process()). Semantics: - // * S9: if the bank generation differs from what we last saw, call reloadFromBank() so a - // recapture/ingest refreshes playback hands-free (atomic swap, glitch-free). + // THE AUDIO THREAD (the editor's UI timer drives this — NEVER process()). This is an + // EDITOR/BROWSER sync path — playback never depends on it (pS). Semantics: + // * S9: if the bank generation differs from what we last saw, call reloadInstrument() so + // a recapture/ingest refreshes playback hands-free (atomic swap, glitch-free). // * S8: if a NEW (generation > last consumed) assignment request names a resolvable // sample AND this instance is the target (isFocusedTarget), apply it as the selection // and reload; an unresolvable request is DROPPED silently (marker advanced, no change); // a non-target instance neither applies nor advances its marker. + // * LEGACY LIFT: a pre-v10 blob restored with intent but no refs retries the (cheap) + // bank read until the blob is parseable, then reloads ONCE to copy the refs in. // The consumed marker advances in component state (marked dirty via the host handler) so a // re-open does not re-apply. `isFocusedTarget` is the shell's thundering-herd policy input // (the editor passes true only for the instance whose editor is open — see the handoff). @@ -162,7 +178,7 @@ public: // editor borrows it (outlives the editor). ReaperBridge& bridge() { return bridge_; } - // The live host sample rate latched from setupProcessing (the SAME rate reloadFromBank + // The live host sample rate latched from setupProcessing (the SAME rate reloadInstrument // resolves seconds->frames against). The editor's S-VIEW-3 envelope overlay reads it to place // its wall-clock seconds on the same time base the voice engine plays them over. 0.0 before // setupProcessing runs (the editor guards). Read on the UI thread; a plain load — sampleRate_ @@ -177,14 +193,14 @@ public: // The performance map (Tier 1: the zoned keymap the instrument owns; D-B). Read/written // by the editor on the UI thread; snapshotted under performanceMutex_. NEVER read on the - // audio thread — reloadFromBank bakes it into the LoadedInstrument's Keymap off-thread. + // audio thread — reloadInstrument bakes it into the LoadedInstrument's Keymap off-thread. PerformanceMap performanceMap(); void setPerformanceMap(const PerformanceMap& map); // The per-instance channel mode (S7, D-E: mono | stereo). Read/written on the UI thread - // (the editor toggle) and read off-thread by getState/reloadFromBank; guarded by + // (the editor toggle) and read off-thread by getState/reloadInstrument; guarded by // channelModeMutex_. NEVER read on the audio thread — process() renders against the host's - // negotiated output channel count, and reloadFromBank bakes the mode into the decode. + // negotiated output channel count, and reloadInstrument bakes the mode into the decode. // GA fix: the mode is a DECODE policy only (downmix vs L/R split). The output bus is a // FIXED stereo bus — mono mode renders dual-mono through it (centered) — so a mode change // never renegotiates host I/O (the mono<->stereo bus flip's live pin remap was the @@ -240,32 +256,12 @@ public: void previewNoteOn(int note); void previewNoteOff(int note); - // Reopen-heal retry tick — the target of the module-internal Win32 heal timer (see - // armHealRetry below), NOT host-facing. Public only because the file-static TIMERPROC - // in the .cpp must reach it. Main thread; re-runs reloadFromBank (which disarms the - // timer itself on success) and disarms when the bounded retry budget runs out. - void healTick(); + // The instance-owned sample refs (pS self-contained playback): a snapshot copy for the + // editor (waveform/loop-intrinsic fallback when the bank blob is not readable). UI + // thread; guarded by refsMutex_. + SampleRefs sampleRefs(); private: - // --- Reopen-heal retry (the NON-editor reload trigger) -------------------------- - // A project-restored instance with intent (a selection or zones) can come up SILENT: - // REAPER runs track-FX setState before the extension's PROJEXTSTATE block is parsed, - // so the setState-time reload reads an empty bank. The editor's WM_TIMER poll heals - // that — but only if the user opens the editor; an instance played via host MIDI with - // the editor never attached stayed silent indefinitely. Mechanism: reloadFromBank - // itself detects "built NOTHING despite restored intent, bridge connected" (off the - // audio thread — it just tried) and arms a BOUNDED, HWND-less Win32 retry timer - // (SetTimer + TIMERPROC: fires on the arming thread's message pump — REAPER's main - // thread, where every reload path already runs). Each tick re-runs reloadFromBank, - // which disarms on success or when the intent is gone; the bound stops the churn for - // an instance whose WAV is genuinely missing. A deliberately-empty instance never - // arms (no intent). process() is untouched — fully RT-safe. Main-thread only. - // No-ops on non-Windows builds (the VST target is Windows-only). - void armHealRetry(); - void disarmHealRetry(); - std::uintptr_t healTimerId_ = 0; // 0 = not armed (main thread only) - int healRetriesLeft_ = 0; // remaining timer-tick retries (main thread only) - // Phase S drain retirement (FA1-review Major #2): if process() has published that the // 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 @@ -283,7 +279,7 @@ private: // 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 + // full reloadInstrument (which re-decodes every zone WAV from disk on the UI thread) was // pure waste — a visible UI stall on a many-zone instrument. Copying the keymap is safe: // it is immutable after construction and, under reloadMutex_, the live instrument can // neither be swapped nor freed while we read it. When nothing is loaded this is a no-op — @@ -293,7 +289,7 @@ private: // Publish `built` (null = install silence) into live_: prune the graveyard by the last // process()-published generation, swap `built` into live_, displace the previous live into // the drain slot, and park the drain-evicted instrument in the graveyard. REQUIRES - // reloadMutex_ held — factored out so reloadFromBank and rebuildVoiceEngine share the ONE + // reloadMutex_ held — factored out so reloadInstrument and rebuildVoiceEngine share the ONE // safety-critical swap dance (see the handoff proof below). void publishBuiltLocked(std::unique_ptr built); @@ -303,7 +299,7 @@ private: // process() atomically loads `live_` AND `draining_` at block start and marshals/renders // against them — two atomic acquires, no lock, no free on the audio thread. // - // reloadFromBank() (off-thread, serialized by reloadMutex_) builds a new + // reloadInstrument() (off-thread, serialized by reloadMutex_) builds a new // LoadedInstrument and atomically swaps it into `live_`. The DISPLACED instrument is // NOT freed and NOT silenced: it moves into `draining_`, where process() keeps // rendering its already-sounding voices (and routes note-offs to it) so a reload — @@ -349,16 +345,25 @@ private: std::string selectedSampleId_; // The performance map (Tier 1: the instrument's owned zoned keymap). Off-thread only; - // guarded against a getState/editor race. NOT read on the audio thread — reloadFromBank + // guarded against a getState/editor race. NOT read on the audio thread — reloadInstrument // bakes it into the LoadedInstrument's Keymap under the reload lock. std::mutex performanceMutex_; PerformanceMap performanceMap_; - // The per-instance channel mode (S7). Off-thread only (UI + getState + reloadFromBank); + // The instance-OWNED sample refs (pS self-contained playback): the path + intrinsics + // per referenced bank sample that setState restores, reloadInstrument resolves/decodes + // from, and getState persists (v10). Refreshed opportunistically from the bank blob + // when it is readable; NEVER a bank dependency for playback. Off-thread only (UI + + // load/save + reload); guarded against a getState/reload race. NOT read on the audio + // thread. + std::mutex refsMutex_; + SampleRefs sampleRefs_; + + // The per-instance channel mode (S7). Off-thread only (UI + getState + reloadInstrument); // guarded against a getState/editor race. Default Mono preserves pre-S7 behavior. NOT read // on the audio thread — process renders against the host's negotiated output channel count. // channelModeExplicit_ (GA, persisted v9): false = the mode is an un-touched default that - // reloadFromBank may auto-default from the loaded capture's channel count; true = the user + // reloadInstrument may auto-default from the loaded capture's channel count; true = the user // deliberately toggled the mode (setChannelMode latches it) and it is never fought. std::mutex channelModeMutex_; ChannelMode channelMode_ = ChannelMode::Mono; @@ -375,17 +380,12 @@ private: // The bank generation this instance last SAW (S9 reader). UI/timer-thread only (pollBankSync // is the sole reader/writer) — no mutex needed, and it is NOT persisted. Initialized to a // -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. - // 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. If that heal reload STILL leaves nothing live, - // pollBankSync resets this back to the -1 sentinel so the next tick re-arms the heal — - // the retry is bounded by the intent check (a deliberately-empty instance never heals), - // and it also covers a bank whose generation counter was never bumped (0 != 0 can never - // fire the generation path). NOT read on the audio thread. + // FIRST poll after an editor open BASELINES the seen value without a redundant reload + // (setState already loaded the instrument from the OWNED refs); a subsequent generation + // CHANGE then drives the reload. Since pS there is NO reopen-heal here: playback never + // depends on this poll — a v10 blob plays from its own refs at setState time. The only + // poll-driven reload besides a generation change is the pre-v10 LEGACY LIFT (see + // pollBankSync). 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 @@ -397,9 +397,9 @@ private: std::uint8_t previewVelocity_ = kPreviewVelocityDefault; // Phase S voice-system parameters (per-instance, persisted in component state v7). Off-thread - // only (UI voice deck + getState/setState + reloadFromBank); guarded against a getState/editor + // only (UI voice deck + getState/setState + reloadInstrument); guarded against a getState/editor // race. Defaults {16, Poly, Retrigger} reproduce pre-Phase-S behavior. NOT read on the audio - // thread — reloadFromBank bakes them into the LoadedInstrument's engine off-thread. + // thread — reloadInstrument bakes them into the LoadedInstrument's engine off-thread. std::mutex voiceParamsMutex_; int voiceCount_ = kDefaultVoiceCount; VoiceMode voiceMode_ = VoiceMode::Poly; @@ -435,7 +435,7 @@ private: // Latched from setupProcessing so setActive/reload can size against it. Read // off-thread only. 0.0 is explicitly invalid — setupProcessing sets the real host rate - // before any audio, and reloadFromBank guards on it before use. + // before any audio, and reloadInstrument guards on it before use. double sampleRate_ = 0.0; Steinberg::int32 maxBlockSize_ = 4096; diff --git a/src/vst/sample_map.cpp b/src/vst/sample_map.cpp index e693b09..43359bf 100644 --- a/src/vst/sample_map.cpp +++ b/src/vst/sample_map.cpp @@ -40,6 +40,32 @@ SelectedSample distill(const Sample& s) { return out; } +// The ONE override-beats-intrinsic fold shared by the bank-side resolvePerformance and the +// refs-side resolvePerformanceFromRefs (pS): a zone's authored fields + the sample's +// intrinsics (already distilled — rootNote carries the middle-C default) -> ResolvedZone. +// Shared so the two resolution paths cannot drift. +ResolvedZone foldZone(const PerformanceZone& z, const SelectedSample& ref) { + ResolvedZone rz; + rz.relativePath = ref.relativePath; + rz.lowNote = z.lowNote; + rz.highNote = z.highNote; + // Effective root: override beats intrinsic (distill already defaulted an empty + // intrinsic to middle C). + rz.rootNote = z.rootOverride ? *z.rootOverride : ref.rootNote; + // S-VIEW-6/S-VIEW-9: key tracking + the velocity->amp curve are instrument state — + // carried straight through and applied at play time. + rz.keyTrack = z.keyTrack; + rz.velocityCurve = z.velocityCurve; + // Effective loop / start (S11): the per-zone override wins over the intrinsic; absent + // -> the intrinsic (loop) / frame 0 (start). The bank is never mutated (D-B). + rz.loop = z.loopOverride ? *z.loopOverride : ref.loop; + rz.startFrame = z.startPoint ? *z.startPoint : 0; + // S15/S16 per-zone play params (SECONDS) carry through unchanged; buildZonedKeymap + // resolves them to frames. + rz.play = z.play; + return rz; +} + } // namespace std::optional selectSample(const std::string& banksJson, @@ -69,6 +95,62 @@ ChannelMode channelModeFor(int channelCount, ChannelMode current, bool isExplici return channelCount >= 2 ? ChannelMode::Stereo : ChannelMode::Mono; } +// --- Instance-owned sample references (pS self-contained playback) ------------- + +const SelectedSample* findRef(const SampleRefs& refs, const std::string& sampleId) { + if (sampleId.empty()) return nullptr; + for (const SampleRefEntry& e : refs) { + if (e.sampleId == sampleId) return &e.ref; + } + return nullptr; +} + +std::vector referencedSampleIds(const std::string& selectionId, + const PerformanceMap& map) { + std::vector ids; + const auto addUnique = [&ids](const std::string& id) { + if (id.empty()) return; + for (const std::string& have : ids) { + if (have == id) return; + } + ids.push_back(id); + }; + addUnique(selectionId); + for (const PerformanceZone& z : map.zones) addUnique(z.sampleId); + return ids; +} + +void refreshRefsFromBank(SampleRefs& refs, const std::string& banksJson, + const std::vector& ids) { + if (ids.empty() || banksJson.empty()) return; + std::optional book = BankBook::deserialize(banksJson); + if (!book) return; // malformed blob -> no-op (the instance keeps its own copies) + for (const std::string& id : ids) { + const Sample* found = nullptr; + for (const Bank& b : book->banks()) { + if (const Sample* s = b.index.query(id)) { found = s; break; } + } + if (!found) continue; // bank miss: NEVER strips a ref — the instance owns its copy + const SelectedSample distilled = distill(*found); + bool updated = false; + for (SampleRefEntry& e : refs) { + if (e.sampleId == id) { e.ref = distilled; updated = true; break; } + } + if (!updated) refs.push_back(SampleRefEntry{id, distilled}); + } +} + +void retainRefs(SampleRefs& refs, const std::vector& ids) { + refs.erase(std::remove_if(refs.begin(), refs.end(), + [&ids](const SampleRefEntry& e) { + for (const std::string& id : ids) { + if (id == e.sampleId) return false; + } + return true; + }), + refs.end()); +} + std::vector listSamples(const std::string& banksJson) { std::vector out; if (banksJson.empty()) return out; @@ -219,28 +301,24 @@ ResolvedPerformance resolvePerformance(const std::string& banksJson, out.droppedSampleIds.push_back(z.sampleId); continue; } - ResolvedZone rz; - rz.relativePath = found->relativePath; - rz.lowNote = z.lowNote; - rz.highNote = z.highNote; - // Effective root: override beats bank intrinsic beats middle-C default. - rz.rootNote = z.rootOverride ? *z.rootOverride - : (found->rootNote ? *found->rootNote : 60); - // S-VIEW-6: the key-tracking scalar is instrument state (not a bank fact) — carried - // straight through to the resolved zone and applied in the repitch math at play time. - rz.keyTrack = z.keyTrack; - // S-VIEW-9: the velocity->amp curve is likewise instrument state — carried through and - // eval'd at Voice::start to set the voice's amp gain from the note-on velocity. - rz.velocityCurve = z.velocityCurve; - // Effective loop / start (S11): the instrument's per-zone override wins over the - // bank's S2 intrinsic; absent -> the intrinsic (loop) / frame 0 (start). The bank is - // never mutated — this only shapes what the core plays for THIS instance (D-B). - rz.loop = z.loopOverride ? *z.loopOverride : loopFromSample(*found); - rz.startFrame = z.startPoint ? *z.startPoint : 0; - // S15/S16 per-zone play params (SECONDS) carry through unchanged (they are instrument - // state, not resolved against the bank); buildZonedKeymap resolves them to frames. - rz.play = z.play; - out.zones.push_back(std::move(rz)); + // Distill the bank Sample to the same intrinsics shape the refs table carries, then + // run the SHARED fold — so the bank path and the refs path resolve identically. + out.zones.push_back(foldZone(z, distill(*found))); + } + return out; +} + +ResolvedPerformance resolvePerformanceFromRefs(const SampleRefs& refs, + const PerformanceMap& map) { + ResolvedPerformance out; + for (const PerformanceZone& z : map.zones) { + if (const SelectedSample* r = findRef(refs, z.sampleId)) { + out.zones.push_back(foldZone(z, *r)); + } else { + // No ref for this id (never copied, or a pre-v10 blob not yet lifted): drop the + // zone cleanly + report — the same shape as the bank path's stale-id policy. + out.droppedSampleIds.push_back(z.sampleId); + } } return out; } @@ -641,6 +719,24 @@ std::vector serializeComponentState(const ComponentState& state) { // v8 lift). 0 = implicit (the shell may auto-default the mode from the loaded capture's // channel count); 1 = the user deliberately toggled the mode (never fought). out.push_back(state.channelModeExplicit ? 1 : 0); + // v10 envelope addition (pS self-contained playback): the instance-owned sample-refs + // table, following the explicit flag so a v9 blob is a strict prefix up to here (see + // the v9 lift). Wire shape per kSelectionZonesRefsV10Version: entry count, then per + // entry id + path (length-prefixed), rootNote, loop (hasLoop + start/end, always + // written), channelCount. + putU32le(out, static_cast(state.sampleRefs.size())); + for (const SampleRefEntry& e : state.sampleRefs) { + putU32le(out, static_cast(e.sampleId.size())); + out.insert(out.end(), e.sampleId.begin(), e.sampleId.end()); + putU32le(out, static_cast(e.ref.relativePath.size())); + out.insert(out.end(), e.ref.relativePath.begin(), e.ref.relativePath.end()); + putU32le(out, static_cast(static_cast(e.ref.rootNote))); + out.push_back(e.ref.loop.hasLoop ? 1 : 0); + putU64le(out, asU64(e.ref.loop.start)); + putU64le(out, asU64(e.ref.loop.end)); + putU32le(out, + static_cast(static_cast(e.ref.channelCount))); + } // Length-prefixed selection id (it precedes the zones payload, so it MUST be framed — // unlike the v1 selection blob where the id ran to end-of-stream). putU32le(out, static_cast(state.selectionId.size())); @@ -717,13 +813,14 @@ ComponentState deserializeComponentState(const std::vector& bytes, return out; // previewVelocity stays at the mid default (pre-S-VIEW-4) } if (version != kComponentStateVersion && + version != kSelectionZonesModeMarkerVelVoiceGainExplicitV9Version && version != kSelectionZonesModeMarkerVelVoiceGainV8Version && version != kSelectionZonesModeMarkerVelVoiceV7Version && version != kSelectionZonesModeMarkerVelV6Version) { return out; // unknown -> empty } - // v6/v7/v8/v9 shared prefix: the channel-mode byte, then the 8-byte consumed-assignment marker, + // v6..v10 shared prefix: the channel-mode byte, then the 8-byte consumed-assignment marker, // then the 1-byte preview velocity, precede the v3 body. A non-{0,1} mode byte is treated // as mono (conservative default) rather than rejected — a corrupt mode never silences the // instance. @@ -774,6 +871,28 @@ ComponentState deserializeComponentState(const std::vector& bytes, if (!r.ok) return out; // truncated before the flag -> empty (implicit holds) out.channelModeExplicit = (explicitByte == 1); } + // v10 (pS self-contained playback): the sample-refs table. A v9-or-older blob skips it — + // the EMPTY-table default holds, and the shell lifts the refs once via the bridge-resolve + // path (then re-saves self-contained). A truncated mid-entry read keeps the entries that + // parsed cleanly and drops the rest (the selection/zones behind it are unreadable anyway). + if (version >= kSelectionZonesRefsV10Version) { + const std::uint32_t refCount = r.u32(); + for (std::uint32_t i = 0; i < refCount && r.ok; ++i) { + SampleRefEntry e; + const std::uint32_t refIdLen = r.u32(); + e.sampleId = r.str(refIdLen); + const std::uint32_t pathLen = r.u32(); + e.ref.relativePath = r.str(pathLen); + e.ref.rootNote = r.i32(); + e.ref.loop.hasLoop = (r.u8() != 0); + e.ref.loop.start = r.i64(); + e.ref.loop.end = r.i64(); + e.ref.channelCount = r.i32(); + if (!r.ok) break; // truncated mid-entry -> keep what parsed, drop the rest + out.sampleRefs.push_back(std::move(e)); + } + if (!r.ok) return out; + } const std::uint32_t idLen = r.u32(); out.selectionId = r.str(idLen); if (!r.ok) { out.selectionId.clear(); return out; } // truncated id -> empty diff --git a/src/vst/sample_map.h b/src/vst/sample_map.h index 483c44a..4f38901 100644 --- a/src/vst/sample_map.h +++ b/src/vst/sample_map.h @@ -63,13 +63,54 @@ std::optional selectSample(const std::string& banksJson, // GA auto-default rule (pure, tested): given the capture's requested channel count, the // instance's current mode, and whether the user has explicitly toggled the mode, return // the mode to apply. Explicit choice is never overridden. An unknown channelCount (0) -// leaves the current mode unchanged. Used by reloadFromBank in the single-capture path. +// leaves the current mode unchanged. Used by reloadInstrument in the single-capture path. // * isExplicit == true -> current (user's choice stands) // * channelCount == 0 -> current (unknown, skip) // * channelCount >= 2 -> Stereo // * channelCount == 1 -> Mono ChannelMode channelModeFor(int channelCount, ChannelMode current, bool isExplicit); +// --- Instance-owned sample references (pS self-contained playback) ------------- +// +// THE ARCHITECTURE CORRECTION: the instrument must never go silent because the extension's +// ext-state has not parsed yet (or the extension is absent). So the instance persists, in +// its OWN component state, a small table of everything it needs to PLAY each referenced +// bank sample: the project-relative WAV path + the decode intrinsics (root note, loop, +// channel count) — exactly a SelectedSample, keyed by the bank sample id. On load the +// shell decodes straight from these refs; the bank blob is a BROWSER SOURCE that also +// refreshes this table opportunistically when readable (recapture/root edits stay live), +// never a runtime lifeline. +// +// POLICY (follows from ownership): a sample deleted from the BANK no longer silences an +// instance that carries its ref — the instance keeps playing while the FILE exists (normal +// sampler behavior; prune deleting the file yields the defined no-play). This deliberately +// supersedes the S10 stale-id-silence rule, which was an artifact of bank-side resolution. +struct PerformanceMap; // defined below (Tier 1); referencedSampleIds spans both tiers + +struct SampleRefEntry { + std::string sampleId; // the bank sample id this ref was copied from (the seam key) + SelectedSample ref; // path + intrinsics, sufficient to decode + play without a bank +}; +using SampleRefs = std::vector; + +// Find the ref for `sampleId` (nullptr on miss). Pointer into `refs` — do not outlive it. +const SelectedSample* findRef(const SampleRefs& refs, const std::string& sampleId); + +// Every bank sample id this instance plays: the selection (when set) + each zone's +// sampleId, de-duplicated, selection first then map order. +std::vector referencedSampleIds(const std::string& selectionId, + const PerformanceMap& map); + +// Upsert a ref for each id in `ids` that resolves in the live bank blob (the same +// distillation selectSample performs). A miss leaves any existing entry untouched — the +// instance owns its copy; a bank deletion never strips a ref. Empty/malformed blob -> no-op. +void refreshRefsFromBank(SampleRefs& refs, const std::string& banksJson, + const std::vector& ids); + +// Keep only the entries whose id is in `ids` (getState hygiene: the persisted table tracks +// exactly what the instance currently plays, so it cannot grow with browsing history). +void retainRefs(SampleRefs& refs, const std::vector& ids); + // One entry in the capture browser's card list: the stable id + display name plus the S2 // intrinsics + bank the browser draws as a card (peak thumbnail + name + root/key badge, // filterable by bank). Peaks are NOT here — they are computed shell-side from the decoded @@ -301,10 +342,17 @@ struct ResolvedPerformance { // (pool + named); a hit yields a ResolvedZone with the effective root note (rootOverride, // else the sample's S2 rootNote, else 60) and the sample's loop intrinsic; a miss appends // the id to droppedSampleIds. Empty/malformed blob or empty map -> empty result (the shell -// then falls back to Tier-0 — see reloadFromBank). +// then falls back to Tier-0 — see reloadInstrument). ResolvedPerformance resolvePerformance(const std::string& banksJson, const PerformanceMap& map); +// Resolve a performance map against the INSTANCE-OWNED refs table (pS self-contained +// playback) — the bank-free mirror of resolvePerformance, sharing the same override- +// beats-intrinsic fold, so the two paths cannot drift. A zone whose sampleId has no ref +// is dropped + reported (same stale-id shape as the bank path). Pure. +ResolvedPerformance resolvePerformanceFromRefs(const SampleRefs& refs, + const PerformanceMap& map); + // Build a zoned Keymap from resolved zones + their decoded mono PCM. `decoded[i]` is the // downmixed frames + sample rate for `zones[i]` (same length + order as `zones`). One // SampleData per zone (Tier 1: one sample per key-region; a sample used by two zones is @@ -459,17 +507,19 @@ PerformanceMap deserializePerformance(const std::vector& bytes, // instance with NO pick and NO zones restores EMPTY (silence + the "pick a capture" empty // state), never auto-playing sample #1. // -// Format (envelope v9): 4-byte LE version tag (== 9), then a 1-byte channel-mode field (0 = mono, +// Format (envelope v10): 4-byte LE version tag (== 10), then a 1-byte channel-mode field (0 = mono, // 1 = stereo), then an 8-byte LE last-consumed-assignment generation (S8/S9 reader marker), then a // 1-byte preview-trigger velocity (S-VIEW-4, MIDI 1..127), then the THREE Phase-S voice-system // bytes: a 1-byte voice count (1..32), a 1-byte voice mode (0 = Poly, 1 = Mono), a 1-byte mono // trigger (0 = Retrigger, 1 = Legato), then the FB1 8-byte LE master-gain LINEAR value (IEEE-754 // double, bit-cast; 0.0 = -inf/silence, 1.0 = unity, cap ~15.849 = +24 dB), then the GA 1-byte // channel-mode-EXPLICIT flag (0 = implicit/auto-default, 1 = the user deliberately toggled the -// mode — see ComponentState::channelModeExplicit), then a 4-byte LE +// mode — see ComponentState::channelModeExplicit), then the pS SAMPLE-REFS table (v10 — the +// instance-owned path + intrinsics per referenced sample; wire shape at +// kSelectionZonesRefsV10Version below), then a 4-byte LE // selection-id length + id bytes, then the CURRENT zones payload (identical to // serializePerformance's body — its own self-describing version, see the ZONES-PAYLOAD block). -// The explicit flag is the ONLY envelope-v9 addition over v8 — the envelope grew a field, +// The refs table is the ONLY envelope-v10 addition over v9 — the envelope grew a field, // the zones payload is untouched (a PARALLEL track owns zone-record extension under its own // versioning; the two version numbers are independent axes — do NOT bump the zones-payload // version for an envelope field). An out-of-range voice byte or a non-finite/out-of-range @@ -477,10 +527,12 @@ PerformanceMap deserializePerformance(const std::vector& bytes, // the instance (the previewVelocity precedent). BACK-COMPAT on read (every older blob lifts to // channelMode = MONO, lastConsumedAssignGeneration = 0, previewVelocity = // kPreviewVelocityDefault, the Phase-S voice defaults {16 voices, Poly, Retrigger}, unity -// master gain, and channelModeExplicit = FALSE — a pre-v9 mode byte is treated as the +// master gain, channelModeExplicit = FALSE — a pre-v9 mode byte is treated as the // un-touched default, so the GA auto-default may follow the loaded capture; a user who HAD -// deliberately chosen a mode re-toggles once and the choice persists explicit from then on): -// * v9 blob -> {channelMode, marker, previewVelocity, voice bytes, masterGainLinear, explicit, selectionId, zones} direct. +// deliberately chosen a mode re-toggles once and the choice persists explicit from then on — +// and an EMPTY sample-refs table, which the shell lifts once via the bridge-resolve path): +// * v10 blob -> {channelMode, marker, previewVelocity, voice bytes, masterGainLinear, explicit, sampleRefs, selectionId, zones} direct. +// * v9 blob -> the v10 fields minus sampleRefs (empty table): pre-pS (bridge-resolve lift). // * v8 blob -> {channelMode, marker, previewVelocity, voice bytes, masterGainLinear, selectionId, zones}: pre-GA (implicit mode). // * v7 blob -> {channelMode, marker, previewVelocity, voiceCount, voiceMode, monoTrigger, selectionId, zones}: pre-FB1 (unity master gain). // * v6 blob -> {channelMode, marker, previewVelocity, selectionId, zones}: pre-Phase-S (voice defaults). @@ -528,9 +580,23 @@ struct ComponentState { // voice, never a keymap fact. Default unity reproduces pre-FB1 output byte-identically, // so an older blob lifting to 1.0 plays exactly as it did. double masterGainLinear = 1.0; + // pS self-contained playback (v10): the instance-OWNED sample refs — path + intrinsics + // for every bank sample this instance plays (see the SampleRefs block above). setState + // decodes straight from these; NO bridge/extension read is required for playback. A + // pre-v10 blob lifts to an EMPTY table, and the shell falls back to the bridge-resolve + // path once (then re-saves self-contained). + SampleRefs sampleRefs; }; -inline constexpr std::uint32_t kComponentStateVersion = 9; +inline constexpr std::uint32_t kComponentStateVersion = 10; + +// The pS self-contained combined-state version (v9 + the instance-owned sample-refs table). +// Wire shape of the refs block (inserted after the v9 explicit flag, before the selection +// id): 4-byte LE entry count, then per entry: 4-byte LE id length + id bytes, 4-byte LE +// path length + path bytes, 4-byte LE rootNote (two's-complement), 1 byte loop.hasLoop, +// 8-byte LE loop.start + 8-byte LE loop.end (two's-complement int64, written regardless of +// hasLoop), 4-byte LE channelCount (two's-complement). +inline constexpr std::uint32_t kSelectionZonesRefsV10Version = 10; // The pre-GA combined-state version (everything through the FB1 master gain, no channel-mode // explicit flag). Retained so deserializeComponentState can lift a v8 blob to implicit mode. diff --git a/tests/test_sample_map.cpp b/tests/test_sample_map.cpp index 74ea740..1912660 100644 --- a/tests/test_sample_map.cpp +++ b/tests/test_sample_map.cpp @@ -1755,7 +1755,7 @@ static void testVelocityCurveEndToEndThroughReloadComposition() { CHECK(rp.zones[0].play.pitchEngine == PitchEngine::Preserve); // 3. Build the zoned keymap from decoded DC-1 PCM and play it through an engine constructed - // the way reloadFromBank constructs it (Preserve voices pre-sized to a real window). + // the way reloadInstrument constructs it (Preserve voices pre-sized to a real window). auto steadyLevelAt = [&](int vel) -> double { DecodedZonePcm pcm; pcm.monoFrames.assign(4000, 1.0f); @@ -2120,6 +2120,230 @@ static void testReconcileBrowseSequenceNoShadowing() { } } +// --- pS self-contained playback: the instance-owned sample refs (envelope v10) --------------- + +static SampleRefEntry refEntry(const std::string& id, const std::string& rel, int root, + bool hasLoop = false, std::int64_t loopStart = 0, + std::int64_t loopEnd = 0, int channels = 0) { + SampleRefEntry e; + e.sampleId = id; + e.ref.relativePath = rel; + e.ref.rootNote = root; + e.ref.loop.hasLoop = hasLoop; + e.ref.loop.start = loopStart; + e.ref.loop.end = loopEnd; + e.ref.channelCount = channels; + return e; +} + +static void testSampleRefsRoundTrip() { + // v10: the owned refs table round-trips — path + every decode intrinsic per entry — + // with the envelope neighbours (selection, zones, explicit flag, gain) intact. + ComponentState s; + s.selectionId = "kick"; + s.channelMode = ChannelMode::Stereo; + s.channelModeExplicit = true; + s.masterGainLinear = 0.5; + s.sampleRefs.push_back(refEntry("kick", "reasampler_bank/kick.wav", 36, + /*hasLoop=*/true, 100, 500, /*channels=*/2)); + s.sampleRefs.push_back(refEntry("pad", "reasampler_bank/pad.wav", 60, + /*hasLoop=*/false, 0, 0, /*channels=*/1)); + s.map.zones.push_back(zone("pad", 48, 72)); + const ComponentState back = deserializeComponentState(serializeComponentState(s), 44100.0); + CHECK(back.sampleRefs.size() == 2); + CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[0].sampleId == "kick"); + CHECK(back.sampleRefs.size() == 2 && + back.sampleRefs[0].ref.relativePath == "reasampler_bank/kick.wav"); + CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[0].ref.rootNote == 36); + CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[0].ref.loop.hasLoop && + back.sampleRefs[0].ref.loop.start == 100 && back.sampleRefs[0].ref.loop.end == 500); + CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[0].ref.channelCount == 2); + CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[1].sampleId == "pad"); + CHECK(back.sampleRefs.size() == 2 && !back.sampleRefs[1].ref.loop.hasLoop); + CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[1].ref.channelCount == 1); + // Envelope neighbours undisturbed (the refs read consumed exactly its own bytes). + CHECK(back.selectionId == "kick"); + CHECK(back.map.zones.size() == 1); + CHECK(back.channelMode == ChannelMode::Stereo && back.channelModeExplicit); + CHECK(std::fabs(back.masterGainLinear - 0.5) < 1e-12); +} + +static void testSampleRefsResolvePlayableKeymapWithoutBank() { + // THE pS architecture correction, end to end in the pure domain: a restored blob + // carrying refs resolves to a PLAYABLE keymap with NO bank blob anywhere in the path — + // deserialize -> resolvePerformanceFromRefs -> buildZonedKeymap. This is the load path + // an instance takes when the extension has not loaded (or does not exist). + ComponentState s; + s.sampleRefs.push_back(refEntry("a", "b/a.wav", 36)); + s.sampleRefs.push_back(refEntry("b", "b/b.wav", 48)); + s.map.zones.push_back(zone("a", 36, 47)); + s.map.zones.push_back(zone("b", 48, 59, /*rootOverride=*/50)); + const ComponentState back = deserializeComponentState(serializeComponentState(s), 44100.0); + const ResolvedPerformance r = resolvePerformanceFromRefs(back.sampleRefs, back.map); + CHECK(r.zones.size() == 2); + CHECK(r.droppedSampleIds.empty()); + CHECK(r.zones.size() == 2 && r.zones[0].relativePath == "b/a.wav"); + CHECK(r.zones.size() == 2 && r.zones[0].rootNote == 36); // ref intrinsic + CHECK(r.zones.size() == 2 && r.zones[1].rootNote == 50); // zone override beats intrinsic + std::vector decoded; + decoded.push_back(DecodedZonePcm{{0.1f, 0.2f}, 44100}); + decoded.push_back(DecodedZonePcm{{0.3f}, 44100}); + const Keymap km = buildZonedKeymap(r.zones, decoded); + CHECK(km.resolve(40, 100).matched && km.resolve(40, 100).zoneIndex == 0); + CHECK(km.resolve(52, 100).matched && km.resolve(52, 100).zoneIndex == 1); +} + +static void testResolveFromRefsMissingRefDrops() { + // MISSING-REF = defined no-play: a zone whose id has no ref (never copied, or a pre-v10 + // blob not yet lifted) drops cleanly + reports; the survivor still plays — the same + // shape as the bank path's stale-id policy. (The shell's missing-FILE no-play is the + // decode seam: an unreadable WAV yields empty PCM and the zone drops in + // buildZonedKeymap — see testBuildZonedKeymapDropsEmptyPcm.) + SampleRefs refs; + refs.push_back(refEntry("a", "b/a.wav", 36)); + PerformanceMap m; + m.zones.push_back(zone("a", 0, 59)); + m.zones.push_back(zone("ghost", 60, 127)); + const ResolvedPerformance r = resolvePerformanceFromRefs(refs, m); + CHECK(r.zones.size() == 1); + CHECK(r.zones.size() == 1 && r.zones[0].relativePath == "b/a.wav"); + CHECK(r.droppedSampleIds.size() == 1); + CHECK(r.droppedSampleIds.size() == 1 && r.droppedSampleIds[0] == "ghost"); +} + +static void testResolveFromRefsMatchesBankResolve() { + // The two resolution paths share ONE fold (foldZone): the same map resolved via the + // bank blob and via a refs table refreshed FROM that bank yields identical effective + // zones — the paths cannot drift. + Sample s1 = makeSample("a", "Pad", "b/a.wav", 40); + s1.loop = LoopPoints{200, 800}; + const std::string json = bookJson({s1}, {}); + PerformanceMap m; + PerformanceZone z = zone("a", 10, 90, /*rootOverride=*/72); + z.startPoint = 512; + m.zones.push_back(z); + SampleRefs refs; + refreshRefsFromBank(refs, json, referencedSampleIds("", m)); + const ResolvedPerformance viaBank = resolvePerformance(json, m); + const ResolvedPerformance viaRefs = resolvePerformanceFromRefs(refs, m); + CHECK(viaBank.zones.size() == 1 && viaRefs.zones.size() == 1); + if (viaBank.zones.size() == 1 && viaRefs.zones.size() == 1) { + CHECK(viaRefs.zones[0].relativePath == viaBank.zones[0].relativePath); + CHECK(viaRefs.zones[0].rootNote == viaBank.zones[0].rootNote); // 72 (override) + CHECK(viaRefs.zones[0].loop.hasLoop == viaBank.zones[0].loop.hasLoop); + CHECK(viaRefs.zones[0].loop.start == viaBank.zones[0].loop.start); // 200 (intrinsic) + CHECK(viaRefs.zones[0].loop.end == viaBank.zones[0].loop.end); + CHECK(viaRefs.zones[0].startFrame == viaBank.zones[0].startFrame); // 512 + } +} + +static void testComponentStateV9LiftsToEmptyRefs() { + // OLD-BLOB FALLBACK: a genuine v9 blob (no refs table) restores with an EMPTY table and + // every other field intact — the shell then lifts via the bridge-resolve path once the + // bank is readable and re-saves self-contained. Hand-built (serializeComponentState now + // emits v10, so it cannot make a v9 blob). + std::vector v9; + v9.push_back(9); v9.push_back(0); v9.push_back(0); v9.push_back(0); // version 9 + v9.push_back(0); // channel mode = mono + for (int i = 0; i < 8; ++i) v9.push_back(0); // marker = 0 + v9.push_back(88); // preview velocity + v9.push_back(7); // voice count + v9.push_back(0); // voice mode = poly + v9.push_back(0); // trigger = retrigger + for (int i = 0; i < 8; ++i) v9.push_back(0); // gain double bytes... + v9[17 + 6] = 0xF0; v9[17 + 7] = 0x3F; // ...= 1.0 (LE IEEE-754) + v9.push_back(1); // explicit flag = true + const std::string id = "saved"; + v9.push_back(static_cast(id.size())); v9.push_back(0); v9.push_back(0); v9.push_back(0); + v9.insert(v9.end(), id.begin(), id.end()); + v9.push_back(0); v9.push_back(0); v9.push_back(0); v9.push_back(0); // zone count 0 + const ComponentState back = deserializeComponentState(v9, 44100.0); + CHECK(back.sampleRefs.empty()); // pre-pS blob -> empty table (bridge-resolve lift) + CHECK(back.selectionId == "saved"); + CHECK(back.channelModeExplicit); + CHECK(back.previewVelocity == 88); + CHECK(back.voiceCount == 7); + CHECK(back.masterGainLinear == 1.0); + CHECK(back.map.zones.empty()); +} + +static void testReferencedSampleIdsDedup() { + // Selection first, then map order, duplicates collapsed; an empty selection contributes + // nothing (no phantom "" id in the refs table). + PerformanceMap m; + m.zones.push_back(zone("a", 0, 59)); + m.zones.push_back(zone("b", 60, 99)); + m.zones.push_back(zone("a", 100, 127)); // duplicate id across zones + const std::vector ids = referencedSampleIds("b", m); // selection dups a zone + CHECK(ids.size() == 2); + CHECK(ids.size() == 2 && ids[0] == "b" && ids[1] == "a"); + const std::vector noSel = referencedSampleIds("", m); + CHECK(noSel.size() == 2); + CHECK(noSel.size() == 2 && noSel[0] == "a" && noSel[1] == "b"); +} + +static void testRefreshRefsFromBankUpsertAndOwnership() { + // Upsert: a resolvable id copies in (the selectSample distillation); a re-refresh after + // a bank edit UPDATES the owned copy (S9 recapture sync); a bank MISS never strips the + // owned entry (a bank deletion cannot silence a self-contained instance); an empty or + // malformed blob is a no-op. + Sample s1 = makeSample("a", "Kick", "b/a.wav", 36); + s1.channelCount = 2; + const std::string json1 = bookJson({s1}, {}); + SampleRefs refs; + refreshRefsFromBank(refs, json1, {"a", "ghost"}); + CHECK(refs.size() == 1); // "ghost" does not resolve -> no entry minted + CHECK(refs.size() == 1 && refs[0].sampleId == "a" && refs[0].ref.rootNote == 36); + CHECK(refs.size() == 1 && refs[0].ref.relativePath == "b/a.wav"); + CHECK(refs.size() == 1 && refs[0].ref.channelCount == 2); + // Recapture-style bank edit: path + root changed -> the owned copy refreshes. + refreshRefsFromBank(refs, bookJson({makeSample("a", "Kick", "b/a2.wav", 40)}, {}), {"a"}); + CHECK(refs.size() == 1 && refs[0].ref.rootNote == 40); + CHECK(refs.size() == 1 && refs[0].ref.relativePath == "b/a2.wav"); + // Bank deletion: the id no longer resolves -> the OWNED copy survives untouched. + refreshRefsFromBank(refs, bookJson({makeSample("x", "Other", "b/x.wav", 60)}, {}), {"a"}); + CHECK(refs.size() == 1 && refs[0].ref.rootNote == 40); + // Malformed / empty blobs: no-op. + refreshRefsFromBank(refs, "{garbage", {"a"}); + refreshRefsFromBank(refs, "", {"a"}); + CHECK(refs.size() == 1 && refs[0].ref.rootNote == 40); +} + +static void testRetainRefsFiltersToPlayedSet() { + // getState hygiene: only the entries the instance currently plays persist — the table + // cannot grow with browsing history. Order of survivors is preserved. + SampleRefs refs; + refs.push_back(refEntry("a", "b/a.wav", 36)); + refs.push_back(refEntry("b", "b/b.wav", 48)); + refs.push_back(refEntry("c", "b/c.wav", 60)); + retainRefs(refs, {"c", "a"}); + CHECK(refs.size() == 2); + CHECK(refs.size() == 2 && refs[0].sampleId == "a" && refs[1].sampleId == "c"); + retainRefs(refs, {}); + CHECK(refs.empty()); +} + +static void testSampleRefsTruncatedMidEntry() { + // A blob cut mid-refs-entry keeps the entries that parsed cleanly and restores the rest + // of the state empty (the selection/zones behind the cut are unreadable anyway) — the + // established truncation posture, never a throw across the host boundary. + ComponentState s; + s.selectionId = "kick"; + s.sampleRefs.push_back(refEntry("kick", "b/k.wav", 36)); + s.sampleRefs.push_back(refEntry("pad", "b/p.wav", 60)); + std::vector bytes = serializeComponentState(s); + // The tail after the refs table is idLen(4) + "kick"(4) + the empty-map zones payload + // (marker 4 + version 4 + count 4) = 20 bytes; entry 2 is 43 bytes (4+3 id, 4+7 path, + // 4 root, 1+8+8 loop, 4 channels). Cutting 40 bytes lands 20 bytes into entry 2. + CHECK(bytes.size() > 40); + bytes.resize(bytes.size() - 40); + const ComponentState back = deserializeComponentState(bytes, 44100.0); + CHECK(back.sampleRefs.size() == 1); + CHECK(back.sampleRefs.size() == 1 && back.sampleRefs[0].sampleId == "kick"); + CHECK(back.selectionId.empty()); + CHECK(back.map.zones.empty()); +} + int main() { testSelectByIdHit(); testSelectEmptyIdIsSilence(); @@ -2245,6 +2469,15 @@ int main() { testReconcileNoOpWhenAlreadyCoherent(); testReconcileGuards(); testReconcileBrowseSequenceNoShadowing(); + testSampleRefsRoundTrip(); + testSampleRefsResolvePlayableKeymapWithoutBank(); + testResolveFromRefsMissingRefDrops(); + testResolveFromRefsMatchesBankResolve(); + testComponentStateV9LiftsToEmptyRefs(); + testReferencedSampleIdsDedup(); + testRefreshRefsFromBankUpsertAndOwnership(); + testRetainRefsFiltersToPlayedSet(); + testSampleRefsTruncatedMidEntry(); if (g_fail == 0) std::printf("sample_map: all tests passed\n"); return g_fail != 0; From cb89dbf5c437887e370a5941d739cf9adcc33254 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Tue, 28 Jul 2026 12:17:36 -0400 Subject: [PATCH 4/4] pS remediation: legacy lift terminates on stale-id proof; displayName in v10 refs; refs-reader range fallbacks; load path keeps owned refs --- src/vst/reasampler_editor.cpp | 30 +++++++++++---- src/vst/reasampler_processor.cpp | 53 ++++++++++++++++++++++---- src/vst/reasampler_processor.h | 24 ++++++++++-- src/vst/sample_map.cpp | 41 +++++++++++++++++--- src/vst/sample_map.h | 37 +++++++++++++++--- tests/test_sample_map.cpp | 64 +++++++++++++++++++++++++++++--- 6 files changed, 213 insertions(+), 36 deletions(-) diff --git a/src/vst/reasampler_editor.cpp b/src/vst/reasampler_editor.cpp index 0a81bb1..a46c222 100644 --- a/src/vst/reasampler_editor.cpp +++ b/src/vst/reasampler_editor.cpp @@ -117,12 +117,19 @@ void drawEnvelope(LICE_IBitmap* bmp, const Rect& r, const Envelope& env) { drawWaveform(bmp, toKitBox(r), env); } -// A display name for a bank sample id from the snapshotted list ("?" if the id no longer -// resolves — e.g. a zone naming a deleted sample). -std::string sampleLabel(const std::vector& samples, const std::string& id) { +// A display name for a bank sample id: the snapshotted bank list first, then the +// instance-OWNED ref's displayName (pS — the label survives with the extension absent / +// bank unreadable, mirroring the waveform + loop-marker ref fallback). "?" only when +// neither source knows the id (a stale zone naming a deleted sample, or a pre-displayName +// refs table not yet back-filled by a bank refresh). +std::string sampleLabel(const std::vector& samples, const SampleRefs& refs, + const std::string& id) { for (const SampleChoice& c : samples) { if (c.id == id) return c.displayName.empty() ? c.id : c.displayName; } + for (const SampleRefEntry& e : refs) { + if (e.sampleId == id && !e.displayName.empty()) return e.displayName; + } return "?"; } @@ -1220,10 +1227,14 @@ void ReaSamplerEditor::paintSample(LICE_IBitmap* bmp, int w, int h) { // signal. std::string title = reasampler::vstPluginName(); // channel-derived (S18) if (processor_ && processor_->bridge().isConnected()) { - if (samples_.empty()) title += " [bank empty]"; - else if (selectedId_.empty() && map_.zones.empty()) title += " [pick a capture]"; - else if (!map_.zones.empty()) title += " [" + std::to_string(map_.zones.size()) + " zone(s)]"; - else title += " [" + sampleLabel(samples_, selectedId_) + "]"; + // The instance's OWN loaded state outranks bank availability (pS: the bank is a + // browser source, not the instrument's identity) — a self-contained instance names + // its sound (refs displayName fallback) even when the bank snapshot is empty. + if (!map_.zones.empty()) title += " [" + std::to_string(map_.zones.size()) + " zone(s)]"; + else if (!selectedId_.empty()) + title += " [" + sampleLabel(samples_, processor_->sampleRefs(), selectedId_) + "]"; + else if (samples_.empty()) title += " [bank empty]"; + else title += " [pick a capture]"; } else { title += " [host: no bridge]"; } @@ -1980,7 +1991,10 @@ void ReaSamplerEditor::paintZone(LICE_IBitmap* bmp, int w, int h) { if (selectedZone_ >= 0 && selectedZone_ < static_cast(map_.zones.size())) { const PerformanceZone& z = map_.zones[static_cast(selectedZone_)]; kitText(bmp, Rect{infoR.left, infoR.top, infoR.left + 120, infoR.bottom}, - sampleLabel(samples_, z.sampleId).c_str(), Font::Label, Role::TextPrimary); + sampleLabel(samples_, processor_ ? processor_->sampleRefs() : SampleRefs{}, + z.sampleId) + .c_str(), + Font::Label, Role::TextPrimary); // Three fields laid out left-to-right after the sample label. A focused field lifts to // the Focus state (accent nudge + ring); values in tabular mono so digits don't jitter. const Rect fields = noteEntryFieldsArea(content); diff --git a/src/vst/reasampler_processor.cpp b/src/vst/reasampler_processor.cpp index 2b0bc69..0a846a8 100644 --- a/src/vst/reasampler_processor.cpp +++ b/src/vst/reasampler_processor.cpp @@ -155,6 +155,16 @@ tresult PLUGIN_API ReaSamplerProcessor::setActive(TBool state) { // Self-contained (pS): this rebuild resolves + decodes from the instance-OWNED // sample refs — it needs no bank read, so it plays regardless of whether the // extension's PROJEXTSTATE has parsed yet (or the extension exists at all). + // + // #B: this unconditional rebuild is ALSO the NON-editor legacy trigger for a + // pre-v10 blob (refs empty + intent): reloadInstrument's opportunistic + // refreshRefsFromBank copies the refs in when the bank blob is readable by + // activation time, so an upgraded project plays on load without the instrument + // ever being opened (and the next save is self-contained). Residual load-order + // race, DAW-verifiable only: if the host activates this instance BEFORE the + // project's ext-state lines parse, the lift misses here and — with no editor open — + // nothing retries until the next activation or editor tick. MIGRATION NOTE: open a + // pre-v10 instrument once after upgrading if it restores silent. reloadInstrument(); } else { std::lock_guard lock(reloadMutex_); @@ -248,6 +258,9 @@ tresult PLUGIN_API ReaSamplerProcessor::setState(IBStream* state) { std::lock_guard lock(refsMutex_); sampleRefs_ = cs.sampleRefs; } + // A new blob is new facts: a staleness proof latched against the PREVIOUS state does + // not carry over (#A — the legacy lift gets one fresh run per restored state). + legacyLiftConcluded_.store(false, std::memory_order_relaxed); // Rebuild from the restored state (off-thread — setState is a load-time call). reloadInstrument(); return kResultOk; @@ -481,9 +494,11 @@ std::string ReaSamplerProcessor::reloadInstrument() { bridge_.readReasamplerExtState(kProjExtBanksKey); std::lock_guard rl(refsMutex_); if (banksJson) refreshRefsFromBank(sampleRefs_, *banksJson, ids); - // Hygiene: the owned table tracks exactly what the instance currently plays, so a - // de-referenced sample's entry drops here (never grows with browsing history). - retainRefs(sampleRefs_, ids); + // The LOAD path never prunes the owned table: dropping entries here on a transient + // bank miss could destroy the owned intrinsics of the previous selection — the ONE + // copy that survives with the extension absent. Entries for de-referenced ids stay + // in memory (bounded by in-session browsing); hygiene lives at the PERSIST boundary, + // where getState filters its snapshot via retainRefs to what the instance plays. refs = sampleRefs_; // snapshot for the decode below (outside the refs lock) } const std::string projectDir = bridge_.activeProjectDir(); @@ -672,6 +687,25 @@ void ReaSamplerProcessor::retireIdleDrain() { graveyard_.end()); } +bool ReaSamplerProcessor::legacyLiftShouldRun() { + // #A terminating guard for the pre-v10 legacy lift. The caller has already established + // refs-empty + intent; this decides whether a lift attempt can MAKE PROGRESS before + // paying for a full reload. Once concluded, the steady state is this one relaxed load — + // no bank read, no parse, no reload churn. + if (legacyLiftConcluded_.load(std::memory_order_relaxed)) return false; + const LegacyLiftDecision decision = legacyLiftDecision( + bridge_.readReasamplerExtState(kProjExtBanksKey), + referencedSampleIds(selectedSampleId(), performanceMap())); + if (decision == LegacyLiftDecision::Stale) { + // Provably stale (the bank parses and knows none of the referenced ids): give up + // PERMANENTLY. A later bank change that re-introduces an id bumps the generation, + // and the genChanged reload refreshes the refs without consulting this latch. + legacyLiftConcluded_.store(true, std::memory_order_relaxed); + return false; + } + return true; // Retry (blob not readable yet) or Lift (a ref can be copied in) +} + ReaSamplerProcessor::BankSyncResult ReaSamplerProcessor::pollBankSync(bool isFocusedTarget) { // OFF THE AUDIO THREAD (the editor's UI timer calls this). Both reads allocate and call @@ -759,13 +793,16 @@ ReaSamplerProcessor::pollBankSync(bool isFocusedTarget) { // editor tick until the lift lands: reloadInstrument folds the bank blob into the refs // when readable, after which the table is non-empty and this never fires again (the // next save is then self-contained). A deliberately-empty instance has no intent and - // never churns; a lift whose bank stays unreadable (or whose id went stale) retries a - // cheap null publish on the editor cadence only. This is a MIGRATION convenience for - // old projects, NOT a playback dependency — a v10 blob plays from its refs with no - // poll at all (pS). + // never churns; a bank that is not readable YET retries a cheap null publish on the + // editor cadence only. TERMINATING GUARD (#A, legacyLiftShouldRun): once the bank blob + // PARSES and no referenced id resolves in it, the ids are provably stale — there is + // nothing to lift, so the lift concludes permanently instead of churning a full bank + // read + reload every tick forever. This is a MIGRATION convenience for old projects, + // NOT a playback dependency — a v10 blob plays from its refs with no poll at all (pS). bool legacyLift = false; if (!genChanged && !result.applied && sampleRefs().empty()) { - legacyLift = !selectedSampleId().empty() || !performanceMap().empty(); + const bool hasIntent = !selectedSampleId().empty() || !performanceMap().empty(); + legacyLift = hasIntent && legacyLiftShouldRun(); } if (genChanged || result.applied || legacyLift) { diff --git a/src/vst/reasampler_processor.h b/src/vst/reasampler_processor.h index 9fe9b22..62b3cc4 100644 --- a/src/vst/reasampler_processor.h +++ b/src/vst/reasampler_processor.h @@ -168,6 +168,9 @@ public: // a non-target instance neither applies nor advances its marker. // * LEGACY LIFT: a pre-v10 blob restored with intent but no refs retries the (cheap) // bank read until the blob is parseable, then reloads ONCE to copy the refs in. + // TERMINATING: once the blob parses and NO referenced id resolves, the ids are + // provably stale — the lift concludes permanently (legacyLiftShouldRun) instead of + // churning a full bank read + reload every tick forever. // The consumed marker advances in component state (marked dirty via the host handler) so a // re-open does not re-apply. `isFocusedTarget` is the shell's thundering-herd policy input // (the editor passes true only for the instance whose editor is open — see the handoff). @@ -286,6 +289,12 @@ private: // the new params bake into the next real reload. Off the audio thread only. void rebuildVoiceEngine(); + // The pre-v10 LEGACY LIFT gate (#A): true when a lift attempt this tick could make + // progress. Latches legacyLiftConcluded_ on a Stale proof (see the member below); the + // pure decision itself is sample_map's legacyLiftDecision. Off the audio thread only + // (bridge read + bank parse). + bool legacyLiftShouldRun(); + // Publish `built` (null = install silence) into live_: prune the graveyard by the last // process()-published generation, swap `built` into live_, displace the previous live into // the drain slot, and park the drain-evicted instrument in the graveyard. REQUIRES @@ -383,11 +392,20 @@ private: // FIRST poll after an editor open BASELINES the seen value without a redundant reload // (setState already loaded the instrument from the OWNED refs); a subsequent generation // CHANGE then drives the reload. Since pS there is NO reopen-heal here: playback never - // depends on this poll — a v10 blob plays from its own refs at setState time. The only - // poll-driven reload besides a generation change is the pre-v10 LEGACY LIFT (see - // pollBankSync). NOT read on the audio thread. + // depends on this poll — a v10 blob plays from its own refs at setState time. Besides a + // generation change, pollBankSync reloads only for an APPLIED S8 assignment and for the + // pre-v10 LEGACY LIFT. NOT read on the audio thread. std::int64_t lastSeenBankGeneration_ = -1; + // The pre-v10 LEGACY LIFT's terminating latch (#A): set once legacyLiftShouldRun proves + // the referenced ids STALE against a readable bank blob (LegacyLiftDecision::Stale) — + // there is nothing to lift, so the lift stops re-firing (the steady state is one relaxed + // load per tick, no bank read). Reset by setState (a new blob = new facts). NOT consulted + // by the genChanged/applied reload paths, so a later bank change that re-introduces an id + // (e.g. an extension-side undo) still refreshes the refs — the latch only gates the lift. + // Atomic: written on the UI-timer thread (pollBankSync) and the host load thread (setState). + std::atomic legacyLiftConcluded_{false}; + // S-VIEW-4 preview-trigger velocity (MIDI 1..127). Persisted in component state (v6) so the // user's chosen strike velocity survives a project save/reload. Since Wave 2 the Sample-view // velocity knob writes it on the UI thread, so it is guarded by previewMutex_; setState and diff --git a/src/vst/sample_map.cpp b/src/vst/sample_map.cpp index 43359bf..f498c04 100644 --- a/src/vst/sample_map.cpp +++ b/src/vst/sample_map.cpp @@ -134,12 +134,32 @@ void refreshRefsFromBank(SampleRefs& refs, const std::string& banksJson, const SelectedSample distilled = distill(*found); bool updated = false; for (SampleRefEntry& e : refs) { - if (e.sampleId == id) { e.ref = distilled; updated = true; break; } + if (e.sampleId == id) { + e.ref = distilled; + e.displayName = found->displayName; // rename sync rides the same refresh + updated = true; + break; + } } - if (!updated) refs.push_back(SampleRefEntry{id, distilled}); + if (!updated) refs.push_back(SampleRefEntry{id, distilled, found->displayName}); } } +LegacyLiftDecision legacyLiftDecision(const std::optional& banksJson, + const std::vector& ids) { + if (!banksJson || banksJson->empty()) return LegacyLiftDecision::Retry; + const std::optional book = BankBook::deserialize(*banksJson); + if (!book) return LegacyLiftDecision::Retry; // present but unparseable: not readable YET + for (const std::string& id : ids) { + for (const Bank& b : book->banks()) { + if (b.index.query(id)) return LegacyLiftDecision::Lift; + } + } + // The blob parses and knows none of the referenced ids (or there are none): provably + // stale — a lift can never make progress against this bank. + return LegacyLiftDecision::Stale; +} + void retainRefs(SampleRefs& refs, const std::vector& ids) { refs.erase(std::remove_if(refs.begin(), refs.end(), [&ids](const SampleRefEntry& e) { @@ -723,7 +743,7 @@ std::vector serializeComponentState(const ComponentState& state) { // table, following the explicit flag so a v9 blob is a strict prefix up to here (see // the v9 lift). Wire shape per kSelectionZonesRefsV10Version: entry count, then per // entry id + path (length-prefixed), rootNote, loop (hasLoop + start/end, always - // written), channelCount. + // written), channelCount, displayName (length-prefixed; display-only). putU32le(out, static_cast(state.sampleRefs.size())); for (const SampleRefEntry& e : state.sampleRefs) { putU32le(out, static_cast(e.sampleId.size())); @@ -736,6 +756,8 @@ std::vector serializeComponentState(const ComponentState& state) { putU64le(out, asU64(e.ref.loop.end)); putU32le(out, static_cast(static_cast(e.ref.channelCount))); + putU32le(out, static_cast(e.displayName.size())); + out.insert(out.end(), e.displayName.begin(), e.displayName.end()); } // Length-prefixed selection id (it precedes the zones payload, so it MUST be framed — // unlike the v1 selection blob where the id ran to end-of-stream). @@ -883,11 +905,20 @@ ComponentState deserializeComponentState(const std::vector& bytes, e.sampleId = r.str(refIdLen); const std::uint32_t pathLen = r.u32(); e.ref.relativePath = r.str(pathLen); - e.ref.rootNote = r.i32(); + // Range fallbacks (the refs table is the ONLY copy on the play path, so a + // corrupt field must degrade to the field's default, never poison playback — + // the previewVelocity/voiceCount posture): an out-of-MIDI-range root falls back + // to the middle-C default distill() uses; a negative channel count falls back + // to 0 = unknown (the GA auto-default then skips it). + const std::int32_t root = r.i32(); + e.ref.rootNote = (root >= 0 && root <= 127) ? root : 60; e.ref.loop.hasLoop = (r.u8() != 0); e.ref.loop.start = r.i64(); e.ref.loop.end = r.i64(); - e.ref.channelCount = r.i32(); + const std::int32_t channels = r.i32(); + e.ref.channelCount = channels >= 0 ? channels : 0; + const std::uint32_t nameLen = r.u32(); + e.displayName = r.str(nameLen); if (!r.ok) break; // truncated mid-entry -> keep what parsed, drop the rest out.sampleRefs.push_back(std::move(e)); } diff --git a/src/vst/sample_map.h b/src/vst/sample_map.h index 4f38901..27344b6 100644 --- a/src/vst/sample_map.h +++ b/src/vst/sample_map.h @@ -90,6 +90,11 @@ struct PerformanceMap; // defined below (Tier 1); referencedSampleIds spans bot struct SampleRefEntry { std::string sampleId; // the bank sample id this ref was copied from (the seam key) SelectedSample ref; // path + intrinsics, sufficient to decode + play without a bank + // The sample's bank display name at copy time — DISPLAY ONLY (the editor's label falls + // back to it when the bank snapshot is unavailable, mirroring the waveform/loop ref + // fallback); never consulted by resolution. Empty for a table written before the field + // existed in-session (it back-fills on the next bank refresh). + std::string displayName; }; using SampleRefs = std::vector; @@ -102,11 +107,26 @@ std::vector referencedSampleIds(const std::string& selectionId, const PerformanceMap& map); // Upsert a ref for each id in `ids` that resolves in the live bank blob (the same -// distillation selectSample performs). A miss leaves any existing entry untouched — the -// instance owns its copy; a bank deletion never strips a ref. Empty/malformed blob -> no-op. +// distillation selectSample performs), copying the bank display name alongside the decode +// intrinsics. A miss leaves any existing entry untouched — the instance owns its copy; a +// bank deletion never strips a ref. Empty/malformed blob -> no-op. void refreshRefsFromBank(SampleRefs& refs, const std::string& banksJson, const std::vector& ids); +// The pre-v10 LEGACY-LIFT terminating decision (pure, so the no-churn rule is provable +// without a host): can a refs lift MAKE PROGRESS against this bank blob for the ids the +// instance references? +// * Retry — the blob is absent/empty/unparseable: not readable YET, keep retrying (the +// project's ext-state may simply not have parsed). +// * Lift — the blob parses and at least one id resolves: a lift copies a ref in (the +// refs table then goes non-empty and the lift never re-fires). +// * Stale — the blob parses and NO id resolves (an empty `ids` included): the ids are +// PROVABLY stale — the bank is readable and does not know them — so there is nothing +// to lift, ever. The shell latches this and stops retrying (no per-tick churn). +enum class LegacyLiftDecision { Retry, Lift, Stale }; +LegacyLiftDecision legacyLiftDecision(const std::optional& banksJson, + const std::vector& ids); + // Keep only the entries whose id is in `ids` (getState hygiene: the persisted table tracks // exactly what the instance currently plays, so it cannot grow with browsing history). void retainRefs(SampleRefs& refs, const std::vector& ids); @@ -341,8 +361,12 @@ struct ResolvedPerformance { // bank_book parse, no host, no PCM. Each zone's sampleId is looked up across every bank // (pool + named); a hit yields a ResolvedZone with the effective root note (rootOverride, // else the sample's S2 rootNote, else 60) and the sample's loop intrinsic; a miss appends -// the id to droppedSampleIds. Empty/malformed blob or empty map -> empty result (the shell -// then falls back to Tier-0 — see reloadInstrument). +// the id to droppedSampleIds. Empty/malformed blob or empty map -> empty result. +// +// NOT the live load path since pS: reloadInstrument resolves via resolvePerformanceFromRefs +// (the instance-owned refs). This bank-side resolver is retained as the TESTED REFERENCE +// the refs path is verified against (testResolveFromRefsMatchesBankResolve) — both share +// foldZone, so the drift test is what keeps the shared fold honest. ResolvedPerformance resolvePerformance(const std::string& banksJson, const PerformanceMap& map); @@ -515,7 +539,7 @@ PerformanceMap deserializePerformance(const std::vector& bytes, // double, bit-cast; 0.0 = -inf/silence, 1.0 = unity, cap ~15.849 = +24 dB), then the GA 1-byte // channel-mode-EXPLICIT flag (0 = implicit/auto-default, 1 = the user deliberately toggled the // mode — see ComponentState::channelModeExplicit), then the pS SAMPLE-REFS table (v10 — the -// instance-owned path + intrinsics per referenced sample; wire shape at +// instance-owned path + intrinsics + display name per referenced sample; wire shape at // kSelectionZonesRefsV10Version below), then a 4-byte LE // selection-id length + id bytes, then the CURRENT zones payload (identical to // serializePerformance's body — its own self-describing version, see the ZONES-PAYLOAD block). @@ -595,7 +619,8 @@ inline constexpr std::uint32_t kComponentStateVersion = 10; // id): 4-byte LE entry count, then per entry: 4-byte LE id length + id bytes, 4-byte LE // path length + path bytes, 4-byte LE rootNote (two's-complement), 1 byte loop.hasLoop, // 8-byte LE loop.start + 8-byte LE loop.end (two's-complement int64, written regardless of -// hasLoop), 4-byte LE channelCount (two's-complement). +// hasLoop), 4-byte LE channelCount (two's-complement), 4-byte LE displayName length + +// displayName bytes (display-only; the editor label's extension-absent fallback). inline constexpr std::uint32_t kSelectionZonesRefsV10Version = 10; // The pre-GA combined-state version (everything through the FB1 master gain, no channel-mode diff --git a/tests/test_sample_map.cpp b/tests/test_sample_map.cpp index 1912660..afba62c 100644 --- a/tests/test_sample_map.cpp +++ b/tests/test_sample_map.cpp @@ -2124,7 +2124,8 @@ static void testReconcileBrowseSequenceNoShadowing() { static SampleRefEntry refEntry(const std::string& id, const std::string& rel, int root, bool hasLoop = false, std::int64_t loopStart = 0, - std::int64_t loopEnd = 0, int channels = 0) { + std::int64_t loopEnd = 0, int channels = 0, + const std::string& name = "") { SampleRefEntry e; e.sampleId = id; e.ref.relativePath = rel; @@ -2133,6 +2134,7 @@ static SampleRefEntry refEntry(const std::string& id, const std::string& rel, in e.ref.loop.start = loopStart; e.ref.loop.end = loopEnd; e.ref.channelCount = channels; + e.displayName = name; return e; } @@ -2145,7 +2147,8 @@ static void testSampleRefsRoundTrip() { s.channelModeExplicit = true; s.masterGainLinear = 0.5; s.sampleRefs.push_back(refEntry("kick", "reasampler_bank/kick.wav", 36, - /*hasLoop=*/true, 100, 500, /*channels=*/2)); + /*hasLoop=*/true, 100, 500, /*channels=*/2, + /*name=*/"Kick Drum")); s.sampleRefs.push_back(refEntry("pad", "reasampler_bank/pad.wav", 60, /*hasLoop=*/false, 0, 0, /*channels=*/1)); s.map.zones.push_back(zone("pad", 48, 72)); @@ -2158,9 +2161,11 @@ static void testSampleRefsRoundTrip() { CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[0].ref.loop.hasLoop && back.sampleRefs[0].ref.loop.start == 100 && back.sampleRefs[0].ref.loop.end == 500); CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[0].ref.channelCount == 2); + CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[0].displayName == "Kick Drum"); CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[1].sampleId == "pad"); CHECK(back.sampleRefs.size() == 2 && !back.sampleRefs[1].ref.loop.hasLoop); CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[1].ref.channelCount == 1); + CHECK(back.sampleRefs.size() == 2 && back.sampleRefs[1].displayName.empty()); // Envelope neighbours undisturbed (the refs read consumed exactly its own bytes). CHECK(back.selectionId == "kick"); CHECK(back.map.zones.size() == 1); @@ -2296,13 +2301,16 @@ static void testRefreshRefsFromBankUpsertAndOwnership() { CHECK(refs.size() == 1 && refs[0].sampleId == "a" && refs[0].ref.rootNote == 36); CHECK(refs.size() == 1 && refs[0].ref.relativePath == "b/a.wav"); CHECK(refs.size() == 1 && refs[0].ref.channelCount == 2); - // Recapture-style bank edit: path + root changed -> the owned copy refreshes. - refreshRefsFromBank(refs, bookJson({makeSample("a", "Kick", "b/a2.wav", 40)}, {}), {"a"}); + CHECK(refs.size() == 1 && refs[0].displayName == "Kick"); // name copied with the ref + // Recapture-style bank edit: path + root + name changed -> the owned copy refreshes. + refreshRefsFromBank(refs, bookJson({makeSample("a", "Kick 2", "b/a2.wav", 40)}, {}), {"a"}); CHECK(refs.size() == 1 && refs[0].ref.rootNote == 40); CHECK(refs.size() == 1 && refs[0].ref.relativePath == "b/a2.wav"); + CHECK(refs.size() == 1 && refs[0].displayName == "Kick 2"); // rename sync // Bank deletion: the id no longer resolves -> the OWNED copy survives untouched. refreshRefsFromBank(refs, bookJson({makeSample("x", "Other", "b/x.wav", 60)}, {}), {"a"}); CHECK(refs.size() == 1 && refs[0].ref.rootNote == 40); + CHECK(refs.size() == 1 && refs[0].displayName == "Kick 2"); // Malformed / empty blobs: no-op. refreshRefsFromBank(refs, "{garbage", {"a"}); refreshRefsFromBank(refs, "", {"a"}); @@ -2333,8 +2341,9 @@ static void testSampleRefsTruncatedMidEntry() { s.sampleRefs.push_back(refEntry("pad", "b/p.wav", 60)); std::vector bytes = serializeComponentState(s); // The tail after the refs table is idLen(4) + "kick"(4) + the empty-map zones payload - // (marker 4 + version 4 + count 4) = 20 bytes; entry 2 is 43 bytes (4+3 id, 4+7 path, - // 4 root, 1+8+8 loop, 4 channels). Cutting 40 bytes lands 20 bytes into entry 2. + // (marker 4 + version 4 + count 4) = 20 bytes; entry 2 is 47 bytes (4+3 id, 4+7 path, + // 4 root, 1+8+8 loop, 4 channels, 4+0 name). Cutting 40 bytes lands 27 bytes into + // entry 2 (inside loop.start). CHECK(bytes.size() > 40); bytes.resize(bytes.size() - 40); const ComponentState back = deserializeComponentState(bytes, 44100.0); @@ -2344,6 +2353,47 @@ static void testSampleRefsTruncatedMidEntry() { CHECK(back.map.zones.empty()); } +static void testSampleRefsReaderRangeFallbacks() { + // Corrupt-blob posture for the refs intrinsics (the refs table is the ONLY copy on the + // play path, so a bad field must degrade to its default, never poison playback): an + // out-of-MIDI-range rootNote falls back to the middle-C default distill() uses; a + // negative channelCount falls back to 0 = unknown (the GA auto-default then skips it). + // The fallback is per-field — in-range neighbours pass through untouched. + ComponentState s; + s.sampleRefs.push_back(refEntry("hi", "b/h.wav", /*root=*/999, false, 0, 0, + /*channels=*/-3)); + s.sampleRefs.push_back(refEntry("lo", "b/l.wav", /*root=*/-5, false, 0, 0, + /*channels=*/1)); + s.sampleRefs.push_back(refEntry("ok", "b/o.wav", /*root=*/36, false, 0, 0, + /*channels=*/2)); + const ComponentState back = deserializeComponentState(serializeComponentState(s), 44100.0); + CHECK(back.sampleRefs.size() == 3); + CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[0].ref.rootNote == 60); + CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[0].ref.channelCount == 0); + CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[1].ref.rootNote == 60); + CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[1].ref.channelCount == 1); + CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[2].ref.rootNote == 36); + CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[2].ref.channelCount == 2); +} + +static void testLegacyLiftDecision() { + // The #A terminating guard, pure: Retry while the blob is not readable YET (absent, + // empty, malformed — the project's ext-state may simply not have parsed); Lift when a + // referenced id resolves (a lift attempt makes progress); Stale — the shell latches + // permanently — when the blob PARSES and knows none of the referenced ids (an empty + // id list included), so a stale-id pre-v10 lift STOPS instead of churning every tick. + const std::string json = bookJson({makeSample("a", "Kick", "b/a.wav", 36)}, {}); + const std::vector ids{"a"}; + CHECK(legacyLiftDecision(std::nullopt, ids) == LegacyLiftDecision::Retry); + CHECK(legacyLiftDecision(std::string(), ids) == LegacyLiftDecision::Retry); + CHECK(legacyLiftDecision(std::string("{garbage"), ids) == LegacyLiftDecision::Retry); + CHECK(legacyLiftDecision(json, ids) == LegacyLiftDecision::Lift); + // One resolvable id among stale ones is still progress (the lift copies what it can). + CHECK(legacyLiftDecision(json, {"ghost", "a"}) == LegacyLiftDecision::Lift); + CHECK(legacyLiftDecision(json, {"ghost"}) == LegacyLiftDecision::Stale); + CHECK(legacyLiftDecision(json, {}) == LegacyLiftDecision::Stale); +} + int main() { testSelectByIdHit(); testSelectEmptyIdIsSilence(); @@ -2478,6 +2528,8 @@ int main() { testRefreshRefsFromBankUpsertAndOwnership(); testRetainRefsFiltersToPlayedSet(); testSampleRefsTruncatedMidEntry(); + testSampleRefsReaderRangeFallbacks(); + testLegacyLiftDecision(); if (g_fail == 0) std::printf("sample_map: all tests passed\n"); return g_fail != 0;