diff --git a/src/vst/reasampler_editor.cpp b/src/vst/reasampler_editor.cpp index 9d2f7e6..0d4915c 100644 --- a/src/vst/reasampler_editor.cpp +++ b/src/vst/reasampler_editor.cpp @@ -269,32 +269,31 @@ ReaSamplerEditor::SetupMarkers ReaSamplerEditor::pickedMarkers(std::int64_t fram return m; } -void ReaSamplerEditor::upsertPickedOverride(const SetupMarkers& m) { +int ReaSamplerEditor::upsertPickedOverride(const SetupMarkers& m) { // Find-or-append the zone for selectedId_ and write the loop/start override fields. // The bank intrinsic is NEVER written (read-only bank consumer, D-B). selectedId_ must // be non-empty; callers are responsible for that guard. + // Returns the zone index (0-based) so callers can update selectedZone_. SampleLoop loop; loop.hasLoop = m.hasLoop; loop.start = m.loopStart; loop.end = m.loopEnd; - bool found = false; - for (PerformanceZone& z : map_.zones) { + for (int i = 0; i < static_cast(map_.zones.size()); ++i) { + PerformanceZone& z = map_.zones[static_cast(i)]; if (z.sampleId == selectedId_) { z.loopOverride = loop; z.startPoint = m.start; - found = true; - break; + return i; } } - if (!found) { - PerformanceZone z; - z.sampleId = selectedId_; - z.lowNote = 0; - z.highNote = 127; - z.loopOverride = loop; - z.startPoint = m.start; - map_.zones.push_back(z); - } + PerformanceZone z; + z.sampleId = selectedId_; + z.lowNote = 0; + z.highNote = 127; + z.loopOverride = loop; + z.startPoint = m.start; + map_.zones.push_back(z); + return static_cast(map_.zones.size()) - 1; } namespace { @@ -1322,6 +1321,14 @@ void ReaSamplerEditor::onMouseDown(int x, int y) { dragParamPanel_ = panel; dragStartMap_ = map_; applyControl(id, z.play, valueAtPoint(r.control, x), 0); + // An explicit ADSR slider touch commits a rate-resolved value (the slider + // maps 0..1 -> editor-domain frames at kEnvTimeMaxFrames, not nominal 44100-Hz + // counts). Mark the zone as no longer needing rate-resolve so buildZonedKeymap + // does not re-rescale the value at reload time. + if (id >= static_cast(ParamControl::kAttack) && + id <= static_cast(ParamControl::kRelease)) { + z.adsrNeedsRateResolve = false; + } invalidate(); // live feedback; commit on WM_LBUTTONUP } break; @@ -1352,9 +1359,18 @@ void ReaSamplerEditor::onMouseMove(int x, int y) { // zone over the whole keyboard) and round-trips through the v3 component state; the // zone becomes visible if the user opens the Zones panel. Upsert by the picked id so a // repeated drag edits the same zone rather than stacking duplicates. + // Upsert the root override on the picked id; track the zone index so the control panel + // stays visible after the zone is materialized on the single-capture face (fix: without + // setting selectedZone_ here, selectedZone_==-1 with a non-empty map hides controls). bool found = false; - for (PerformanceZone& z : map_.zones) { - if (z.sampleId == selectedId_) { z.rootOverride = note; found = true; break; } + for (int i = 0; i < static_cast(map_.zones.size()); ++i) { + PerformanceZone& z = map_.zones[static_cast(i)]; + if (z.sampleId == selectedId_) { + z.rootOverride = note; + selectedZone_ = i; + found = true; + break; + } } if (!found) { PerformanceZone z; @@ -1363,6 +1379,7 @@ void ReaSamplerEditor::onMouseMove(int x, int y) { z.highNote = 127; z.rootOverride = note; map_.zones.push_back(z); + selectedZone_ = static_cast(map_.zones.size()) - 1; } invalidate(); // live feedback; the commit lands on WM_LBUTTONUP return; @@ -1407,8 +1424,10 @@ void ReaSamplerEditor::onMouseMove(int x, int y) { if (m.start > frames - 1) m.start = frames - 1; // Upsert the override on the picked id (mirror of the root-marker path); commit lands on - // release, this is live feedback. - upsertPickedOverride(m); + // release, this is live feedback. Set selectedZone_ so the control panel stays visible + // after the zone is materialized (fix: without this, selectedZone_==-1 with a non-empty + // map hides controls after the first marker drag on the single-capture face). + selectedZone_ = upsertPickedOverride(m); invalidate(); return; } diff --git a/src/vst/reasampler_editor.h b/src/vst/reasampler_editor.h index d8af4af..acf8bd5 100644 --- a/src/vst/reasampler_editor.h +++ b/src/vst/reasampler_editor.h @@ -172,8 +172,9 @@ private: // Write `m` as a loop/start override upsert into map_ for selectedId_ (find-or-append). // Does NOT call commitAndReload — callers decide whether this is a live-drag update or a - // final commit. selectedId_ must be non-empty before calling. - void upsertPickedOverride(const SetupMarkers& m); + // final commit. selectedId_ must be non-empty before calling. Returns the zone index + // (0-based) that was updated or appended, so callers can set selectedZone_. + int upsertPickedOverride(const SetupMarkers& m); // --- S12/S15/S16 parameter surface (Zones panel, keyed to selectedZone_) ------ // diff --git a/src/vst/sample_map.cpp b/src/vst/sample_map.cpp index bfa7275..d0d44e8 100644 --- a/src/vst/sample_map.cpp +++ b/src/vst/sample_map.cpp @@ -147,6 +147,23 @@ Keymap buildTier0Keymap(std::vector frames, int sampleRate, data.rootNote = rootNote; data.loop = loop; data.play = play; // S15/S16 single-capture play params (product defaults unless overridden) + + // The default `play` arg carries 44100-Hz nominal ADSR frame counts (kTier0Nominal*). + // Rescale A/D/R by (sampleRate / 44100) so the wall-clock ADSR matches tier0Adsr(sampleRate) + // exactly. Sustain (a level, not a frame count) is unchanged. At 44100 the factor is 1.0 — + // bit-identical to the pre-fix build. The tier-0 single-capture path never carries user-edited + // ADSR (users edit ADSR through zones, which go through buildZonedKeymap), so always rescaling + // here is correct and safe. + if (data.sampleRate != 44100) { + const double factor = static_cast(data.sampleRate) / 44100.0; + data.play.adsr.attackFrames = static_cast( + static_cast(data.play.adsr.attackFrames) * factor + 0.5); + data.play.adsr.decayFrames = static_cast( + static_cast(data.play.adsr.decayFrames) * factor + 0.5); + data.play.adsr.releaseFrames = static_cast( + static_cast(data.play.adsr.releaseFrames) * factor + 0.5); + } + return Keymap::singleSampleChromatic(std::move(data)); } @@ -190,6 +207,8 @@ ResolvedPerformance resolvePerformance(const std::string& banksJson, // S15/S16 per-zone play params carry through unchanged (they are instrument state, not // resolved against the bank) so the keymap build can stamp them onto the SampleData. rz.play = z.play; + // Carry the rate-resolve flag so buildZonedKeymap can rescale 44100-nominal ADSR counts. + rz.adsrNeedsRateResolve = z.adsrNeedsRateResolve; out.zones.push_back(std::move(rz)); } return out; @@ -215,6 +234,20 @@ Keymap buildZonedKeymap(const std::vector& zones, data.loop = zones[i].loop; data.startFrame = zones[i].startFrame; // S11 effective start (override, else 0) data.play = zones[i].play; // S15/S16 per-zone play mode + engine + envelopes + + // If the zone's ADSR came from a v3 lift or a new-zone default (adsrNeedsRateResolve), + // its A/D/R frame counts are 44100-Hz nominals. Rescale to the WAV's actual rate so + // wall-clock ADSR durations match tier0Adsr(sampleRate) exactly. v4 zones (user-edited + // frame counts) carry adsrNeedsRateResolve=false and are left unchanged. + if (zones[i].adsrNeedsRateResolve && data.sampleRate != 44100) { + const double factor = static_cast(data.sampleRate) / 44100.0; + data.play.adsr.attackFrames = static_cast( + static_cast(data.play.adsr.attackFrames) * factor + 0.5); + data.play.adsr.decayFrames = static_cast( + static_cast(data.play.adsr.decayFrames) * factor + 0.5); + data.play.adsr.releaseFrames = static_cast( + static_cast(data.play.adsr.releaseFrames) * factor + 0.5); + } const std::size_t sampleIndex = km.samples.size(); km.samples.push_back(std::move(data)); KeyZone zone; @@ -420,10 +453,12 @@ void readZonesPayload(ByteReader& r, PerformanceMap& map) { } if (hasAdsr) { // S12 review fix (v4): the full per-zone A/D/S/R tail — read in the emit order. + // These values were authored at the DAW's rate; mark them resolved (no rescaling). 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(); + z.adsrNeedsRateResolve = false; // already rate-resolved; do NOT rescale at keymap build } 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). diff --git a/src/vst/sample_map.h b/src/vst/sample_map.h index 866bfef..76edc4b 100644 --- a/src/vst/sample_map.h +++ b/src/vst/sample_map.h @@ -108,11 +108,13 @@ std::vector extractChannel(const std::vector& interlea 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). +// blob whose per-zone A/D/S/R fields are absent, and as the initializer for new zones / the +// buildTier0Keymap default play arg. These are 44100-Hz nominal frame counts; buildTier0Keymap +// and buildZonedKeymap both rescale the A/D/R frame counts by (sampleRate / 44100) at build +// time when adsrNeedsRateResolve is set on the zone — so a v3-lifted or default zone plays with +// the same wall-clock ADSR as tier0Adsr(liveRate) did before the fix. Bit-identical at 44100 Hz. +// Zones loaded from a v4 blob (explicitly user-edited) carry adsrNeedsRateResolve=false and are +// never rescaled — their stored frame counts already reflect the rate at which they were authored. // 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 @@ -129,8 +131,9 @@ inline constexpr std::int64_t kTier0NominalReleaseFrames = 2646; // 0.060 * 441 // pair never half-plays. `sampleRate` is the WAV's rate. // `play` carries the S15/S16 per-zone play params for the single-capture path; it defaults to // the PRODUCT defaults (Gate + Preserve engine, S16-F1) so a picked single capture plays under -// the same default engine as a zone would. The editor will surface per-capture overrides later -// (S15-F2 one-zone-map lean); until then this is the one place the single-capture default lives. +// the same default engine as a zone would. The A/D/R frame counts in the default play arg are +// 44100-Hz nominals; this function always rescales them by (sampleRate / 44100) before stamping +// them on the SampleData so the ADSR wall-clock durations match tier0Adsr(sampleRate) exactly. Keymap buildTier0Keymap(std::vector frames, int sampleRate, int rootNote, const SampleLoop& loop, std::vector framesR = {}, @@ -184,6 +187,13 @@ struct PerformanceZone { kTier0NominalDecayFrames, kTier0NominalSustainLevel, kTier0NominalReleaseFrames}, TriggerParams{}, kDefaultPitchEngine, PitchEnvParams{}}; + + // When true, the A/D/R frame counts in play.adsr are 44100-Hz nominals (either lifted from a + // v3 blob or defaulted for a new zone) that must be rescaled by (sampleRate / 44100) at keymap + // build time (buildZonedKeymap). Set to false when a v4 blob explicitly provides A/D/S/R (the + // stored counts already reflect the DAW rate at the time the user edited them) or when the user + // edits an ADSR slider (the committed value is already editor-domain). Never serialized. + bool adsrNeedsRateResolve = true; }; // The instrument's performance map: an ordered list of zones. Order is authoritative for @@ -213,6 +223,9 @@ struct ResolvedZone { kTier0NominalReleaseFrames}, TriggerParams{}, kDefaultPitchEngine, PitchEnvParams{}}; // S15/S16 per-zone play params (carried through as-is) + // Carried from PerformanceZone::adsrNeedsRateResolve — buildZonedKeymap rescales A/D/R + // frames by (sampleRate / 44100) when true. False for v4-explicit or user-edited values. + bool adsrNeedsRateResolve = true; }; // The result of resolving a performance map against the live bank blob. `zones` are the diff --git a/src/vst/sampler_core.cpp b/src/vst/sampler_core.cpp index 6cd2336..82734fe 100644 --- a/src/vst/sampler_core.cpp +++ b/src/vst/sampler_core.cpp @@ -279,16 +279,15 @@ void Voice::start(int note, int velocity, const SampleData& sample, int rootNote // --- 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). + // All five AHDSR fields come from sample.play.adsr, stamped by buildTier0Keymap / + // buildZonedKeymap at reload time (with rate-rescaling for v3-lifted / default zones). + // gateAdsr (the VoiceEngine's instrument-wide ADSR) is accepted for interface compat + // but is NOT read here — it is vestigial since the S12 review fix moved A/D/S/R fully + // onto the per-zone SampleData. // - // 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. --- + // Back-compat invariant: a zone whose adsr fields carry the tier-0 nominal values + // (rescaled to the live sample rate by buildZonedKeymap) sounds bit-identical to the + // pre-fix build at every DAW rate. --- if (playMode_ == PlayMode::Gate) { env_.configure(p.adsr); env_.noteOn(); diff --git a/src/vst/sampler_core.h b/src/vst/sampler_core.h index b4a09d7..e552190 100644 --- a/src/vst/sampler_core.h +++ b/src/vst/sampler_core.h @@ -340,11 +340,13 @@ class Voice { public: // Starts this voice on `note` at `velocity`, playing `sample` (a stable reference // the caller must keep alive for the voice's lifetime — the Keymap owns it), repitched - // from `rootNote`. `gateAdsr` is the effective Gate AHDSR (the engine supplies the - // instrument-wide attack/decay/sustain/release timing; the per-zone HOLD stage comes from - // sample.play.adsr.holdFrames, folded in here). The S15 play MODE + Trigger params and the - // S16 pitch ENGINE + pitch envelope are read from `sample.play`. The Preserve shifters MUST - // already be pre-sized (presizePreserveShifters, off-thread) — start() only reset()s + warm()s + // from `rootNote`. All five AHDSR fields (A/H/D/S/R) are read directly from + // sample.play.adsr — the per-zone values stamped by buildTier0Keymap / buildZonedKeymap. + // `gateAdsr` is the VoiceEngine's instrument-wide ADSR parameter, accepted for interface + // compatibility but NOT used by start() (vestigial since the S12 review fix moved A/D/S/R + // onto the per-zone SampleData). The S15 play MODE + Trigger params and the S16 pitch + // ENGINE + pitch envelope are read from `sample.play`. The Preserve shifters MUST already + // be pre-sized (presizePreserveShifters, off-thread) — start() only reset()s + warm()s // them (RT-safe, no allocation) since it runs on the audio thread inside process(). The warm // silence pass settles the OLA taps before the first output frame (no cold-start click). // Byte-identical to the pre-S15 engine when sample.play is default (Gate + Varispeed + no diff --git a/tests/test_sample_map.cpp b/tests/test_sample_map.cpp index 5fcd4a3..339972d 100644 --- a/tests/test_sample_map.cpp +++ b/tests/test_sample_map.cpp @@ -1194,6 +1194,84 @@ static void testV3BlobLiftsAdsrToNominalDefaults() { CHECK(a.decayFrames == kTier0NominalDecayFrames); // 0 CHECK(a.sustainLevel == kTier0NominalSustainLevel); // 1.0 CHECK(a.releaseFrames == kTier0NominalReleaseFrames); // 2646 (0.060 s at 44100) + // The lifted zone must be flagged for rate-resolve so buildZonedKeymap rescales at the live rate. + CHECK(back.zones[0].adsrNeedsRateResolve == true); +} + +// A v4 blob (explicit A/D/S/R tail) must NOT set adsrNeedsRateResolve — those values +// were authored at the DAW's rate and must not be rescaled again at keymap build time. +static void testV4BlobClearsAdsrNeedsRateResolve() { + PerformanceMap m; + PerformanceZone z = zone("pad", 0, 127); + z.play.adsr.attackFrames = 441; + z.play.adsr.releaseFrames = 8820; + m.zones.push_back(z); + // A round-trip through serialize/deserialize writes a v4 payload (current version). + const PerformanceMap back = deserializePerformance(serializePerformance(m)); + CHECK(back.zones.size() == 1); + if (back.zones.size() != 1) return; + // v4 tail was explicitly read — adsrNeedsRateResolve must be false. + CHECK(back.zones[0].adsrNeedsRateResolve == false); +} + +// buildTier0Keymap at 48k must produce ADSR frame counts equal to tier0Adsr(48000): +// attack = round(kTier0NominalAttackFrames * 48000 / 44100) = round(143.67) = 144, +// release = round(kTier0NominalReleaseFrames * 48000 / 44100) = round(2880.0) = 2880. +// This is the "pre-fix, the Gate voice used tier0Adsr(sampleRate_)" invariant restored +// for the single-capture fast path at any DAW rate. +static void testBuildTier0KeymapRescalesAdsrAt48k() { + const Keymap km = buildTier0Keymap({0.5f}, 48000, 60, SampleLoop{}); + CHECK(km.samples.size() == 1); + if (km.samples.empty()) return; + const AdsrParams& a = km.samples[0].play.adsr; + // Rescaled from 44100-nominal at 48000 Hz: + CHECK(a.attackFrames == 144); // round(132 * 48000.0 / 44100.0) + CHECK(a.decayFrames == 0); // 0 * factor = 0 (no change) + CHECK(a.sustainLevel == 1.0); // level, not frames (no rescale) + CHECK(a.releaseFrames == 2880); // round(2646 * 48000.0 / 44100.0) +} + +// buildZonedKeymap at 48k with adsrNeedsRateResolve=true must rescale ADSR to match +// tier0Adsr(48000), mirroring what Gate voices saw before the per-zone-ADSR fix. +static void testBuildZonedKeymapRescalesNominalAdsrAt48k() { + // Build a zone with nominal 44100-Hz ADSR and the needs-resolve flag (the default). + ResolvedZone z; + z.lowNote = 0; z.highNote = 127; z.rootNote = 60; + z.adsrNeedsRateResolve = true; // 44100-nominal values, rescale needed + z.play.adsr.attackFrames = kTier0NominalAttackFrames; // 132 + z.play.adsr.decayFrames = kTier0NominalDecayFrames; // 0 + z.play.adsr.sustainLevel = kTier0NominalSustainLevel; // 1.0 + z.play.adsr.releaseFrames = kTier0NominalReleaseFrames; // 2646 + const DecodedZonePcm pcm{{0.5f}, 48000}; // 48k WAV + const Keymap km = buildZonedKeymap({z}, {pcm}); + CHECK(km.samples.size() == 1); + if (km.samples.empty()) return; + const AdsrParams& a = km.samples[0].play.adsr; + CHECK(a.attackFrames == 144); // round(132 * 48000.0 / 44100.0) + CHECK(a.decayFrames == 0); // 0 * factor = 0 + CHECK(a.sustainLevel == 1.0); // level, not rescaled + CHECK(a.releaseFrames == 2880); // round(2646 * 48000.0 / 44100.0) +} + +// buildZonedKeymap must NOT rescale a zone whose adsrNeedsRateResolve is false — +// those frame counts are explicitly user-authored at the DAW's rate. +static void testBuildZonedKeymapDoesNotRescaleV4Adsr() { + ResolvedZone z; + z.lowNote = 0; z.highNote = 127; z.rootNote = 60; + z.adsrNeedsRateResolve = false; // v4 blob or user-edited — do not rescale + z.play.adsr.attackFrames = 441; // 0.01 s at 44100 Hz (non-nominal) + z.play.adsr.decayFrames = 4410; + z.play.adsr.sustainLevel = 0.7; + z.play.adsr.releaseFrames = 8820; + const DecodedZonePcm pcm{{0.5f}, 48000}; // 48k WAV — rescale would change values + const Keymap km = buildZonedKeymap({z}, {pcm}); + CHECK(km.samples.size() == 1); + if (km.samples.empty()) return; + const AdsrParams& a = km.samples[0].play.adsr; + CHECK(a.attackFrames == 441); // unchanged (not rescaled) + CHECK(a.decayFrames == 4410); + CHECK(a.sustainLevel == 0.7); + CHECK(a.releaseFrames == 8820); } int main() { @@ -1249,6 +1327,10 @@ int main() { testPlayParamsThroughComponentEnvelope(); testFullAdsrV4RoundTrip(); testV3BlobLiftsAdsrToNominalDefaults(); + testV4BlobClearsAdsrNeedsRateResolve(); + testBuildTier0KeymapRescalesAdsrAt48k(); + testBuildZonedKeymapRescalesNominalAdsrAt48k(); + testBuildZonedKeymapDoesNotRescaleV4Adsr(); testComponentStateRoundTrip(); testComponentStateLoopStartRoundTrip(); testComponentStateSelectionOnlyNoZones();