Bound the automation hold to the window the model has not caught up on, and make that authority model stated, enforced and tested

This commit is contained in:
2026-08-02 17:16:02 -04:00
parent de5654fb6f
commit 1fd38bbd57
35 changed files with 1155 additions and 302 deletions
+27 -7
View File
@@ -1270,7 +1270,13 @@ value semantics, any deck geometry, or the bake's reset *membership* (W3-T2's).
`stepCount` is asserted **zero on all 44**.
- **A host automation lane moving a Live parameter moves a sounding note; a lane moving a
NoteOnLatched parameter takes effect on the next note and does NOT trigger a reload or an
engine rebuild** — assert the tier, not just the sound.
engine rebuild** — assert the tier, not just the sound. All three NoteOnLatched controls now
have a delivery test in `tests/test_live_delivery.cpp` (rate, key-track, Trigger length), each
asserting BOTH halves: the sounding note byte-identical, the next note taking the value.
- **A host automation point's AUTHORITY IS BOUNDED**, and its release is where both review passes
went wrong: it outranks the model only until the UI thread has folded it in and republished.
`shell/instrument/CLAUDE.md`'s Authority section states the model, `param_merge` enforces it,
`test_param_merge` fails if a hold is never released or is released too early.
- **No automation path reaches `reloadInstrument` or `rebuildVoiceEngine`.**
- **A project saved before this change opens with every parameter reading the blob's value
and sounds identical**; a project saved by this build opens in an older binary with its
@@ -1309,10 +1315,23 @@ value semantics, any deck geometry, or the bake's reset *membership* (W3-T2's).
`ProcessData::inputParameterChanges` in `process()` while also implementing
`setParamNormalized`. **Both are serviced.** This was never a DAW question — the headers
answer it, and building on the paragraph alone is exactly what the first pass did.
(b) The ORDERING of `setState` against the first parameter block: no longer a question. An
automation point held by the audio thread is re-applied over every merge, so a written lane
outranks the restore whichever way round the two arrive — VST3's own rule. What survives as
DAW work is recorded in `docs/TODO.md`, and none of it can change the frozen contract.
(b) The ORDERING of `setState` against the first parameter block: no longer a question, but not
for the reason the second pass gave. A point held by the audio thread is re-applied over every
merge only until the UI thread has folded it into the model — the hold is bounded, and a lane
that is genuinely DRIVING re-sends and so keeps outranking the restore, while a lane that sent
one point and had it folded does not. That is the correct reading of the rule "a lane in
read/write mode outranks a plug-in-side set" (stated in `docs/product/parameter-automation.md`
§7 as reasoning from the host's replay behaviour, not from a header — the SDK does not spell it
out). The second pass's unbounded latch made the claim true by making every later writer
permanently deaf; `shell/instrument/CLAUDE.md`'s Authority section is the model now.
(c) **STILL OPEN, and it is the half the bundled `[verify, FIRST]` originally asked**: that
REAPER calls `setState` (not `setComponentState`) on a single-component plug-in — a state
ENTRY-POINT question, not a delivery-channel one. Recorded in `docs/TODO.md` rather than closed:
`vstsinglecomponenteffect.h:41-47` does collapse the names as §6.1 claims, our overrides land on
the `IComponent` pair with `setEditorState`/`getEditorState` left at the base's `kNotImplemented`,
and the blob has shipped through payload v1…v16, so the behaviour is very likely fine — but that
is inference, not observation. The rest of the DAW work is in `docs/TODO.md` too, and none of it
can change the frozen contract.
- **[verify]** whether REAPER renders `ParameterInfo::units` beside the string
`getParamStringByValue` returns, or shows the string alone. **We ship the SDK's own
convention** — digits in the string, unit carried separately, which is what
@@ -1328,8 +1347,9 @@ value semantics, any deck geometry, or the bake's reset *membership* (W3-T2's).
structurally could not see it and id 1000 would have no-oped in both directions with no
compile-time guard; both controls also had to reach the engine, which meant widening
`LiveValues` and `foldLive`'s input and handing `Voice::start` the two latched values as
arguments beside the rate. The guard that closes the class is `param::valueHomeFor`, asserted
over the exposed set.
arguments beside the rate. The guard that closes the class is `param::valueHomeFor` asserted
over the exposed set (every control has a home, the instance-scalar set has exactly two
members) AND branched on by the shell's own read and write paths, so the three cannot drift.
- **[propose at review]** whether to ship a default `IMidiMapping` CC table here or leave MIDI
control to REAPER's host-side learn. Either is defensible; **skipping it silently is not.**
- **[propose at review]** whether this track spends the reserved payload rung. §6.1 says
+1
View File
@@ -832,6 +832,7 @@ in the `.rpp`", and does not gain a second home for the distinction.
2. **Whether REAPER's own per-parameter MIDI learn covers what a shipped `IMidiMapping` CC table would have.** The decision to ship no default map rests on it; if learn does not reach these parameters, a CC table is additive and frozen by nothing.
3. **That the three migration round trips hold**: a pre-parameter project opens with every parameter reading the blob's value and sounds identical; a project saved by this build restores fully in an older binary; a project with automation drawn, saved and reopened, replays against the same plain values.
4. **That an offline render replays automation** — the sharpest case for the audio-side drain, because the host drives `process()` and may never touch the controller.
5. **That REAPER restores instance state through `setState`, not `setComponentState`.** This is the ENTRY-POINT half of the original bundled `[verify, FIRST]`; the pass that closed that item closed only its delivery-channel half, which is a different question. The evidence short of a DAW is strong but is inference: `vstsinglecomponenteffect.h:41-47` collapses the two names on a single-component plug-in, our `setState`/`getState` overrides land on the `IComponent` pair with `setEditorState`/`getEditorState` left at the base's `kNotImplemented`, and the blob has round-tripped through payload v1…v16 in real projects. Exercising it costs one save/reopen.
**Priority / risk.** Low. Nothing here is load-bearing on the frozen contract: the id table, the plain ranges and the norm↔plain laws are all decided and tested without a host.
+15 -5
View File
@@ -342,9 +342,15 @@ the GUI-update channel (*"should update the according GUI element(s) only"*,
`ivsteditcontroller.h`), and `ProcessData::inputParameterChanges` is the audio-side one — the
SDK's own `SingleComponentEffect` sample services BOTH
(`public.sdk/samples/vst/again/source/againsimple.cpp`), and so do we. The `setState` ordering
half dissolves with it: an automation point held by the audio thread is re-applied over every
merge, so a written lane outranks the restore whichever way round the two arrive, which is
VST3's own authority rule rather than a race. **The audio thread cannot run the model path**
half dissolves with it, but only because the hold is BOUNDED: an automation point held by the
audio thread is re-applied over every merge until the UI thread folds it into the model, so a
lane that is genuinely driving outranks the restore whichever way round the two arrive, while a
lane that sent one point and had it folded does not. That is the authority rule read correctly —
and note it is reasoning from the host's replay behaviour, not a header quote: the SDK does not
state it. An unbounded hold makes the ordering claim true by making every later writer
permanently deaf, which is not the same property. `shell/instrument/CLAUDE.md`'s Authority
section is the model, and `core/instrument/param/param_merge` is where it is enforced.
**The audio thread cannot run the model path**
(`resolvePlay` copies velocity curves and spline contours, so it allocates), so the drain patches
the live block in place through one pure RT-safe function whose routing is pinned by an
exhaustive equivalence test against the model path.
@@ -913,8 +919,12 @@ makes them automatable, since otherwise they would re-decode a WAV per automatio
`InstrumentParams`, not `PlaySeconds`, so the host's write path could not reach it and id 1000
would have no-oped in both directions with nothing failing to compile; both controls also had to
reach the engine, which widened `LiveValues` and `foldLive`'s input and gave `Voice::start` the
two latched values as arguments beside the rate. `param::valueHomeFor`, asserted over the
exposed set, is what makes the next promotion of this shape a test failure instead of a silence.
two latched values as arguments beside the rate. `param::valueHomeFor` is what makes the next
promotion of this shape a test failure instead of a silence, and it earns that claim in three
places rather than one: `test_param_live` asserts every exposed control HAS a home and that the
instance-scalar set has exactly two members, and the shell's own read and write paths
(`modelParamNormalized`, `writeDeckParamToModel`) now BRANCH on it rather than on a hardcoded
control id — so a third instance scalar cannot appear without failing that count.
**Not promoted, and not proposed for promotion: Rate to Live.** §3.5 records the cost;
that paragraph is the first thing to read if it is ever proposed.