From b5a573aebb4822d4b838ff693ada7746df6dddbb Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 26 Jul 2026 15:29:57 -0400 Subject: [PATCH] feat(bank_model): add Phase S seam fields (rootNote + loop points) to Sample Additive optional MIDI root note and sustain-loop points with JSON round-trip and deserialize-boundary validation; capture leaves them empty (not derivable). Mirrors the provenance addition; BankIndex behavior unchanged. --- src/bank_model.cpp | 57 +++++++++++++++- src/bank_model.h | 28 ++++++++ src/capture.cpp | 5 ++ src/realtime_record.cpp | 3 + tests/test_bank_model.cpp | 134 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 226 insertions(+), 1 deletion(-) diff --git a/src/bank_model.cpp b/src/bank_model.cpp index e451219..a5286ec 100644 --- a/src/bank_model.cpp +++ b/src/bank_model.cpp @@ -35,6 +35,10 @@ bool Levels::operator==(const Levels& o) const { return peakDb == o.peakDb && rmsDb == o.rmsDb && lufs == o.lufs; } +bool LoopPoints::operator==(const LoopPoints& o) const { + return start == o.start && end == o.end; +} + bool Sample::operator==(const Sample& o) const { return id == o.id && displayName == o.displayName && relativePath == o.relativePath && sourceMode == o.sourceMode && sourceRange == o.sourceRange && @@ -43,7 +47,8 @@ bool Sample::operator==(const Sample& o) const { lengthSeconds == o.lengthSeconds && lengthBeats == o.lengthBeats && captureTempo == o.captureTempo && captureTimeSigNum == o.captureTimeSigNum && - captureTimeSigDenom == o.captureTimeSigDenom && key == o.key && levels == o.levels && + captureTimeSigDenom == o.captureTimeSigDenom && key == o.key && + rootNote == o.rootNote && loop == o.loop && levels == o.levels && clipped == o.clipped && tier == o.tier && contentHash == o.contentHash && provenance == o.provenance && createdTimestamp == o.createdTimestamp; } @@ -253,6 +258,21 @@ void writeSample(std::string& out, const Sample& s) { w.keyBegin("key"); if (s.key) writeEscaped(out, *s.key); else out += "null"; + // Phase S seam fields (D-B). Emitted as null when absent (same shape as `key` + // and `provenance`) so pre-Phase-S JSON — which lacks these keys entirely — + // parses to empty optionals and re-serializes without invention. + w.keyBegin("rootNote"); + if (s.rootNote) out += numToStr(*s.rootNote); else out += "null"; + + w.keyBegin("loop"); + if (s.loop) { + ObjWriter lp(out); + lp.keyRaw("start", numToStr(s.loop->start)); + lp.keyRaw("end", numToStr(s.loop->end)); + } else { + out += "null"; + } + w.keyBegin("levels"); { ObjWriter l(out); @@ -608,6 +628,41 @@ bool Parser::parseSample(Sample& s) { if (!parseString(k)) return false; s.key = k; } + } else if (key == "rootNote") { + bool wasNull = false; + if (!expectNullOr(wasNull)) return false; + if (wasNull) { + s.rootNote.reset(); + } else { + int v = 0; + if (!parseInt(v)) return false; + // Valid MIDI note range: 0..127 inclusive (boundaries valid). + if (v < 0 || v > 127) return false; + s.rootNote = v; + } + } else if (key == "loop") { + bool wasNull = false; + if (!expectNullOr(wasNull)) return false; + if (wasNull) { + s.loop.reset(); + } else { + if (!consume('{')) return false; + LoopPoints lp; + do { + std::string lk; + if (!parseKey(lk)) return false; + std::int64_t lv = 0; + if (!parseInt64(lv)) return false; + if (lk == "start") lp.start = lv; + else if (lk == "end") lp.end = lv; + } while (consume(',')); + if (!consume('}')) return false; + // Invariant: 0 <= start <= end. start == end is a valid zero-length + // marker; a negative index or start > end is malformed, not silently + // clamped (mirrors the enum-range rejection above). + if (lp.start < 0 || lp.end < lp.start) return false; + s.loop = lp; + } } else if (key == "levels") { if (!consume('{')) return false; do { diff --git a/src/bank_model.h b/src/bank_model.h index 19ce437..5ad8472 100644 --- a/src/bank_model.h +++ b/src/bank_model.h @@ -63,6 +63,22 @@ struct Levels { bool operator==(const Levels& o) const; }; +// Sample-accurate sustain-loop bounds, as frame indices into the captured file +// (Phase S seam field, D-B). A bank intrinsic — a fact about the file, like +// sampleRate or length — consumed by the future MIDI-playback instrument to hold +// notes past the recorded length. Modeled as one optional struct (not two loose +// optionals) so "both points or neither" is a structural invariant, not a rule to +// re-check at every boundary. Frame indices, not seconds, because the loop is a +// per-sample-frame contract; the instrument reads the file's sample rate to relate +// them to time. Invariant (enforced at the deserialize boundary): 0 <= start <= end. +// start == end is a valid zero-length loop marker. +struct LoopPoints { + std::int64_t start = 0; + std::int64_t end = 0; + + bool operator==(const LoopPoints& o) const; +}; + // The metadata record for one captured sample. The audio itself lives in a // project-relative file; `relativePath` is ALWAYS relative (enforced at the // BankIndex::add boundary — see AddResult). @@ -95,6 +111,18 @@ struct Sample { std::optional key; // musical key, when known + // Phase S seam fields (D-B) — bank intrinsics for the MIDI-playback instrument, + // additive like `provenance` (M1). Both default cleanly empty: pre-Phase-S + // samples deserialize without them and re-serialize without inventing values. + // - rootNote: MIDI note (0..127) the sample was recorded at, so the instrument + // can repitch it across the keyboard. DISTINCT from the musical `key` above: + // `key` is a human label ("F#m"); `rootNote` is the exact pitch for repitch. + // Populated at/after capture only where derivable — left empty (never guessed) + // when the source is not a single played note. + // - loop: sustain-loop bounds, populated only where explicitly set. + std::optional rootNote; + std::optional loop; + Levels levels; bool clipped = false; diff --git a/src/capture.cpp b/src/capture.cpp index 48bb3b6..61c9113 100644 --- a/src/capture.cpp +++ b/src/capture.cpp @@ -530,6 +530,11 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { } } s.createdTimestamp = static_cast(std::time(nullptr)); + // Phase S seam fields (rootNote / loop) left empty (D-B). An offline render of a + // master mix / track / time-selection is not a single played note, so no root + // note is derivable here — we do NOT guess one. Loop points are set later by an + // explicit user action, not at capture. Leaving them empty is the honest default; + // the instrument (Phase S) treats an absent root note as "not a pitched sample". result.status = CaptureStatus::Ok; result.sample = s; diff --git a/src/realtime_record.cpp b/src/realtime_record.cpp index 9175005..9b287d1 100644 --- a/src/realtime_record.cpp +++ b/src/realtime_record.cpp @@ -58,6 +58,9 @@ Sample sampleFromRecordedCapture(const RecordedCapture& cap) { // finalized and on disk — the hash is over the finished file bytes. Left empty // here because sampleFromRecordedCapture runs before the file exists (the // mapping is pure / DAW-free); the shell patches it in after the move+trim. + // Phase S seam fields (rootNote / loop) left empty (D-B) — same reasoning as the + // offline path: a realtime record of wet output is not a single played note, so + // no root note is derivable; loop points are set by a later explicit action. s.createdTimestamp = cap.createdTimestamp; return s; } diff --git a/tests/test_bank_model.cpp b/tests/test_bank_model.cpp index 53aaf22..5de98db 100644 --- a/tests/test_bank_model.cpp +++ b/tests/test_bank_model.cpp @@ -36,6 +36,8 @@ static Sample fullSample(const std::string& seed) { s.captureTimeSigNum = 6; // L7 F1 meter stamp (non-4/4 to prove it round-trips) s.captureTimeSigDenom = 8; s.key = "F#m"; + s.rootNote = 60; // Phase S seam field (present) + s.loop = LoopPoints{4096, 65536}; // Phase S seam field (present) s.levels = {-0.3, -12.7, -14.2}; s.clipped = true; s.tier = Tier::Archive; @@ -86,11 +88,18 @@ static void testFullFieldRoundTrip() { CHECK(minMeter && minMeter->captureTimeSigNum == 0 && minMeter->captureTimeSigDenom == 0); CHECK(full && full->provenance.has_value()); CHECK(full && full->provenance->fxChainSnapshot == ""); + // Phase S seam fields survive round-trip exactly. + CHECK(full && full->rootNote.has_value() && *full->rootNote == 60); + CHECK(full && full->loop.has_value()); + CHECK(full && full->loop && full->loop->start == 4096 && full->loop->end == 65536); const Sample* min = back->query("min-b"); CHECK(min && !min->key.has_value()); CHECK(min && !min->provenance.has_value()); CHECK(min && min->trackGuids.empty()); + // Seam fields absent on the minimal sample and stay absent. + CHECK(min && !min->rootNote.has_value()); + CHECK(min && !min->loop.has_value()); } } @@ -417,6 +426,128 @@ static void testEnumRangeValidation() { CHECK(!r2.has_value()); } +// S2 test case 2: a legacy Sample JSON — written before the Phase S seam fields +// existed, so it has NO "rootNote" or "loop" keys at all — parses to clean empty +// optionals (no loss, no migration) and re-serializes without inventing values. +// (The parser's forward-compat unknown-key skipping is what makes the reverse case +// — new keys ignored by an old parser — safe too; here we test old-JSON→new-parser.) +static void testLegacyJsonDefaults() { + const char* legacy = + "{\"samples\":[{\"id\":\"leg1\",\"relativePath\":\"bank/leg.wav\"," + "\"displayName\":\"legacy\"," + "\"sourceMode\":0,\"sourceRange\":{\"startSeconds\":1.5,\"endSeconds\":2.5," + "\"startPpq\":0.0,\"endPpq\":0.0},\"trackGuids\":[]," + "\"wetDry\":1.0,\"channelCount\":2,\"sampleRate\":48000," + "\"lengthSeconds\":1.0,\"lengthBeats\":0.0,\"captureTempo\":120.0," + "\"key\":null,\"levels\":{\"peakDb\":0.0,\"rmsDb\":0.0,\"lufs\":0.0}," + "\"clipped\":false,\"tier\":0,\"contentHash\":\"h-leg1\"," + "\"provenance\":null,\"createdTimestamp\":0}]}"; + auto r = BankIndex::deserialize(legacy); + CHECK(r.has_value()); + if (r) { + const Sample* s = r->query("leg1"); + CHECK(s != nullptr); + CHECK(s && !s->rootNote.has_value()); // clean default, not a guessed value + CHECK(s && !s->loop.has_value()); + + // Re-serialize is lossless: parsing it again yields an equal index. This + // proves the absent fields did not silently gain values on the way out. + std::string out = r->serialize(); + auto again = BankIndex::deserialize(out); + CHECK(again.has_value()); + CHECK(again && *again == *r); + if (again) { + const Sample* s2 = again->query("leg1"); + CHECK(s2 && !s2->rootNote.has_value()); + CHECK(s2 && !s2->loop.has_value()); + } + } +} + +// S2 test case 4: boundary values for the seam fields are representable and +// round-trip. rootNote 0 and 127 (the MIDI edges); loopStart == loopEnd (a valid +// zero-length marker); a loop whose end sits at the file's last frame. Also asserts +// the deserialize-boundary validation rules reject out-of-range input rather than +// storing a bogus value. +static void testSeamFieldBoundaries() { + // rootNote at both MIDI edges + equal-and-end-anchored loop points round-trip. + BankIndex idx; + Sample lo = minimalSample("lo"); lo.contentHash = "h-lo"; + lo.rootNote = 0; + lo.loop = LoopPoints{0, 0}; // zero-length marker at frame 0 + Sample hi = minimalSample("hi"); hi.contentHash = "h-hi"; + hi.rootNote = 127; + hi.loop = LoopPoints{100, 100}; // start == end elsewhere + Sample end = minimalSample("end"); end.contentHash = "h-end"; + end.loop = LoopPoints{0, 9223372036854775807LL}; // end at max int64 frame + CHECK(idx.add(lo) == AddResult::Added); + CHECK(idx.add(hi) == AddResult::Added); + CHECK(idx.add(end) == AddResult::Added); + + auto back = BankIndex::deserialize(idx.serialize()); + CHECK(back.has_value()); + CHECK(back && *back == idx); + if (back) { + CHECK(back->query("min-lo")->rootNote == 0); + CHECK(back->query("min-hi")->rootNote == 127); + // Named local: a brace-init with a comma inside CHECK(...) would be parsed + // as two macro arguments by the preprocessor. + const LoopPoints zeroLen{0, 0}; + CHECK(back->query("min-lo")->loop == zeroLen); + CHECK(back->query("min-end")->loop->end == 9223372036854775807LL); + } + + // Validation rule (chosen for this design, surfaced in the handoff): + // rootNote must be 0..127; loop must satisfy 0 <= start <= end. + // Out-of-range input is rejected at the deserialize boundary (nullopt), mirroring + // the existing enum-range and integer-overflow rejections — never clamped. + const char* head = + "{\"samples\":[{\"id\":\"bad\",\"relativePath\":\"bank/b.wav\"," + "\"displayName\":\"\",\"sourceMode\":0," + "\"sourceRange\":{\"startSeconds\":0.0,\"endSeconds\":0.0," + "\"startPpq\":0.0,\"endPpq\":0.0},\"trackGuids\":[]," + "\"wetDry\":1.0,\"channelCount\":1,\"sampleRate\":44100," + "\"lengthSeconds\":0.0,\"lengthBeats\":0.0,\"captureTempo\":0.0," + "\"key\":null,"; + const char* tail = + "\"levels\":{\"peakDb\":0.0,\"rmsDb\":0.0,\"lufs\":0.0}," + "\"clipped\":false,\"tier\":0,\"contentHash\":\"h-bad\"," + "\"provenance\":null,\"createdTimestamp\":0}]}"; + + CHECK(!BankIndex::deserialize(std::string(head) + "\"rootNote\":128," + tail).has_value()); + CHECK(!BankIndex::deserialize(std::string(head) + "\"rootNote\":-1," + tail).has_value()); + CHECK(!BankIndex::deserialize( + std::string(head) + "\"loop\":{\"start\":10,\"end\":5}," + tail).has_value()); // start > end + CHECK(!BankIndex::deserialize( + std::string(head) + "\"loop\":{\"start\":-1,\"end\":5}," + tail).has_value()); // negative start +} + +// S2 test case 3: the seam-field addition is purely additive — dedup-by-hash, tier +// moves/filtering, and BankIndex ordering are byte-for-byte unchanged by the +// presence (or absence) of rootNote/loop. Two samples differing ONLY in seam fields +// but sharing a content hash still collapse; a seam-populated sample tiers exactly +// like any other. +static void testSeamFieldsAdditiveInvariant() { + BankIndex idx; + Sample a = fullSample("z"); // has rootNote + loop populated + CHECK(idx.add(a) == AddResult::Added); + + // Same hash, seam fields cleared — dedup keys off contentHash only, so this + // still collapses. Seam fields do NOT enter the dedup identity. + Sample dup = fullSample("z2"); + dup.contentHash = a.contentHash; + dup.rootNote.reset(); + dup.loop.reset(); + CHECK(idx.add(dup) == AddResult::Collapsed); + CHECK(idx.size() == 1); + + // Tier move on a seam-populated sample behaves exactly as before. + CHECK(idx.query("id-z")->tier == Tier::Archive); + CHECK(idx.moveTier("id-z", Tier::Scratch)); + CHECK(idx.query("id-z")->tier == Tier::Scratch); + CHECK(idx.query("id-z")->rootNote == 60); // move did not disturb seam fields +} + int main() { testFullFieldRoundTrip(); testDedupByHash(); @@ -430,6 +561,9 @@ int main() { testUnicodeEscapeDecoding(); testIntegerOverflow(); testEnumRangeValidation(); + testLegacyJsonDefaults(); + testSeamFieldBoundaries(); + testSeamFieldsAdditiveInvariant(); if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0;