diff --git a/src/core/instrument/CLAUDE.md b/src/core/instrument/CLAUDE.md index b27d5c3..98e4a70 100644 --- a/src/core/instrument/CLAUDE.md +++ b/src/core/instrument/CLAUDE.md @@ -160,23 +160,28 @@ Daniel's ruling, verbatim: *"hell no, I was going to bring that up for the other must live compute, latching the parameters at note on is not acceptable. long term these will be automatable parameters."* It rejects the precedent, not one instance of it. -- **Which controls are live is ONE decision, recorded in ONE place** — `isLiveDeckParam` - (`ui/deck_groups`). Continuous playback controls go live: the six filter tone/modulation - knobs, and every stage time and stage level on all three envelopes. Everything else reloads - or drops to the voice-param rebuild. Two controls look continuous but are deliberately not - live — the pitch key-track and the velocity→cutoff depth feed values a voice latches at - note-on (the pitch ratio, the velocity-curve result), so making them live would retune or - re-gain a note already struck. The three capture-anchored overrides (root, loop span, start - frame) reload because they name positions in the decoded PCM. +- **Which controls are live is ONE decision, recorded in ONE place** — `isLiveDeckParam` and + `liveCommitFor` (`ui/deck_groups`), whose header is the home for why each excluded control is + excluded. Continuous playback controls go live: the six filter tone/modulation knobs, and + every stage time and stage level on all three envelopes. Everything else reloads or drops to + the voice-param rebuild. **FIVE continuous controls are outside the live set** — pitch + key-track, velocity→cutoff depth, and Trigger's %-length, fade-in and fade-out — so a + Trigger-mode instance gets no live delivery on its amplitude controls at all; only the filter + and pitch-envelope knobs move a sounding one-shot. - **Ownership sits ABOVE every snapshot.** `SampleData::live` is a NON-OWNING pointer to the one - block the shell owns per instance; a block owned by a snapshot would leave the drain slot's - still-sounding voices deaf to the knob under them. A drain voice tracking the knob is the - DESIRED behaviour — it is the note the user is hearing. + block the shell owns per instance. The member-ordering constraint that enforces it, and why, + are recorded at `liveParams_` in `shell/instrument/reasampler_processor.h`. A drain voice + tracking the knob is the DESIRED behaviour — it is the note the user is hearing. - **Null is the bare engine.** `live == nullptr` is byte-identical to the pre-live core, which is why `sampler_core`'s regression baselines needed no change. - **Observation is at block boundaries, never per frame.** `VoiceEngine` reads the seqlock once - per `render()` and once per note-on; the per-sample path gained one predicted branch - (`filterRamping_`) and no indirection. + per `render()` and once per note-on; the per-sample path gained three predicted branches (the + voice's filter-ramp check and each envelope smoother's active check), all false at rest, and + no indirection. +- **A fresh note SNAPS, a sounding one holds φ.** They are different entry points on purpose + (`snapLive` vs `applyLive`): a voice that has rendered nothing has no phase to hold, and the + φ rule reads its stage-0 position under a stale zero-length stage as a completed stage. One + function serving both silently discarded every newly-dialled attack. - **The mid-stage rule is HOLD NORMALIZED STAGE POSITION** (Daniel's pick among six candidates): φ = elapsed/duration is held across a stage-time change, so the level is continuous by construction and the remainder takes its share of the new duration. Stated over normalized @@ -232,7 +237,7 @@ slider couldn't. Two pure modules split the forward (draw) and inverse (edit) ma - The engine is the `sampler_core` CMake target over FOUR headers and TWO TUs, split on its own responsibility seam — cold note routing vs the hot per-sample render: - `play_params.h` — the value layer: `PlayParams`/`AdsrParams`/`TriggerParams`/`PitchEnvParams`/`FilterParams`, the per-instance mode enums (`ChannelMode`/`VoiceMode`/`MonoTrigger`), and `SampleData` (the ONE loaded capture: decoded PCM + root + loop + start + keyTrack + velocity curve + play params). Shared by the engine, the codec, and the editor, so a UI/codec TU reading a param struct doesn't recompile when a `Voice` member changes. `FilterParams` stores the filter module's own `FilterSettings` by value rather than a parallel copy of its normalized positions. - - `envelopes.h` — the three per-frame evaluators (`AdsrEnvelope` AHDSR, `TriggerEnvelope` fade shape, `PitchEnvelope` AD offset), CONCRETE and fully header-inline. Never give them a common base or a virtual `tick()`: they are called per-voice-per-sample. The filter envelope is a SECOND `AdsrEnvelope` instance on the voice, not a fourth class. `AdsrEnvelope`/`PitchEnvelope` also own `applyLive` (the φ-holding mid-stage rule) and `StepSmoother`, the bounded offset that absorbs the two level steps φ cannot cover. + - `envelopes.h` — the three per-frame evaluators (`AdsrEnvelope` AHDSR, `TriggerEnvelope` fade shape, `PitchEnvelope` AD offset), CONCRETE and fully header-inline. Never give them a common base or a virtual `tick()`: they are called per-voice-per-sample. The filter envelope is a SECOND `AdsrEnvelope` instance on the voice, not a fourth class. `AdsrEnvelope`/`PitchEnvelope` also own `applyLive` (the φ-holding mid-stage rule), its fresh-note peer `snapLive`, and `StepSmoother`, the bounded offset that absorbs the two level steps φ cannot cover. - `live_params.h` / `live_params.cpp` — the live-parameter block: `LiveValues` (the plain, trivially-copyable bundle the audio thread observes), the single-writer `LiveParams` seqlock that publishes it without a lock or a torn read, `foldLive` (the ONE derivation from `PlayParams` — every publisher goes through it so the two representations cannot drift), and `ValueRamp`, the per-frame glide whose EXACT termination is what lets the filter's equality-compare cutoff skip re-engage. Links no engine: the block is a value the voice observes, not a thing the engine owns. - `voice.h` / `voice.cpp` — one voice. The per-SAMPLE render half (`advanceFrame` and everything it calls) is INLINE IN THE HEADER by RT constraint; the per-NOTE half (note-on setup incl. the Preserve ring prime, legato retune, gate-off, the off-thread shifter presize) is out of line in the TU. The voice owns its own `VoiceFilter` and filter envelope, run between the pitch stage and the amp multiply — see `engine/filter/CLAUDE.md`. - `voice_engine.h` / `voice_engine.cpp` — `VoiceEngine`: note routing, bounded-stealing allocation, user-parameterized voice count (1–32, default 16), `VoiceMode` Poly/Mono (last-note held-note stack, `MonoTrigger` Retrigger/Legato), two-tier panic (CC 123 = all-notes-off release, CC 120 = immediate hard-stop including Trigger one-shots), and the block render loops. Preview injects a synthetic note-on at the loaded capture's root note into the main `VoiceEngine` — no dedicated `PreviewCard`; preview obeys polyphony/mono/voice-stealing/envelopes. @@ -261,7 +266,7 @@ slider couldn't. Two pure modules split the forward (draw) and inverse (edit) ma - `param_slider` — parameter control-panel: vertical stack of TOGGLE (two-segment selector) and SLIDER (horizontal track) rows; maps normalized value to/from handle pixel. - `embed_strip` — compact single-row control layout for embed mode in the track FX chain. - `knob_deck` — pure knob-deck layout + hit-test (FB1): group-box / caption-row / compact-toggle / knob-cell geometry, deterministic whole-group wrap, `DeckLayout` / `DeckHit`. Mirror of `action_bar`/`param_slider`; no LICE or REAPER types. -- `deck_groups` — also home to `isLiveDeckParam`, the editor's commit-tier routing predicate (see "Live parameter delivery" above); WHICH groups the Sample face's deck carries, split from `knob_deck`'s HOW they lay out: the `DeckParam` control-id space (the editor's `ParamControl` is an alias of it), the `DeckGroupId` list, `sampleDeckGroups` in signal-flow order (**pitch → filter → amp**, then voice/master), and the deck's bipolar-knob law. Reads `PlayMode` for the AMP group's Gate/Trigger face, which is why this and not `knob_deck` is the module that touches the engine's value layer. +- `deck_groups` — also home to `isLiveDeckParam` and `liveCommitFor`, the editor's whole commit-tier routing decision (see "Live parameter delivery" above); WHICH groups the Sample face's deck carries, split from `knob_deck`'s HOW they lay out: the `DeckParam` control-id space (the editor's `ParamControl` is an alias of it), the `DeckGroupId` list, `sampleDeckGroups` in signal-flow order (**pitch → filter → amp**, then voice/master), and the deck's bipolar-knob law. Reads `PlayMode` for the AMP group's Gate/Trigger face, which is why this and not `knob_deck` is the module that touches the engine's value layer. - `curve_popup` — pure curve-popup geometry + dismissal test (FB1): centered sheet over the Sample face — width/height clamps, title row, Close button rect, curve-box rect, outside-sheet dismissal test. Mirror of `overflow_menu`; no LICE or REAPER types. - `envelope_overlay` — pure amp-envelope→polyline geometry for the Sample-view envelope overlay (read from `envelope_overlay.h`): maps Gate's AHDSR shape or Trigger's fade-in/unity/%-length/fade-out shape to a polyline inside a rect at the shared time base (Gate: a bounded param-domain schematic, sample-length-free; Trigger: PCM-aligned wall-clock), every vertex clamped in-canvas (`x`/`y` inside the rect). Shares the `EnvNode`/`AmpEnvelope`/`timeToX`/`levelToY` vocabulary with `envelope_edit` so the drawn handle and its grab region agree pixel-for-pixel. No VST3/REAPER/LICE types at the boundary. - `envelope_edit` — pure node hit-test + pixel-delta→clamped-param inverse map for the draggable envelope nodes (read from `envelope_edit.h`): `nodeAtPoint` resolves a grab to the nearest node within a pick radius (Chebyshev distance, draw-order tie-break); `resolveNodeDrag` maps a pixel delta since grab to a new `AmpEnvelope`, enforcing monotonic-in-time ordering between neighbouring nodes and the same caller-supplied per-param clamp bounds the sliders use — a drag can never produce a param a slider couldn't. Mirror of `card_drag`/`waveform_view`; the inverse of `envelope_overlay`'s params→polyline forward map, so node-drag and slider-edit read/write one shared model and can never diverge. @@ -280,13 +285,14 @@ slider couldn't. Two pure modules split the forward (draw) and inverse (edit) ma - **A filter envelope only advances while its depth is non-zero.** `tickFilterCutoff`'s exact skip at `modAmount == 0` skips the envelope tick along with the solve, so dialling depth up mid-note starts the envelope from the note's stage-0 position rather than from where it would - have been. Continuous either way (the contribution starts at 0), and keeping the skip is what - holds the at-rest per-sample path byte-identical — but don't read a live depth move as - "resuming" an envelope that was never running. + have been. Its step smoother is frozen with it — an absorbed step sits in the offset and + emits when depth is next dialled up (bounded, and scaled by a depth ramping from 0). + Continuous either way (the contribution starts at 0), and keeping the skip is what holds the + at-rest per-sample path byte-identical — but don't read a live depth move as "resuming" an + envelope that was never running. - **A live edit leaves the snapshot's own `sample.play` stale, on purpose.** The block, not the snapshot, is the audio thread's source; a new voice latches the stale copy and is corrected by - `applyLive(snap)` before its first frame. The persisted `params_` is written by the same - commit, so `getState` is never stale. + `snapLive` before its first frame. - **`keyboard_strip`'s width-uniformity guarantee is client-pixel only.** Its test sweep covers client-pixel widths (including multiples standing in for larger client areas); nothing in the instrument implements `IPlugViewContentScaleSupport`, so host-side DPI diff --git a/src/core/instrument/engine/envelopes.h b/src/core/instrument/engine/envelopes.h index 4aa176d..8608cf8 100644 --- a/src/core/instrument/engine/envelopes.h +++ b/src/core/instrument/engine/envelopes.h @@ -90,6 +90,17 @@ public: stagePos_ = 0.0; } + // Live parameter delivery to a fresh voice — one that has NOT yet rendered a frame, whose + // latched copy may predate the newest edit. It takes the params outright: there is no + // phase to hold and nothing to be continuous with. applyLive cannot serve here in either + // direction — with a stale duration of 0 its phi rule reads stagePos_ == 0 as a COMPLETED + // stage and discards the newly-dialled time, and with a stale duration > 0 against a new 0 + // it absorbs a full-scale step into a voice that has emitted nothing, fading the onset in. + void snapLive(const AdsrParams& params) { + params_ = params; + smooth_.clear(); + } + // Live parameter delivery to a SOUNDING voice. The mid-stage rule is HOLD NORMALIZED // STAGE POSITION: phi = elapsed/duration is kept fixed across the change, so this frame's // level is unchanged by construction and the remainder of the stage takes its share of the @@ -117,6 +128,11 @@ public: // Once Release completes the envelope latches Finished and returns 0.0 forever (until // the next noteOn). A single, monotonic per-frame step — the caller pulls one value per // output frame. + // + // While the smoother runs the return may sit OUTSIDE [0,1] by the offset it is decaying + // (bounded by the step it absorbed). finished() ignores that residue, so a Release that + // completes with an offset still decaying is hard-cut when the voice frees — the audible + // remainder of a step the smoother had already taken most of. double tick() { const double out = tickStage(); return smooth_.active() ? out + smooth_.advance() : out; @@ -127,8 +143,11 @@ public: double level() const { return level_; } private: - // The level tick() would emit right now under `params`, without advancing anything — the - // prediction applyLive compares across the change to size the smoother. + // The level tick() would emit right now under `params` without advancing anything. THE one + // home for every segment's shape: tickStage owns only the advance and the stage + // transitions and reads its output from here, so a per-segment curve added later lands in + // one place and the smoother can never size a step against a different curve than the + // output takes. double stageLevel(const AdsrParams& params) const { switch (stage_) { case Stage::Attack: { @@ -178,12 +197,7 @@ private: return 0.0; case Stage::Attack: { - if (params_.attackFrames <= 0) { - level_ = 1.0; - } else { - level_ = stagePos_ / static_cast(params_.attackFrames); - if (level_ > 1.0) level_ = 1.0; - } + level_ = stageLevel(params_); const double out = level_; stagePos_ += 1.0; if (stagePos_ >= static_cast(params_.attackFrames)) { @@ -208,7 +222,7 @@ private: // smoother is applied exactly once per frame. return tickStage(); } - level_ = 1.0; + level_ = stageLevel(params_); const double out = level_; stagePos_ += 1.0; if (stagePos_ >= static_cast(params_.holdFrames)) { @@ -220,12 +234,7 @@ private: } case Stage::Decay: { - if (params_.decayFrames <= 0) { - level_ = params_.sustainLevel; - } else { - const double t = stagePos_ / static_cast(params_.decayFrames); - level_ = 1.0 + (params_.sustainLevel - 1.0) * t; - } + level_ = stageLevel(params_); const double out = level_; stagePos_ += 1.0; if (stagePos_ >= static_cast(params_.decayFrames)) { @@ -237,7 +246,7 @@ private: } case Stage::Sustain: - level_ = params_.sustainLevel; + level_ = stageLevel(params_); return level_; case Stage::Release: { @@ -246,9 +255,7 @@ private: stage_ = Stage::Finished; return 0.0; } - const double t = stagePos_ / static_cast(params_.releaseFrames); - level_ = releaseFrom_ * (1.0 - t); - if (level_ < 0.0) level_ = 0.0; + level_ = stageLevel(params_); const double out = level_; stagePos_ += 1.0; if (stagePos_ >= static_cast(params_.releaseFrames)) { @@ -348,6 +355,15 @@ public: void configure(const PitchEnvParams& params) { params_ = params; pos_ = 0.0; } void noteOn() { pos_ = 0.0; smooth_.clear(); } + // Peer of AdsrEnvelope::snapLive (see it for why the two paths cannot share code): a voice + // that has rendered nothing takes the new times and depth outright. + void snapLive(std::int64_t attackFrames, std::int64_t decayFrames, double peakSemitones) { + params_.attackFrames = attackFrames; + params_.decayFrames = decayFrames; + params_.peakSemitones = peakSemitones; + smooth_.clear(); + } + // Live parameter delivery, same rule as AdsrEnvelope::applyLive: hold the normalized // position within whichever leg the envelope is in, and absorb the depth step (peak is a // level, not a duration). `enabled` is a discrete toggle and travels by reload, so it is diff --git a/src/core/instrument/engine/live_params.h b/src/core/instrument/engine/live_params.h index 253e4f0..bfb7941 100644 --- a/src/core/instrument/engine/live_params.h +++ b/src/core/instrument/engine/live_params.h @@ -52,6 +52,16 @@ LiveValues foldLive(const PlayParams& params); // reader: after the retry budget it reports "nothing new" and the caller keeps its last good // snapshot rather than spinning on the audio thread. // +// SINGLE-WRITER IS THE CALLER'S JOB and is load-bearing: two concurrent writers can leave the +// generation EVEN mid-write (A stores gen+1, B reads odd and stores gen+2) while both copy the +// block, and a reader then accepts a torn block as coherent. Every publisher must serialize. +// +// The plain (non-atomic) block copied across the fences is the standard pragmatic seqlock: +// the fences give correct ordering, but the concurrent read of a non-atomic object is a data +// race under the C++ object model, so TSan/UBSan will report it. That report is expected, not +// a defect — there is no clean lock-free standard-C++ alternative that keeps the block a plain +// value the audio thread can copy in one shot. +// // The writer interface deliberately assumes NO particular thread beyond single-writer, so a // host's own parameter-change queue (delivered on the audio thread with sample offsets) can // drive it later without a redesign. @@ -64,7 +74,10 @@ public: std::atomic_thread_fence(std::memory_order_release); values_ = values; std::atomic_thread_fence(std::memory_order_release); - seq_.store(gen + 2, std::memory_order_release); // even: complete and coherent + // Skip 0 on wrap (~2^31 publishes): landing there would read as "never published" and + // stall every reader until the NEXT publish — a silent mode, unlike a loud one. + const std::uint32_t next = (gen + 2 == 0u) ? 2u : gen + 2; + seq_.store(next, std::memory_order_release); // even: complete and coherent } // Copies the block into `out` and returns the generation actually observed, or 0 when diff --git a/src/core/instrument/engine/voice.cpp b/src/core/instrument/engine/voice.cpp index 4ff4e9c..b04066c 100644 --- a/src/core/instrument/engine/voice.cpp +++ b/src/core/instrument/engine/voice.cpp @@ -197,13 +197,24 @@ void Voice::start(int note, int velocity, const SampleData& sample, bool declick void Voice::applyLive(const instrument::engine::LiveValues& live, bool snap) { // Gate's amplitude envelope is the AHDSR; Trigger's fade shape is anchored to a play span - // resolved at note-on and is not a live control, so it is deliberately untouched here. - if (playMode_ == PlayMode::Gate) env_.applyLive(live.adsr); - pitchEnv_.applyLive(live.pitchEnvAttackFrames, live.pitchEnvDecayFrames, - live.pitchEnvPeakSemitones); + // resolved at note-on and travels by reload instead (deck_groups.h names why). + // + // A fresh note and a sounding one take DIFFERENT envelope entry points, never one with a + // flag: a voice that has rendered nothing has no phase to hold and nothing to be + // continuous with, and the mid-stage rule misreads its stage-0 position (envelopes.h). + if (snap) { + if (playMode_ == PlayMode::Gate) env_.snapLive(live.adsr); + pitchEnv_.snapLive(live.pitchEnvAttackFrames, live.pitchEnvDecayFrames, + live.pitchEnvPeakSemitones); + } else { + if (playMode_ == PlayMode::Gate) env_.applyLive(live.adsr); + pitchEnv_.applyLive(live.pitchEnvAttackFrames, live.pitchEnvDecayFrames, + live.pitchEnvPeakSemitones); + } if (!filterOn_) return; // filter enable is a discrete toggle: it travels by reload - filterEnv_.applyLive(live.filterEnv); + if (snap) filterEnv_.snapLive(live.filterEnv); + else filterEnv_.applyLive(live.filterEnv); filterCutoffNorm_ = static_cast(live.filterSettings.cutoffNorm); filterKeyTrack_ = live.filterKeyTrack; filterSettings_.morphLaw = live.filterSettings.morphLaw; diff --git a/src/core/instrument/engine/voice.h b/src/core/instrument/engine/voice.h index 7004ac8..58b6496 100644 --- a/src/core/instrument/engine/voice.h +++ b/src/core/instrument/engine/voice.h @@ -119,8 +119,8 @@ public: // Applies the live-parameter block to a voice that is already sounding (or, with `snap`, // to one just started). Called at BLOCK boundaries by VoiceEngine — never per frame — so // the per-sample shape is unchanged; every continuous control glides toward its new value - // from here rather than jumping to it. `snap` takes the values outright: a fresh note has - // nothing to glide from, and its latched copy may predate the newest edit. + // from here rather than jumping to it. `snap` takes the values outright — glides AND + // envelopes: a fresh note has nothing to glide from, and its copy may predate the edit. // // What is NOT here is the point: velocity and its curve result, the note number and the // pitch ratio, and the decoded PCM stay latched at note-on. diff --git a/src/core/instrument/ui/deck_groups.cpp b/src/core/instrument/ui/deck_groups.cpp index 1144d96..8f144be 100644 --- a/src/core/instrument/ui/deck_groups.cpp +++ b/src/core/instrument/ui/deck_groups.cpp @@ -119,9 +119,40 @@ bool isLiveDeckParam(DeckParam id) { case DeckParam::kFilterEnvSustain: case DeckParam::kFilterEnvRelease: return true; - default: + // Listed rather than defaulted so a newly added control is a COMPILE error here (the + // -Wswitch gate is GCC/Clang; MSVC's C4062 is off at this project's warning level) + // instead of silently defaulting to non-live. Reasons live in the header. + case DeckParam::kPlayMode: + case DeckParam::kPitchEngine: + case DeckParam::kTrigLength: + case DeckParam::kTrigFadeIn: + case DeckParam::kTrigFadeOut: + case DeckParam::kPitchEnvEnable: + case DeckParam::kKeyTrack: + case DeckParam::kFilterEnable: + case DeckParam::kFilterVel: + case DeckParam::kFilterLaw: + case DeckParam::kVoiceCount: + case DeckParam::kVoiceMode: + case DeckParam::kMonoTrigger: + case DeckParam::kMasterGain: + case DeckParam::kCount: // not a control return false; } + return false; // unreachable for a valid enumerator; silences a warning. +} + +bool liveCommitFor(LiveDragKind kind, int paramId, PlayMode playMode) { + switch (kind) { + case LiveDragKind::kDeckKnob: + return paramId >= 0 && paramId < static_cast(DeckParam::kCount) && + isLiveDeckParam(static_cast(paramId)); + case LiveDragKind::kEnvNode: + return playMode == PlayMode::Gate; + case LiveDragKind::kOther: + return false; + } + return false; } } // namespace reasampler::instrument::ui diff --git a/src/core/instrument/ui/deck_groups.h b/src/core/instrument/ui/deck_groups.h index 7560945..8974dcd 100644 --- a/src/core/instrument/ui/deck_groups.h +++ b/src/core/instrument/ui/deck_groups.h @@ -77,15 +77,35 @@ std::vector sampleDeckGroups(PlayMode playMode); // controls, so this is a routing decision at the editor's commit site rather than a property // of any one knob; moving a control across the line is a change here and nowhere else. // -// Deliberately NOT live, each for its own reason: the discrete toggles (play mode, pitch -// engine, filter enable/law, pitch-envelope enable) name a different sound rather than a -// different setting of one; the three capture-anchored overrides (root, loop span, start -// frame) name positions in the decoded PCM; pitch key-track and the velocity->cutoff depth -// feed values a voice latches at note-on by design (the pitch ratio and the velocity-curve -// result), so making them live would retune or re-gain a note already struck; and Trigger's -// %-length and fades resolve a play span that is a fact about the note. +// THE home for why each excluded control is excluded. Five continuous controls are outside the +// live set, plus every discrete toggle: +// - the discrete toggles (play mode, pitch engine, filter enable/law, pitch-envelope enable) +// name a different sound rather than a different setting of one; +// - the three capture-anchored overrides (root, loop span, start frame) name positions in +// the decoded PCM; +// - kKeyTrack and kFilterVel feed values a voice latches at note-on by design (the pitch +// ratio and the velocity-curve result), so live delivery would retune or re-gain a note +// already struck; +// - kTrigLength resolves playEnd_, a fact about the note. kTrigFadeIn/kTrigFadeOut are pure +// amplitude shape and would be live-able in principle, but they live in `sample.play` and +// are baked into SampleData at build time — the engine rebuild copies that verbatim, so +// only a reload can deliver them without widening LiveValues. They fold into the AHD +// alongside Gate's, at which point they inherit its routing; until then they reload. +// Consequence, stated plainly: a Trigger-mode instance gets NO live delivery on its amplitude +// controls. Only the filter and pitch-envelope knobs move a sounding Trigger one-shot. bool isLiveDeckParam(DeckParam id); +// The editor drag kinds that can commit live, in this pure module's own vocabulary (the +// shell's DragKind maps onto it) so the WHOLE routing decision — not just the predicate — is +// testable without a host. +enum class LiveDragKind { kOther, kDeckKnob, kEnvNode }; + +// Whether a drag of `kind` commits live. A deck knob is live per isLiveDeckParam (negative ids +// are the shell's processor-side sentinels and out-of-range ids are not controls, so neither +// reaches the enum); an envelope-node drag is live only in Gate, where it edits the AHDSR — +// in Trigger the same drag rewrites the play span, which is not a live control. +bool liveCommitFor(LiveDragKind kind, int paramId, PlayMode playMode); + // The deck's BIPOLAR knob law: 0.5 of the knob's travel is zero depth, the ends are -1 and // +1. Exact inverses, and exact at the centre detent (0.5 -> 0 -> 0.5), so a knob parked at // centre can never persist a hair of modulation. Out-of-range norm clamps to the endpoints. diff --git a/src/shell/instrument/CLAUDE.md b/src/shell/instrument/CLAUDE.md index 71b0185..7d66eea 100644 --- a/src/shell/instrument/CLAUDE.md +++ b/src/shell/instrument/CLAUDE.md @@ -68,7 +68,8 @@ scattered `#ifdef`s in the VST shell, except the one described below). (`DEF_CLASS2` / `INLINE_UID` / `FUID` from `pluginfactory.h` + `funknown.h`). **The three commit tiers (Θ-W3).** An edit reaches the audio by exactly one of three routes, and -which route a control takes is decided once, by the pure `isLiveDeckParam` predicate — see +which route a control takes is decided once, by the pure `isLiveDeckParam` / `liveCommitFor` pair +(`core/instrument/ui/deck_groups`) that the editor's `dragCommitsLive` only maps onto — see `core/instrument/CLAUDE.md`'s "Live parameter delivery" for the rule and its rationale. 1. **Full reload** — `reloadInstrument`: bridge read, WAV re-decode, fresh engine, snapshot swap. 2. **Engine rebuild** — `rebuildVoiceEngine`: same drain-slot swap around the already-decoded @@ -76,10 +77,9 @@ which route a control takes is decided once, by the pure `isLiveDeckParam` predi 3. **Live** — `publishLiveParams` (and `masterGain_`, the original of the shape): a lock-free publish the audio thread observes at block boundaries. No rebuild, no snapshot, no disk. -`liveParams_` is declared ahead of the instrument slots so it outlives every snapshot pointing at -it. The editor's `commitLive` is the tier-3 peer of `commitAndReload` and still writes the -parameter set, so persistence is unchanged — `getState` serializes `params_`, never a reload -artifact, and the reload was never the persistence trigger. +The editor's `commitLive` is the tier-3 peer of `commitAndReload`; why it still writes the +parameter set is recorded at its declaration in `reasampler_editor.h`, and why `liveParams_` is +declared ahead of the instrument slots at that member in `reasampler_processor.h`. **Non-goals / guardrails.** - The instrument never captures and never inserts into the arrange. Playback is a diff --git a/src/shell/instrument/editor_input.cpp b/src/shell/instrument/editor_input.cpp index 3d99601..322db2e 100644 --- a/src/shell/instrument/editor_input.cpp +++ b/src/shell/instrument/editor_input.cpp @@ -105,7 +105,7 @@ void ReaSamplerEditor::onMouseUp(int x, int y) { return; } // A live control already reached the voices during the drag; its release commits the - // final value the same way — no bridge read, no WAV re-decode, no snapshot rebuild. + // final value through the same tier. if (dragCommitsLive(kind, paramId)) { commitLive(); invalidate(); diff --git a/src/shell/instrument/editor_platform.cpp b/src/shell/instrument/editor_platform.cpp index 0d6d2c5..9f0c391 100644 --- a/src/shell/instrument/editor_platform.cpp +++ b/src/shell/instrument/editor_platform.cpp @@ -205,8 +205,8 @@ LRESULT CALLBACK ReaSamplerEditor::wndProc(HWND hwnd, UINT msg, WPARAM wParam, case WM_RBUTTONUP: return 0; // claimed so the pair never reaches DefWindowProc (no context menu) case WM_CAPTURECHANGED: - // Capture stolen mid-drag (modal dialog, alt-tab, etc.) — restore map_ to its - // pre-grab snapshot so the in-flight live-drag mutation is rolled back, then reset + // Capture stolen mid-drag (modal dialog, alt-tab, etc.) — restore params_ to its + // pre-grab snapshot so the in-flight drag mutation is rolled back, then reset // the drag state machine so stale capture-less WM_MOUSEMOVEs don't keep editing. // Mirror of the panel shell's WM_CAPTURECHANGED handler (panel_window.cpp). if (self) { @@ -219,15 +219,25 @@ LRESULT CALLBACK ReaSamplerEditor::wndProc(HWND hwnd, UINT msg, WPARAM wParam, } if (self->drag_ != DragKind::kNone) { // A scrollbar drag + the processor-side deck knobs (preview velocity -2 / - // voice count / master gain) are transient (they mutate no parameter, so - // dragStartParams_ is not a rollback target) — reset drag state only. - // Every parameter-editing drag rolls its live mutation back to the snapshot. + // voice count / master gain) mutate no parameter, so dragStartParams_ is + // not a rollback target for them — reset drag state only. Every + // parameter-editing drag restores the pre-grab snapshot. const bool transient = self->drag_ == DragKind::kScrollThumb || (self->drag_ == DragKind::kDeckKnob && (self->dragParamId_ == -2 || self->dragParamId_ == static_cast(ParamControl::kVoiceCount) || self->dragParamId_ == static_cast(ParamControl::kMasterGain))); - if (!transient) self->params_ = self->dragStartParams_; + if (!transient) { + self->params_ = self->dragStartParams_; + // A live drag already reached the voices AND the processor's own + // parameter set on every move, so restoring params_ alone would leave + // the face painting one value while the audio plays — and getState + // persists — the abandoned one. Roll back through the same tier the + // drag used. + if (self->dragCommitsLive(self->drag_, self->dragParamId_)) { + self->commitLive(); + } + } self->drag_ = DragKind::kNone; self->dragParamId_ = -1; self->curvePointIndex_ = -1; // curve-node drag state (peer reset) diff --git a/src/shell/instrument/editor_session.cpp b/src/shell/instrument/editor_session.cpp index b647d46..fc50a46 100644 --- a/src/shell/instrument/editor_session.cpp +++ b/src/shell/instrument/editor_session.cpp @@ -144,13 +144,14 @@ void ReaSamplerEditor::commitLive() { } bool ReaSamplerEditor::dragCommitsLive(DragKind kind, int paramId) const { - if (kind == DragKind::kDeckKnob) { - // Negative ids are the processor-side sentinels (preview velocity), not parameter-set - // controls, so they never reach the enum. - return paramId >= 0 && - instrument::ui::isLiveDeckParam(static_cast(paramId)); - } - return kind == DragKind::kEnvNode && params_.play.playMode == PlayMode::Gate; + // The decision itself is the pure liveCommitFor's; this is only the shell's drag-kind + // vocabulary mapped onto it, so the routing is pinned by deck_groups' tests rather than + // by inspection of this file. + using instrument::ui::LiveDragKind; + const LiveDragKind k = kind == DragKind::kDeckKnob ? LiveDragKind::kDeckKnob + : kind == DragKind::kEnvNode ? LiveDragKind::kEnvNode + : LiveDragKind::kOther; + return instrument::ui::liveCommitFor(k, paramId, params_.play.playMode); } void ReaSamplerEditor::loadSelection(const std::string& id) { diff --git a/src/shell/instrument/processor_state.cpp b/src/shell/instrument/processor_state.cpp index 21682d0..b08648e 100644 --- a/src/shell/instrument/processor_state.cpp +++ b/src/shell/instrument/processor_state.cpp @@ -153,6 +153,13 @@ void ReaSamplerProcessor::setInstrumentParams(const InstrumentParams& params) { } void ReaSamplerProcessor::publishLiveParams() { + // reloadMutex_ enforces the block's SINGLE-WRITER contract (live_params.h), not the + // reload's slot bookkeeping: reloadInstrument publishes the block too, and two concurrent + // seqlock writers can leave the generation even mid-write, which a reader would accept as + // a coherent — but torn — block. The audio thread never takes this mutex, so the cost is + // an off-thread wait behind a reload. Lock order matches reloadInstrument's + // (reloadMutex_ then paramsMutex_, taken by instrumentParams below). + std::lock_guard lock(reloadMutex_); const int rate = builtSampleRate_.load(std::memory_order_relaxed); if (rate <= 0) return; liveParams_.publish( diff --git a/src/shell/instrument/reasampler_processor.h b/src/shell/instrument/reasampler_processor.h index 7fe68a5..7524b64 100644 --- a/src/shell/instrument/reasampler_processor.h +++ b/src/shell/instrument/reasampler_processor.h @@ -154,11 +154,10 @@ public: // Republishes the live-parameter block from the stored parameter set, resolved against the // rate the loaded capture was built at so an unmoved value folds to exactly the frames the - // voices already latched. THE tier-3 commit: no bridge read, no WAV re-decode, no engine - // rebuild, no snapshot swap — the sounding note follows the knob. Persistence is - // unaffected: getState still serializes params_, so 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. + // 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(); // Per-instance channel mode (mono | stereo), guarded by channelModeMutex_, never read @@ -243,6 +242,13 @@ private: // The rate the loaded capture was decoded/built at, so a live republish resolves the // stored wall-clock seconds to exactly the frames the built SampleData carries. 0 = nothing // built yet. + // + // ONE BLOCK, ONE RATE: this is stamped by whichever capture built last, and a reload + // publishes the new block before installing the new instrument. Swapping to a capture at a + // different rate therefore hands drain voices still ringing from the old-rate capture + // envelope frame counts resolved at the NEW rate (~8.8% timing shift on a 48k->44.1k swap). + // Unavoidable while one block sits above every snapshot, and it touches a release tail + // only. std::atomic builtSampleRate_{0}; // --- The audio-thread handoff (drain slot) --- diff --git a/tests/test_deck_groups.cpp b/tests/test_deck_groups.cpp index 7be2d19..f0994ff 100644 --- a/tests/test_deck_groups.cpp +++ b/tests/test_deck_groups.cpp @@ -2,8 +2,9 @@ // framework. knob_deck's own tests pin how a descriptor list LAYS OUT; these pin WHICH // descriptors the Sample face carries: the signal-flow group order (pitch -> filter -> amp), // the Filter group's contents, the wrapped deck height at the editor's floor width and its fit -// inside the floor window, the hit-test reaching the new filter controls, and the bipolar knob -// law's inverse pair. +// inside the floor window, the hit-test reaching the new filter controls, the bipolar knob +// law's inverse pair, and the commit-tier routing — which controls are live, and which drags +// take the live tier. #include "../src/core/instrument/ui/deck_groups.h" #include "../src/core/instrument/ui/sample_bands.h" @@ -186,7 +187,7 @@ static void testBipolarKnobLawRoundTripsAndIsExactAtCentre() { CHECK(deckNormFromBipolar(3.0) == 1.0); } -static void testLiveRoutingCoversEveryContinuousPlaybackControl() { +static void testEveryDeckControlIsClassifiedLiveOrReloading() { // The live set: the six filter tone/modulation knobs, plus every stage time and stage // level on all three envelopes. const DeckParam live[] = { @@ -200,9 +201,8 @@ static void testLiveRoutingCoversEveryContinuousPlaybackControl() { }; for (DeckParam p : live) CHECK(isLiveDeckParam(p)); - // Everything else reloads or rebuilds. kFilterVel and kKeyTrack are the two that look - // continuous but feed values a voice latches at note-on (the velocity-curve result and - // the pitch ratio) — making them live would re-gain or retune a note already struck. + // Everything else reloads or rebuilds; deck_groups.h is the home for why each exclusion + // is excluded. const DeckParam reloads[] = { DeckParam::kPlayMode, DeckParam::kPitchEngine, DeckParam::kPitchEnvEnable, DeckParam::kFilterEnable, DeckParam::kFilterLaw, DeckParam::kFilterVel, @@ -212,14 +212,49 @@ static void testLiveRoutingCoversEveryContinuousPlaybackControl() { }; for (DeckParam p : reloads) CHECK(!isLiveDeckParam(p)); - // Every id in the space is classified by one of the two lists above, so a control added - // later cannot slip through unclassified. - const std::size_t classified = (sizeof(live) + sizeof(reloads)) / sizeof(DeckParam); - CHECK(classified == static_cast(DeckParam::kCount)); + // COVERAGE, not cardinality: every id appears in EXACTLY ONE of the two lists. A sum check + // would stay green if an edit duplicated one id and dropped another, leaving that one + // unclassified. + for (int i = 0; i < static_cast(DeckParam::kCount); ++i) { + const DeckParam p = static_cast(i); + int seen = 0; + for (DeckParam q : live) if (q == p) ++seen; + for (DeckParam q : reloads) if (q == p) ++seen; + if (seen != 1) std::printf(" (deck id %d classified %d times)\n", i, seen); + CHECK(seen == 1); + } +} + +static void testOnlyALiveControlsDragTakesTheLiveTier() { + // isLiveDeckParam alone is not what a user experiences — liveCommitFor is, at the editor's + // commit site. Inverting it has to FAIL a test rather than merely read wrong. + CHECK(liveCommitFor(LiveDragKind::kDeckKnob, static_cast(DeckParam::kFilterCutoff), + PlayMode::Gate)); + // A knob's routing is the knob's, not the play mode's. + CHECK(liveCommitFor(LiveDragKind::kDeckKnob, static_cast(DeckParam::kAttack), + PlayMode::Trigger)); + CHECK(!liveCommitFor(LiveDragKind::kDeckKnob, static_cast(DeckParam::kTrigFadeIn), + PlayMode::Trigger)); + CHECK(!liveCommitFor(LiveDragKind::kDeckKnob, static_cast(DeckParam::kMasterGain), + PlayMode::Gate)); + // The shell's processor-side sentinels (preview velocity is -2) and any out-of-range id + // are not parameter-set controls, so they must never reach the enum. + CHECK(!liveCommitFor(LiveDragKind::kDeckKnob, -2, PlayMode::Gate)); + CHECK(!liveCommitFor(LiveDragKind::kDeckKnob, -1, PlayMode::Gate)); + CHECK(!liveCommitFor(LiveDragKind::kDeckKnob, static_cast(DeckParam::kCount), + PlayMode::Gate)); + // An envelope-node drag edits the AHDSR in Gate; the same drag in Trigger rewrites the + // play span, which is not a live control. + CHECK(liveCommitFor(LiveDragKind::kEnvNode, -1, PlayMode::Gate)); + CHECK(!liveCommitFor(LiveDragKind::kEnvNode, -1, PlayMode::Trigger)); + // Every other drag (markers, scrollbar, curve nodes) commits through a reload. + CHECK(!liveCommitFor(LiveDragKind::kOther, static_cast(DeckParam::kFilterCutoff), + PlayMode::Gate)); } int main() { - testLiveRoutingCoversEveryContinuousPlaybackControl(); + testEveryDeckControlIsClassifiedLiveOrReloading(); + testOnlyALiveControlsDragTakesTheLiveTier(); testDeckReadsPitchThenFilterThenAmpLeftToRight(); testFilterGroupCarriesItsFiveToneControlsPlusModulation(); testAmpGroupWidthSurvivesAGateTriggerFlip(); diff --git a/tests/test_live_delivery.cpp b/tests/test_live_delivery.cpp index 60b4273..89b7527 100644 --- a/tests/test_live_delivery.cpp +++ b/tests/test_live_delivery.cpp @@ -1,9 +1,10 @@ // Standalone tests for LIVE PARAMETER DELIVERY into a sounding voice — no VST3, no REAPER, no // framework. The block's own publication contract is live_params_tests; this file asserts what // reaches the audio: the mid-stage rule holds normalized position, a level move glides, a -// filter knob moves the note that is already playing, two snapshots sharing one block behave -// identically (the drain slot), what stays latched at note-on stays latched, and an unmoved -// block renders byte-identically to the engine with no block at all. +// fresh note takes the newest block outright, every stage time and stage level on all three +// envelopes moves the note already sounding, a filter knob does too, two snapshots sharing one +// block behave identically (the drain slot), what stays latched at note-on stays latched, and +// an unmoved block renders byte-identically to the engine with no block at all. #include "../src/core/instrument/engine/voice_engine.h" @@ -48,6 +49,62 @@ static double maxAbsDelta(const std::vector& v, std::size_t from, s return worst; } +static double peakOf(const std::vector& v, std::size_t from, std::size_t to) { + double peak = 0.0; + for (std::size_t i = from; i < to && i < v.size(); ++i) { + peak = (std::max)(peak, std::fabs(static_cast(v[i]))); + } + return peak; +} + +static SampleData filteredSine() { + SampleData s = periodicSine(200000, 64.0); + s.play.filter.enabled = true; + s.play.filter.settings.cutoffNorm = 0.8f; + s.play.filter.settings.resonanceNorm = 0.9f; + s.play.filter.settings.morphNorm = 1.0f; + s.play.filter.env.sustainLevel = 1.0; + return s; +} + +// A low corner with real envelope depth, so the filter ENVELOPE's shape is what the timbre +// depends on rather than the static knob position. +static void filterSweep(SampleData& s) { + s.play.filter.enabled = true; + s.play.filter.settings.cutoffNorm = 0.15f; + s.play.filter.settings.resonanceNorm = 0.6f; + s.play.filter.settings.morphNorm = 1.0f; + s.play.filter.modAmount = 0.8; +} + +// Renders `blocks` blocks of `blockFrames` through a one-voice engine over `sample`, +// republishing `changed` at the top of block `changeAfter` and gating the note off at the top +// of `noteOffBlock` (-1 holds it). Voice-major render order is the engine's, so a fixed block +// size is what makes two runs comparable. +struct Run { + std::vector out; +}; + +// The note is an octave above the root on purpose: key-tracking scales (note - root), so a +// root-note test would leave the key-track control with nothing to move. +constexpr int kTestNote = 72; + +static Run renderWithLive(SampleData& sample, LiveParams* block, int blockFrames, int blocks, + int changeAfter, const LiveValues* changed, int noteOffBlock = -1, + int velocity = 100) { + sample.live = block; + if (block) block->publish(foldLive(sample.play)); + VoiceEngine engine(1, sample); + engine.noteOn(kTestNote, velocity); + Run r; + for (int b = 0; b < blocks; ++b) { + if (block && changed && b == changeAfter) block->publish(*changed); + if (b == noteOffBlock) engine.noteOff(kTestNote); + engine.render(r.out, static_cast(blockFrames)); + } + return r; +} + // --- The mid-stage rule (candidate iv): hold normalized stage position ------------------ static void testStageDurationChangeHoldsPhase() { @@ -120,7 +177,10 @@ static void testSustainLevelChangeGlides() { double v = 0.0; for (int i = 0; i < 600; ++i) { v = env.tick(); - if (i == 0) CHECK(v == 1.0); // the first frame reproduces the pre-change level exactly + // The first frame reproduces the pre-change level. Bounded rather than compared + // exactly: 0.2 + fl(1.0 - 0.2) does round to exactly 1.0 for THESE operands, but the + // property under test is continuity, not a bit-exactness the smoother never promised. + if (i == 0) CHECK(std::fabs(v - 1.0) < 1e-15); const double step = std::fabs(v - prev); if (step > worstStep) worstStep = step; prev = v; @@ -162,6 +222,222 @@ static void testPitchEnvelopeHoldsPhaseAndGlidesDepth() { CHECK(c.tick() == 0.0); } +// --- The fresh-note path: snap, never the phi rule --------------------------------------- + +static void testAFreshEnvelopeTakesANewlyDialledStageTimeOutright() { + // Regression, both directions. The snap path once ran applyLive's phi rule, which reads a + // stale duration of 0 as "this stage is already complete" and threw the newly-dialled + // attack away for every note until the next reload. + AdsrParams stale; // the AdsrParams default: every stage zero + stale.sustainLevel = 1.0; + AdsrEnvelope env; + env.configure(stale); + env.noteOn(); + AdsrParams dialled = stale; + dialled.attackFrames = 100; + env.snapLive(dialled); + CHECK(env.tick() == 0.0); // frame 0 of a 100-frame attack, not an instant 1.0 + for (int i = 0; i < 49; ++i) env.tick(); + CHECK(std::fabs(env.tick() - 0.5) < 1e-12); + + // Reverse: a stale non-zero attack against a newly-dialled ZERO one must not absorb a + // full-scale step into a voice that has emitted nothing — that fades in a note the user + // asked to be instant. + AdsrParams staleLong; + staleLong.attackFrames = 1000; + staleLong.sustainLevel = 1.0; + AdsrEnvelope instant; + instant.configure(staleLong); + instant.noteOn(); + AdsrParams zeroAttack = staleLong; + zeroAttack.attackFrames = 0; + instant.snapLive(zeroAttack); + CHECK(instant.tick() == 1.0); +} + +static void testAFreshPitchEnvelopeTakesTheNewTimesOutright() { + PitchEnvParams stale; // enabled, but every leg zero + stale.enabled = true; + PitchEnvelope env; + env.configure(stale); + env.noteOn(); + env.snapLive(0, 1000, 12.0); + CHECK(env.tick() == 12.0); // at the top of the new decay leg, not past the envelope + for (int i = 0; i < 499; ++i) env.tick(); + CHECK(std::fabs(env.tick() - 6.0) < 1e-12); +} + +static void testANoteStartedAfterAPublishSoundsThePublishedEnvelope() { + // End-to-end shape of the snap path: a live commit deliberately leaves the snapshot's own + // sample.play stale, so the ONLY thing standing between a new note and a stale envelope is + // the snap. This is the coverage whose absence let the phi-on-snap bug through. + SampleData s = periodicSine(200000, 64.0); // adsr default: attack 0, sustain 1.0 + LiveParams block; + s.live = █ + LiveValues dialled = foldLive(s.play); + dialled.adsr.attackFrames = 24000; // half a second of attack, dialled before the note + block.publish(dialled); + VoiceEngine engine(1, s); + engine.noteOn(kTestNote, 100); + std::vector out; + engine.render(out, 512); + + // Control: the same stale snapshot with no block at all speaks at full level immediately. + SampleData bare = periodicSine(200000, 64.0); + VoiceEngine bareEngine(1, bare); + bareEngine.noteOn(kTestNote, 100); + std::vector bareOut; + bareEngine.render(bareOut, 512); + const double barePeak = peakOf(bareOut, 0, bareOut.size()); + const double peak = peakOf(out, 0, out.size()); + CHECK(barePeak > 0.9); + CHECK(peak < barePeak * 0.1); // 512 frames into a 24000-frame attack: ~2% of full scale + + // Reverse: a stale LONG attack against a published zero one. The note must speak at full + // level within its first cycle rather than fading in over the smoother's decay. + SampleData slow = periodicSine(200000, 64.0); + slow.play.adsr.attackFrames = 24000; + LiveParams block2; + slow.live = &block2; + LiveValues snappy = foldLive(slow.play); + snappy.adsr.attackFrames = 0; + block2.publish(snappy); + VoiceEngine fast(1, slow); + fast.noteOn(kTestNote, 100); + std::vector fastOut; + fast.render(fastOut, 512); + // Source period 64 read at ratio 2 peaks at output frame 8; a spurious smoother fade-in + // would still be at ~0.34 there. + CHECK(peakOf(fastOut, 0, 32) > 0.9); +} + +// --- Every envelope stage, end to end through the engine --------------------------------- + +// Renders the same note twice — once untouched, once with `mutate` published mid-note — and +// asserts the field reached the SOUNDING voice (the tail diverges) and only after its publish. +static void assertLiveFieldMovesTheSoundingNote(const char* name, void (*rig)(SampleData&), + void (*mutate)(LiveValues&), int noteOffBlock) { + SampleData still = periodicSine(200000, 64.0); + SampleData moved = periodicSine(200000, 64.0); + rig(still); + rig(moved); + LiveParams blockA, blockB; + LiveValues target = foldLive(moved.play); + mutate(target); + + const Run baseline = renderWithLive(still, &blockA, 512, 24, -1, nullptr, noteOffBlock); + const Run edited = renderWithLive(moved, &blockB, 512, 24, 8, &target, noteOffBlock); + + CHECK(baseline.out.size() == edited.out.size()); + double tailDiff = 0.0; + for (std::size_t i = 512 * 9; i < baseline.out.size() && i < edited.out.size(); ++i) { + tailDiff += std::fabs(static_cast(edited.out[i]) - + static_cast(baseline.out[i])); + } + if (!(tailDiff > 1.0)) std::printf(" (never reached the voice: %s)\n", name); + CHECK(tailDiff > 1.0); + + bool preChangeIdentical = true; + for (std::size_t i = 0; i < 512 * 8 && i < baseline.out.size(); ++i) { + if (edited.out[i] != baseline.out[i]) { preChangeIdentical = false; break; } + } + if (!preChangeIdentical) std::printf(" (moved before its publish: %s)\n", name); + CHECK(preChangeIdentical); +} + +static void testEveryEnvelopeStageTimeAndLevelMovesTheSoundingNote() { + // Each rig puts the voice INSIDE the stage under test at the publish (block 8, output + // frame 4096) — a stage already passed cannot move, which is the physics, not a gap. + struct Case { + const char* name; + void (*rig)(SampleData&); + void (*mutate)(LiveValues&); + int noteOffBlock; + }; + const Case cases[] = { + {"amp attack", + [](SampleData& s) { s.play.adsr.attackFrames = 48000; }, + [](LiveValues& v) { v.adsr.attackFrames = 4000; }, -1}, + {"amp hold", + [](SampleData& s) { + s.play.adsr.holdFrames = 48000; + s.play.adsr.decayFrames = 4000; + s.play.adsr.sustainLevel = 0.1; + }, + [](LiveValues& v) { v.adsr.holdFrames = 5000; }, -1}, + {"amp decay", + [](SampleData& s) { + s.play.adsr.decayFrames = 48000; + s.play.adsr.sustainLevel = 0.0; + }, + [](LiveValues& v) { v.adsr.decayFrames = 8000; }, -1}, + {"amp sustain", + [](SampleData& s) { s.play.adsr.sustainLevel = 1.0; }, + [](LiveValues& v) { v.adsr.sustainLevel = 0.2; }, -1}, + {"amp release", + [](SampleData& s) { s.play.adsr.releaseFrames = 48000; }, + [](LiveValues& v) { v.adsr.releaseFrames = 6000; }, 2}, + + // The filter envelope: swept over a low corner with real depth, so its shape is the + // only thing the timbre depends on. The amp release is long so a gated-off voice + // keeps sounding while the filter release is measured. + {"filter env attack", + [](SampleData& s) { filterSweep(s); s.play.filter.env.attackFrames = 48000; }, + [](LiveValues& v) { v.filterEnv.attackFrames = 4000; }, -1}, + {"filter env hold", + [](SampleData& s) { + filterSweep(s); + s.play.filter.env.holdFrames = 48000; + s.play.filter.env.decayFrames = 4000; + s.play.filter.env.sustainLevel = 0.0; + }, + [](LiveValues& v) { v.filterEnv.holdFrames = 5000; }, -1}, + {"filter env decay", + [](SampleData& s) { + filterSweep(s); + s.play.filter.env.decayFrames = 48000; + s.play.filter.env.sustainLevel = 0.0; + }, + [](LiveValues& v) { v.filterEnv.decayFrames = 8000; }, -1}, + {"filter env sustain", + [](SampleData& s) { filterSweep(s); }, + [](LiveValues& v) { v.filterEnv.sustainLevel = 0.0; }, -1}, + {"filter env release", + [](SampleData& s) { + filterSweep(s); + s.play.filter.env.releaseFrames = 48000; + s.play.adsr.releaseFrames = 480000; + }, + [](LiveValues& v) { v.filterEnv.releaseFrames = 6000; }, 2}, + + {"pitch env attack", + [](SampleData& s) { + s.play.pitchEnv.enabled = true; + s.play.pitchEnv.attackFrames = 48000; + s.play.pitchEnv.decayFrames = 48000; + s.play.pitchEnv.peakSemitones = 12.0; + }, + [](LiveValues& v) { v.pitchEnvAttackFrames = 4000; }, -1}, + {"pitch env decay", + [](SampleData& s) { + s.play.pitchEnv.enabled = true; + s.play.pitchEnv.decayFrames = 48000; + s.play.pitchEnv.peakSemitones = 12.0; + }, + [](LiveValues& v) { v.pitchEnvDecayFrames = 8000; }, -1}, + {"pitch env depth", + [](SampleData& s) { + s.play.pitchEnv.enabled = true; + s.play.pitchEnv.decayFrames = 480000; + s.play.pitchEnv.peakSemitones = 12.0; + }, + [](LiveValues& v) { v.pitchEnvPeakSemitones = 0.0; }, -1}, + }; + for (const Case& c : cases) { + assertLiveFieldMovesTheSoundingNote(c.name, c.rig, c.mutate, c.noteOffBlock); + } +} + // --- The filter DSP's glide property, finally exercised --------------------------------- static void testCutoffMoveAcrossPrepareDoesNotStep() { @@ -203,39 +479,6 @@ static void testCutoffMoveAcrossPrepareDoesNotStep() { // --- Delivery into a sounding voice ------------------------------------------------------ -// Renders `blocks` blocks of `blockFrames` through a one-voice engine over `sample`, applying -// `mutate` to the published block after `changeAfter` blocks. Voice-major render order is the -// engine's, so a fixed block size is what makes two runs comparable. -struct Run { - std::vector out; -}; - -// The note is an octave above the root on purpose: key-tracking scales (note - root), so a -// root-note test would leave the key-track control with nothing to move. -static Run renderWithLive(SampleData& sample, LiveParams* block, int blockFrames, int blocks, - int changeAfter, const LiveValues* changed) { - sample.live = block; - if (block) block->publish(foldLive(sample.play)); - VoiceEngine engine(1, sample); - engine.noteOn(72, 100); - Run r; - for (int b = 0; b < blocks; ++b) { - if (block && changed && b == changeAfter) block->publish(*changed); - engine.render(r.out, static_cast(blockFrames)); - } - return r; -} - -static SampleData filteredSine() { - SampleData s = periodicSine(200000, 64.0); - s.play.filter.enabled = true; - s.play.filter.settings.cutoffNorm = 0.8f; - s.play.filter.settings.resonanceNorm = 0.9f; - s.play.filter.settings.morphNorm = 1.0f; - s.play.filter.env.sustainLevel = 1.0; - return s; -} - static void testUnmovedBlockIsByteIdenticalToNoBlockAtAll() { SampleData bare = filteredSine(); SampleData blocked = filteredSine(); @@ -290,10 +533,6 @@ static void testEveryLiveFilterControlMovesTheSoundingNote() { } CHECK(preChangeIdentical); - // And it glided rather than stepping. Measured against the signal's OWN local scale - // frame by frame, because a resonant sweep legitimately grows the output as the corner - // passes the tone — an absolute delta bound would flag that as a click. A step shows - // up instead as one frame far outside the range its own neighbourhood was moving in. // And it ARRIVED as a glide, not as a step. Measured as how far the swept render has // departed from the untouched one in the first frames after the publish, against how // far it departs once settled: a glide has barely begun to diverge, a snapped delivery @@ -303,8 +542,21 @@ static void testEveryLiveFilterControlMovesTheSoundingNote() { // TPT filter preserves state across prepare(), so even an instantaneous coefficient // jump produces no isolated output spike — measured, by defeating the ramp and // re-running, the spike statistic was unchanged while these two numbers converged. + // + // The window is a FRACTION OF THE GLIDE, not a frame count: kLiveRampSeconds is the + // full travel time, so at 1/240 of it a working glide has barely started when the + // window closes. kGlideMargin then puts the bound at the geometric middle of the two + // MEASURED populations — with the ramp in place these six controls ratio 0.0005..0.060; + // with it defeated (every live move delivered as a snap, run) they ratio 0.52..1.10. + // The bound lands at 0.175: ~3x above the worst glide, ~3x below the tamest snap. + const std::size_t rampFrames = + static_cast(instrument::engine::kLiveRampSeconds * kRate); + const std::size_t window = rampFrames / 240; + const double kGlideMargin = 42.0; + const double bound = kGlideMargin * static_cast(window) / + static_cast(rampFrames); double immediate = 0.0; - for (std::size_t i = 512 * 8; i < 512 * 8 + 16; ++i) { + for (std::size_t i = 512 * 8; i < 512 * 8 + window; ++i) { immediate = (std::max)(immediate, std::fabs(static_cast(swept.out[i]) - static_cast(baseline.out[i]))); } @@ -313,15 +565,18 @@ static void testEveryLiveFilterControlMovesTheSoundingNote() { settled = (std::max)(settled, std::fabs(static_cast(swept.out[i]) - static_cast(baseline.out[i]))); } - if (!(immediate <= settled * 0.4)) std::printf(" (glide: %s %f vs %f)\n", c.name, - immediate, settled); - CHECK(immediate <= settled * 0.4); + if (!(immediate <= settled * bound)) + std::printf(" (glide: %s ratio %f vs bound %f)\n", c.name, + settled > 0.0 ? immediate / settled : -1.0, bound); + CHECK(immediate <= settled * bound); } } -static void testDrainSlotVoiceTracksTheSameBlock() { - // Two snapshots, one block — exactly the processor's live_/draining_ shape. A note ringing - // out of the displaced snapshot must answer the knob identically to a live one. +static void testOneBlockServesTwoIndependentObservers() { + // Two snapshots, one block — exactly the processor's live_/draining_ shape. The claim is + // narrow and specific: read() does NOT consume the generation, so the second engine to + // observe a publish sees it as fully as the first. Two identically-built engines are + // otherwise identical by construction, so that is the only thing the comparison pins. SampleData liveSnapshot = filteredSine(); SampleData drainSnapshot = filteredSine(); LiveParams block; @@ -348,18 +603,27 @@ static void testDrainSlotVoiceTracksTheSameBlock() { if (a[i] != b[i]) { same = false; break; } } CHECK(same); - // Non-tautological: the shared block genuinely moved the sound, so "identical" is a claim - // about the drain tracking, not about nothing having happened. + // The shared block genuinely moved the sound, so "identical" is a claim about both + // observers having seen it rather than about nothing having happened. double moveEnergy = 0.0; for (std::size_t i = 512 * 12; i < a.size(); ++i) moveEnergy += std::fabs(a[i]); CHECK(moveEnergy > 1.0); + // And a THIRD observer, after both engines have read it, still sees the same publish. + LiveValues seen; + CHECK(block.read(seen) != 0); + CHECK(seen.filterSettings.cutoffNorm == 0.2f); } // --- What stays latched at note-on ------------------------------------------------------- -static void testVelocityNoteAndPitchStayLatched() { +static void testPitchRatioAndVelocityGainStayLatched() { // A ramp source read under Varispeed: every output frame is (source at readPos) * velocity // gain, so a moved pitch ratio or a moved velocity gain would show up directly. + // + // The filter and the pitch envelope are OFF here on purpose — that is what makes the read + // rate provable arithmetic. It also means the block's filter and pitch-envelope fields + // cannot land on this voice; that they DO land on a voice that has them enabled, and still + // leave the velocity gain alone, is the next test's job. SampleData s; s.frames.resize(100000); for (std::size_t i = 0; i < s.frames.size(); ++i) { @@ -422,16 +686,74 @@ static void testVelocityNoteAndPitchStayLatched() { CHECK(std::fabs(static_cast(out2.back()) - static_cast(out.back())) > 1e-4); } +static void testVelocityGainSurvivesAHostilePublishThatReallyLands() { + // Filter AND pitch envelope enabled, so every field the block carries actually reaches the + // voice. velAmount is 0, so velocity enters the render exactly once — as the amp gain + // latched at note-on — which makes two runs at different velocities exactly proportional + // unless the publish moved that gain (a re-derived gain would have to preserve the ratio + // 100:64 to slip through). + SampleData rig = periodicSine(200000, 64.0); + rig.velocityCurve = VelocityCurve::linear(); + filterSweep(rig); + rig.play.filter.velAmount = 0.0; + rig.play.pitchEnv.enabled = true; + rig.play.pitchEnv.decayFrames = 24000; + rig.play.pitchEnv.peakSemitones = 3.0; + + LiveValues hostile = foldLive(rig.play); + hostile.filterKeyTrack = 2.0; + hostile.filterSettings.cutoffNorm = 0.9f; + hostile.filterModAmount = -1.0; + hostile.filterEnv.decayFrames = 4800; + hostile.filterEnv.sustainLevel = 0.0; + hostile.pitchEnvAttackFrames = 4800; + hostile.pitchEnvDecayFrames = 4800; + hostile.pitchEnvPeakSemitones = 24.0; + hostile.adsr.sustainLevel = 0.4; + + SampleData quiet = rig, loud = rig, untouched = rig; + LiveParams blockQuiet, blockLoud, blockUntouched; + const Run atQuiet = renderWithLive(quiet, &blockQuiet, 512, 16, 2, &hostile, -1, 64); + const Run atLoud = renderWithLive(loud, &blockLoud, 512, 16, 2, &hostile, -1, 100); + const Run noPublish = renderWithLive(untouched, &blockUntouched, 512, 16, -1, nullptr, -1, 64); + + // The publish is not inert: it moved the note it was published into. + double landed = 0.0; + for (std::size_t i = 512 * 3; i < atQuiet.out.size() && i < noPublish.out.size(); ++i) { + landed += std::fabs(static_cast(atQuiet.out[i]) - + static_cast(noPublish.out[i])); + } + CHECK(landed > 1.0); + + // ...and through all of it the two velocities differ by exactly the curve's ratio. + const double ratio = rig.velocityCurve.eval(100.0) / rig.velocityCurve.eval(64.0); + CHECK(ratio > 1.5); // the curve really does separate these two velocities + bool proportional = true; + for (std::size_t i = 0; i < atQuiet.out.size() && i < atLoud.out.size(); ++i) { + if (std::fabs(static_cast(atLoud.out[i]) - + static_cast(atQuiet.out[i]) * ratio) > 1e-6) { + proportional = false; + break; + } + } + CHECK(proportional); +} + int main() { testStageDurationChangeHoldsPhase(); testShortenedStageStillLandsContinuously(); testSustainLevelChangeGlides(); testPitchEnvelopeHoldsPhaseAndGlidesDepth(); + testAFreshEnvelopeTakesANewlyDialledStageTimeOutright(); + testAFreshPitchEnvelopeTakesTheNewTimesOutright(); testCutoffMoveAcrossPrepareDoesNotStep(); testUnmovedBlockIsByteIdenticalToNoBlockAtAll(); + testANoteStartedAfterAPublishSoundsThePublishedEnvelope(); + testEveryEnvelopeStageTimeAndLevelMovesTheSoundingNote(); testEveryLiveFilterControlMovesTheSoundingNote(); - testDrainSlotVoiceTracksTheSameBlock(); - testVelocityNoteAndPitchStayLatched(); + testOneBlockServesTwoIndependentObservers(); + testPitchRatioAndVelocityGainStayLatched(); + testVelocityGainSurvivesAHostilePublishThatReallyLands(); if (g_fail == 0) std::printf("live_delivery tests passed\n"); return g_fail == 0 ? 0 : 1; }