diff --git a/CMakeLists.txt b/CMakeLists.txt index 21fdc03..d37baef 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -934,12 +934,6 @@ add_library(browser_scroll STATIC src/core/instrument/ui/browser_scroll.cpp) target_include_directories(browser_scroll PUBLIC src) target_link_libraries(browser_scroll PUBLIC capture_browser sample_chrome) -# note_entry (Phase S12) — PURE text->clamped-MIDI-note parse for the direct numeric entry of -# a zone's low/high/root (decimal integer OR note name under the C4==60 convention, clamped to -# [0,127]). No dependency beyond the standard library. NEITHER SDK. -add_library(note_entry STATIC src/core/instrument/map/note_entry.cpp) -target_include_directories(note_entry PUBLIC src) - # param_slider (Phase S12 + the S15/S16 control surfaces deferred here) — PURE control-surface # layout + hit-test + normalized value<->pixel mapping for the editor parameter panel (the # Gate|Trigger + Varispeed|Preserve toggles and the AHDSR / Trigger / pitch-env sliders). The @@ -961,8 +955,8 @@ target_include_directories(trigger_seam PUBLIC src) # envelope overlay: AHDSR (Gate) / fade+%-length (Trigger) params + the sample's wall-clock # duration -> a breakpoint polyline in the waveform rect, at the same time base waveform_view maps. # The mirror of waveform_view / param_slider; links editor_geometry for the shared Rect. -# Deliberately engine-free (no sample_map / sampler_core) — the shell packs the zone's stored -# AdsrSeconds / TriggerParams into the small AmpEnvelope view struct. NEITHER SDK. +# Deliberately engine-free (no sample_map / sampler_core) — the shell packs the one parameter +# set's stored AdsrSeconds / TriggerParams into the small AmpEnvelope view struct. NEITHER SDK. add_library(envelope_overlay STATIC src/core/instrument/ui/envelope_overlay.cpp) target_include_directories(envelope_overlay PUBLIC src) target_link_libraries(envelope_overlay PUBLIC editor_geometry) @@ -1057,11 +1051,6 @@ add_executable(browser_scroll_tests tests/test_browser_scroll.cpp) target_link_libraries(browser_scroll_tests PRIVATE browser_scroll) add_test(NAME browser_scroll_tests COMMAND browser_scroll_tests) -# note_entry (S12): the pure text->clamped-MIDI-note parse for direct numeric entry. -add_executable(note_entry_tests tests/test_note_entry.cpp) -target_link_libraries(note_entry_tests PRIVATE note_entry) -add_test(NAME note_entry_tests COMMAND note_entry_tests) - # param_slider (S12 + S15/S16 control surfaces): the pure control-panel layout + slider/toggle # value<->pixel mapping the editor parameter surface draws + routes against. add_executable(param_slider_tests tests/test_param_slider.cpp) @@ -1319,11 +1308,12 @@ if(WIN32 AND EXISTS "${VST3_SDK}/public.sdk/source/main/pluginfactory.cpp") ${LICE_SRC} ) # editor_geometry + bridge_marshal: the pure spike helpers. sample_map (S4): the pure - # bank->keymap mapping + state (de)ser the processor drives off the audio thread; - # linking it pulls its pure deps (bank_book, wav_codec, sampler_core, bank_model, - # peaks) transitively. capture_paths: the shared M4 path resolution (resolveBankFile / - # projectDirOfRpp) the bridge + processor use. Its PUBLIC include dir (src) - # gives the shell TUs their headers (ext_keys.h, bank_book.h, sampler_core.h, ...). + # bank -> one-capture resolve + SampleData build the processor drives off the audio + # thread; linking it pulls its pure deps (bank_book, wav_codec, velocity_curve, peaks) + # transitively — deliberately NOT sampler_core (the voice engine). capture_paths: the + # shared M4 path resolution (resolveBankFile / projectDirOfRpp) the bridge + processor + # use. Its PUBLIC include dir (src) + # gives the shell TUs their headers (ext_keys.h, bank_book.h, voice_engine.h, ...). # embed_strip (S6): the pure inline-strip layout + hit-test the embed shell marshals # into; it links editor_geometry transitively (shared Rect). # app_version: ext_keys.h's channel-derived namespace accessor (V4) delegates to it, so @@ -1342,11 +1332,11 @@ if(WIN32 AND EXISTS "${VST3_SDK}/public.sdk/source/main/pluginfactory.cpp") # bank_sync (S9/S8 reader): the pure generation-compare + assignment-consume decision the # processor's off-thread poll runs; links assignment_request transitively (the decoded # request it consumes) — the same key the extension writes, shared via the pure module. - # browser_scroll + note_entry + param_slider (S12 + S15/S16 control surfaces): the pure - # scroll/search geometry over the capture browser, the numeric-note-entry parse, and the - # control-panel layout + slider/toggle value<->pixel mapping the editor's parameter surface - # draws + routes against. browser_scroll pulls capture_browser transitively; param_slider + - # note_entry link editor_geometry / the stdlib only. All engine-free, DAW-verified in the shell. + # browser_scroll + param_slider (S12 + S15/S16 control surfaces): the pure scroll/search + # geometry over the capture browser, and the control-panel layout + slider/toggle + # value<->pixel mapping the editor's parameter surface draws + routes against. + # browser_scroll pulls capture_browser transitively; param_slider links editor_geometry / + # the stdlib only. All engine-free, DAW-verified in the shell. # theme + component_geometry + bank_grid: the Phase L (L1) draw-kit's PURE deps (L3). The # kit draws every editor/embed surface by palette ROLE via draw_kit.cpp (compiled into the # module above): theme supplies role->KitColor + spectralColor, component_geometry the @@ -1364,7 +1354,7 @@ if(WIN32 AND EXISTS "${VST3_SDK}/public.sdk/source/main/pluginfactory.cpp") target_link_libraries(reasampler_vst PRIVATE vst3_sdk editor_geometry bridge_marshal sampler_core sample_map component_state_io capture_paths embed_strip app_version capture_browser keyboard_strip sample_bands sample_chrome - waveform_view bank_sync browser_scroll note_entry param_slider + waveform_view bank_sync browser_scroll param_slider theme component_geometry bank_grid trigger_seam envelope_overlay envelope_edit knob_deck curve_popup master_gain sample_usage file_bytes) # SDK_INC gives reaper_vst3_interfaces.h + reaper_plugin_functions.h for the bridge; diff --git a/src/core/instrument/CLAUDE.md b/src/core/instrument/CLAUDE.md index 46bf695..3ae371f 100644 --- a/src/core/instrument/CLAUDE.md +++ b/src/core/instrument/CLAUDE.md @@ -212,7 +212,6 @@ slider couldn't. Two pure modules split the forward (draw) and inverse (edit) ma - `component_state_io` (`core/instrument/map`) — the `ComponentState` envelope + params-payload binary codec (envelope v1…v11, params payload v1…v8), split out of `sample_map` (Q-W2v, T4-13 ≡ T2-07) so BOTH artifacts can link the codec without the extension pulling in the whole voice engine to serialize one preset blob — the extension's `instrument_drop` and the instrument's processor read/write the identical bytes, so the cross-artifact contract cannot drift. Payload v1…v7 are the RETIRED per-zone lists: still read, lifting by adopting zone one's capture + parameters (that first zone is what the old first-match resolve actually played, so it is also what supersedes the envelope's stored selection id). - `bank_sync` — generation change-detection + assignment-request consume: owns the yes/no decision logic so the rules are provable without a host. The processor shell owns cadence and side effects. - `bridge_marshal` — pure marshalling helper for the REAPER VST-host bridge read: interprets the `GetProjExtState` int return against its filled buffer. -- `note_entry` — parses a raw string into a clamped MIDI note [0,127]; accepts plain decimal integers or note names (C4==60, DAW convention). - `trigger_seam` — pure Trigger frames↔fraction converter: owns the shared formula for converting between engine source-frame fade counts and the overlay's fractional representation, threading `startFrame` correctly through pack and unpack directions. ### `ui/` diff --git a/src/core/instrument/map/note_entry.cpp b/src/core/instrument/map/note_entry.cpp deleted file mode 100644 index f2119a7..0000000 --- a/src/core/instrument/map/note_entry.cpp +++ /dev/null @@ -1,111 +0,0 @@ -// note_entry.cpp — see note_entry.h. - -#include "core/instrument/map/note_entry.h" - -#include -#include - -namespace reasampler::instrument::map { - -namespace { -char asciiUpper(char c) { - return static_cast(std::toupper(static_cast(c))); -} - -std::string trim(const std::string& s) { - std::size_t a = 0; - std::size_t b = s.size(); - while (a < b && std::isspace(static_cast(s[a]))) ++a; - while (b > a && std::isspace(static_cast(s[b - 1]))) --b; - return s.substr(a, b - a); -} - -int clampNote(long long n) { - if (n < 0) return 0; - if (n > 127) return 127; - return static_cast(n); -} - -// Semitone offset within an octave for a note letter (C..B), or -1 for a non-letter. -int letterSemitone(char up) { - switch (up) { - case 'C': return 0; - case 'D': return 2; - case 'E': return 4; - case 'F': return 5; - case 'G': return 7; - case 'A': return 9; - case 'B': return 11; - default: return -1; - } -} - -// Parse a note name like "C4", "F#3", "Bb-1" (case-insensitive, DAW convention: -// MIDI 0 == C-1, 60 == C4). Returns nullopt if it is not a note name. -std::optional parseNoteName(const std::string& s) { - if (s.empty()) return std::nullopt; - std::size_t i = 0; - const int base = letterSemitone(asciiUpper(s[i])); - if (base < 0) return std::nullopt; // not a letter -> not a note name - ++i; - int semitone = base; - // Optional accidental(s): # / b only (not 's'/'f'). - while (i < s.size() && (s[i] == '#' || s[i] == 'b' || s[i] == 'B')) { - if (s[i] == '#') ++semitone; - else --semitone; - ++i; - } - // The octave: an optional sign then digits, running to the end. - if (i >= s.size()) return std::nullopt; // a bare "C" has no octave -> reject (ambiguous) - bool neg = false; - if (s[i] == '+' || s[i] == '-') { - neg = (s[i] == '-'); - ++i; - } - if (i >= s.size()) return std::nullopt; - int octave = 0; - bool anyDigit = false; - for (; i < s.size(); ++i) { - if (!std::isdigit(static_cast(s[i]))) return std::nullopt; - octave = octave * 10 + (s[i] - '0'); - anyDigit = true; - } - if (!anyDigit) return std::nullopt; - if (neg) octave = -octave; - // MIDI note = (octave + 1) * 12 + semitone (C-1 == 0, C4 == 60). - const long long note = static_cast(octave + 1) * 12 + semitone; - return clampNote(note); -} - -std::optional parseInteger(const std::string& s) { - if (s.empty()) return std::nullopt; - std::size_t i = 0; - bool neg = false; - if (s[i] == '+' || s[i] == '-') { - neg = (s[i] == '-'); - ++i; - } - if (i >= s.size()) return std::nullopt; - long long v = 0; - for (; i < s.size(); ++i) { - if (!std::isdigit(static_cast(s[i]))) return std::nullopt; - v = v * 10 + (s[i] - '0'); - if (v > 1000000) v = 1000000; // saturate; clampNote takes it to 127 anyway - } - if (neg) v = -v; - return clampNote(v); -} -} // namespace - -std::optional parseNoteEntry(const std::string& text) { - const std::string s = trim(text); - if (s.empty()) return std::nullopt; - // Try a plain integer first (the common MIDI-number case); fall back to a note name. - if (std::isdigit(static_cast(s[0])) || s[0] == '+' || - (s[0] == '-' && s.size() > 1 && std::isdigit(static_cast(s[1])))) { - if (auto n = parseInteger(s)) return n; - } - return parseNoteName(s); -} - -} // namespace reasampler::instrument::map diff --git a/src/core/instrument/map/note_entry.h b/src/core/instrument/map/note_entry.h deleted file mode 100644 index 3ecb4d4..0000000 --- a/src/core/instrument/map/note_entry.h +++ /dev/null @@ -1,18 +0,0 @@ -// note_entry — parse + clamp for direct numeric/note-name entry of a zone's low/high/root -// MIDI note (a drag on the keyboard strip can't hit a precise note reliably). -// -// Accepts a plain decimal integer ("60", "+5") or a note name ("C4", "f#3", "Bb-1", DAW -// convention: MIDI 0 == C-1, 60 == C4). Out-of-range CLAMPS to [0,127] rather than -// rejecting; unparseable input returns nullopt (shell keeps the old value). - -#pragma once - -#include -#include - -namespace reasampler::instrument::map { - -// Leading/trailing whitespace ignored. Empty or unparseable input returns nullopt. -std::optional parseNoteEntry(const std::string& text); - -} // namespace reasampler::instrument::map diff --git a/src/core/instrument/ui/curve_popup.h b/src/core/instrument/ui/curve_popup.h index 3eea28d..f9fb146 100644 --- a/src/core/instrument/ui/curve_popup.h +++ b/src/core/instrument/ui/curve_popup.h @@ -6,7 +6,7 @@ // focused sub-editor, not a view change): width/height each clamp to a fraction of the // window within min/max bounds. A title row sits over the curve box. The curve box rect // here is the border rect — the shell derives the mapping box via its curveBoxFromRect -// formula, so the popup editor and the Zone-panel inline editor share coordinates. +// formula. #pragma once diff --git a/src/core/instrument/ui/envelope_edit.h b/src/core/instrument/ui/envelope_edit.h index 8437fcc..55a48a7 100644 --- a/src/core/instrument/ui/envelope_edit.h +++ b/src/core/instrument/ui/envelope_edit.h @@ -3,8 +3,9 @@ // outside the DAW; the shell draws handles, captures the grab, and feeds pixel deltas back in. // // envelope_overlay owns the params->polyline forward (draw) map; this module owns the inverse -// (edit) map + hit-test. Both read/write the same AmpEnvelope fields (shell re-reads the zone -// every paint), so a node drag and a slider edit are two views on one source of truth. +// (edit) map + hit-test. Both read/write the same AmpEnvelope fields (shell re-reads the one +// parameter set every paint), so a node drag and a slider edit are two views on one source of +// truth. // // A drag can never produce a param a slider couldn't: nodes are monotonic in time (clamped // between time predecessor/successor) and range-clamped to the same per-param [min,max] the diff --git a/src/core/instrument/ui/envelope_overlay.h b/src/core/instrument/ui/envelope_overlay.h index d65f9e2..0b88197 100644 --- a/src/core/instrument/ui/envelope_overlay.h +++ b/src/core/instrument/ui/envelope_overlay.h @@ -1,7 +1,8 @@ // envelope_overlay.h — amp-envelope -> polyline geometry for the Sample-view envelope overlay. // Engine-free by design (no sample_map/sampler_core dependency); mirror of waveform_view / -// param_slider. The shell packs the zone's AdsrSeconds/TriggerParams into AmpEnvelope and draws -// the polyline plus a handle at each node (envelope_edit does the hit-test). +// param_slider. The shell packs the one parameter set's AdsrSeconds/TriggerParams into +// AmpEnvelope and draws the polyline plus a handle at each node (envelope_edit does the +// hit-test). #pragma once diff --git a/src/shell/instrument/CLAUDE.md b/src/shell/instrument/CLAUDE.md index 1929c69..d6fe147 100644 --- a/src/shell/instrument/CLAUDE.md +++ b/src/shell/instrument/CLAUDE.md @@ -10,7 +10,7 @@ two small identity/helper headers this directory owns outright The pure engine/geometry core this shell wraps (`sampler_core`, `pitch_shift`, `sample_map`, `component_state_io`, `play_params.h`, `editor_geometry`, `sample_bands`, `sample_chrome`, `keyboard_strip`, `waveform_view`, `capture_browser`, `browser_scroll`, -`note_entry`, `param_slider`, `trigger_seam`, `velocity_curve`, `embed_strip`, `knob_deck`, +`param_slider`, `trigger_seam`, `velocity_curve`, `embed_strip`, `knob_deck`, `curve_popup`, `master_gain`, `reasampler_uid.h`) lives in `core/instrument/*` and `core/wire` and is documented there — this directory consumes it but does not own it. diff --git a/src/shell/instrument/processor_reload.cpp b/src/shell/instrument/processor_reload.cpp index a1bfc68..c34a1b3 100644 --- a/src/shell/instrument/processor_reload.cpp +++ b/src/shell/instrument/processor_reload.cpp @@ -232,7 +232,7 @@ void ReaSamplerProcessor::publishBuiltLocked(std::unique_ptr b void ReaSamplerProcessor::rebuildVoiceEngine() { // Off the audio thread. A voice-param change touches no audio data, so this rebuilds - // the engine around a copy of the live instrument's already-decoded keymap — no + // the engine around a copy of the live instrument's already-decoded SampleData — no // bridge, no disk — and publishes through the same drain-slot swap. std::lock_guard lock(reloadMutex_); LoadedInstrument* cur = live_.load(std::memory_order_acquire); diff --git a/src/shell/instrument/reasampler_embed.cpp b/src/shell/instrument/reasampler_embed.cpp index 690e208..4381a8e 100644 --- a/src/shell/instrument/reasampler_embed.cpp +++ b/src/shell/instrument/reasampler_embed.cpp @@ -78,10 +78,13 @@ void ReaSamplerEmbed::refreshLoaded() { const InstrumentParams params = processor_->instrumentParams(); if (params.rootOverride) { rootNote_ = *params.rootOverride; - } else if (const SelectedSample* ref = findRef(processor_->sampleRefs(), loadedId_)) { - rootNote_ = ref->rootNote; } else { - rootNote_ = 60; + const SampleRefs refs = processor_->sampleRefs(); + if (const SelectedSample* ref = findRef(refs, loadedId_)) { + rootNote_ = ref->rootNote; + } else { + rootNote_ = 60; + } } } diff --git a/src/shell/instrument/reasampler_processor.h b/src/shell/instrument/reasampler_processor.h index c1ccfeb..568f166 100644 --- a/src/shell/instrument/reasampler_processor.h +++ b/src/shell/instrument/reasampler_processor.h @@ -165,8 +165,8 @@ public: void setPreviewVelocity(std::uint8_t velocity); // Voice-system parameters (per-instance), guarded by voiceParamsMutex_, not read on the - // audio thread — each setter rebuilds via rebuildVoiceEngine (already-decoded keymap, no - // bridge/WAV re-read) through the same drain-slot swap, so a change never cuts a tail. + // audio thread — each setter rebuilds via rebuildVoiceEngine (already-decoded SampleData, + // no bridge/WAV re-read) through the same drain-slot swap, so a change never cuts a tail. int voiceCount(); void setVoiceCount(int count); // clamped to kMinVoiceCount..kMaxVoiceCount VoiceMode voiceMode(); diff --git a/tests/test_component_state_io.cpp b/tests/test_component_state_io.cpp index adbe7f5..76d086d 100644 --- a/tests/test_component_state_io.cpp +++ b/tests/test_component_state_io.cpp @@ -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& 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 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 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 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 current = serializeComponentState(in); legacy::Zone z; @@ -685,18 +888,19 @@ static void testTruncationDegradesCleanly() { const std::vector part(blob->begin(), blob->begin() + static_cast(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(); diff --git a/tests/test_note_entry.cpp b/tests/test_note_entry.cpp deleted file mode 100644 index 1107f36..0000000 --- a/tests/test_note_entry.cpp +++ /dev/null @@ -1,74 +0,0 @@ -// Standalone tests for reasampler::instrument::map::note_entry — no VST3, no REAPER, no framework. -// Assert the S12 direct-numeric-entry parse for a zone's low/high/root MIDI note. -// -// Covers: plain decimal integers (with +/- sign + surrounding whitespace); note names under the -// C4==60 convention (C-1==0, sharps + flats, negative octaves); out-of-range values CLAMPING to -// [0,127] rather than rejecting; empty / whitespace-only / unparseable input returning nullopt; -// the integer path taking precedence over the note-name path for a leading digit. - -#include "../src/core/instrument/map/note_entry.h" - -#include - -using namespace reasampler; -using namespace reasampler::instrument::map; - -static int g_fail = 0; -#define CHECK(cond) do { if(!(cond)) { \ - std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) - -static void testPlainIntegers() { - CHECK(parseNoteEntry("60") == 60); - CHECK(parseNoteEntry("0") == 0); - CHECK(parseNoteEntry("127") == 127); - CHECK(parseNoteEntry(" 64 ") == 64); // surrounding whitespace ignored - CHECK(parseNoteEntry("+5") == 5); -} - -static void testIntegerClamps() { - CHECK(parseNoteEntry("200") == 127); // over-range clamps to the ceiling - CHECK(parseNoteEntry("-10") == 0); // under-range clamps to the floor - CHECK(parseNoteEntry("99999") == 127); -} - -static void testNoteNames() { - // C4 == 60 (MIDI 0 == C-1). - CHECK(parseNoteEntry("C4") == 60); - CHECK(parseNoteEntry("c4") == 60); // case-insensitive - CHECK(parseNoteEntry("A4") == 69); // A4 = 69 (concert A) - CHECK(parseNoteEntry("C-1") == 0); // lowest MIDI note - CHECK(parseNoteEntry("G9") == 127); // G9 = 127 -} - -static void testAccidentals() { - CHECK(parseNoteEntry("C#4") == 61); - CHECK(parseNoteEntry("Db4") == 61); // enharmonic of C#4 - CHECK(parseNoteEntry("F#3") == 54); - CHECK(parseNoteEntry("Bb3") == 58); // Bb3 = 58 -} - -static void testNoteNameClamps() { - CHECK(parseNoteEntry("C10") == 127); // above the range clamps - CHECK(parseNoteEntry("C-5") == 0); // below the range clamps -} - -static void testRejects() { - CHECK(parseNoteEntry("") == std::nullopt); - CHECK(parseNoteEntry(" ") == std::nullopt); - CHECK(parseNoteEntry("hello") == std::nullopt); - CHECK(parseNoteEntry("C") == std::nullopt); // a bare letter with no octave is ambiguous - CHECK(parseNoteEntry("H4") == std::nullopt); // H is not a note letter - CHECK(parseNoteEntry("+") == std::nullopt); -} - -int main() { - testPlainIntegers(); - testIntegerClamps(); - testNoteNames(); - testAccidentals(); - testNoteNameClamps(); - testRejects(); - - if (g_fail == 0) std::printf("note_entry: all tests passed\n"); - return g_fail != 0; -}