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();