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.
This commit is contained in:
@@ -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<std::uint8_t> 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<std::uint8_t> 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();
|
||||
|
||||
@@ -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<AudioSample> 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<AudioSample> 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;
|
||||
|
||||
Reference in New Issue
Block a user