From 7fb778206c7c067858f4e0e0a72c828db0c7436d Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Mon, 27 Jul 2026 01:38:48 -0400 Subject: [PATCH] fix(S12 review): per-zone A/D/S/R reaches voice; zones payload v4 + v3 back-compat Voice::start reads full zone ADSR instead of folding only holdFrames into instrument-wide gateAdsr. Wire format bumped to v4 (four new fields); v3 blobs lift A/D/S/R to kTier0Nominal* constants. Single-capture editor controls now reachable. Minor comment and geometry fixes. --- src/vst/reasampler_editor.cpp | 67 +++++++++++++++++++++++------- src/vst/sample_map.cpp | 27 +++++++++++- src/vst/sample_map.h | 58 +++++++++++++++++++------- src/vst/sampler_core.cpp | 19 ++++++--- tests/test_sample_map.cpp | 60 +++++++++++++++++++++++++++ tests/test_sampler_core.cpp | 77 +++++++++++++++++++++++++++++++---- 6 files changed, 266 insertions(+), 42 deletions(-) diff --git a/src/vst/reasampler_editor.cpp b/src/vst/reasampler_editor.cpp index f0e7df8..9d2f7e6 100644 --- a/src/vst/reasampler_editor.cpp +++ b/src/vst/reasampler_editor.cpp @@ -299,10 +299,13 @@ void ReaSamplerEditor::upsertPickedOverride(const SetupMarkers& m) { namespace { // The S12/S15/S16 control-surface value DOMAINS (the shell owns these — param_slider is -// engine-free and maps only 0..1). Time sliders span [0, max] frames at a nominal rate so a -// full-throw reaches a musically generous ceiling; the exact wall-clock is DAW-verified. These -// are build-time residuals (one place to retune), not persisted. -constexpr double kEnvTimeMaxFrames = 2.0 * 44100.0; // AHDSR A/H/D/R + pitch A/D throw ceiling +// engine-free and maps only 0..1). Time sliders span [0, max] frames at the NOMINAL 44100 Hz +// rate; the stored frame count is host-rate-independent, so at other DAW rates the same slider +// position maps to a slightly different wall-clock duration. The ceiling is kept nominal-only +// because the host rate is not reachable inside the editor without a processor callback, and the +// approximation is musically negligible (±1-2 ms at typical rates). Build-time residual — one +// place to retune; not persisted. +constexpr double kEnvTimeMaxFrames = 2.0 * 44100.0; // AHDSR A/H/D/R + pitch A/D throw ceiling (44100 nominal) constexpr double kPitchDepthMaxSemis = 24.0; // AD pitch depth throw: +/-24 st, centered double clamp01(double v) { return v < 0.0 ? 0.0 : (v > 1.0 ? 1.0 : v); } @@ -600,12 +603,11 @@ Rect zonesStripArea(const EditorBands& bands) { // The S12 numeric-entry field ROW area inside the Zones legend: a band to the right of the // sample label on the legend row. Three equal fields (low/high/root) tile it. Both draw + -// hit-test use this single formula so they never drift. +// hit-test use this single formula so they never drift. Anchored off zonesStripArea.bottom so +// the legend top tracks the strip bottom without re-inlining the strip arithmetic here. Rect noteEntryFieldsArea(const EditorBands& bands) { - const Rect strip = Rect{bands.content.left + 8, - bands.content.top + 4 + 20 + 12 + kStripBandHeight, - bands.content.right - 8, 0}; - const int top = strip.top + 8; // legendTop (== zonesStripArea.bottom + 8) + const int stripBottom = zonesStripArea(bands).bottom; + const int top = stripBottom + 8; // legendTop (== zonesStripArea.bottom + 8) return Rect{bands.content.left + 8 + 128, top, bands.content.right - 8, top + 18}; } @@ -976,8 +978,13 @@ void ReaSamplerEditor::paintZones(LICE_IBitmap* bmp, int w, int h) { } // The S12/S15/S16 parameter surface for the selected zone (play mode + AHDSR / Trigger + - // pitch engine + AD pitch envelope). Only when a zone is selected. - if (selectedZone_ >= 0 && selectedZone_ < static_cast(map_.zones.size())) { + // pitch engine + AD pitch envelope). Shown for an explicit zone selection OR for the + // single-capture face when the map is empty but a capture is picked (S15-F2 lean: the + // single capture is already a one-zone map — one storage site serves both). + const bool haveControlTarget = + (selectedZone_ >= 0 && selectedZone_ < static_cast(map_.zones.size())) || + (map_.zones.empty() && !selectedId_.empty()); + if (haveControlTarget) { paintControls(bmp, zonesControlPanel(computeBands(w, h))); } } @@ -989,8 +996,18 @@ struct ControlLabels { const char* label; const char* seg0; const char* seg1; }; } // namespace void ReaSamplerEditor::paintControls(LICE_IBitmap* bmp, const Rect& panel) { - if (selectedZone_ < 0 || selectedZone_ >= static_cast(map_.zones.size())) return; - const ZonePlayParams play = map_.zones[static_cast(selectedZone_)].play; + // Resolve the play params: from the selected zone when one is chosen, or from the + // PerformanceZone product defaults when the map is empty but a capture is picked + // (S15-F2 lean: the single-capture face shares the same storage site as a one-zone map; + // see paintZones for the gate that reaches here). + ZonePlayParams play; + if (selectedZone_ >= 0 && selectedZone_ < static_cast(map_.zones.size())) { + play = map_.zones[static_cast(selectedZone_)].play; + } else if (map_.zones.empty() && !selectedId_.empty()) { + play = PerformanceZone{}.play; // product defaults (Gate + Preserve + tier-0 ADSR) + } else { + return; // no control target + } const std::vector descs = controlDescs(play); const std::vector rows = layoutControls(panel, descs); @@ -1259,7 +1276,29 @@ void ReaSamplerEditor::onMouseDown(int x, int y) { entryField_ = -1; // a click elsewhere in the Zones view cancels an in-progress entry // The S12/S15/S16 parameter panel: a toggle segment flips at once (commit); a slider grab - // starts a live drag (commit on release). Only when a zone is selected. + // starts a live drag (commit on release). Reachable for an explicit zone selection OR for + // the single-capture face when the map is empty but a capture is picked (S15-F2 lean). + // In the empty-map+picked case, auto-create a full-keyboard zone for selectedId_ on first + // control interaction (same path as "+ Add Zone"), then apply the control — the zone is + // committed as part of the control edit. + if (selectedZone_ < 0 && map_.zones.empty() && !selectedId_.empty()) { + // Synthesize a probe layout with the product defaults to see if the click is in the + // panel before committing to creating the zone. + const Rect panel = zonesControlPanel(bands); + const ZonePlayParams defaultPlay = PerformanceZone{}.play; + const std::vector probeDescs = controlDescs(defaultPlay); + const std::vector probeRows = layoutControls(panel, probeDescs); + if (controlAtPoint(probeRows, x, y) >= 0) { + // The click lands in the control panel — materialize the zone now. + PerformanceZone z; + z.sampleId = selectedId_; + z.lowNote = 0; + z.highNote = 127; + map_.zones.push_back(z); + selectedZone_ = 0; + // Fall through to the control handler below which will process the click. + } + } if (selectedZone_ >= 0 && selectedZone_ < static_cast(map_.zones.size())) { PerformanceZone& z = map_.zones[static_cast(selectedZone_)]; const Rect panel = zonesControlPanel(bands); diff --git a/src/vst/sample_map.cpp b/src/vst/sample_map.cpp index 90a937c..bfa7275 100644 --- a/src/vst/sample_map.cpp +++ b/src/vst/sample_map.cpp @@ -356,6 +356,13 @@ void putZonesPayload(std::vector& out, const PerformanceMap& map) putU64le(out, asU64(pp.pitchEnv.attackFrames)); putU64le(out, asU64(pp.pitchEnv.decayFrames)); putU64le(out, doubleToBits(pp.pitchEnv.peakSemitones)); + // S12 review fix (PAYLOAD v4): the full per-zone AHDSR A/D/S/R tail (always present in v4). + // Voice::start now uses the zone's full ADSR; old v3 blobs lift to tier-0 nominal defaults + // at read time (see readZonesPayload) so the voice sounds bit-identical to the pre-fix build. + putU64le(out, asU64(pp.adsr.attackFrames)); + putU64le(out, asU64(pp.adsr.decayFrames)); + putU64le(out, doubleToBits(pp.adsr.sustainLevel)); + putU64le(out, asU64(pp.adsr.releaseFrames)); } } @@ -367,11 +374,13 @@ void putZonesPayload(std::vector& out, const PerformanceMap& map) void readZonesPayload(ByteReader& r, PerformanceMap& map) { bool extended = false; // v2+: the S11 loop/start tail is present bool hasPlay = false; // v3+: the S15/S16 play-params tail is present + bool hasAdsr = false; // v4+: the full A/D/S/R per-zone tail is present if (r.peekU32() == kZonesFormatMarker) { r.u32(); // consume the marker const std::uint32_t pv = r.u32(); // payload version extended = (pv >= 2); // v2+ carries the loop/start tail hasPlay = (pv >= 3); // v3+ carries the S15/S16 play-params tail + hasAdsr = (pv >= 4); // v4+ carries the full A/D/S/R tail } const std::uint32_t count = r.u32(); for (std::uint32_t i = 0; i < count && r.ok; ++i) { @@ -397,7 +406,7 @@ void readZonesPayload(ByteReader& r, PerformanceMap& map) { if (hasStart) z.startPoint = r.i64(); } if (hasPlay) { - // S15/S16 play params, always present in a v3 record (read in the emit order). + // S15/S16 play params, always present in a v3+ record (read in the emit order). z.play.playMode = (r.u8() != 0) ? PlayMode::Trigger : PlayMode::Gate; z.play.adsr.holdFrames = r.i64(); z.play.trigger.lengthFraction = bitsToDouble(r.u64()); @@ -409,6 +418,22 @@ void readZonesPayload(ByteReader& r, PerformanceMap& map) { z.play.pitchEnv.decayFrames = r.i64(); z.play.pitchEnv.peakSemitones = bitsToDouble(r.u64()); } + if (hasAdsr) { + // S12 review fix (v4): the full per-zone A/D/S/R tail — read in the emit order. + z.play.adsr.attackFrames = r.i64(); + z.play.adsr.decayFrames = r.i64(); + z.play.adsr.sustainLevel = bitsToDouble(r.u64()); + z.play.adsr.releaseFrames = r.i64(); + } else if (hasPlay) { + // v3 blob: A/D/S/R fields are absent. Lift to the tier-0 nominal defaults (44100 Hz) + // so a voice playing this zone sounds bit-identical to the pre-v4 build (back-compat). + // Voice::start now uses the zone's full ADSR; a zone with these values reproduces + // the instrument-wide tier0Adsr behavior the pre-fix code applied unconditionally. + z.play.adsr.attackFrames = kTier0NominalAttackFrames; + z.play.adsr.decayFrames = kTier0NominalDecayFrames; + z.play.adsr.sustainLevel = kTier0NominalSustainLevel; + z.play.adsr.releaseFrames = kTier0NominalReleaseFrames; + } if (!r.ok) break; // truncated mid-zone -> keep what parsed cleanly, drop the rest map.zones.push_back(std::move(z)); } diff --git a/src/vst/sample_map.h b/src/vst/sample_map.h index b51430d..866bfef 100644 --- a/src/vst/sample_map.h +++ b/src/vst/sample_map.h @@ -107,6 +107,19 @@ std::vector downmixToMono(const std::vector& interleav std::vector extractChannel(const std::vector& interleaved, int channelCount, int which); +// Nominal tier-0 ADSR defaults (frames at 44100 Hz) — used when lifting a pre-v4 zones payload +// blob whose per-zone A/D/S/R fields are absent. These reproduce the instrument-wide tier0Adsr +// behavior (0.003 s attack, 0.0 s decay, sustain 1.0, 0.060 s release at 44100 Hz) so that a +// zone loaded from an old blob sounds bit-identical to the pre-v4 build. DAWs at other sample +// rates will be close but not exact (the same approximation the instrument already makes when +// building tier0Adsr from its compile-time kAttackSeconds / kReleaseSeconds constants). +// Must be declared before buildTier0Keymap (default arg) and PerformanceZone / ResolvedZone +// (member initializers) — both of which reference these values. +inline constexpr std::int64_t kTier0NominalAttackFrames = 132; // 0.003 * 44100, rounded +inline constexpr std::int64_t kTier0NominalDecayFrames = 0; +inline constexpr double kTier0NominalSustainLevel = 1.0; +inline constexpr std::int64_t kTier0NominalReleaseFrames = 2646; // 0.060 * 44100 + // Build the Tier-0 chromatic keymap for one decoded sample: one zone spanning the whole // keyboard, repitched from `rootNote`, looped per `loop`. The single-sample degenerate case // (Keymap::singleSampleChromatic) with the S2 intrinsics threaded in. `frames` is channel 0 @@ -122,8 +135,11 @@ Keymap buildTier0Keymap(std::vector frames, int sampleRate, int rootNote, const SampleLoop& loop, std::vector framesR = {}, const ZonePlayParams& play = ZonePlayParams{ - PlayMode::Gate, AdsrParams{}, TriggerParams{}, kDefaultPitchEngine, - PitchEnvParams{}}); + PlayMode::Gate, + AdsrParams{kTier0NominalAttackFrames, 0, + kTier0NominalDecayFrames, kTier0NominalSustainLevel, + kTier0NominalReleaseFrames}, + TriggerParams{}, kDefaultPitchEngine, PitchEnvParams{}}); // --- Performance map (Tier 1, D-B: the instrument's OWN state) --------------- // @@ -155,16 +171,19 @@ struct PerformanceZone { std::optional loopOverride; // instrument-owned sustain loop; absent -> bank intrinsic std::optional startPoint; // instrument-owned initial read frame; absent -> 0 - // S15/S16 per-zone play parameters (play mode + AHDSR hold + Trigger %-length/fades; pitch + // S15/S16 per-zone play parameters (play mode + AHDSR + Trigger %-length/fades; pitch // engine + AD pitch envelope). Instrument-owned (D-B), never a bank fact — mirror of the // loop/start overrides. Defaults to the PRODUCT defaults for a NEW zone: Gate play mode, - // hold 0, no fades, and the PRESERVE pitch engine (S16-F1 — Daniel's directive; the one - // flippable default is sampler_core::kDefaultPitchEngine), pitch envelope off. An older - // zone-payload blob (no S15/S16 tail) lifts to exactly these defaults on read (see the - // PAYLOAD v3 versioning in the (de)serialize section), so a pre-S15 instrument opens with - // Gate + Preserve — the deliberate, spec-flagged behavior change. - ZonePlayParams play{PlayMode::Gate, AdsrParams{}, TriggerParams{}, kDefaultPitchEngine, - PitchEnvParams{}}; + // AHDSR with the tier-0 nominal A/D/S/R (kTier0Nominal* at 44100 Hz — the same values a + // v3 blob lifts to), hold 0, no fades, and the PRESERVE pitch engine (S16-F1), pitch env + // off. An older zone-payload blob (no S15/S16 tail or no v4 A/D/S/R tail) lifts to exactly + // these defaults on read (see the PAYLOAD v3/v4 versioning), so a pre-v4 instrument opens + // with Gate + Preserve + tier-0 ADSR — the deliberate back-compat path. + ZonePlayParams play{PlayMode::Gate, + AdsrParams{kTier0NominalAttackFrames, 0, + kTier0NominalDecayFrames, kTier0NominalSustainLevel, + kTier0NominalReleaseFrames}, + TriggerParams{}, kDefaultPitchEngine, PitchEnvParams{}}; }; // The instrument's performance map: an ordered list of zones. Order is authoritative for @@ -188,7 +207,11 @@ struct ResolvedZone { int rootNote = 60; // effective: override, else bank intrinsic, else 60 SampleLoop loop; // effective: loopOverride, else bank S2 intrinsic (S11) std::int64_t startFrame = 0; // effective initial read frame: startPoint, else 0 (S11) - ZonePlayParams play{PlayMode::Gate, AdsrParams{}, TriggerParams{}, kDefaultPitchEngine, + ZonePlayParams play{PlayMode::Gate, + AdsrParams{kTier0NominalAttackFrames, 0, + kTier0NominalDecayFrames, kTier0NominalSustainLevel, + kTier0NominalReleaseFrames}, + TriggerParams{}, kDefaultPitchEngine, PitchEnvParams{}}; // S15/S16 per-zone play params (carried through as-is) }; @@ -273,8 +296,7 @@ DecodedZonePcm decodeChannels(const std::vector& interleaved, // appended to each zone record after the S11 startPoint tail (the S15/S16 per-zone play // params — always present, NOT flag-gated, since every zone has a play mode + engine): // 1 byte playMode (0 = Gate, 1 = Trigger); -// 8-byte LE adsr.holdFrames (int64) — the S15 AHDSR hold stage (A/D/S/R timing stays -// instrument-wide; only hold is per-zone); +// 8-byte LE adsr.holdFrames (int64) — the S15 AHDSR hold stage; // 8-byte LE trigger.lengthFraction as an IEEE-754 double (bit-cast to u64 LE); // 8-byte LE trigger.fadeInFrames (int64); 8-byte LE trigger.fadeOutFrames (int64); // 1 byte pitchEngine (0 = Varispeed, 1 = Preserve); @@ -283,6 +305,14 @@ DecodedZonePcm decodeChannels(const std::vector& interleaved, // A v1/v2 payload (no v3 tail) lifts each zone to the PRODUCT defaults (Gate + Preserve + // no fades + disabled pitch env) — the deliberate S16-F1 behavior change for already-saved // instruments. A truncated mid-v3-tail record keeps the zones that parsed and drops the rest. +// * PAYLOAD v4 (S12 review fix): the same marker + payload version (== 4), THEN the v3 body +// PLUS, appended to each zone record after the v3 pitch-env tail, the full per-zone A/D/S/R: +// 8-byte LE adsr.attackFrames (int64); 8-byte LE adsr.decayFrames (int64); +// 8-byte LE adsr.sustainLevel as an IEEE-754 double (bit-cast to u64 LE); +// 8-byte LE adsr.releaseFrames (int64). +// A v3 payload (no v4 A/D/S/R tail) lifts those fields to the tier-0 nominal defaults at +// 44100 Hz (kTier0Nominal* constants) so a voice using the zone ADSR sounds bit-identical to +// the pre-v4 build. Voice::start now uses the zone's full ADSR for all five AHDSR fields. // BACK-COMPAT: a v1 ENVELOPE blob (the S4 single-selection format: version tag 1 + id bytes) is // lifted to a single full-keyboard zone playing that id (no override) — so an instance saved // under Tier 0 restores as a one-zone Tier-1 map. A truncated/unknown/empty blob deserializes @@ -302,7 +332,7 @@ inline constexpr std::uint32_t kPerformanceStateVersion = 2; // (marker + version 2, no play tail) for back-compat, lifting the missing fields to defaults. // The marker is a high sentinel that a legitimate zone count (bounded by 128 MIDI zones in // practice, always tiny) can never collide with. -inline constexpr std::uint32_t kZonesPayloadVersion = 3; // S15/S16: per-zone play params tail +inline constexpr std::uint32_t kZonesPayloadVersion = 4; // S15/S16 A/D/S/R per-zone tail inline constexpr std::uint32_t kZonesFormatMarker = 0xFFFFFF00u; // The performance map serialized to bytes for IBStream (getState). diff --git a/src/vst/sampler_core.cpp b/src/vst/sampler_core.cpp index 4aa2f16..6cd2336 100644 --- a/src/vst/sampler_core.cpp +++ b/src/vst/sampler_core.cpp @@ -276,12 +276,21 @@ void Voice::start(int note, int velocity, const SampleData& sample, int rootNote readPos_ = static_cast(start); startFrame_ = start; // Trigger fade offset origin (readPos - startFrame = span offset) - // --- Amplitude envelope: Gate = AHDSR (instrument A/D/S/R + per-zone HOLD); Trigger = the - // time-boxed fade-in/out over the % play length. --- + // --- Amplitude envelope: Gate = AHDSR (fully per-zone: A/H/D/S/R all read from the zone's + // play.adsr); Trigger = the time-boxed fade-in/out over the % play length. + // + // gateAdsr (instrument-wide tier0) is the fallback for zones deserialized from a pre-v4 + // payload blob (see sample_map zones-payload v4): those zones carry their adsr fields + // pre-populated at deserialize time with the tier0 nominal defaults so back-compat holds + // (see sample_map.cpp readZonesPayload). For a new or fully-round-tripped zone the field + // is purely from the zone's own state; gateAdsr is still passed here for the Tier-0 + // single-capture path (buildTier0Keymap supplies a ZonePlayParams that already seeds the + // ADSR defaults from the product defaults; the voice reads those directly). + // + // Back-compat invariant: a zone whose adsr fields carry the product defaults (the tier0 + // nominal at 44100) sounds bit-identical to the pre-fix build. --- if (playMode_ == PlayMode::Gate) { - AdsrParams a = gateAdsr; - a.holdFrames = p.adsr.holdFrames; // per-zone hold folds into the instrument-wide AHDSR - env_.configure(a); + env_.configure(p.adsr); env_.noteOn(); playEnd_ = 0; // unused in Gate } else { diff --git a/tests/test_sample_map.cpp b/tests/test_sample_map.cpp index 3607d11..5fcd4a3 100644 --- a/tests/test_sample_map.cpp +++ b/tests/test_sample_map.cpp @@ -1138,6 +1138,64 @@ static void testPlayParamsThroughComponentEnvelope() { CHECK(back.map.zones[0].play.pitchEngine == PitchEngine::Varispeed); } +// --- S12 review fix (PAYLOAD v4): full A/D/S/R per-zone round-trip. --------------------- +// +// Before the fix, per-zone A/D/S/R (attack/decay/sustain/release) was not serialized; +// only holdFrames was written. These two tests assert the corrected v4 path. + +// All five AHDSR fields (including the four new A/D/S/R) must round-trip through the v4 payload. +static void testFullAdsrV4RoundTrip() { + PerformanceMap m; + PerformanceZone z = zone("pad", 0, 127); + z.play.playMode = PlayMode::Gate; + z.play.adsr.attackFrames = 441; // 0.01 s at 44100 Hz (a non-default value) + z.play.adsr.holdFrames = 882; + z.play.adsr.decayFrames = 4410; // 0.1 s + z.play.adsr.sustainLevel = 0.7; + z.play.adsr.releaseFrames = 8820; // 0.2 s + z.play.pitchEngine = PitchEngine::Preserve; + m.zones.push_back(z); + const PerformanceMap back = deserializePerformance(serializePerformance(m)); + CHECK(back.zones.size() == 1); + if (back.zones.size() != 1) return; + const AdsrParams& a = back.zones[0].play.adsr; + CHECK(a.attackFrames == 441); + CHECK(a.holdFrames == 882); + CHECK(a.decayFrames == 4410); + CHECK(a.sustainLevel == 0.7); // exact double round-trip via bit-cast + CHECK(a.releaseFrames == 8820); + CHECK(back.zones[0].play.pitchEngine == PitchEngine::Preserve); +} + +// A genuine PAYLOAD v3 blob (S15/S16 build — has holdFrames but not A/D/S/R) must lift +// attackFrames / decayFrames / sustainLevel / releaseFrames to the tier-0 nominal defaults +// (kTier0Nominal* constants) so the voice sounds bit-identical to the pre-fix behavior. +// We reuse handBuildV3PayloadOneZone which emits a valid marker-versioned v3 payload. +static void testV3BlobLiftsAdsrToNominalDefaults() { + // Build a PERFORMANCE blob: 4-byte kPerformanceStateVersion header + v3 zones payload. + // deserializePerformance strips the 4-byte header and passes the rest to readZonesPayload, + // which self-selects the v3 record shape from the payload marker+version — exercising the + // real production lift path for a user who saved on the S15 build. + std::vector blob; + auto u32 = [&](std::uint32_t v) { + blob.push_back(v & 0xFF); blob.push_back((v >> 8) & 0xFF); + blob.push_back((v >> 16) & 0xFF); blob.push_back((v >> 24) & 0xFF); + }; + u32(kPerformanceStateVersion); // envelope version 2 header + const std::vector payload = handBuildV3PayloadOneZone("old"); + blob.insert(blob.end(), payload.begin(), payload.end()); + const PerformanceMap back = deserializePerformance(blob); + CHECK(back.zones.size() == 1); + if (back.zones.size() != 1) return; + const AdsrParams& a = back.zones[0].play.adsr; + // holdFrames comes from the v3 record itself; A/D/S/R must lift to the nominal tier-0 values. + CHECK(a.holdFrames == 2048); // from the hand-built v3 record + CHECK(a.attackFrames == kTier0NominalAttackFrames); // 132 (0.003 s at 44100) + CHECK(a.decayFrames == kTier0NominalDecayFrames); // 0 + CHECK(a.sustainLevel == kTier0NominalSustainLevel); // 1.0 + CHECK(a.releaseFrames == kTier0NominalReleaseFrames); // 2646 (0.060 s at 44100) +} + int main() { testSelectByIdHit(); testSelectEmptyIdIsSilence(); @@ -1189,6 +1247,8 @@ int main() { testPlayParamsComposeWithLoopStart(); testPlayParamsV2BackCompatLiftsToDefaults(); testPlayParamsThroughComponentEnvelope(); + testFullAdsrV4RoundTrip(); + testV3BlobLiftsAdsrToNominalDefaults(); testComponentStateRoundTrip(); testComponentStateLoopStartRoundTrip(); testComponentStateSelectionOnlyNoZones(); diff --git a/tests/test_sampler_core.cpp b/tests/test_sampler_core.cpp index b7282a4..7622d22 100644 --- a/tests/test_sampler_core.cpp +++ b/tests/test_sampler_core.cpp @@ -383,10 +383,14 @@ static void testOutOfZoneNoteConsumesNoVoice() { // --------------------------------------------------------------------------- static void testStealsReleasingVoiceFirst() { - Keymap km = Keymap::singleSampleChromatic(dcSample(100000, 60)); - AdsrParams a = flatAdsr(); - a.releaseFrames = 100000; // long release so a released voice stays "active". - VoiceEngine eng(2, km, a); + // Long per-zone release so the voice stays active through the release tail. + // Per the S12 fix, Voice::start uses sample.play.adsr — not the engine's gateAdsr — + // so the long release must live on the SampleData, not on the VoiceEngine constructor arg. + SampleData s = dcSample(100000, 60); + s.play.adsr = flatAdsr(); + s.play.adsr.releaseFrames = 100000; // long release so a released voice stays "active" + Keymap km = Keymap::singleSampleChromatic(std::move(s)); + VoiceEngine eng(2, km, flatAdsr()); std::size_t vA = eng.noteOn(60, 100); // startOrder 1 std::size_t vB = eng.noteOn(62, 100); // startOrder 2 @@ -406,10 +410,12 @@ static void testStealsReleasingVoiceFirst() { } static void testStealsOldestWhenNoneReleasing() { - Keymap km = Keymap::singleSampleChromatic(dcSample(100000, 60)); - AdsrParams a = flatAdsr(); - a.releaseFrames = 100000; - VoiceEngine eng(2, km, a); + // Long per-zone release — placed on SampleData.play.adsr per the S12 fix. + SampleData s = dcSample(100000, 60); + s.play.adsr = flatAdsr(); + s.play.adsr.releaseFrames = 100000; + Keymap km = Keymap::singleSampleChromatic(std::move(s)); + VoiceEngine eng(2, km, flatAdsr()); std::size_t vA = eng.noteOn(60, 100); // startOrder 1 (oldest) std::size_t vB = eng.noteOn(62, 100); // startOrder 2 @@ -1177,6 +1183,57 @@ static void testPreserveVoiceCap() { CHECK(eng.activeVoiceCount() == 2); } +// --- S12 review fix: per-zone A/D/S/R actually reaches the voice envelope. --- +// +// Before the fix, Voice::start used the instrument-wide gateAdsr for A/D/S/R and only +// folded the per-zone holdFrames. These two tests assert the corrected path. + +// The zone's attackFrames drives the envelope ramp — NOT the VoiceEngine's gateAdsr. +// Strategy: give the VoiceEngine a FLAT gateAdsr (instant attack) but put an explicit +// 10-frame attack on the SampleData.play.adsr. If Voice::start reads the zone ADSR, the +// DC-1 output will be 0 at frame 0 and 1.0 after the 10-frame ramp. If it instead used +// gateAdsr (flat = instant), frame 0 would already be 1.0. This is the load-bearing proof. +static void testPerZoneAdsrReachesVoiceEnvelope() { + SampleData s = dcSample(500, 60); + // Per-zone attack = 10 frames, zero decay, sustain 1.0, zero release. + s.play.adsr.attackFrames = 10; + s.play.adsr.holdFrames = 0; + s.play.adsr.decayFrames = 0; + s.play.adsr.sustainLevel = 1.0; + s.play.adsr.releaseFrames = 0; + s.play.pitchEngine = PitchEngine::Varispeed; // isolate from pitch engine machinery + Keymap km = Keymap::singleSampleChromatic(std::move(s)); + VoiceEngine eng(1, km, flatAdsr()); // instrument-wide gateAdsr = flat (instant attack) + eng.noteOn(60, 127); // unity pitch, full velocity -> gain 1.0 + std::vector out; + eng.render(out, 20); + // Frame 0: attack start, envelope near 0. If gateAdsr (flat) were used, this would be 1.0. + CHECK(approx(out[0], 0.0, 1e-9)); // env still at bottom of ramp + // Frame 9: still ramping (last attack frame, linear ramp reaches 0.9). + CHECK(out[9] < 1.0 - 1e-9); + // Frame 10+: attack complete, sustain at 1.0. + CHECK(approx(out[10], 1.0, 1e-9)); + CHECK(approx(out[19], 1.0, 1e-9)); +} + +// Default-valued zone (AdsrParams all zeros) is behavior-identical to the pre-fix flat path. +// A zero-init AdsrParams (attackFrames=0, decayFrames=0, sustainLevel=1.0, releaseFrames=0) must +// yield an instant-attack/instant-sustain voice — frame 0 immediately at 1.0. This preserves the +// back-compat invariant: an old zone with no A/D/S/R storage sounds the same as before. +static void testZeroAdsrIsInstantSustain() { + SampleData s = dcSample(20, 60); + // Default AdsrParams{}: all zeros, sustainLevel = 1.0 (struct default). No attack ramp. + s.play.adsr = AdsrParams{}; + s.play.pitchEngine = PitchEngine::Varispeed; + Keymap km = Keymap::singleSampleChromatic(std::move(s)); + VoiceEngine eng(1, km, flatAdsr()); + eng.noteOn(60, 127); + std::vector out; + eng.render(out, 5); + // All frames must be 1.0: zero attack + sustain 1.0 = instantly at full level. + for (std::size_t i = 0; i < out.size(); ++i) CHECK(approx(out[i], 1.0, 1e-9)); +} + int main() { testChromaticSingleRoot(); testZonedRangesBoundaries(); @@ -1228,6 +1285,10 @@ int main() { testPreserveGateStereoLoopComposes(); testPreserveVoiceCap(); + // S12 review fix — per-zone A/D/S/R reaches the voice envelope. + testPerZoneAdsrReachesVoiceEnvelope(); + testZeroAdsrIsInstantSustain(); + if (g_fail == 0) { std::printf("all sampler_core tests passed\n"); return 0;