diff --git a/src/shell/instrument/CLAUDE.md b/src/shell/instrument/CLAUDE.md index ac2ae49..bdef065 100644 --- a/src/shell/instrument/CLAUDE.md +++ b/src/shell/instrument/CLAUDE.md @@ -107,7 +107,7 @@ declared ahead of the instrument slots at that member in `reasampler_processor.h ## Modules - `reaper_bridge` — READ-ONLY bank consumer: receives bank snapshots from the extension and exposes them as a read-only view. **Never writes to the extension's bank** — this is a load-bearing invariant; no mutation path exists in this module. It owns TWO prefix-guarded ext-state write entry points, `writeUsageExtState` (`rsusage_`) and `writeBakeExtState` (`rsbake_`), each refusing every other key; neither weakens the read-only-*bank* invariant, because neither payload is bank state and `banks`/`view`/`tail`/`assign` stay structurally unwritable. Both PROVE the write by reading the key back (`wire::extStateWriteLanded`) — `SetProjExtState`'s own return cannot speak for one key, so testing it was a guard that could never fire, and the bake's "could not publish" refusal was consequently unreachable. It also owns the bake crossing — `extensionActionAvailable` / `invokeExtensionAction` (`NamedCommandLookup` + `Main_OnCommandEx` with `getReaperParent(3)`, the instance's OWN project tab, as `proj` — a request, not a DAW-verified guarantee; see the header) and `projectTempoBpm`. -- `reasampler_processor` (`shell/instrument/`: `reasampler_processor.cpp` lifecycle + `process()`, `processor_state.cpp` component-state I/O + UI-thread parameter accessors, `processor_reload.cpp` the off-audio-thread `reloadInstrument`/publish family — Q-W2v, T4-12 split; `process()` and its per-block work stay ONE TU on purpose, no cross-TU call on the per-sample path) — VST3 `SingleComponentEffect` shell: declares event-input bus + **permanently stereo** output (GA fix: dynamic mono↔stereo bus renegotiation deleted; `ChannelMode` is now decode-only), marshals MIDI note-on/off into the VoiceEngine, renders audio; owns off-audio-thread `reloadInstrument` + atomic pointer swap so `process()` does no allocation, no file I/O, no bridge calls. The instance state is `{loaded capture id, one InstrumentParams}`, and `reloadInstrument` resolves + decodes exactly that one capture into the `SampleData` the engine plays. **Self-contained playback (pS):** `ComponentState` v10 adds a `SampleRefs` table — per referenced sample, a project-relative path + decode intrinsics (root, loop, channels, displayName); `reloadInstrument` decodes directly from `SampleRefs`, bank-free (plays with the extension absent). The bank/bridge is a browser source: loading a capture copies its reference in; the reopen-heal timer + poll-to-play apparatus are removed. `retireIdleDrain()` retires fully-idle drain snapshots on the UI-timer cadence. Voice-param edits (`setVoiceCount`/`setVoiceMode`/`setMonoTrigger`) rebuild the engine from the already-decoded `SampleData` via the drain-slot swap — no bank re-read, no WAV re-decode, no audible cut to ringing tails. **FB1:** applies the post-mixer `masterGainLinear` (from `ComponentState` v8) as a per-sample ramp over the summed output — no zipper noise. **GA v9:** `channelModeExplicit_` flag persisted; `channelModeFor()` auto-defaults the mode from the loaded capture's channel count when the flag is not set. **pS:** `ComponentState` bumped v9→v10 (`SampleRefs` table); pre-v10 blobs lift to empty refs and re-save self-contained. **pS-usage:** publishes instance usage (held `SampleRefs` paths) to `rsusage_` at the tail of `reloadInstrument` (off audio thread) via `reaper_bridge::writeUsageExtState`; `ComponentState` bumped v10→**v11** (`instanceGuid` field); pre-v11 blobs mint guid on first publish. **The master bus:** the summed output runs `voice mixer → master gain → limiter (core/instrument/engine/limiter) → output bus`, with the meter tapped at the bus output POST-limiter and published as relaxed atomics — per-channel peak and smallest limiter gain ACCUMULATED across every block since the UI last read, plus the latched clip (the meter Gotcha below owns why). The limiter's enable is persisted in the parameter set (params payload v15) and mirrored onto the audio thread by `setInstrumentParams`, the single funnel every writer already goes through. That mirror is also what `getLatencySamples()` answers from — the plugin's FIRST latency reporting: 0 bypassed, the lookahead engaged. `setLimiterEnabled` requests the host's `restartComponent(kLatencyChanged)`, UI thread only and never from `process()`; it is a LATENCY restart with the bus untouched, NOT the retired per-mode `kIoChanged` bus renegotiation the invariant above forbids. +- `reasampler_processor` (`shell/instrument/`: `reasampler_processor.cpp` lifecycle + `process()`, `processor_state.cpp` component-state I/O + UI-thread parameter accessors, `processor_reload.cpp` the off-audio-thread `reloadInstrument`/publish family — Q-W2v, T4-12 split; `process()` and its per-block work stay ONE TU on purpose, no cross-TU call on the per-sample path) — VST3 `SingleComponentEffect` shell: declares event-input bus + **permanently stereo** output (GA fix: dynamic mono↔stereo bus renegotiation deleted; `ChannelMode` is now decode-only), marshals MIDI note-on/off into the VoiceEngine, renders audio; owns off-audio-thread `reloadInstrument` + atomic pointer swap so `process()` does no allocation, no file I/O, no bridge calls. The instance state is `{loaded capture id, one InstrumentParams}`, and `reloadInstrument` resolves + decodes exactly that one capture into the `SampleData` the engine plays. **Self-contained playback (pS):** `ComponentState` v10 adds a `SampleRefs` table — per referenced sample, a project-relative path + decode intrinsics (root, loop, channels, displayName); `reloadInstrument` decodes directly from `SampleRefs`, bank-free (plays with the extension absent). The bank/bridge is a browser source: loading a capture copies its reference in; the reopen-heal timer + poll-to-play apparatus are removed. `retireIdleDrain()` retires fully-idle drain snapshots on the UI-timer cadence. Voice-param edits (`setVoiceCount`/`setVoiceMode`/`setMonoTrigger`) rebuild the engine from the already-decoded `SampleData` via the drain-slot swap — no bank re-read, no WAV re-decode, no audible cut to ringing tails. **FB1:** applies the post-mixer `masterGainLinear` (from `ComponentState` v8) as a per-sample ramp over the summed output — no zipper noise. **GA v9:** `channelModeExplicit_` flag persisted; `channelModeFor()` auto-defaults the mode from the loaded capture's channel count when the flag is not set. **pS:** `ComponentState` bumped v9→v10 (`SampleRefs` table); pre-v10 blobs lift to empty refs and re-save self-contained. **pS-usage:** publishes instance usage (held `SampleRefs` paths) to `rsusage_` at the tail of `reloadInstrument` (off audio thread) via `reaper_bridge::writeUsageExtState`; `ComponentState` bumped v10→**v11** (`instanceGuid` field); pre-v11 blobs mint guid on first publish. **The master bus:** the summed output runs `voice mixer → master gain → limiter (core/instrument/engine/limiter) → output bus`, with the meter tapped at the bus output POST-limiter and published as relaxed atomics — per-channel peak and smallest limiter gain ACCUMULATED across every block since the UI last read, plus the latched clip (the meter Gotcha below owns why). The limiter's enable is persisted in the parameter set (params payload v15) and mirrored onto the audio thread by `setInstrumentParams`, the single funnel every writer already goes through. That mirror is also what `getLatencySamples()` answers from — the plugin's FIRST latency reporting: 0 bypassed, the lookahead engaged. The host's `restartComponent(kLatencyChanged)` is issued by `flushLatencyRestart` alone, UI thread only and never from `process()`; it is a LATENCY restart with the bus untouched, NOT the retired per-mode `kIoChanged` bus renegotiation the invariant above forbids. Why it is split from the commit is the Gotcha below. - `reasampler_editor` — VST3 `IPlugView` LICE editor shell: hosts a LICE-drawn child window; the Sample face is home and Browse is a modal picker over it. Split on the Sample face's BAND axis, mirroring the pure `sample_bands` allocator: `editor_session` (session/bridge state, caches, commit-and-reload), `editor_controls` (the ONE `faceLayout` band resolve every paint and hit-test path shares, the node-drag bounds, the value labels, and the per-instance controls the parameter set does not carry — the parameter-set binding itself is the pure `core/instrument/ui/deck_values` module this only adapts int ids onto), `editor_models` (the orthogonal half: which stored struct each transient editor selection names — the staged-envelope pack/unpack, the drawn contour, and the three velocity curves), then matching paint and input sets — `editor_paint`/`editor_input` (dispatch + drag router + hover dispatch), `_chrome`, `_waveform`, `_deck` — plus the two band-independent surfaces (`_browse` for the modal picker, `_curve` for the velocity-curve popup) and `editor_platform` (IPlugView/Win32 window plumbing). Shared internals in `editor_internal.h`, no TU of its own. Drop-onto-editor ingest is NOT shipped (deferred). - `reasampler_embed` — implements `IReaperUIEmbedInterface` so the instrument draws inline in the TCP/MCP without a plugin-owned HWND; delegates layout to `embed_strip`. A read-only readout: the loaded capture across the keyboard span with its root marked, plus the activity level. It takes no mouse input (there is nothing on the strip to select). - `editor_stroke` — the editor's LICE side of the analytic stroker: builds a coverage mask with the pure `core/ui/stroke_aa` and blends it into the bitmap ONCE, writing straight to the bitmap's bits (the arithmetic matches LICE's own mode-0 combine, so a stroke composites identically to every other kit draw). Every radial and spline stroke on the editor routes through `strokeArcAA` / `strokePolylineAA` / `strokeLineAA`. Holds the draw-thread-only scratch mask and arc point list — reuse, not a hidden dependency: threading a canvas through the eight paint sites would grow those signatures to carry an allocation detail. Deliberately does NOT touch `shell/panel/draw_kit`: the waveform stroke, the docked bank panel and the browse cards are out of this seam's blast radius. @@ -125,16 +125,22 @@ declared ahead of the instrument slots at that member in `reasampler_processor.h the very instance whose frame is on the stack. Deferring by one tick is same-thread and in-instance — it is NOT a cross-process poller/nonce handshake, and it must not grow into one. -- **The limiter toggle arms on the same principle, and for the same reason.** Its commit - requests the host's `restartComponent(kLatencyChanged)`; a host that services that +- **The limiter toggle splits its commit: the sound is inline, the HOST NOTIFICATION arms.** + Its commit needs the host's `restartComponent(kLatencyChanged)`; a host that services that synchronously runs `setActive(false)`/`setActive(true)`, and OUR `setActive(true)` calls - `reloadInstrument()` — a WAV re-decode plus disk I/O. Inline from `WM_LBUTTONDOWN` that - whole cycle runs with `SetCapture` held. The click writes the editor's own snapshot and - paints at once; the sync tick calls `setLimiterEnabled`, so **the audio and the reported - latency follow the click by up to one tick.** It sits AFTER the drag guard with the bake: - the restart rebuilds the instance, which mid-drag would yank the edit surface exactly as a - reload would. Every other writer of the parameter set (`setState`, the bake's adopt) still - commits and restarts immediately — none of them is inside a mouse handler. + `reloadInstrument()` — a WAV re-decode plus disk I/O, which inline from `WM_LBUTTONDOWN` + would run nested in a mouse handler. So the click commits the parameter set, the audio-thread + mirror and the latency reader at once, and `setInstrumentParams` only ARMS a pending restart + that `flushLatencyRestart` delivers. The editor's sync tick is the general drain and sits + AFTER the drag guard with the bake (the restart rebuilds the instance, which mid-drag would + yank the edit surface exactly as a reload would); `setState` and the bake's adopt flush at + their own tails, because they can commit with no editor open. The arm is a sticky bool, so + toggling twice inside one tick still costs exactly one restart. + **The residual:** between the commit and the flush the host's delay compensation is out of + step with the plugin by `limiterLookaheadSamples` (2 ms — `round(0.002 · rate)`, the + detector's 4-sample group delay INSIDE that budget, not on top), bounded by one 500 ms tick. + Narrowing it further means a second deferral mechanism (a posted window message) rather than + the tick — deliberately not built. - **The MASTER meter's ballistics ride the sync tick, and that tick is 500 ms.** They run BEFORE the tick's in-flight-drag guard on purpose — a drag suppresses the reload poll, but the bus keeps sounding. Elapsed time is measured (`GetTickCount64`), never assumed from the diff --git a/src/shell/instrument/editor_input_deck.cpp b/src/shell/instrument/editor_input_deck.cpp index 57f6993..009a50a 100644 --- a/src/shell/instrument/editor_input_deck.cpp +++ b/src/shell/instrument/editor_input_deck.cpp @@ -68,12 +68,11 @@ bool ReaSamplerEditor::mouseDownDeck(const FaceLayout& fl, int x, int y) { const bool on = (hit.segment == 1); if (on != params_.limiterEnabled) { params_.limiterEnabled = on; - // ARMED here, run on the sync tick — the same treatment the bake gets, and - // for the same reason: the processor's funnel requests the host's latency - // restart, whose deactivate/reactivate calls setActive(true) and re-decodes - // the WAV. Inline, that whole cycle would run nested inside this mouse - // handler with SetCapture held. - limiterPending_ = on; + // Commits the audible state and the persisted state together, here, because + // this is a control the user A/Bs. The funnel only ARMS the host's latency + // restart — the sync tick delivers it — so nothing on this path calls into + // the host from inside a mouse handler. + processor_->setLimiterEnabled(on); } invalidate(); break; diff --git a/src/shell/instrument/editor_session.cpp b/src/shell/instrument/editor_session.cpp index 9ec7c7d..5b543fa 100644 --- a/src/shell/instrument/editor_session.cpp +++ b/src/shell/instrument/editor_session.cpp @@ -128,14 +128,12 @@ void ReaSamplerEditor::onSyncTimer() { if (drag_ != DragKind::kNone) return; // defer past the in-flight edit - // The limiter click armed it; this is where it runs. Past the drag guard with the bake, - // because the restart it requests makes the host rebuild this instance — mid-drag that - // would yank the edit surface exactly as a reload would. - if (limiterPending_) { - const bool on = *limiterPending_; - limiterPending_.reset(); - processor_->setLimiterEnabled(on); - } + // A parameter commit that flipped the limiter already changed the sound; what waits for this + // tick is only telling the host to re-ask for the latency. Past the drag guard with the bake, + // because the restart makes the host rebuild this instance — mid-drag that would yank the + // edit surface exactly as a reload would. Unconditional: it self-cancels when nothing is + // armed, so no commit site has to remember to ask for it. + processor_->flushLatencyRestart(); // Resolve the bake affordance's availability on the SAME tick that paints it, so it // can never be enabled on one tick and refuse on the next. diff --git a/src/shell/instrument/processor_reload.cpp b/src/shell/instrument/processor_reload.cpp index eb5b769..d032f0d 100644 --- a/src/shell/instrument/processor_reload.cpp +++ b/src/shell/instrument/processor_reload.cpp @@ -246,6 +246,10 @@ void ReaSamplerProcessor::adoptBakedCapture(const SampleRefEntry& entry, // ONE reload for the re-point and the reset together: it decodes the new file and // publishes the neutral parameters in the same swap. reloadInstrument(); + // The bake's reset may have flipped the limiter; deliver the host's latency restart here + // rather than leaving it to the editor's tick, so an adopt is correct with no editor open. + // At the tail for the same reason setState's is (see there). + flushLatencyRestart(); } void ReaSamplerProcessor::publishUsage(const SampleRefs& refs, diff --git a/src/shell/instrument/processor_state.cpp b/src/shell/instrument/processor_state.cpp index 33ea8ab..784a258 100644 --- a/src/shell/instrument/processor_state.cpp +++ b/src/shell/instrument/processor_state.cpp @@ -86,6 +86,10 @@ tresult PLUGIN_API ReaSamplerProcessor::setState(IBStream* state) { // A new blob is new facts — the legacy lift gets one fresh run per restored state. legacyLiftConcluded_.store(false, std::memory_order_relaxed); reloadInstrument(); + // This caller has no editor to flush for it. At the TAIL on purpose: a host that services the + // restart synchronously deactivates/reactivates, and our setActive(true) reloads — from the + // refs above, which are only fully restored once this function has run to here. + flushLatencyRestart(); return kResultOk; } @@ -157,27 +161,40 @@ void ReaSamplerProcessor::setInstrumentParams(const InstrumentParams& params) { } // Every writer of the parameter set — setState, the editor's commits, the bake's adopt — // funnels through here, so mirroring the limiter flag at this one point is what keeps the - // audio thread's copy and the latency report from ever lagging what is persisted, and - // requesting the restart here (not just from setLimiterEnabled) is what keeps the host's - // PDC from lagging it too. Coalesced: writing the value already held requests nothing. + // audio thread's copy and the latency report from ever lagging what is persisted. The MIRROR + // is inline, because that is the sound the user clicked for; the host notification is not, + // because this funnel is reachable from inside a mouse handler and restartComponent is not + // safe there (see this directory's CLAUDE.md). publishLimiterEnabled(params.limiterEnabled); - if (limiterFlagChanged && componentHandler) { - // The SDK requires this on the UI thread and answers getLatencySamples only after the - // host's own deactivate/reactivate — so the flag above is already committed by the time - // the host asks. This is a kLatencyChanged restart with the bus untouched, NOT the - // retired per-mode kIoChanged bus renegotiation (see initialize()); do not conflate. - componentHandler->restartComponent(kLatencyChanged); + // Armed AFTER the mirror, so getLatencySamples already answers the new value for the whole + // window the arm stays outstanding. Sticky and idempotent: any number of changes before one + // flush cost one restart, and the flush is the only thing that clears it. + if (limiterFlagChanged) { + latencyRestartPending_.store(true, std::memory_order_release); } } +void ReaSamplerProcessor::flushLatencyRestart() { + // Cleared only once it can actually be delivered — an arm raised before the host connected + // its handler waits for a later flush instead of evaporating. + if (!componentHandler) return; + if (!latencyRestartPending_.exchange(false, std::memory_order_acquire)) return; + // The SDK requires this on the UI thread and answers getLatencySamples only after the host's + // own deactivate/reactivate — so the flag is long committed by the time the host asks. This + // is a kLatencyChanged restart with the bus untouched, NOT the retired per-mode kIoChanged + // bus renegotiation (see initialize()); do not conflate. + componentHandler->restartComponent(kLatencyChanged); +} + void ReaSamplerProcessor::publishLimiterEnabled(bool on) { limiterEnabled_.store(on, std::memory_order_relaxed); limiter_.setEnabled(on); } void ReaSamplerProcessor::setLimiterEnabled(bool on) { - // Thin wrapper: setInstrumentParams is the one funnel that mirrors the flag AND requests - // the restart, so every writer of the parameter set — this one included — agrees. + // Rebased off the PROCESSOR's copy rather than taking a caller-supplied set: an editor + // snapshot may carry edits it has not committed, and writing one back here would clobber + // them. Everything else is setInstrumentParams', the one funnel every writer agrees through. InstrumentParams params = instrumentParams(); params.limiterEnabled = on; setInstrumentParams(params); diff --git a/src/shell/instrument/reasampler_editor.h b/src/shell/instrument/reasampler_editor.h index b6ca274..f67fc69 100644 --- a/src/shell/instrument/reasampler_editor.h +++ b/src/shell/instrument/reasampler_editor.h @@ -446,11 +446,6 @@ private: std::string bakeMessage_; // last outcome, shown in the title band int bakeMessageTicks_ = 0; // sync ticks the message survives - // The limiter toggle, armed by the click and run on the sync tick — the commit requests a - // host latency restart, which is the same nested-inside-a-mouse-handler hazard the bake - // defers for. Empty = nothing armed. - std::optional limiterPending_; - // The editor-drop -> extension-ingest relay is not shipped (the bridge is read-only): // an OS drop just flashes a "drop on the panel instead" banner (dropHintTicks_ counts // down via the sync tick). Never ingests, never inserts a timeline item. diff --git a/src/shell/instrument/reasampler_processor.h b/src/shell/instrument/reasampler_processor.h index 4e1f65b..d6f93fd 100644 --- a/src/shell/instrument/reasampler_processor.h +++ b/src/shell/instrument/reasampler_processor.h @@ -236,15 +236,23 @@ public: void setMasterGainLinear(double linear); // clamped to [0, masterGainMaxLinear()] // The master-bus limiter's single enable (persisted in the parameter set). UI thread only: - // a thin wrapper over setInstrumentParams, the one funnel that both mirrors the flag and - // requests the host's kLatencyChanged restart, which the SDK requires be issued from the UI - // thread and which process() must therefore never trigger. Setting the value it already - // holds is a no-op, so repeated clicks on one segment cost no restart. + // a thin wrapper over setInstrumentParams, the one funnel that mirrors the flag onto the + // audio thread INLINE — the sound follows the click — and only ARMS the host's latency + // restart. Setting the value it already holds is a no-op, so repeated clicks on one segment + // cost no restart. bool limiterEnabled() const { return limiterEnabled_.load(std::memory_order_relaxed); } void setLimiterEnabled(bool on); + // Delivers the armed kLatencyChanged restart, at most once per armed window, and does + // nothing when none is armed. Split off the commit because the SDK requires this on the UI + // thread AND a host may service it synchronously — deactivate/reactivate, which reaches our + // setActive(true) and its reloadInstrument — so it must never run nested inside a mouse + // handler. The editor's sync tick is the general drain; the two callers that can commit with + // no editor open (setState, adoptBakedCapture) flush themselves at their own tails. + void flushLatencyRestart(); + // Fires a one-shot preview note-on/off through the live VoiceEngine — the same // noteOn/noteOff host MIDI uses, so a preview is a real voice (counts against voice // count, can steal/be stolen, respects Poly/Mono + Retrigger/Legato). Off the audio @@ -471,6 +479,11 @@ private: // getLatencySamples answers from. instrument::engine::Limiter limiter_; std::atomic limiterEnabled_{false}; + // Set by the commit funnel when the enable actually changed, cleared only by + // flushLatencyRestart. A sticky bool and not a count on purpose: the host is being told to + // re-ASK, so N changes before one flush need exactly one restart, and whatever + // getLatencySamples answers at that moment is the truth being announced. + std::atomic latencyRestartPending_{false}; // What the audio thread publishes about the output bus each block, relaxed. The peaks and // minGain ACCUMULATE (max / min) across every block since the UI last read, and