diff --git a/src/shell/instrument/automation_channel.h b/src/shell/instrument/automation_channel.h index 06ea8a8..5c2deff 100644 --- a/src/shell/instrument/automation_channel.h +++ b/src/shell/instrument/automation_channel.h @@ -11,6 +11,7 @@ #include #include #include +#include #include "core/instrument/param/param_merge.h" @@ -21,6 +22,29 @@ inline constexpr std::size_t kDeckParamSlots = instrument::param::kDeckParamSlot class AutomationChannel { public: + // Evidence release() below requires: the model has actually caught up with the + // point being released. The private constructor means the only way to obtain one + // is through a factory here, and both are named for the case they cover — + // `fromPublish` for an observed model republish, `noRepublishNeeded` for the two + // cases drainAutomationToModel finds nothing to publish (the fold left the model + // unchanged, or no engine exists yet to read the live block). A caller that has + // not published cannot spell the argument release() needs, which is what turns + // reordering the release ahead of the publish into a compile error. + class ReleaseProof { + public: + static ReleaseProof fromPublish(std::uint32_t before, std::uint32_t after) { + // publish() is guaranteed to advance the generation; landing here means + // that guarantee broke, worth crashing on unconditionally rather than + // tolerating silently. + if (after == before) std::abort(); + return ReleaseProof{}; + } + static ReleaseProof noRepublishNeeded() { return ReleaseProof{}; } + + private: + ReleaseProof() = default; + }; + // --- AUDIO THREAD ------------------------------------------------------------------- // A point landed for `slot`. `ridesTheBlock` is false for a control that reaches the audio // beside the live block (master gain, whose route is the processor's own atomic): such a @@ -76,10 +100,10 @@ public: return true; } - // Releases `slot`'s hold. ONLY legal once the model carrying that point has been republished - // — calling it earlier would let the audio thread drop the hold ahead of the block that - // carries its value, which is a one-block revert to the superseded value. - void release(std::size_t slot, std::uint32_t seq) { + // Releases `slot`'s hold. The `ReleaseProof` argument is the enforcement: it can only be + // constructed once the model carrying this point has been republished (or shown not to need + // it), so a caller earlier in that ordering has no value to pass. + void release(std::size_t slot, std::uint32_t seq, ReleaseProof) { folded_[slot].store(seq, std::memory_order_release); } diff --git a/src/shell/instrument/instrument_params.cpp b/src/shell/instrument/instrument_params.cpp index 619f268..50ab140 100644 --- a/src/shell/instrument/instrument_params.cpp +++ b/src/shell/instrument/instrument_params.cpp @@ -6,8 +6,6 @@ #include "shell/instrument/reasampler_processor.h" -#include - #include "base/source/fstring.h" #include "pluginterfaces/base/ustring.h" #include "pluginterfaces/vst/ivstparameterchanges.h" // IParameterChanges / IParamValueQueue @@ -307,28 +305,20 @@ void ReaSamplerProcessor::drainAutomationToModel() { // controller cache is still refreshed, so the host's display and the editor follow. const bool wasSuppressed = paramNotifySuppressed_; paramNotifySuppressed_ = true; - // Captured before the publish below, so the assert at the release loop can tell "the model's - // publish already landed" from "it merely happened to be in flight for some other reason". - const std::uint32_t generationBeforeFold = liveParams_.generation(); if (moved) { setInstrumentParams(params); - publishLiveParams(); } + // The hold outranks the model only until the model carries the point (this directory's + // CLAUDE.md, THE AUTHORITY MODEL) — `releaseProof` is that ordering enforced structurally: + // `automation_.release` below cannot compile without one, and the only ways to obtain one are + // `publishLiveParams`'s return (the `moved` branch) or `noRepublishNeeded` (nothing to + // publish because the fold left the model already matching the point). + const AutomationChannel::ReleaseProof releaseProof = + moved ? publishLiveParams() : AutomationChannel::ReleaseProof::noRepublishNeeded(); syncParamsFromModel(); paramNotifySuppressed_ = wasSuppressed; - // LAST, and that is the whole authority rule: the hold outranks the model only until the - // model carries the point. Released any earlier and the audio thread could drop the hold - // ahead of the block that carries its value; never released at all — the defect this - // replaces — and one point would defeat every later restore, reset and knob move. - // Enforced, not just commented: two prior passes inverted this order and every test in the - // tree still passed, because nothing exercises `process()`. `publishLiveParams` is a no-op - // before the engine has a sample rate (`builtSampleRate_`), which is the one case this assert - // must not fire for. - assert((!moved || builtSampleRate_.load(std::memory_order_relaxed) <= 0 || - liveParams_.generation() != generationBeforeFold) && - "release ran ahead of the publish that gives it authority"); for (std::size_t i = 0; i < foldedCount; ++i) { - automation_.release(foldedSlots[i], foldedSeqs[i]); + automation_.release(foldedSlots[i], foldedSeqs[i], releaseProof); } } diff --git a/src/shell/instrument/processor_state.cpp b/src/shell/instrument/processor_state.cpp index 6dacac4..c33fa27 100644 --- a/src/shell/instrument/processor_state.cpp +++ b/src/shell/instrument/processor_state.cpp @@ -276,16 +276,21 @@ void ReaSamplerProcessor::clearMasterBusClip() { meterClip_.store(false, std::memory_order_relaxed); } -void ReaSamplerProcessor::publishLiveParams() { +AutomationChannel::ReleaseProof ReaSamplerProcessor::publishLiveParams() { const int rate = builtSampleRate_.load(std::memory_order_relaxed); - if (rate <= 0) return; + if (rate <= 0) return AutomationChannel::ReleaseProof::noRepublishNeeded(); + const std::uint32_t before = liveParams_.generation(); const InstrumentParams params = instrumentParams(); const instrument::engine::LiveValues block = instrument::engine::foldLive(resolvePlay(params.play, rate), params.keyTrack); - // livePublishMutex_ enforces the seqlock's single-writer contract (live_params.h) against - // reloadInstrument's publish — held for the publish call only, not the fold above. - std::lock_guard lock(livePublishMutex_); - liveParams_.publish(block); + { + // livePublishMutex_ enforces the seqlock's single-writer contract (live_params.h) + // against reloadInstrument's publish — held for the publish call only, not the fold + // above. + std::lock_guard lock(livePublishMutex_); + liveParams_.publish(block); + } + return AutomationChannel::ReleaseProof::fromPublish(before, liveParams_.generation()); } SampleRefs ReaSamplerProcessor::sampleRefs() { diff --git a/src/shell/instrument/reasampler_processor.h b/src/shell/instrument/reasampler_processor.h index e3b9cc6..8b0f177 100644 --- a/src/shell/instrument/reasampler_processor.h +++ b/src/shell/instrument/reasampler_processor.h @@ -203,8 +203,8 @@ public: // voices already latched. THE tier-3 commit (the three tiers are listed in this // directory's CLAUDE.md). Callers pair this with setInstrumentParams exactly as they // paired it with reloadInstrument. No-op before anything has been decoded (the next reload - // bakes and publishes). UI thread; serialized against reloadInstrument's own publish. - void publishLiveParams(); + // bakes and publishes). UI thread; serialized against reloadInstrument's own publish. Returns drainAutomationToModel's release proof (automation_channel.h); other callers ignore it. + AutomationChannel::ReleaseProof publishLiveParams(); // Per-instance channel mode (mono | stereo), guarded by channelModeMutex_, never read // on the audio thread. Decode policy only (downmix vs L/R split) — the output bus is