Fix embed use-after-free, restore golden fixture + refs tests, drop dead note_entry
This commit is contained in:
@@ -65,6 +65,12 @@ struct Zone {
|
||||
int highNote = 127;
|
||||
int rootOverride = -1; // < 0 = absent
|
||||
bool hasLoopOverride = false;
|
||||
// The override's OWN hasLoop bit — distinct from hasLoopOverride above. An override can
|
||||
// itself say "disable the loop" (loopOverrideHasLoop = false): the field is present but
|
||||
// sets no sustain loop, as opposed to no override at all (the sample's own intrinsic loop
|
||||
// applies). Defaults true so existing callers that only set hasLoopOverride keep writing
|
||||
// the enabled-loop shape they always did.
|
||||
bool loopOverrideHasLoop = true;
|
||||
std::int64_t loopStart = 0;
|
||||
std::int64_t loopEnd = 0;
|
||||
std::int64_t startPoint = -1; // < 0 = absent
|
||||
@@ -95,7 +101,7 @@ static void putZone(std::vector<std::uint8_t>& out, const Zone& z, std::uint32_t
|
||||
if (pv >= 2) {
|
||||
u8v(out, z.hasLoopOverride ? 1 : 0);
|
||||
if (z.hasLoopOverride) {
|
||||
u8v(out, 1);
|
||||
u8v(out, z.loopOverrideHasLoop ? 1 : 0);
|
||||
i64v(out, z.loopStart);
|
||||
i64v(out, z.loopEnd);
|
||||
}
|
||||
@@ -185,6 +191,24 @@ static std::vector<std::uint8_t> envelopeWithZones(const std::string& selectionI
|
||||
|
||||
// --- The current format -------------------------------------------------------
|
||||
|
||||
// Builds a SampleRefEntry with the intrinsics fields the refs-robustness tests below need to
|
||||
// set individually (root/loop/channels), mirroring the codec's own field names.
|
||||
static SampleRefEntry refEntry(const std::string& id, const std::string& rel, int root,
|
||||
bool hasLoop = false, std::int64_t loopStart = 0,
|
||||
std::int64_t loopEnd = 0, int channels = 0,
|
||||
const std::string& name = "") {
|
||||
SampleRefEntry e;
|
||||
e.sampleId = id;
|
||||
e.ref.relativePath = rel;
|
||||
e.ref.rootNote = root;
|
||||
e.ref.loop.hasLoop = hasLoop;
|
||||
e.ref.loop.start = loopStart;
|
||||
e.ref.loop.end = loopEnd;
|
||||
e.ref.channelCount = channels;
|
||||
e.displayName = name;
|
||||
return e;
|
||||
}
|
||||
|
||||
// A full round-trip through the CURRENT envelope (v11) + params payload (v8): every field
|
||||
// survives. This is the "one parameter set round-trips save/reload intact" contract.
|
||||
static void testComponentStateRoundTrip() {
|
||||
@@ -281,6 +305,114 @@ static void testComponentStateRoundTrip() {
|
||||
CHECK(p.play.pitchEnv.peakSemitones == 5.0);
|
||||
}
|
||||
|
||||
// GOLDEN FULL-BLOB FIXTURE (reviewer follow-up). testEnvelopePrefixBytesFrozen below only
|
||||
// pins the first 5 bytes of a near-EMPTY blob; it cannot catch a drift anywhere past the mode
|
||||
// byte (a field re-ordered or dropped inside the voice/gain/refs/guid/params tail would still
|
||||
// pass it). This builds a canonical v11 ComponentState/v8-params blob that exercises every
|
||||
// field family at once (a two-entry sample-refs table — one with a loop, one without — every
|
||||
// optional param field present, a non-flat velocity curve, Trigger mode with a pitch envelope)
|
||||
// and asserts the encoded bytes equal an EXACT expected vector, captured from the current
|
||||
// writer's output and checked field-for-field against the v8/v11 layout documented in
|
||||
// component_state_io.h.
|
||||
static void testGoldenFullBlobFixture() {
|
||||
ComponentState in;
|
||||
in.selectionId = "kick";
|
||||
in.channelMode = ChannelMode::Stereo;
|
||||
in.channelModeExplicit = true;
|
||||
in.lastConsumedAssignGeneration = 12345;
|
||||
in.previewVelocity = 100;
|
||||
in.voiceCount = 24;
|
||||
in.voiceMode = VoiceMode::Mono;
|
||||
in.monoTrigger = MonoTrigger::Legato;
|
||||
in.masterGainLinear = 2.0;
|
||||
in.instanceGuid = "guid-1234-5678-abcd";
|
||||
|
||||
SampleRefEntry kickRef;
|
||||
kickRef.sampleId = "kick";
|
||||
kickRef.ref.relativePath = "bank/kick.wav";
|
||||
kickRef.ref.rootNote = 36;
|
||||
kickRef.ref.loop.hasLoop = true;
|
||||
kickRef.ref.loop.start = 1000;
|
||||
kickRef.ref.loop.end = 5000;
|
||||
kickRef.ref.channelCount = 2;
|
||||
kickRef.displayName = "Kick Drum";
|
||||
in.sampleRefs.push_back(kickRef);
|
||||
|
||||
SampleRefEntry snareRef;
|
||||
snareRef.sampleId = "snare";
|
||||
snareRef.ref.relativePath = "bank/snare.wav";
|
||||
snareRef.ref.rootNote = 38;
|
||||
snareRef.ref.loop.hasLoop = false;
|
||||
snareRef.ref.loop.start = 0;
|
||||
snareRef.ref.loop.end = 0;
|
||||
snareRef.ref.channelCount = 1;
|
||||
snareRef.displayName = "Snare";
|
||||
in.sampleRefs.push_back(snareRef);
|
||||
|
||||
in.params.rootOverride = 36;
|
||||
SampleLoop loopA;
|
||||
loopA.hasLoop = true;
|
||||
loopA.start = 1000;
|
||||
loopA.end = 5000;
|
||||
in.params.loopOverride = loopA;
|
||||
in.params.startPoint = 250;
|
||||
in.params.keyTrack = 0.5;
|
||||
in.params.velocityCurve = reasampler::instrument::engine::VelocityCurve::fromPoints(
|
||||
{VelocityPoint{0.0, 0.2}, VelocityPoint{64.0, 0.6}, VelocityPoint{127.0, 1.0}});
|
||||
in.params.play.playMode = PlayMode::Trigger;
|
||||
in.params.play.adsr.attackSeconds = 0.01;
|
||||
in.params.play.adsr.holdSeconds = 0.05;
|
||||
in.params.play.adsr.decaySeconds = 0.02;
|
||||
in.params.play.adsr.sustainLevel = 0.8;
|
||||
in.params.play.adsr.releaseSeconds = 0.15;
|
||||
in.params.play.trigger.lengthFraction = 0.75;
|
||||
in.params.play.trigger.fadeInFrames = 100;
|
||||
in.params.play.trigger.fadeOutFrames = 200;
|
||||
in.params.play.pitchEngine = PitchEngine::Preserve;
|
||||
in.params.play.pitchEnv.enabled = true;
|
||||
in.params.play.pitchEnv.attackSeconds = 0.02;
|
||||
in.params.play.pitchEnv.decaySeconds = 0.03;
|
||||
in.params.play.pitchEnv.peakSemitones = 5.0;
|
||||
|
||||
const std::vector<std::uint8_t> bytes = serializeComponentState(in);
|
||||
// clang-format off
|
||||
static const std::uint8_t kGolden[] = {
|
||||
0x0b,0x00,0x00,0x00,0x01,0x39,0x30,0x00,0x00,0x00,0x00,0x00,0x00,0x64,0x18,0x01,
|
||||
0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x40,0x01,0x02,0x00,0x00,0x00,0x04,0x00,
|
||||
0x00,0x00,0x6b,0x69,0x63,0x6b,0x0d,0x00,0x00,0x00,0x62,0x61,0x6e,0x6b,0x2f,0x6b,
|
||||
0x69,0x63,0x6b,0x2e,0x77,0x61,0x76,0x24,0x00,0x00,0x00,0x01,0xe8,0x03,0x00,0x00,
|
||||
0x00,0x00,0x00,0x00,0x88,0x13,0x00,0x00,0x00,0x00,0x00,0x00,0x02,0x00,0x00,0x00,
|
||||
0x09,0x00,0x00,0x00,0x4b,0x69,0x63,0x6b,0x20,0x44,0x72,0x75,0x6d,0x05,0x00,0x00,
|
||||
0x00,0x73,0x6e,0x61,0x72,0x65,0x0e,0x00,0x00,0x00,0x62,0x61,0x6e,0x6b,0x2f,0x73,
|
||||
0x6e,0x61,0x72,0x65,0x2e,0x77,0x61,0x76,0x26,0x00,0x00,0x00,0x00,0x00,0x00,0x00,
|
||||
0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x01,0x00,0x00,
|
||||
0x00,0x05,0x00,0x00,0x00,0x53,0x6e,0x61,0x72,0x65,0x13,0x00,0x00,0x00,0x67,0x75,
|
||||
0x69,0x64,0x2d,0x31,0x32,0x33,0x34,0x2d,0x35,0x36,0x37,0x38,0x2d,0x61,0x62,0x63,
|
||||
0x64,0x04,0x00,0x00,0x00,0x6b,0x69,0x63,0x6b,0x00,0xff,0xff,0xff,0x08,0x00,0x00,
|
||||
0x00,0x01,0x24,0x00,0x00,0x00,0x01,0x01,0xe8,0x03,0x00,0x00,0x00,0x00,0x00,0x00,
|
||||
0x88,0x13,0x00,0x00,0x00,0x00,0x00,0x00,0x01,0xfa,0x00,0x00,0x00,0x00,0x00,0x00,
|
||||
0x00,0x01,0x9a,0x99,0x99,0x99,0x99,0x99,0xa9,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,
|
||||
0xe8,0x3f,0x64,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0xc8,0x00,0x00,0x00,0x00,0x00,
|
||||
0x00,0x00,0x01,0x01,0x7b,0x14,0xae,0x47,0xe1,0x7a,0x94,0x3f,0xb8,0x1e,0x85,0xeb,
|
||||
0x51,0xb8,0x9e,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x14,0x40,0x7b,0x14,0xae,0x47,
|
||||
0xe1,0x7a,0x84,0x3f,0x7b,0x14,0xae,0x47,0xe1,0x7a,0x94,0x3f,0x9a,0x99,0x99,0x99,
|
||||
0x99,0x99,0xe9,0x3f,0x33,0x33,0x33,0x33,0x33,0x33,0xc3,0x3f,0x00,0x00,0x00,0x00,
|
||||
0x00,0x00,0xe0,0x3f,0x03,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,
|
||||
0x9a,0x99,0x99,0x99,0x99,0x99,0xc9,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x50,0x40,
|
||||
0x33,0x33,0x33,0x33,0x33,0x33,0xe3,0x3f,0x00,0x00,0x00,0x00,0x00,0xc0,0x5f,0x40,
|
||||
0x00,0x00,0x00,0x00,0x00,0x00,0xf0,0x3f,
|
||||
};
|
||||
// clang-format on
|
||||
CHECK(bytes.size() == sizeof(kGolden));
|
||||
if (bytes.size() == sizeof(kGolden)) {
|
||||
bool same = true;
|
||||
for (std::size_t i = 0; i < bytes.size(); ++i) {
|
||||
if (bytes[i] != kGolden[i]) { same = false; break; }
|
||||
}
|
||||
CHECK(same);
|
||||
}
|
||||
}
|
||||
|
||||
// A DEFAULT parameter set must round-trip to defaults — the "no pick, nothing configured"
|
||||
// blob restores as the silent empty state, not as a set of accidental values.
|
||||
static void testDefaultStateRoundTripsToDefaults() {
|
||||
@@ -398,6 +530,26 @@ static void testSingleZoneMigrationIsLossless() {
|
||||
CHECK(p.play.pitchEnv.peakSemitones == 5.0);
|
||||
}
|
||||
|
||||
// A legacy OVERRIDE THAT DISABLES THE LOOP migrates as a PRESENT loopOverride with hasLoop
|
||||
// false — distinct from no override at all (which leaves the sample's own intrinsic loop in
|
||||
// force). The writer always emitted the override's inner hasLoop bit as true; this is the
|
||||
// disabled shape it never exercised.
|
||||
static void testSingleZoneMigrationLiftsLoopDisablingOverride() {
|
||||
legacy::Zone z;
|
||||
z.sampleId = "kick";
|
||||
z.hasLoopOverride = true;
|
||||
z.loopOverrideHasLoop = false;
|
||||
z.loopStart = 1000;
|
||||
z.loopEnd = 5000;
|
||||
|
||||
const ComponentState out =
|
||||
deserializeComponentState(legacy::envelopeWithZones("kick", {z}, 7), 48000.0);
|
||||
|
||||
const InstrumentParams& p = out.params;
|
||||
CHECK(p.loopOverride.has_value());
|
||||
CHECK(p.loopOverride && !p.loopOverride->hasLoop);
|
||||
}
|
||||
|
||||
// A lifted single-zone instance RE-SAVES in the current format and survives a second
|
||||
// round-trip unchanged — the lift is a one-way door, not a per-open re-derivation.
|
||||
static void testLiftedStateReSavesInCurrentFormat() {
|
||||
@@ -628,6 +780,52 @@ static void testCorruptFieldsFallBackToDefaults() {
|
||||
CHECK(out.masterGainLinear == 1.0);
|
||||
}
|
||||
|
||||
// CORRUPT-BLOB posture for the refs-table intrinsics: the refs table is the ONLY copy on the
|
||||
// play path, so a bad field must degrade to its own default, never poison playback. An
|
||||
// out-of-MIDI-range rootNote falls back to the middle-C default distill() uses; a negative
|
||||
// channelCount falls back to 0 = unknown (the GA auto-default then skips it). The fallback is
|
||||
// per-field — in-range neighbours pass through untouched.
|
||||
static void testSampleRefsReaderRangeFallbacks() {
|
||||
ComponentState s;
|
||||
s.sampleRefs.push_back(refEntry("hi", "b/h.wav", /*root=*/999, false, 0, 0,
|
||||
/*channels=*/-3));
|
||||
s.sampleRefs.push_back(refEntry("lo", "b/l.wav", /*root=*/-5, false, 0, 0,
|
||||
/*channels=*/1));
|
||||
s.sampleRefs.push_back(refEntry("ok", "b/o.wav", /*root=*/36, false, 0, 0,
|
||||
/*channels=*/2));
|
||||
const ComponentState back = deserializeComponentState(serializeComponentState(s), 44100.0);
|
||||
CHECK(back.sampleRefs.size() == 3);
|
||||
CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[0].ref.rootNote == 60);
|
||||
CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[0].ref.channelCount == 0);
|
||||
CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[1].ref.rootNote == 60);
|
||||
CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[1].ref.channelCount == 1);
|
||||
CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[2].ref.rootNote == 36);
|
||||
CHECK(back.sampleRefs.size() == 3 && back.sampleRefs[2].ref.channelCount == 2);
|
||||
}
|
||||
|
||||
// A blob cut mid-refs-entry keeps the entries that parsed cleanly and restores the rest of
|
||||
// the state empty (the selection/params behind the cut are unreadable anyway) — the
|
||||
// established truncation posture, never a throw across the host boundary.
|
||||
static void testSampleRefsTruncatedMidEntry() {
|
||||
ComponentState s;
|
||||
s.selectionId = "kick";
|
||||
s.sampleRefs.push_back(refEntry("kick", "b/k.wav", 36));
|
||||
s.sampleRefs.push_back(refEntry("pad", "b/p.wav", 60));
|
||||
std::vector<std::uint8_t> bytes = serializeComponentState(s);
|
||||
// The tail after the refs table is instanceGuid(4, empty) + selectionId(4+4="kick") +
|
||||
// the current params payload for DEFAULT params (marker4+version4 + overrides3 + the
|
||||
// 91-byte play tail + keyTrack8 + curve(4+2*16, the flat 2-point default)) = 158 bytes;
|
||||
// entry two is 47 bytes (id 4+3, path 4+7, root4, loop 1+8+8, channels4, name 4+0).
|
||||
// Cutting 178 bytes keeps the first 27 of entry two's 47 — mid loop.start (offset 23..31).
|
||||
CHECK(bytes.size() > 178);
|
||||
bytes.resize(bytes.size() - 178);
|
||||
const ComponentState back = deserializeComponentState(bytes, 44100.0);
|
||||
CHECK(back.sampleRefs.size() == 1);
|
||||
CHECK(back.sampleRefs.size() == 1 && back.sampleRefs[0].sampleId == "kick");
|
||||
CHECK(back.selectionId.empty());
|
||||
CHECK(!back.params.rootOverride);
|
||||
}
|
||||
|
||||
// The WRITER never emits an out-of-range voice count or master gain, so a blob this codec
|
||||
// produced always re-reads as itself.
|
||||
static void testWriterClampsOutOfRangeFields() {
|
||||
@@ -669,11 +867,16 @@ static void testV1SelectionLift() {
|
||||
|
||||
// Truncation degrades to a partial/empty parse — never out-of-bounds, never throws. Run
|
||||
// over BOTH the current format and a retired zone-list blob, since the migration path has
|
||||
// its own bounded-read walk.
|
||||
// its own bounded-read walk. Beyond mere survival, a cut read must never RETAIN more refs
|
||||
// than the blob actually carried (the "keep what parsed, drop the rest" contract could not
|
||||
// silently start fabricating entries) — see testSampleRefsTruncatedMidEntry for the exact
|
||||
// mid-entry retention case this bounds only loosely across every cut point.
|
||||
static void testTruncationDegradesCleanly() {
|
||||
ComponentState in;
|
||||
in.selectionId = "smp-2";
|
||||
in.params.rootOverride = 61;
|
||||
in.sampleRefs.push_back(refEntry("smp-2", "b/s.wav", 61));
|
||||
in.sampleRefs.push_back(refEntry("smp-3", "b/t.wav", 62));
|
||||
const std::vector<std::uint8_t> current = serializeComponentState(in);
|
||||
|
||||
legacy::Zone z;
|
||||
@@ -685,18 +888,19 @@ static void testTruncationDegradesCleanly() {
|
||||
const std::vector<std::uint8_t> part(blob->begin(),
|
||||
blob->begin() + static_cast<long>(cut));
|
||||
const ComponentState out = deserializeComponentState(part, 48000.0);
|
||||
(void)out; // reaching here without UB/throw is the contract under test
|
||||
CHECK(out.sampleRefs.size() <= in.sampleRefs.size());
|
||||
}
|
||||
}
|
||||
CHECK(true);
|
||||
}
|
||||
|
||||
int main() {
|
||||
testComponentStateRoundTrip();
|
||||
testGoldenFullBlobFixture();
|
||||
testDefaultStateRoundTripsToDefaults();
|
||||
testEnvelopePrefixBytesFrozen();
|
||||
testWriterEmitsCurrentPayloadVersion();
|
||||
testSingleZoneMigrationIsLossless();
|
||||
testSingleZoneMigrationLiftsLoopDisablingOverride();
|
||||
testLiftedStateReSavesInCurrentFormat();
|
||||
testMultiZoneMigrationAdoptsFirstZone();
|
||||
testFirstZoneSupersedesStoredSelection();
|
||||
@@ -706,6 +910,8 @@ int main() {
|
||||
testEnvelopeLadderLiftsEachVersion();
|
||||
testV2ZonesOnlyBlobAdoptsBothFromZoneOne();
|
||||
testCorruptFieldsFallBackToDefaults();
|
||||
testSampleRefsReaderRangeFallbacks();
|
||||
testSampleRefsTruncatedMidEntry();
|
||||
testWriterClampsOutOfRangeFields();
|
||||
testUnknownEnvelopeVersionIsEmpty();
|
||||
testV1SelectionLift();
|
||||
|
||||
Reference in New Issue
Block a user