diff --git a/src/core/instrument/map/params_payload.cpp b/src/core/instrument/map/params_payload.cpp index c3251e3..2984da3 100644 --- a/src/core/instrument/map/params_payload.cpp +++ b/src/core/instrument/map/params_payload.cpp @@ -244,6 +244,7 @@ void readBakeHold(ByteReader& r, InstrumentParams& p) { // bipolar at EVERY version — a pre-v12 blob's y values are already valid bipolar ones, so its // v12 domain re-tag needs no version branch (see component_state_io.h). void readFilterTail(ByteReader& r, InstrumentParams& p) { + const FilterSeconds fallback; // the construction defaults, read rather than restated FilterSeconds& f = p.play.filter; f.enabled = (r.u8() != 0); f.settings.cutoffNorm = static_cast(bitsToDouble(r.u64())); @@ -260,11 +261,11 @@ void readFilterTail(ByteReader& r, InstrumentParams& p) { f.modAmount = std::isfinite(modAmount) ? modAmount : 0.0; f.velAmount = std::isfinite(velAmount) ? velAmount : 0.0; f.keyTrack = std::isfinite(keyTrack) ? keyTrack : 0.0; - f.env.attackSeconds = bitsToDouble(r.u64()); - f.env.holdSeconds = bitsToDouble(r.u64()); - f.env.decaySeconds = bitsToDouble(r.u64()); - f.env.sustainLevel = bitsToDouble(r.u64()); - f.env.releaseSeconds = bitsToDouble(r.u64()); + f.env.attackSeconds = finiteOr(bitsToDouble(r.u64()), fallback.env.attackSeconds); + f.env.holdSeconds = finiteOr(bitsToDouble(r.u64()), fallback.env.holdSeconds); + f.env.decaySeconds = finiteOr(bitsToDouble(r.u64()), fallback.env.decaySeconds); + f.env.sustainLevel = finiteOr(bitsToDouble(r.u64()), fallback.env.sustainLevel); + f.env.releaseSeconds = finiteOr(bitsToDouble(r.u64()), fallback.env.releaseSeconds); readCurveTail(r, f.velocityCurve, reasampler::instrument::engine::CurveDomain::Bipolar); } @@ -371,8 +372,10 @@ PayloadRead readLegacyZonePayload(ByteReader& r, std::uint32_t pv, double projec readSecondsPlayTail(r, p, projectRate); } // A pre-v6 payload leaves keyTrack = 1.0 (100% ET), so an already-saved instance - // repitches BIT-IDENTICALLY. A pre-v7 payload leaves VelocityCurve::flat(). - if (keyTrackTail) p.keyTrack = bitsToDouble(r.u64()); + // repitches BIT-IDENTICALLY. A pre-v7 payload leaves VelocityCurve::flat(). A NaN + // reaches keyTrackedRatio -> baseRatio_ -> readPos_'s per-sample cast (voice.h) — the + // same guard the v8+ single-record reader applies to its own keyTrack below. + if (keyTrackTail) p.keyTrack = finiteOr(bitsToDouble(r.u64()), InstrumentParams{}.keyTrack); if (curveTail) { readCurveTail(r, p.velocityCurve, reasampler::instrument::engine::CurveDomain::Unipolar); @@ -500,7 +503,10 @@ PayloadRead readParamsPayload(ByteReader& r, double projectRate) { const std::uint8_t hasStart = r.u8(); if (hasStart) p.startPoint = r.i64(); readSecondsPlayTail(r, p, projectRate); - p.keyTrack = bitsToDouble(r.u64()); + // NaN reaches keyTrackedRatio (voice.h) -> baseRatio_ -> readPos_'s per-sample + // static_cast — UB on the per-sample path. Guarded here, codec-side, so + // that path needs no check of its own. + p.keyTrack = finiteOr(bitsToDouble(r.u64()), InstrumentParams{}.keyTrack); readCurveTail(r, p.velocityCurve, reasampler::instrument::engine::CurveDomain::Unipolar); if (pv >= kParamsFilterVersion) readFilterTail(r, p); if (pv >= kParamsCurveVersion) readCurveStageTail(r, p); diff --git a/src/core/instrument/note/musical_division.cpp b/src/core/instrument/note/musical_division.cpp index deec8ab..c1babad 100644 --- a/src/core/instrument/note/musical_division.cpp +++ b/src/core/instrument/note/musical_division.cpp @@ -54,27 +54,6 @@ int divisionIndex(Division d) { + static_cast(d.modifier()); } -Division shortestDivisionAtLeast(double beats) { - // Picker order is NOT length order — a rung's triplet is shorter than the previous rung's - // dotted — so both the fit and the fallback are found by scanning. 39 entries. - Division longest = divisionAt(0); - double longestBeats = divisionBeats(longest); - Division best = longest; - double bestBeats = 0.0; - bool found = false; - for (int i = 0; i < kDivisionCount; ++i) { - const Division d = divisionAt(i); - const double b = divisionBeats(d); - if (b > longestBeats) { longest = d; longestBeats = b; } - if (b >= beats && (!found || b < bestBeats)) { best = d; bestBeats = b; found = true; } - } - // NaN names no length to be at least, so it takes the never-short direction rather than - // the bottom rung: a corrupt value must not resolve to a near-instant note. - if (std::isnan(beats)) return longest; - if (!(beats > 0.0)) return divisionAt(0); - return found ? best : longest; -} - std::string divisionLabel(Division d) { const int e = d.quarterExponent(); // Both branches meet at e == 2 ("1/1"): a division's written form is its length in diff --git a/src/core/instrument/note/musical_division.h b/src/core/instrument/note/musical_division.h index e2257b1..bef6fc8 100644 --- a/src/core/instrument/note/musical_division.h +++ b/src/core/instrument/note/musical_division.h @@ -69,13 +69,6 @@ double divisionBeats(Division d); Division divisionAt(int index); int divisionIndex(Division d); -// The shortest ladder length that is at least `beats` — for a caller quantizing a duration ONTO -// the ladder, where overshooting is the safe direction. It is NOT how a derived duration reaches -// the bake: nothing on a finite ladder covers an arbitrarily long source (see note/CLAUDE.md). -// Nothing long enough, or a non-finite `beats`, yields the top rung; a non-positive one yields -// the bottom. -Division shortestDivisionAtLeast(double beats); - // The notation divisions are named in: "1/16", "1/8.", "1/4t", "4/1". std::string divisionLabel(Division d); diff --git a/tests/test_component_state_io.cpp b/tests/test_component_state_io.cpp index 202e76c..85daaac 100644 --- a/tests/test_component_state_io.cpp +++ b/tests/test_component_state_io.cpp @@ -954,25 +954,41 @@ static void testV13HardFlagCountThatStrandsAlignmentLeavesTheHoldAbsentNotFabric // Numeric domains are established at the DOOR, not at each consumer. A NaN pitch depth reaches // the bake's pow() and the voice's ratio multiply; a NaN %-length and a NaN stage time reach -// narrowing casts that are undefined on one; and a root override outside MIDI range makes the +// narrowing casts that are undefined on one; a NaN keyTrack reaches keyTrackedRatio -> +// baseRatio_ -> readPos_'s per-sample static_cast (voice.h); a NaN v9 filter +// env second reaches secToFrames the same way the trigAhd/filter.trigEnv pair already covered +// by testNonFiniteAhdSecondsLiftToZero do; and a root override outside MIDI range makes the // bake's render note and the sample's own root disagree, which is a read rate other than 1 and // therefore a window sized in the truncating direction. static void testOutOfDomainWireValuesAreBoundedAtTheCodec() { const PlaySeconds defaults; + const InstrumentParams paramDefaults; ComponentState in; in.selectionId = "pad"; - in.params.keyTrack = 0.5; // a neighbouring field, to show the guards are per-field + in.params.keyTrack = std::numeric_limits::quiet_NaN(); in.params.rootOverride = 9999; in.params.play.pitchEnv.peakSemitones = std::numeric_limits::quiet_NaN(); in.params.play.trigger.lengthFraction = std::numeric_limits::quiet_NaN(); in.params.play.adsr.releaseSeconds = std::numeric_limits::infinity(); + in.params.play.filter.enabled = true; + in.params.play.filter.env.attackSeconds = std::numeric_limits::quiet_NaN(); + in.params.play.filter.env.holdSeconds = std::numeric_limits::infinity(); + in.params.play.filter.env.decaySeconds = -std::numeric_limits::infinity(); + in.params.play.filter.env.sustainLevel = std::numeric_limits::quiet_NaN(); + in.params.play.filter.env.releaseSeconds = std::numeric_limits::quiet_NaN(); const ComponentState out = deserializeComponentState(serializeComponentState(in), 48000.0); CHECK(out.params.rootOverride && *out.params.rootOverride == 127); CHECK(out.params.play.pitchEnv.peakSemitones == defaults.pitchEnv.peakSemitones); CHECK(out.params.play.trigger.lengthFraction == defaults.trigger.lengthFraction); CHECK(out.params.play.adsr.releaseSeconds == defaults.adsr.releaseSeconds); - CHECK(out.params.keyTrack == 0.5); + CHECK(out.params.keyTrack == paramDefaults.keyTrack); + CHECK(out.params.play.filter.enabled); // the fallback is per-field, not per-record + CHECK(out.params.play.filter.env.attackSeconds == defaults.filter.env.attackSeconds); + CHECK(out.params.play.filter.env.holdSeconds == defaults.filter.env.holdSeconds); + CHECK(out.params.play.filter.env.decaySeconds == defaults.filter.env.decaySeconds); + CHECK(out.params.play.filter.env.sustainLevel == defaults.filter.env.sustainLevel); + CHECK(out.params.play.filter.env.releaseSeconds == defaults.filter.env.releaseSeconds); ComponentState low = in; low.params.rootOverride = -5; @@ -980,6 +996,20 @@ static void testOutOfDomainWireValuesAreBoundedAtTheCodec() { CHECK(lowOut.params.rootOverride && *lowOut.params.rootOverride == 0); } +// The keyTrack guard's OTHER site: a pre-v7 (legacy zone-list) blob's own keyTrack field +// (params_payload.cpp's readLegacyZonePayload, pv >= 6) is a second, independent read of the +// same wire double — same hazard, same fallback, must not be missed just because the v8+ +// single-record reader above was fixed. +static void testLegacyZoneKeyTrackNaNLiftsToDefault() { + legacy::Zone z; + z.sampleId = "kick"; + z.keyTrack = std::numeric_limits::quiet_NaN(); + + const ComponentState out = + deserializeComponentState(legacy::envelopeWithZones("kick", {z}, 7), 48000.0); + CHECK(out.params.keyTrack == InstrumentParams{}.keyTrack); +} + // A hard-flag tail truncated mid-COUNT-FIELD (only 2 of its 4 length bytes present, and // nothing else after) is a different failure shape than a declared-huge count: the u32 read // itself fails, tripping r.ok inside readHardFlags rather than its own bound check. That must @@ -1935,6 +1965,7 @@ int main() { testV13HardFlagOutOfBoundsCountSurvivesWithoutWipingTheRecord(); testV13HardFlagCountThatStrandsAlignmentLeavesTheHoldAbsentNotFabricated(); testOutOfDomainWireValuesAreBoundedAtTheCodec(); + testLegacyZoneKeyTrackNaNLiftsToDefault(); testV13HardFlagTailTruncatedMidCountSurvivesWithoutWipingTheRecord(); testBakeHoldRoundTripsAndDisturbsNothingElse(); testV13BlobLiftsToTheDefaultHold(); diff --git a/tests/test_musical_division.cpp b/tests/test_musical_division.cpp index c308928..87548f8 100644 --- a/tests/test_musical_division.cpp +++ b/tests/test_musical_division.cpp @@ -3,15 +3,13 @@ // // Covers: the beat length of all 39 divisions against a literal rung table (NOT the module's // own exponent formula); the 1/64 and 64/1 extremes; the four named example divisions; the -// label notation; picker order and index round-trip; off-ladder clamping of BOTH persisted -// fields, measured through the readers rather than by comparing two clamped values; and -// shortestDivisionAtLeast's never-short contract across the rung boundaries. +// label notation; picker order and index round-trip; and off-ladder clamping of BOTH persisted +// fields, measured through the readers rather than by comparing two clamped values. #include "../src/core/instrument/note/musical_division.h" #include #include -#include using namespace reasampler::instrument::note; @@ -206,54 +204,7 @@ static void testOutOfRangeIndexClampsIntoTheSet() { CHECK(divisionAt(kDivisionCount) == divisionAt(kDivisionCount - 1)); } -// --- shortestDivisionAtLeast -------------------------------------------------- - -// The contract is never-short: whatever comes back is at least the requested length, and -// nothing shorter on the ladder also is. Swept over every rung and over the gaps between them. -static void testShortestAtLeastIsNeverShortAndNeverLonger() { - for (int i = 0; i < kDivisionCount; ++i) { - const double target = divisionBeats(divisionAt(i)); - for (const double want : {target, target * 0.99, target * 0.5}) { - const Division got = shortestDivisionAtLeast(want); - CHECK(divisionBeats(got) >= want - 1e-12); - // Nothing on the ladder is both long enough and shorter than the answer. - for (int j = 0; j < kDivisionCount; ++j) { - const double other = divisionBeats(divisionAt(j)); - CHECK(!(other >= want && other < divisionBeats(got) - 1e-12)); - } - } - } -} - -// Picker order is NOT length order — a rung's triplet is shorter than the previous rung's -// dotted — so the answer is not simply "the next index up". -static void testShortestAtLeastCrossesRungBoundaries() { - // 5 beats: the 2/1 triplet (8 * 2/3 == 5.33) beats the dotted half (6) above it. - CHECK(shortestDivisionAtLeast(5.0) == makeDivision(3, DivisionModifier::Triplet)); - // An exact rung length answers with that rung, not the one above it. - CHECK(shortestDivisionAtLeast(4.0) == makeDivision(2, DivisionModifier::Straight)); -} - -// Degenerate inputs land on an end rather than anywhere in the middle. -static void testShortestAtLeastDegenerateInputs() { - CHECK(shortestDivisionAtLeast(0.0) == divisionAt(0)); - CHECK(shortestDivisionAtLeast(-10.0) == divisionAt(0)); - // Past the longest programmable note (the dotted top rung), the top rung is the answer. - const Division longest = makeDivision(kMaxQuarterExponent, DivisionModifier::Dotted); - CHECK(almostEqual(divisionBeats(longest), kMaxDivisionBeats)); - CHECK(shortestDivisionAtLeast(kMaxDivisionBeats * 2.0) == longest); - // A non-finite request names no length to be at least, so it takes the SAME never-short - // answer as one nothing covers. Landing on the bottom rung instead would turn a corrupt - // value into a near-instant note. - CHECK(shortestDivisionAtLeast(std::nan("")) == longest); - CHECK(shortestDivisionAtLeast(std::numeric_limits::infinity()) == longest); -} - int main() { - testShortestAtLeastIsNeverShortAndNeverLonger(); - testShortestAtLeastCrossesRungBoundaries(); - testShortestAtLeastDegenerateInputs(); - testLadderSpansSixtyfourthToSixtyFourWhole(); testEveryStraightRungHasItsWrittenBeatLength(); testDottedIsHalfAgainAndTripletIsTwoThirds(); diff --git a/tests/test_note_program.cpp b/tests/test_note_program.cpp index 35949ac..3ac7819 100644 --- a/tests/test_note_program.cpp +++ b/tests/test_note_program.cpp @@ -219,8 +219,10 @@ static void testAnExactLengthCarriesDurationsPastTheLaddersTopRung() { const Tempo t = at(120.0); const double pastTheLadder = t.beatsToSeconds(kMaxDivisionBeats) * 3.0; CHECK(almostEqual(noteLengthSeconds(lengthOfSeconds(pastTheLadder), t), pastTheLadder)); - // …and the ladder really does saturate there, which is what makes the seam load-bearing. - CHECK(almostEqual(divisionBeats(shortestDivisionAtLeast(kMaxDivisionBeats * 3.0)), + // …and the ladder really does saturate there, which is what makes the seam load-bearing: + // its longest rung (the dotted top) IS kMaxDivisionBeats, so nothing on it reaches + // pastTheLadder. + CHECK(almostEqual(divisionBeats(makeDivision(kMaxQuarterExponent, DivisionModifier::Dotted)), kMaxDivisionBeats)); } diff --git a/tests/test_sampler_core.cpp b/tests/test_sampler_core.cpp index 00a6762..16cc49c 100644 --- a/tests/test_sampler_core.cpp +++ b/tests/test_sampler_core.cpp @@ -1380,6 +1380,34 @@ static void testTriggerEdgeCases() { } } +// --- Trigger out-of-domain lengthFraction: Voice::start's inline copy of trigger_seam's +// formula (map/trigger_seam.h) must clamp the same way the shared formula does — a +// frac > 1.0 never plays past the post-start span, and a NaN frees immediately rather than +// reaching the round()/static_cast undefined on a non-finite value. Pins the second +// implementation directly; test_trigger_seam.cpp pins the first. +static void testTriggerFracAboveOneClampsToSpan() { + // 150% length on a 200-frame sample -> clamped to the full 200-frame post-start span, + // not 300 frames (which would read off the end of the PCM). + SampleData km = (triggerSample(200, 1.5, 0, 0)); + VoiceEngine eng(1, km); + eng.noteOn(60, 127); + std::vector out; + eng.render(out, 220); + for (std::size_t i = 0; i < 200; ++i) CHECK(out[i] > 0.5f); + for (std::size_t i = 200; i < 220; ++i) CHECK(approx(out[i], 0.0, 1e-6)); + CHECK(eng.activeVoiceCount() == 0); // frees exactly at frameCount +} + +static void testTriggerFracNaNFreesImmediately() { + SampleData km = (triggerSample(200, std::nan(""), 5, 5)); + VoiceEngine eng(1, km); + eng.noteOn(60, 127); + std::vector out; + eng.render(out, 50); + for (float v : out) CHECK(approx(v, 0.0, 1e-6)); + CHECK(eng.activeVoiceCount() == 0); // playLen 0 -> frees at start, no sound +} + // --- Trigger ignores note-off (S15): the one-shot plays through regardless. --- static void testTriggerIgnoresNoteOff() { SampleData km = (triggerSample(200, 0.5, 0, 0)); @@ -2908,6 +2936,8 @@ int main() { testTriggerLengthWithStart(); testTriggerAhdFadeShape(); testTriggerEdgeCases(); + testTriggerFracAboveOneClampsToSpan(); + testTriggerFracNaNFreesImmediately(); testTriggerIgnoresNoteOff(); // S16 — pitch engine + pitch envelope.