From 66d47fb4109c679192a58db61c74068af778612b Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 26 Jul 2026 23:46:06 -0400 Subject: [PATCH] feat(S9/S8): bank-generation change-detection + instrument-side assignment reader Extension stamps a monotonic bank_generation counter, bumped at content mutations; the VST3 instrument polls it off-thread on an editor timer and consumes assignment requests, refreshing playback hands-free. --- CMakeLists.txt | 24 ++++- src/actions.cpp | 23 +++- src/actions.h | 10 +- src/bank_panel.cpp | 11 +- src/ext_keys.h | 13 +++ src/ingest.cpp | 7 ++ src/main.cpp | 26 ++++- src/persist.cpp | 21 ++++ src/persist.h | 27 +++++ src/vst/bank_sync.cpp | 70 ++++++++++++ src/vst/bank_sync.h | 105 ++++++++++++++++++ src/vst/reasampler_editor.cpp | 50 +++++++++ src/vst/reasampler_editor.h | 9 ++ src/vst/reasampler_embed.cpp | 39 ++++++- src/vst/reasampler_embed.h | 13 +++ src/vst/reasampler_processor.cpp | 87 ++++++++++++++- src/vst/reasampler_processor.h | 37 +++++++ src/vst/sample_map.cpp | 26 ++++- src/vst/sample_map.h | 41 ++++--- tests/test_bank_sync.cpp | 179 +++++++++++++++++++++++++++++++ tests/test_sample_map.cpp | 56 ++++++++++ 21 files changed, 838 insertions(+), 36 deletions(-) create mode 100644 src/vst/bank_sync.cpp create mode 100644 src/vst/bank_sync.h create mode 100644 tests/test_bank_sync.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index a0328a4..580bf2e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -685,6 +685,17 @@ add_library(waveform_view STATIC src/vst/waveform_view.cpp) target_include_directories(waveform_view PUBLIC src/vst src) target_link_libraries(waveform_view PUBLIC editor_geometry peaks) +# bank_sync (Phase S9/S8 reader) — PURE decision logic for the instrument's off-audio-thread +# poll: parse/compare the S9 bank-generation stamp, and the S8 assignment-request CONSUME +# decision (new-and-resolvable-and-target -> apply; unresolvable -> drop-and-mark; non-target +# -> stay eligible). The shell owns the timer cadence + side effects (reloadFromBank, +# setSelectedSampleId, component-state marker); this owns only the yes/no maths, unit-tested +# outside the DAW. Links assignment_request for the decoded AssignmentRequest it consumes. +# NEITHER SDK. +add_library(bank_sync STATIC src/vst/bank_sync.cpp) +target_include_directories(bank_sync PUBLIC src/vst src) +target_link_libraries(bank_sync PUBLIC assignment_request) + add_executable(editor_geometry_tests tests/test_editor_geometry.cpp) target_link_libraries(editor_geometry_tests PRIVATE editor_geometry) add_test(NAME editor_geometry_tests COMMAND editor_geometry_tests) @@ -718,6 +729,12 @@ add_executable(waveform_view_tests tests/test_waveform_view.cpp) target_link_libraries(waveform_view_tests PRIVATE waveform_view) add_test(NAME waveform_view_tests COMMAND waveform_view_tests) +# bank_sync (S9/S8 reader): the pure generation-parse + assignment-consume decision. Links +# ONLY bank_sync (+ its assignment_request dep) — NEITHER SDK — the plain-data-boundary proof. +add_executable(bank_sync_tests tests/test_bank_sync.cpp) +target_link_libraries(bank_sync_tests PRIVATE bank_sync) +add_test(NAME bank_sync_tests COMMAND bank_sync_tests) + # --------------------------------------------------------------------------- # 4) The REAPER extension — a loadable module (dlopen'd by REAPER, not linked). # --------------------------------------------------------------------------- @@ -770,7 +787,7 @@ add_library(reaper_reasampler MODULE src/card_meta.cpp src/card_drag.cpp ) -target_link_libraries(reaper_reasampler PRIVATE bank_model capture_paths peaks bank_grid mode_switch tab_strip view_mode_model insert_plan render_settings batch_capture tail_control realtime_record bank_book wav_trim owned_manifest prune_reconcile prune_button app_version provenance action_buttons drag_out theme component_geometry action_bar footer_bar overflow_menu mode_enable tooltip card_meta card_drag assignment_request) +target_link_libraries(reaper_reasampler PRIVATE bank_model capture_paths peaks bank_grid mode_switch tab_strip view_mode_model insert_plan render_settings batch_capture tail_control realtime_record bank_book wav_trim owned_manifest prune_reconcile prune_button app_version provenance action_buttons drag_out theme component_geometry action_bar footer_bar overflow_menu mode_enable tooltip card_meta card_drag assignment_request bank_sync) target_include_directories(reaper_reasampler PRIVATE ${SDK_INC} ${WDL_INC}) # OUTPUT_NAME is channel-derived (Phase V, V4): "reaper_reasampler" (stable, default) or # "reaper_reasampler_beta" (beta). REAPER dlopen's any reaper_* module, so both channels' @@ -901,9 +918,12 @@ if(WIN32 AND EXISTS "${VST3_SDK}/public.sdk/source/main/pluginfactory.cpp") # waveform_view (S11): the pure frame<->pixel marker geometry + zero-crossing snap the # editor's waveform surface draws + hit-tests against; links editor_geometry + peaks # transitively (shared Rect + AudioSample). + # 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. target_link_libraries(reasampler_vst PRIVATE vst3_sdk editor_geometry bridge_marshal sample_map capture_paths embed_strip app_version capture_browser keyboard_strip - waveform_view) + waveform_view bank_sync) # SDK_INC gives reaper_vst3_interfaces.h + reaper_plugin_functions.h for the bridge; # WDL_INC gives LICE for the editor. The VST3 SDK headers come from vst3_sdk PUBLIC. target_include_directories(reasampler_vst PRIVATE ${SDK_INC} ${WDL_INC}) diff --git a/src/actions.cpp b/src/actions.cpp index 1868561..be69618 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -645,7 +645,9 @@ void doBankDelete() { ShowConsoleMsg("ReaSampler: cannot delete that bank (the pool is un-deletable).\n"); return; } - persistBankOp("ReaSampler: delete bank"); + // S9: bump only when the deleted bank held samples — dropping them changes what a live + // instance referencing one could play. Deleting an EMPTY bank is purely organizational. + persistBankOp("ReaSampler: delete bank", /*bumpGeneration=*/members > 0); } // Evacuate a named bank: move every member back to the pool (index-only, collapse by @@ -666,7 +668,8 @@ void doBankEvacuate() { "destination, not a source).\n"); return; } - persistBankOp("ReaSampler: evacuate bank"); + // S9: evacuate moves members between banks (bank membership changes) -> bump. + persistBankOp("ReaSampler: evacuate bank", /*bumpGeneration=*/true); } // Cycle the active bank forward in ordinal order (pool -> named -> ... -> pool), @@ -752,7 +755,9 @@ void doBankTransferSelected(bool copy) { if (mutated) { const std::string label = std::string("ReaSampler: ") + verb + " sample(s)"; - persistBankOp(label.c_str()); + // S9: a move/copy changes bank membership (a sample arrives in / leaves a bank an + // instance may reference) -> bump so assigned instances refresh hands-free. + persistBankOp(label.c_str(), /*bumpGeneration=*/true); } } @@ -794,7 +799,9 @@ void doBankRemoveSelected() { } // No-op guardrail (R-B): open an undo point only if the index actually mutated. - if (removed > 0) persistBankOp("ReaSampler: remove sample(s)"); + // S9: a remove drops a sample from a bank (an instance referencing it must refresh — it + // will resolve to silence, per the stale-id policy) -> bump. + if (removed > 0) persistBankOp("ReaSampler: remove sample(s)", /*bumpGeneration=*/true); } // Prune bank folder — Phase R (Reclaim), R3: the guarded DESTRUCTIVE step, and the SOLE @@ -884,8 +891,14 @@ void doBankPruneFolder() { // change stands and persists on the user's next save; it just earns no undo point until // there is a project to persist into (undo of an unsaved bank op has nothing to roll // back to anyway). The Begin/End must still be balanced, hence the close-either-way. -void persistBankOp(const char* label) { +void persistBankOp(const char* label, bool bumpGeneration) { Undo_BeginBlock2(nullptr); + // S9: bump the bank-generation counter INSIDE the block, before persistBook(), so the + // fresh generation rides the same ext-state write the persist makes (persistBook() -> + // saveToActiveProject() stamps bankGeneration()). Bumped only for content-changing verbs + // (the caller decides); a pure-organizational verb passes false and leaves the counter be, + // so a rename/activate does not needlessly refresh live instances. + if (bumpGeneration) g_session->bumpBankGeneration(); const bool persisted = persistBook(); if (persisted) Undo_EndBlock2(nullptr, label, UNDO_STATE_MISCCFG); diff --git a/src/actions.h b/src/actions.h index 4ca2a74..e9d9bfe 100644 --- a/src/actions.h +++ b/src/actions.h @@ -79,6 +79,14 @@ int bankPruneCommandId(); // must invoke this ONLY after a successful/effective mutation — rejected ops (duplicate // name, un-deletable pool, etc.) must return before reaching here so no empty undo // point is ever opened for a no-op. Defined in actions.cpp alongside persistBook(). -void persistBankOp(const char* label); +// +// S9 bank-generation bump: pass `bumpGeneration = true` for a verb that changes what a live +// instance would PLAY — move / copy / remove / evacuate / delete-with-members (a sample left, +// arrived, or dropped out of a bank an instance may reference). Leave it false (the default) +// for a PURELY ORGANIZATIONAL verb — create / rename / activate / reorder — which changes no +// existing (bankId, sampleId) -> content mapping, so no instance need refresh. The bump (when +// requested) happens INSIDE the block, BEFORE persistBook(), so the stamped counter rides the +// same ext-state write and undo captures the pre/post generation with the rest of the blob. +void persistBankOp(const char* label, bool bumpGeneration = false); } // namespace reasampler diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 7eef9ba..146389f 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -2059,7 +2059,10 @@ void doDeleteBank(const std::string& bankId) { // r == 6 (Yes) falls through to a plain delete (drops members). } if (!book()->deleteBank(bankId)) return; - persistBankOp("ReaSampler: delete bank"); + // S9: bump when the bank held samples (either the Yes-drop path or the No-evacuate-then- + // delete path moved/dropped members) — both change what a live instance could play. An + // empty-bank delete is purely organizational, no bump. + persistBankOp("ReaSampler: delete bank", /*bumpGeneration=*/members > 0); // shownBankId is reconciled by the next fingerprint pass. If no named banks remain, // nudge focus to the pool so the selection has a valid home. if (namedBanks().empty()) g_panel.focusedRegion = Region::Pool; @@ -2071,7 +2074,7 @@ void doEvacuateBank(const std::string& bankId) { const Bank* bk = book()->bank(bankId); if (!bk || bk->isPool()) return; if (!book()->evacuate(bankId)) return; - persistBankOp("ReaSampler: evacuate bank"); + persistBankOp("ReaSampler: evacuate bank", /*bumpGeneration=*/true); // S9: membership changed invalidatePanel(); } @@ -2115,7 +2118,7 @@ void transferSamples(const std::vector& sampleIds, if (!mutated) return; // nothing changed — no persist, no undo point const char* label = copy ? "ReaSampler: copy sample(s)" : "ReaSampler: move sample(s)"; - persistBankOp(label); + persistBankOp(label, /*bumpGeneration=*/true); // S9: bank membership changed // The selection indexed into the source; after a move those indices are stale, so // clear it (the fingerprint pass will also clear, but do it now for immediacy). g_panel.selection = Selection{}; @@ -2140,7 +2143,7 @@ void removeSamples(const std::vector& sampleIds, ++removed; if (removed == 0) return; // nothing changed — no persist, no undo point - persistBankOp("ReaSampler: remove sample(s)"); + persistBankOp("ReaSampler: remove sample(s)", /*bumpGeneration=*/true); // S9: sample dropped // The selection indexed into the source; after a remove those indices are stale, so // clear it (the fingerprint pass will also clear, but do it now for immediacy). g_panel.selection = Selection{}; diff --git a/src/ext_keys.h b/src/ext_keys.h index 3db31ef..9f8595e 100644 --- a/src/ext_keys.h +++ b/src/ext_keys.h @@ -45,6 +45,19 @@ inline constexpr const char* kProjExtTailKey = "tail_setting"; // The per-project minted-GUID identity key. inline constexpr const char* kProjExtGuidKey = "project_guid"; +// The S9 BANK-GENERATION key. The EXTENSION stamps a monotonic decimal counter here that it +// bumps on every bank-content mutation that changes what a live instance would PLAY (capture +// add, re-capture-in-place, sample remove, move/copy affecting banks, ingest import). The VST3 +// instrument READS it off the audio thread on a UI-timer cadence and, when the value differs +// from what it last saw, calls reloadFromBank() so a recapture/ingest refreshes playing +// instances hands-free (the S9 change-detection trigger). WIRE-SHARED (instrument reads it); +// the instrument never WRITES it (the extension owns it, same read-only-over-bank rule as the +// assignment request). Additive to the persist blob — an absent stamp reads as generation 0 +// (a pre-S9 project), and the first bump (>= 1) then reads as a change. FOREVER-STABLE once +// shipped: changing this spelling resets every already-shipped instance's change-detection +// baseline (a one-time spurious reload), so it is fixed like every sibling key. +inline constexpr const char* kProjExtBankGenKey = "bank_generation"; + // The S8 ingest ASSIGNMENT-REQUEST key. The EXTENSION writes an assignment request here // after an ingest-with-assign (arrange capture / Media-Explorer import / drop-onto-panel): // "the active sampler instance should now play THIS sample." The value is the pure diff --git a/src/ingest.cpp b/src/ingest.cpp index 6211bb3..78d1955 100644 --- a/src/ingest.cpp +++ b/src/ingest.cpp @@ -458,6 +458,10 @@ void doImportFromMediaExplorer() { // zero flag so REAPER discards the undo entry (the house pattern from actions.cpp). if (r.added) { Undo_BeginBlock2(nullptr); + // S9: an ingest import adds a sample to the active bank -> bump inside the block so + // the stamped generation refreshes the assigned instance hands-free (and undo rolls + // the generation back with the banks/assign_request keys). + g_session->bumpBankGeneration(); const bool persisted = g_session->saveToActiveProject(); // Assign request inside the same block: undo rolls back both keys together. ingestAssignActiveInstance(g_session->book().activeBankId(), r.sampleId); @@ -531,6 +535,9 @@ void ingestDroppedFiles(const std::vector& absolutePaths) { if (!firstAssignId.empty()) { if (importedNew > 0) { Undo_BeginBlock2(nullptr); + // S9: one coalesced bump for the whole drop (>=1 new sample landed) inside the + // block so the generation refreshes the assigned instance and undo rolls it back. + g_session->bumpBankGeneration(); const bool persisted = g_session->saveToActiveProject(); // Assign inside the block: undo restores both keys atomically. ingestAssignActiveInstance(firstAssignBank, firstAssignId); diff --git a/src/main.cpp b/src/main.cpp index eeecdb3..668eba4 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -216,7 +216,10 @@ static void CommitRealtimeResult(const reasampler::CaptureResult& res) // index AddResult — even a hash-collapse still WROTE a file the tool owns, and the // manifest dedups a repeat path itself (Phase R prune reconciles manifest vs index). g_session.owned().add(res.sample.relativePath); - g_session.saveToActiveProject(); // persist book + manifest + MarkProjectDirty (travels with .rpp) + // S9: a capture add changes what a live instance could play (a new sample landed in the + // active bank) -> bump before the persist so the stamped generation refreshes instances. + g_session.bumpBankGeneration(); + g_session.saveToActiveProject(); // persist book + manifest + generation + MarkProjectDirty (travels with .rpp) } // Advance any in-flight realtime capture one tick. Cheap when none is running (a @@ -852,6 +855,9 @@ static std::string RunCapture(const reasampler::CaptureActionDef& def) // `banks` + `owned_files` keys) so the capture survives Save / close+reopen (M4) and // travels with the .rpp. saveToActiveProject also clears the retired legacy key and // calls MarkProjectDirty. Non-destructive: writes only our own ext-state keys. + // S9: a capture add is a bank-content change -> bump before the persist so an assigned + // live instance refreshes hands-free (the S8 capture+assign path builds on this). + g_session.bumpBankGeneration(); g_session.saveToActiveProject(); // Hand the LANDED bank-index id back to the assign path (S8): captureAndIndexOne @@ -1042,8 +1048,12 @@ static void RunBatchCaptureItems() } // selGuard restores the original selection here, on every path // Persist ONCE for the whole batch (one ext-state write) — only if something landed. - if (anyAdded) + // S9: one bump for the whole batch (coalesced) — the counter is monotonic, not per-sample, + // so a single increment past the last-seen value is enough to trigger one instance reload. + if (anyAdded) { + g_session.bumpBankGeneration(); g_session.saveToActiveProject(); + } ShowConsoleMsg((outcome.summaryLine("item") + "\n").c_str()); } @@ -1159,8 +1169,11 @@ static void RunBatchCaptureRazor() } } // selGuard restores the original track selection here, on every path - if (anyAdded) + // S9: one coalesced bump for the whole razor batch (see the item-batch note above). + if (anyAdded) { + g_session.bumpBankGeneration(); g_session.saveToActiveProject(); + } ShowConsoleMsg((outcome.summaryLine("razor area") + "\n").c_str()); } @@ -1344,7 +1357,12 @@ static void RunRecaptureFromSource() // Record the regenerated file in the owned manifest (a new file the tool wrote); // the superseded old file becomes an orphan reclaimed by Phase R prune. g_session.owned().add(updated.relativePath); - const bool persisted = g_session.saveToActiveProject(); // book + manifest + MarkProjectDirty + // S9: re-capture-in-place regenerates the SAME id's audio — the exact case the + // hands-free refresh exists for (an instance referencing this id keeps playing the + // OLD audio until it reloads). Bump inside the undo block so undo rolls back the + // generation with the rest of the blob. + g_session.bumpBankGeneration(); + const bool persisted = g_session.saveToActiveProject(); // book + manifest + generation + MarkProjectDirty Undo_EndBlock2(nullptr, persisted ? "ReaSampler: re-capture from source" : "", persisted ? UNDO_STATE_MISCCFG : 0); } diff --git a/src/persist.cpp b/src/persist.cpp index 74fe848..e1f6e09 100644 --- a/src/persist.cpp +++ b/src/persist.cpp @@ -96,6 +96,7 @@ #include "app_version.h" #include "capture_paths.h" #include "prune_reconcile.h" +#include "vst/bank_sync.h" // parseBankGeneration / formatBankGeneration (SHARED with the instrument reader) #define REAPERAPI_MINIMAL #define REAPERAPI_WANT_EnumProjects @@ -252,6 +253,16 @@ bool ReaSamplerSession::saveToActiveProject() { SetProjExtState(static_cast(proj), projExtNamespace(), kProjExtVersionKey, stampVersion().c_str()); + // S9: stamp the current bank-generation counter under its own wire-shared key, on the SAME + // seam so the counter and MarkProjectDirty stay paired. The value is whatever + // bumpBankGeneration() advanced it to since the last save (0 if never bumped / pre-S9), so + // every content mutation's own save carries the fresh generation the instrument reads. The + // format is the SHARED pure encoder (vst::formatBankGeneration) so writer and reader agree + // byte-for-byte — a decimal integer. Additive: does not disturb the blobs above. + SetProjExtState(static_cast(proj), projExtNamespace(), + kProjExtBankGenKey, + vst::formatBankGeneration(bankGeneration_).c_str()); + MarkProjectDirty(static_cast(proj)); return true; } @@ -580,6 +591,16 @@ void ReaSamplerSession::loadFromProject(void* proj, const std::string& projectDi kProjExtVersionKey) : std::string{}); + // S9: recover the bank-generation counter on EVERY load path (peer-symmetry with + // writingVersion_/tail_/view_ above), so it continues monotonic from the stored value + // rather than resetting to 0 on reopen — a next bump then reads > the stored value. A + // project switch reads THAT project's counter, not the previous one's; an absent/malformed + // stamp (pre-S9 or corrupt) parses to 0 via the SHARED decoder. proj == nullptr -> 0. + bankGeneration_ = vst::parseBankGeneration( + proj ? getProjExtStateString(static_cast(proj), projExtNamespace(), + kProjExtBankGenKey) + : std::string{}); + if (!proj) { book_ = BankBook{}; return; diff --git a/src/persist.h b/src/persist.h index 723e483..6b67f56 100644 --- a/src/persist.h +++ b/src/persist.h @@ -17,6 +17,7 @@ // calls live in persist.cpp. It depends on bank_model (pure) for JSON round-trip // and capture_paths (pure) for the path arithmetic it drives. +#include #include #include "app_version.h" @@ -149,6 +150,24 @@ public: // reason about the origin build without re-reading ext state. const WritingVersion& writingVersion() const { return writingVersion_; } + // The S9 bank-generation counter (the value stamped under `bank_generation`). Monotonic + // per project: recovered on load (so it continues from the stored value rather than + // resetting), bumped by bank-content mutations via bumpBankGeneration(), and written on + // every saveToActiveProject(). Exposed const for the writer sites to read/log. + std::int64_t bankGeneration() const { return bankGeneration_; } + + // Bump the S9 bank-generation counter — call at every bank-CONTENT mutation that changes + // what a live instance would PLAY (capture add, re-capture-in-place, sample remove, + // move/copy affecting banks, ingest import). NOT the pure-organizational verbs (create / + // rename / activate / reorder a bank), which change no existing (bankId, sampleId) -> + // content mapping. The bumped value is persisted by the NEXT saveToActiveProject() call + // the same mutation already makes (the counter rides the persist blob, so there is no + // separate write). In-memory only here — cheap and REAPER-free; the persist is the write. + // Over-bumping is safe (a reload that finds unchanged content atomically re-installs the + // same instrument, no glitch); under-bumping misses a hands-free refresh, so the sites err + // toward bumping. Idempotent per logical op — call once per mutation, before the persist. + void bumpBankGeneration() { ++bankGeneration_; } + // Serialize the current book (under the `banks` key), view model, and tail setting // to the active project's ext state (namespace "reasampler"), and clear the retired // legacy `bank_index` key. Non-destructive beyond writing our own ext-state keys. @@ -282,6 +301,14 @@ private: // the previous project's stamp. Read-only to consumers via writingVersion(). WritingVersion writingVersion_; + // The S9 bank-generation counter (peer to writingVersion_). Recovered on EVERY load path + // from the stored `bank_generation` stamp (parseBankGeneration; absent -> 0), so it + // continues monotonic from the persisted value across reopen and resets cleanly on a + // project switch (a different project's counter, not the previous project's). bumped by + // bumpBankGeneration() at bank-content mutations and stamped by saveToActiveProject(). + // Default 0 for an unsaved / never-loaded / pre-S9 session. + std::int64_t bankGeneration_ = 0; + // The project identity last observed by poll(), used to detect load/Save-As. // The GUID is the PRIMARY signal (a different stored GUID = a different project // of record = Load, immune to pointer recycling). The pointer disambiguates the diff --git a/src/vst/bank_sync.cpp b/src/vst/bank_sync.cpp new file mode 100644 index 0000000..8ca894b --- /dev/null +++ b/src/vst/bank_sync.cpp @@ -0,0 +1,70 @@ +// bank_sync.cpp — see bank_sync.h. Pure; standard library only. + +#include "bank_sync.h" + +#include +#include +#include + +namespace reasampler::vst { + +std::int64_t parseBankGeneration(const std::string& raw) { + if (raw.empty()) return kBankGenerationAbsent; + + // Whole-string, non-negative decimal parse WITHOUT exceptions or locale surprises. + // A leading '+' / '-' , any non-digit, an empty digit run, or overflow past int64 max + // all reject to the absent default (0). Manual accumulation with an overflow guard so a + // pathologically long digit run can never wrap into a bogus small value. + std::int64_t value = 0; + constexpr std::int64_t kMax = std::numeric_limits::max(); + for (const char c : raw) { + if (c < '0' || c > '9') return kBankGenerationAbsent; // any non-digit -> reject whole + const int digit = c - '0'; + // Guard value*10 + digit against overflow before performing it. + if (value > (kMax - digit) / 10) return kBankGenerationAbsent; // would overflow -> reject + value = value * 10 + digit; + } + return value; +} + +std::string formatBankGeneration(std::int64_t generation) { + // Non-negative decimal; a negative (should never be produced by the writer) formats as + // its std::to_string form and would parse back to 0, so the writer's monotonic counter + // stays in the >= 0 domain by construction. + return std::to_string(generation); +} + +bool bankGenerationChanged(std::int64_t seen, std::int64_t current) { + return current != seen; +} + +AssignConsumeDecision consumeDecision(const std::optional& request, + std::int64_t lastConsumed, bool resolves, + bool isFocusedTarget) { + AssignConsumeDecision d; + d.consumedGeneration = lastConsumed; // default: nothing changes + + // Rule 1: no request, or not newer than what we already consumed -> nothing new. + if (!request) return d; + if (request->generation <= lastConsumed) return d; + + // Rule 2: a new request, but this instance is not the target -> do not act, do NOT + // advance the marker (stay eligible if focus later lands here). No thundering herd. + if (!isFocusedTarget) return d; + + // The request is new AND we are the target: it will be consumed-as-seen either way, so + // advance the marker to its generation so it is never re-evaluated. + d.consumedGeneration = request->generation; + + // Rule 3: unresolvable (bankId, sampleId) -> DROP silently (reader requirement): marker + // advanced above, but no selection change. + if (!resolves) return d; + + // Rule 4: new, target, resolvable -> apply the selection. + d.apply = true; + d.bankId = request->bankId; + d.sampleId = request->sampleId; + return d; +} + +} // namespace reasampler::vst diff --git a/src/vst/bank_sync.h b/src/vst/bank_sync.h new file mode 100644 index 0000000..acf49ad --- /dev/null +++ b/src/vst/bank_sync.h @@ -0,0 +1,105 @@ +#pragma once +// bank_sync — PURE decision logic for the S9 bank-generation change-detection and the +// S8 instrument-side assignment-request consume. NO VST3, NO REAPER, NO SWELL, NO +// vendor/ includes. Standard library only. Unit-tested outside the DAW — the mirror of +// sample_map / bridge_marshal splitting the fiddly, testable arithmetic out of a +// host-facing shell. +// +// WHY IT EXISTS (S9/S8 reader seams). The instrument polls two "reasampler" ext-state +// keys off the audio thread: the S9 bank-generation counter (has the bank changed?) and +// the S8 assignment request (should I switch to a just-ingested sample?). The RAW string +// read crosses the bridge in the shell; every DECISION after — parse the generation +// stamp, decide whether it differs from what we last saw, decide whether a decoded +// assignment request is NEW-and-resolvable-and-worth-applying — is pure and lives here. +// +// The processor shell owns the cadence (a UI-thread timer, NEVER process) and the side +// effects (reloadFromBank, setSelectedSampleId); this module owns only the yes/no maths so +// the reader's rules are provable without a host. assignment_request.h owns the WIRE format +// (encode/decode); this module owns the CONSUME decision layered over a decoded request. + +#include +#include +#include + +#include "assignment_request.h" // AssignmentRequest (the decoded request this consumes) + +namespace reasampler::vst { + +// The S9 bank-generation "generation 0 = never stamped" default. A project saved before +// S9 shipped carries no bank_generation key; the bridge read yields an absent/empty value +// which parses to this, and the first real bump (>= 1) then reads as a change. Matches the +// writer's monotonic-from-1 counter (the extension bumps to 1 on the first mutation). +inline constexpr std::int64_t kBankGenerationAbsent = 0; + +// Parse the raw bank-generation ext-state value the bridge read. The writer stamps a +// non-negative decimal integer (formatBankGeneration). Absent / empty / malformed / negative +// / overflowing all yield kBankGenerationAbsent (0) — the reader treats any unreadable stamp +// as "generation 0", so a pre-S9 or corrupt value is a clean default, never a crash and never +// a spurious reload storm (0 vs a previously-seen 0 is no change). Whole-string parse: trailing +// garbage after the digits rejects the value (returns 0), so a torn/partial write is ignored +// until the next clean poll (the read tolerates staleness by design — it reloads on the NEXT +// poll once the value is clean). +std::int64_t parseBankGeneration(const std::string& raw); + +// Format a bank-generation counter for the ext-state stamp. The inverse of +// parseBankGeneration for a non-negative value: a plain decimal, no sign, no padding, so +// the stamp is byte-stable across writes of the same value. +std::string formatBankGeneration(std::int64_t generation); + +// Has the bank generation changed since the reader last saw `seen`? True when `current` +// differs from `seen` — the reader then triggers a reload. Any difference counts (not just +// an increase): the writer is monotonic, but a project switch or reload can legitimately +// lower the value, and the reader should re-read the bank in that case too. `seen` starts at +// kBankGenerationAbsent so the first non-zero generation reads as a change (the pre-S9 / +// first-bump refresh the spec requires). +bool bankGenerationChanged(std::int64_t seen, std::int64_t current); + +// The verdict of the S8 assignment-request consume decision (below). A pure value the +// processor shell acts on: apply the selection (or not) and advance the consumed marker +// (or not). Distinct booleans because the two are NOT the same event — a request may be +// consumed-as-seen (marker advances) without being applied (it named an unresolvable +// sample and was DROPPED per the reader requirement), so the shell must not re-evaluate it +// every poll. +struct AssignConsumeDecision { + bool apply = false; // set this instance's selection to (bankId, sampleId) + reload + std::string bankId; // the request's bank (valid only when apply) + std::string sampleId; // the request's sample (valid only when apply) + std::int64_t consumedGeneration = 0; // the marker to persist (== lastConsumed when nothing new) +}; + +// Decide whether to CONSUME a decoded assignment request (S8 instrument-side reader). +// +// `request` — the decoded assignment request (nullopt when the assign_request key +// is absent / malformed — nothing pending). +// `lastConsumed` — the generation this instance last consumed (persisted in component +// state so a re-open does not re-apply a request the user already got, +// then manually changed away from). Defaults to 0 for a fresh instance. +// `resolves` — whether the request's (bankId, sampleId) resolves to an existing bank +// sample RIGHT NOW (the shell computed this against the live bank blob). +// `isFocusedTarget` — whether THIS instance is the assignment target under the shell's +// thundering-herd policy (e.g. only the focused-editor instance applies). +// The shell passes true when this instance should act; false suppresses +// consumption entirely so a non-target instance neither applies nor +// advances its marker (it stays eligible if it later becomes the target). +// +// RULES (all pure, order matters): +// 1. No request, or an OLDER/equal generation (<= lastConsumed): nothing new — do not +// apply, marker unchanged. (Covers the re-open case: the persisted marker == the +// request's generation, so it is not re-applied.) +// 2. A NEW request (generation > lastConsumed) but NOT this instance's target: do not +// apply and do NOT advance the marker — a non-target instance must stay able to consume +// the request if focus later lands on it. (No thundering herd: only the target acts.) +// 3. A NEW request, this instance IS the target, but the (bankId, sampleId) does NOT +// resolve: DROP it silently (assignment_request.h reader requirement) — do not apply, +// but DO advance the marker to the request's generation so a stale/unresolvable request +// is consumed-as-seen and never re-evaluated (no error state, no selection change). +// 4. A NEW request, target, and resolvable: APPLY (selection <- (bankId, sampleId)) and +// advance the marker to the request's generation. +// +// The shell then: if apply, setSelectedSampleId + reloadFromBank; always persist +// consumedGeneration into component state when it advanced. +AssignConsumeDecision consumeDecision(const std::optional& request, + std::int64_t lastConsumed, bool resolves, + bool isFocusedTarget); + +} // namespace reasampler::vst diff --git a/src/vst/reasampler_editor.cpp b/src/vst/reasampler_editor.cpp index 7ae8fa9..526bb2d 100644 --- a/src/vst/reasampler_editor.cpp +++ b/src/vst/reasampler_editor.cpp @@ -38,6 +38,14 @@ namespace { #ifdef _WIN32 constexpr const wchar_t* kChildClassName = L"ReaSampler9000VstEditor"; +// The S9/S8 change-detection poll (WM_TIMER on the child window). A low-frequency UI-thread +// timer: responsive enough that a recapture/ingest/assign refreshes "within a bounded cadence" +// (the S9 verify criterion) yet cheap — three small ext-state reads per tick, coalescing many +// bumps between ticks into one reload. 500 ms is a deliberate build-time residual: fast enough +// to feel hands-free, slow enough to be free. The id is a per-window SetTimer id (any nonzero). +constexpr UINT_PTR kSyncTimerId = 1; +constexpr UINT kSyncTimerIntervalMs = 500; + // Top-level band metrics (shell arithmetic — the load-bearing card/tab/key/zone geometry // is in capture_browser / keyboard_strip). The title band names the plugin + a live // readout; the toggle band carries the Browser/Zones switch; the setup band (single- @@ -173,6 +181,35 @@ void ReaSamplerEditor::rebuildVisible() { } } +#ifdef _WIN32 +// Windows-only (the WM_TIMER cadence + invalidate() are the Win32 child-window path). Declared +// under the same _WIN32 guard in the header; keep the definition guarded to match (D5 makes +// Windows the only build target, but the TU must still compile elsewhere). +void ReaSamplerEditor::onSyncTimer() { + // UI thread (WM_TIMER). Poll the S9 bank generation + the S8 assignment request via the + // processor (off the audio thread — the poll itself never touches process()). NEVER while a + // drag is in flight: a reload mid-drag would rebuild the instrument and repaint under the + // user's cursor, yanking the edit. The next tick (500 ms) picks up the change after release. + if (!processor_) return; + if (drag_ != DragKind::kNone) return; // defer past the in-flight edit + + // An open editor marks THIS instance the focused assignment target (the thundering-herd + // policy — only an editor-open instance applies a pending assign; see the handoff). Pass + // true so this instance consumes the request; instances with no editor open do not poll at + // all (the timer is bound to the child window), so they never contend for the request. + const ReaSamplerProcessor::BankSyncResult r = processor_->pollBankSync(/*isFocusedTarget=*/true); + + // Re-snapshot the editor's own view only when something changed (a reload from a bank + // content change, or an applied assignment). refreshFromBank re-reads the bank blob + the + // processor's (possibly just-updated) selection/map and drops the stale thumbnail/PCM + // caches, then repaints — so the browser + setup surface reflect the new bank hands-free. + if (r.reloaded || r.applied) { + refreshFromBank(); + invalidate(); + } +} +#endif // _WIN32 + void ReaSamplerEditor::commitAndReload() { // UI thread only. Publish the edited selection + zones to the processor, then rebuild // the instrument off the audio thread (reloadFromBank bakes them into the live Keymap). @@ -372,11 +409,21 @@ void ReaSamplerEditor::attachedToParent() { r.getWidth(), r.getHeight(), parent, nullptr, hInst, nullptr); if (childHwnd_) { SetWindowLongPtr(childHwnd_, GWLP_USERDATA, reinterpret_cast(this)); + // Start the S9/S8 change-detection poll (UI thread). Tied to the child window's + // lifetime — created here, killed in removedFromParent — so an instance whose editor + // is closed does NOT poll (the editor-open-only cadence; see the handoff limitation). + SetTimer(childHwnd_, kSyncTimerId, kSyncTimerIntervalMs, nullptr); + // Poll ONCE immediately so a pending assignment (an S8 ingest fired while this editor + // was closed) or a bank change applies the instant the editor opens, rather than waiting + // up to one timer interval. refreshFromBank above already primed the view; this folds in + // any pending assign/generation so the just-opened editor shows the assigned capture. + onSyncTimer(); } } void ReaSamplerEditor::removedFromParent() { if (childHwnd_) { + KillTimer(childHwnd_, kSyncTimerId); // stop the poll before the window goes away DestroyWindow(childHwnd_); childHwnd_ = nullptr; } @@ -1063,6 +1110,9 @@ LRESULT CALLBACK ReaSamplerEditor::wndProc(HWND hwnd, UINT msg, WPARAM wParam, self->invalidate(); } return 0; + case WM_TIMER: + if (self && wParam == kSyncTimerId) self->onSyncTimer(); + return 0; case WM_ERASEBKGND: return 1; // fully repaint in WM_PAINT; skip the flicker-inducing erase default: diff --git a/src/vst/reasampler_editor.h b/src/vst/reasampler_editor.h index bf84b71..6959bd3 100644 --- a/src/vst/reasampler_editor.h +++ b/src/vst/reasampler_editor.h @@ -88,6 +88,15 @@ private: void onMouseMove(int x, int y); void onMouseUp(int x, int y); + // The S9/S8 change-detection tick (WM_TIMER on the child window — the UI thread, NEVER the + // audio thread). Polls the processor's bank-sync (generation change -> hands-free reload; + // a new assignment request -> apply as this instance's selection) and, when anything + // changed, re-snapshots the editor's own view (refreshFromBank) + repaints so the browser / + // setup surface reflect the new bank. An open editor means THIS instance is the focused + // assignment target (the thundering-herd policy — see the handoff), so it passes true. + // Suppressed WHILE A DRAG IS IN FLIGHT so a mid-drag reload does not yank the edit surface. + void onSyncTimer(); + static LRESULT CALLBACK wndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam); void invalidate(); diff --git a/src/vst/reasampler_embed.cpp b/src/vst/reasampler_embed.cpp index ca4b5b9..51e62f7 100644 --- a/src/vst/reasampler_embed.cpp +++ b/src/vst/reasampler_embed.cpp @@ -7,9 +7,10 @@ #include #include +#include "bank_sync.h" // parseBankGeneration (S9 dirty-guard over the per-paint refresh) #include "editor_geometry.h" // Rect (shared with embed_strip) #include "embed_strip.h" // the pure strip layout + hit-test -#include "ext_keys.h" // kProjExtBanksKey +#include "ext_keys.h" // kProjExtBanksKey / kProjExtBankGenKey #include "reaper_bridge.h" #include "reasampler_processor.h" @@ -91,6 +92,36 @@ void ReaSamplerEmbed::refresh() { if (selectedZone_ >= static_cast(map_.zones.size())) selectedZone_ = -1; } +void ReaSamplerEmbed::maybeRefresh() { + if (!processor_) { refresh(); return; } // clears state; cheap + + // The performance map is a cheap in-process accessor (mutex + copy), and the editor may + // have edited zones with NO bank-content change — always re-snapshot it so a zone edit + // reflects immediately. + map_ = processor_->performanceMap(); + if (selectedZone_ >= static_cast(map_.zones.size())) selectedZone_ = -1; + + // The EXPENSIVE part is the bank-blob bridge read (samples_). Gate it on the S9 bank- + // generation stamp (a small ext-state read): only re-read the bank when the generation + // changed since the last paint (a recapture / ingest / remove), or on the first paint + // (lastSeenBankGeneration_ == -1). A pre-S9 project reads generation 0; the first paint + // folds it and subsequent idle paints skip the bank read entirely. + std::int64_t currentGen = lastSeenBankGeneration_; + if (auto rawGen = + processor_->bridge().readReasamplerExtState(reasampler::kProjExtBankGenKey)) { + currentGen = parseBankGeneration(*rawGen); + } else if (lastSeenBankGeneration_ < 0) { + currentGen = 0; // unprimed + no stamp (pre-S9): treat as generation 0 for the first read + } + + if (lastSeenBankGeneration_ < 0 || currentGen != lastSeenBankGeneration_) { + auto banks = + processor_->bridge().readReasamplerExtState(reasampler::kProjExtBanksKey); + samples_ = banks ? listSamples(*banks) : std::vector{}; + lastSeenBankGeneration_ = currentGen; + } +} + TPtrInt ReaSamplerEmbed::embed_message(int msg, TPtrInt parm2, TPtrInt parm3) { switch (msg) { case REAPER_FXEMBED_WM_IS_SUPPORTED: @@ -141,8 +172,10 @@ bool ReaSamplerEmbed::paint(TPtrInt bitmap, TPtrInt drawInfo) { if (w <= 0 || h <= 0) return false; // Re-read live state each paint (UI thread) so the strip reflects keymap edits + bank - // changes without its own timer — REAPER repaints the embed surface on its cadence. - refresh(); + // changes without its own timer — REAPER repaints the embed surface on its cadence. S9 + // dirty-guard: maybeRefresh does the EXPENSIVE bank-blob read only when the bank generation + // changed (the flagged S6 follow-up), always refreshing the cheap performance map. + maybeRefresh(); // REAPER hands us its own bitmap sized to the embed area; draw directly into it (unlike // the editor, which owns a LICE_SysBitmap and BitBlt's). Origin is the bitmap's (0,0). diff --git a/src/vst/reasampler_embed.h b/src/vst/reasampler_embed.h index 8db5d0b..33a017c 100644 --- a/src/vst/reasampler_embed.h +++ b/src/vst/reasampler_embed.h @@ -32,6 +32,7 @@ #pragma once +#include #include #include @@ -86,7 +87,19 @@ private: // as the editor's refreshSampleList does (bridge read + processor accessors, UI thread). void refresh(); + // The S9 dirty-guard over refresh() (the S6 flagged follow-up): read the cheap bank- + // generation stamp; do the EXPENSIVE bank-blob bridge read (refresh()) only when the + // generation changed since the last paint (or on the first paint) — the strip re-read + // per paint was wasteful now that a generation counter exists. The performance map (a + // cheap in-process accessor, edited by the editor independently of bank content) is + // ALWAYS refreshed so a zone edit still reflects immediately. UI thread only. + void maybeRefresh(); + ReaSamplerProcessor* processor_ = nullptr; + // The bank generation last folded into samples_ (S9 dirty-guard). -1 forces the first + // maybeRefresh() to do a full read (no generation can be negative — parseBankGeneration + // yields >= 0 — so -1 is an "unprimed" sentinel distinct from a real generation 0). + std::int64_t lastSeenBankGeneration_ = -1; // Snapshotted for the current paint (refreshed each paint off the audio thread). std::vector samples_; PerformanceMap map_; diff --git a/src/vst/reasampler_processor.cpp b/src/vst/reasampler_processor.cpp index 7d65188..f9903f8 100644 --- a/src/vst/reasampler_processor.cpp +++ b/src/vst/reasampler_processor.cpp @@ -17,8 +17,10 @@ #include "public.sdk/source/vst/vstbus.h" // Vst::AudioBus::setArrangement (S7 output arr) +#include "assignment_request.h" // decodeAssignmentRequest (S8 request wire parse) +#include "bank_sync.h" // S9/S8 pure decisions: parseBankGeneration, consumeDecision #include "capture_paths.h" // resolveBankFile (shared M4 path resolution) -#include "ext_keys.h" // kProjExtBanksKey (shared wire contract) +#include "ext_keys.h" // kProjExtBanksKey / kProjExtBankGenKey / kProjExtAssignKey (shared wire contract) #include "reasampler_editor.h" #include "reasampler_embed.h" // S6 embed shell + IReaperUIEmbedInterface (its iid DEF'd there) #include "sample_map.h" // selectSample, resolvePerformance, buildZonedKeymap, state (de)ser @@ -184,6 +186,12 @@ tresult PLUGIN_API ReaSamplerProcessor::setState(IBStream* state) { const ComponentState cs = deserializeComponentState(bytes); setSelectedSampleId(cs.selectionId); setPerformanceMap(cs.map); + // S8: restore the last-consumed assignment generation so a re-open does not re-apply a + // stale assign_request (the user may have manually changed the selection after the assign). + { + std::lock_guard lock(assignMarkerMutex_); + lastConsumedAssignGeneration_ = cs.lastConsumedAssignGeneration; + } // Restore the S7 channel mode and point the output bus at its arrangement so a reopened // project comes back in the saved mode. setState runs before the host queries bus info, so // seeding the arrangement here (rather than re-negotiating) is enough — no restartComponent. @@ -208,6 +216,10 @@ tresult PLUGIN_API ReaSamplerProcessor::getState(IBStream* state) { state_out.selectionId = selectedSampleId(); state_out.map = performanceMap(); state_out.channelMode = channelMode(); // S7: persist the per-instance mono/stereo mode + { + std::lock_guard lock(assignMarkerMutex_); + state_out.lastConsumedAssignGeneration = lastConsumedAssignGeneration_; // S8 reader marker + } const std::vector bytes = serializeComponentState(state_out); if (!bytes.empty()) { const tresult wr = state->write(const_cast(bytes.data()), @@ -392,6 +404,79 @@ std::string ReaSamplerProcessor::reloadFromBank() { return resolvedId; } +ReaSamplerProcessor::BankSyncResult +ReaSamplerProcessor::pollBankSync(bool isFocusedTarget) { + // OFF THE AUDIO THREAD (the editor's UI timer calls this). Both reads allocate and call + // REAPER via the bridge — never invoked from process(). A disconnected bridge (non-REAPER + // host, or before connect) yields nullopt for both reads, so this no-ops cleanly. + BankSyncResult result; + + // --- S8: assignment-request consume FIRST ------------------------------------- + // Decode the pending assignment request (nullopt when absent/malformed). Resolve its + // (bankId, sampleId) against the live bank blob: selectSample returns non-nullopt only when + // the sampleId names an existing sample (the reader requirement — an unresolvable pair is + // dropped). Then run the pure consume decision against this instance's persisted marker. + std::optional request; + if (auto raw = bridge_.readReasamplerExtState(kProjExtAssignKey)) { + request = decodeAssignmentRequest(*raw); + } + + bool resolves = false; + if (request) { + // Resolve the assigned sample against the CURRENT bank blob (a fresh read, so a request + // whose sample was rolled back by an extension undo resolves to nullopt -> dropped). + if (auto banksJson = bridge_.readReasamplerExtState(kProjExtBanksKey)) { + resolves = selectSample(*banksJson, request->sampleId).has_value(); + } + } + + std::int64_t lastConsumed = 0; + { + std::lock_guard lock(assignMarkerMutex_); + lastConsumed = lastConsumedAssignGeneration_; + } + const AssignConsumeDecision decision = + consumeDecision(request, lastConsumed, resolves, isFocusedTarget); + + // Advance the persisted consumed marker whenever the decision consumed the request + // (applied OR dropped-as-seen). getState will persist it on the next project save so a + // re-open does not re-apply. A non-target instance leaves the marker (decision returns it + // unchanged) so it stays eligible if focus later lands here. + if (decision.consumedGeneration != lastConsumed) { + std::lock_guard lock(assignMarkerMutex_); + lastConsumedAssignGeneration_ = decision.consumedGeneration; + } + + if (decision.apply) { + // Apply the assignment as this instance's own selection (the same path a user card-pick + // takes) — the instrument updates its OWN state, never the bank. reloadFromBank below + // rebuilds against the new selection, so skip a redundant reload here. + setSelectedSampleId(decision.sampleId); + result.applied = true; + } + + // --- S9: bank-generation change-detection ------------------------------------- + // Read the generation stamp; parse (absent/malformed -> 0, the pre-S9 default). FIRST poll + // (lastSeenBankGeneration_ == -1 sentinel): BASELINE the seen value without a reload — setState + // already loaded the current bank, so a redundant reload on open would only churn. A later + // generation CHANGE (a recapture/ingest/remove, or an undo that lowers it) then drives the + // reload. An assignment we just applied also needs a reload; fold both into ONE (coalesced). + std::int64_t currentGen = kBankGenerationAbsent; + if (auto rawGen = bridge_.readReasamplerExtState(kProjExtBankGenKey)) { + currentGen = parseBankGeneration(*rawGen); + } + const bool firstPoll = (lastSeenBankGeneration_ < 0); + const bool genChanged = + !firstPoll && bankGenerationChanged(lastSeenBankGeneration_, currentGen); + lastSeenBankGeneration_ = currentGen; + + if (genChanged || result.applied) { + reloadFromBank(); // atomic pointer-swap handoff — glitch-free mid-play (S4 graveyard) + result.reloaded = genChanged; // report S9 vs S8 distinctly for the editor's reaction + } + return result; +} + tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { // REAL-TIME: no allocation, no IO, no locks. Load the live instrument once for the // whole block (a single atomic acquire), then publish inst->installedAt so the off- diff --git a/src/vst/reasampler_processor.h b/src/vst/reasampler_processor.h index a16e283..0b1dde0 100644 --- a/src/vst/reasampler_processor.h +++ b/src/vst/reasampler_processor.h @@ -122,6 +122,27 @@ public: // resolved selection id ("" if nothing was loaded) for the editor to reflect. std::string reloadFromBank(); + // The result of a bank-sync poll (S9/S8): what pollBankSync did this tick, so the editor + // can react (repaint / re-snapshot its own view) only when something actually changed. + struct BankSyncResult { + bool reloaded = false; // the bank generation changed -> reloadFromBank ran + bool applied = false; // a new assignment request was applied -> selection changed + }; + + // Poll the S9 bank-generation counter and the S8 assignment request over the bridge, OFF + // THE AUDIO THREAD (the editor's UI timer drives this — NEVER process()). Semantics: + // * S9: if the bank generation differs from what we last saw, call reloadFromBank() so a + // recapture/ingest refreshes playback hands-free (atomic swap, glitch-free). + // * S8: if a NEW (generation > last consumed) assignment request names a resolvable + // sample AND this instance is the target (isFocusedTarget), apply it as the selection + // and reload; an unresolvable request is DROPPED silently (marker advanced, no change); + // a non-target instance neither applies nor advances its marker. + // The consumed marker advances in component state (marked dirty via the host handler) so a + // re-open does not re-apply. `isFocusedTarget` is the shell's thundering-herd policy input + // (the editor passes true only for the instance whose editor is open — see the handoff). + // Idempotent on an idle tick (generation unchanged + no new request -> no work). + BankSyncResult pollBankSync(bool isFocusedTarget); + // The bridge, for the editor's live-state readout + sample list. Owned here; the // editor borrows it (outlives the editor). ReaperBridge& bridge() { return bridge_; } @@ -210,6 +231,22 @@ private: std::mutex channelModeMutex_; ChannelMode channelMode_ = ChannelMode::Mono; + // The last assignment-request generation this instance CONSUMED (S8 reader). Persisted in + // component state (v5) so a re-open does not re-apply a request the user already got and + // then changed away from. Written by pollBankSync (UI/timer thread) and getState; read by + // pollBankSync + getState; seeded by setState. Guarded against a getState/poll race. NEVER + // read on the audio thread. Default 0 -> a genuinely new first assign (gen >= 1) applies. + std::mutex assignMarkerMutex_; + std::int64_t lastConsumedAssignGeneration_ = 0; + + // The bank generation this instance last SAW (S9 reader). UI/timer-thread only (pollBankSync + // is the sole reader/writer) — no mutex needed, and it is NOT persisted. Initialized to a + // -1 SENTINEL (no real generation can be negative — parseBankGeneration yields >= 0) so the + // FIRST poll after an editor open BASELINES the seen value without a redundant reload (setState + // already loaded the current bank); a subsequent generation CHANGE then drives the reload. + // NOT read on the audio thread. + std::int64_t lastSeenBankGeneration_ = -1; + // Latched from setupProcessing so setActive/reload can size against it. Read // off-thread only. double sampleRate_ = 44100.0; diff --git a/src/vst/sample_map.cpp b/src/vst/sample_map.cpp index 64c95b8..1f27924 100644 --- a/src/vst/sample_map.cpp +++ b/src/vst/sample_map.cpp @@ -406,6 +406,10 @@ std::vector serializeComponentState(const ComponentState& state) { putU32le(out, kComponentStateVersion); // v4 envelope addition: the channel mode (0 = mono, 1 = stereo) precedes the v3 body. out.push_back(state.channelMode == ChannelMode::Stereo ? 1 : 0); + // v5 envelope addition (S8/S9 reader): the last-consumed assignment generation, 8-byte LE + // two's-complement, precedes the selection id. Follows the mode byte so a v4 reader that + // stops at the mode byte is a strict prefix (see the v4 lift below). + putU64le(out, asU64(state.lastConsumedAssignGeneration)); // Length-prefixed selection id (it precedes the zones payload, so it MUST be framed — // unlike the v1 selection blob where the id ran to end-of-stream). putU32le(out, static_cast(state.selectionId.size())); @@ -447,15 +451,31 @@ ComponentState deserializeComponentState(const std::vector& bytes) out.selectionId = r.str(idLen); if (!r.ok) { out.selectionId.clear(); return out; } // truncated id -> empty readZonesPayload(r, out.map); - return out; // channelMode stays Mono (pre-S7) + return out; // channelMode stays Mono, marker stays 0 (pre-S7/S8/S9) + } + // BACK-COMPAT: a v4 blob (pre-S8/S9 reader {mode, selection, zones}, no consumed marker): + // mode byte, then the id + zones body — no 8-byte marker. lastConsumedAssignGeneration + // defaults to 0, so a first assign still applies for a pre-marker instance. + if (version == kSelectionZonesModeV4Version) { + const std::uint8_t modeByte = r.u8(); + if (!r.ok) return out; // truncated before the mode byte -> empty (mono default holds) + out.channelMode = (modeByte == 1) ? ChannelMode::Stereo : ChannelMode::Mono; + const std::uint32_t idLen = r.u32(); + out.selectionId = r.str(idLen); + if (!r.ok) { out.selectionId.clear(); return out; } // truncated id -> empty + readZonesPayload(r, out.map); + return out; // marker stays 0 (pre-S8/S9 reader) } if (version != kComponentStateVersion) return out; // unknown -> empty - // v4: the channel-mode byte precedes the v3 body. A non-{0,1} byte is treated as mono - // (conservative default) rather than rejected — a corrupt mode never silences the instance. + // v5: the channel-mode byte, then the 8-byte consumed-assignment marker, precede the v3 + // body. A non-{0,1} mode byte is treated as mono (conservative default) rather than + // rejected — a corrupt mode never silences the instance. const std::uint8_t modeByte = r.u8(); if (!r.ok) return out; // truncated before the mode byte -> empty (mono default holds) out.channelMode = (modeByte == 1) ? ChannelMode::Stereo : ChannelMode::Mono; + out.lastConsumedAssignGeneration = r.i64(); + if (!r.ok) return out; // truncated before/inside the marker -> empty (marker 0 holds) const std::uint32_t idLen = r.u32(); out.selectionId = r.str(idLen); if (!r.ok) { out.selectionId.clear(); return out; } // truncated id -> empty diff --git a/src/vst/sample_map.h b/src/vst/sample_map.h index f527135..e05b5f2 100644 --- a/src/vst/sample_map.h +++ b/src/vst/sample_map.h @@ -285,25 +285,40 @@ PerformanceMap deserializePerformance(const std::vector& bytes); // instance with NO pick and NO zones restores EMPTY (silence + the "pick a capture" empty // state), never auto-playing sample #1. // -// Format (v4): 4-byte LE version tag (== 4), then a 1-byte channel-mode field (0 = mono, -// 1 = stereo), then a 4-byte LE selection-id length + id bytes, then the v2 zones payload -// (4-byte LE zone count + per-zone records, identical to serializePerformance's body). The -// channel-mode field is the ONLY v4 addition over v3 — the envelope grew a field, the zones -// payload is untouched (a PARALLEL track owns zone-record extension under the map's own -// versioning). BACK-COMPAT on read (every older blob lifts to channelMode = MONO, preserving -// current behavior for already-saved instances): -// * v4 blob -> {channelMode, selectionId, zones} parsed directly. -// * v3 blob -> {mono, selectionId, zones}: pre-S7 had no channel mode. -// * v2 blob -> {mono, "", zones}: an S5 instance had zones but no separate selection. -// * v1 blob -> {mono, id, one full-keyboard zone}: the S4 single-selection lift. -// * empty/unknown -> {mono, "", no zones}: EMPTY (the S10 silent empty state). +// Format (v5): 4-byte LE version tag (== 5), then a 1-byte channel-mode field (0 = mono, +// 1 = stereo), then an 8-byte LE last-consumed-assignment generation (S8/S9 reader marker), +// then a 4-byte LE selection-id length + id bytes, then the v2 zones payload (4-byte LE zone +// count + per-zone records, identical to serializePerformance's body). The 8-byte marker is +// the ONLY v5 addition over v4 — the envelope grew a field, the zones payload is untouched +// (a PARALLEL track owns zone-record extension under the map's own versioning). BACK-COMPAT on +// read (every older blob lifts to channelMode = MONO and lastConsumedAssignGeneration = 0, +// preserving current behavior for already-saved instances): +// * v5 blob -> {channelMode, lastConsumedAssignGeneration, selectionId, zones} direct. +// * v4 blob -> {channelMode, 0, selectionId, zones}: pre-S8/S9 reader (no marker). +// * v3 blob -> {mono, 0, selectionId, zones}: pre-S7 had no channel mode. +// * v2 blob -> {mono, 0, "", zones}: an S5 instance had zones but no separate selection. +// * v1 blob -> {mono, 0, id, one full-keyboard zone}: the S4 single-selection lift. +// * empty/unknown -> {mono, 0, "", no zones}: EMPTY (the S10 silent empty state). +// +// WHY THE MARKER PERSISTS (S8 reader requirement). The last-consumed assignment generation is +// the disambiguator that stops a re-opened instance re-applying a stale assign_request the user +// already got and then manually changed away from: on re-open the instance re-reads the pending +// request, and only a generation STRICTLY GREATER than this stored marker re-applies (see +// bank_sync::consumeDecision). A fresh instance defaults to 0, so a genuinely new first assign +// (generation >= 1) still applies. It is the instrument's OWN state (D-B), never written to the +// bank — the extension owns the assign_request key; the instrument only tracks what it consumed. struct ComponentState { std::string selectionId; // the single-capture pick; "" = no pick PerformanceMap map; // the opt-in zones; empty = no zones ChannelMode channelMode = ChannelMode::Mono; // S7 output mode; default mono (D-E) + std::int64_t lastConsumedAssignGeneration = 0; // S8/S9: last assign_request generation consumed }; -inline constexpr std::uint32_t kComponentStateVersion = 4; +inline constexpr std::uint32_t kComponentStateVersion = 5; + +// The pre-S8/S9-reader combined-state version (selection + zones + channel mode, no consumed +// marker). Retained so deserializeComponentState can lift a v4 blob to {mode, 0, sel, zones}. +inline constexpr std::uint32_t kSelectionZonesModeV4Version = 4; // The pre-S7 combined-state version (selection + zones, no channel mode). Retained as a named // constant so deserializeComponentState can lift a v3 blob to {mono, selection, zones}. diff --git a/tests/test_bank_sync.cpp b/tests/test_bank_sync.cpp new file mode 100644 index 0000000..f18af93 --- /dev/null +++ b/tests/test_bank_sync.cpp @@ -0,0 +1,179 @@ +// Standalone tests for reasampler::vst::bank_sync — no REAPER, no VST3, no framework. +// The S9 bank-generation change-detection + the S8 assignment-request consume DECISION +// (the yes/no maths the instrument's off-audio-thread poll runs). The shell owns the +// cadence + side effects; this proves the decision rules without a host. +// +// Covers: parseBankGeneration (absent/malformed/overflow/negative/valid whole-string), +// formatBankGeneration round-trip, bankGenerationChanged, and every consumeDecision rule +// (no request / not-newer / non-target / unresolvable-drop / apply), asserting both the +// apply flag AND the advanced-marker value so a stale request is never re-evaluated. + +#include "../src/vst/bank_sync.h" + +#include +#include +#include +#include + +using namespace reasampler; +using namespace reasampler::vst; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +// --- parseBankGeneration ----------------------------------------------------- + +static void testParseAbsentAndMalformed() { + // Absent / empty -> generation 0 (the pre-S9 default; a project with no stamp). + CHECK(parseBankGeneration("") == 0); + CHECK(parseBankGeneration("") == kBankGenerationAbsent); + // Malformed -> 0, never a crash, never a partial value. + CHECK(parseBankGeneration("abc") == 0); + CHECK(parseBankGeneration("12x") == 0); // trailing garbage rejects whole + CHECK(parseBankGeneration("x12") == 0); // leading garbage + CHECK(parseBankGeneration("1 2") == 0); // embedded space + CHECK(parseBankGeneration("+5") == 0); // sign rejected + CHECK(parseBankGeneration("-5") == 0); // negative rejected + CHECK(parseBankGeneration(" 5") == 0); // leading space + CHECK(parseBankGeneration("5.0") == 0); // decimal point +} + +static void testParseValid() { + CHECK(parseBankGeneration("0") == 0); + CHECK(parseBankGeneration("1") == 1); + CHECK(parseBankGeneration("42") == 42); + CHECK(parseBankGeneration("00042") == 42); // leading zeros are still digits -> 42 + CHECK(parseBankGeneration("9007199254740993") == 9007199254740993LL); // > 2^53 +} + +static void testParseOverflow() { + // A 19-digit int64-max is fine; anything past it rejects to 0 (never wraps). + CHECK(parseBankGeneration("9223372036854775807") == 9223372036854775807LL); // INT64_MAX + CHECK(parseBankGeneration("9223372036854775808") == 0); // INT64_MAX + 1 -> reject + CHECK(parseBankGeneration("99999999999999999999") == 0); // 20 nines -> reject +} + +static void testFormatRoundTrip() { + CHECK(formatBankGeneration(0) == "0"); + CHECK(formatBankGeneration(1) == "1"); + CHECK(formatBankGeneration(123456789) == "123456789"); + // Round-trips: format then parse yields the original for the valid domain. + for (std::int64_t g : {std::int64_t{0}, std::int64_t{1}, std::int64_t{7}, + std::int64_t{9007199254740993LL}}) { + CHECK(parseBankGeneration(formatBankGeneration(g)) == g); + } +} + +// --- bankGenerationChanged --------------------------------------------------- + +static void testGenerationChanged() { + CHECK(!bankGenerationChanged(0, 0)); // pre-S9 idle: no stamp seen, no stamp now + CHECK(bankGenerationChanged(0, 1)); // first bump after a pre-S9 baseline -> change + CHECK(bankGenerationChanged(5, 6)); // normal increment + CHECK(!bankGenerationChanged(6, 6)); // idle poll (coalesced): no change + CHECK(bankGenerationChanged(6, 3)); // a project switch/reload can lower it -> change +} + +// --- consumeDecision --------------------------------------------------------- + +static AssignmentRequest makeReq(const std::string& bank, const std::string& sample, + std::int64_t gen) { + AssignmentRequest r; + r.bankId = bank; + r.sampleId = sample; + r.generation = gen; + return r; +} + +// Rule 1a: no pending request -> nothing to do, marker unchanged. +static void testNoRequest() { + const auto d = consumeDecision(std::nullopt, /*lastConsumed*/ 5, + /*resolves*/ true, /*isFocusedTarget*/ true); + CHECK(!d.apply); + CHECK(d.consumedGeneration == 5); // marker held +} + +// Rule 1b: a request no newer than what we already consumed (re-open case) -> no re-apply. +static void testNotNewerNotReapplied() { + // The persisted marker equals the request generation: the user already got this assign, + // possibly changed away from it. It MUST NOT re-apply on re-open. + const auto same = consumeDecision(makeReq("b", "s", 100), 100, true, true); + CHECK(!same.apply); + CHECK(same.consumedGeneration == 100); // unchanged + // An older request (a stale value lingering) is likewise ignored. + const auto older = consumeDecision(makeReq("b", "s", 90), 100, true, true); + CHECK(!older.apply); + CHECK(older.consumedGeneration == 100); +} + +// Rule 2: a NEW request but this instance is not the target -> do not apply AND do not +// advance the marker (must stay eligible if focus later lands here — no thundering herd). +static void testNonTargetStaysEligible() { + const auto d = consumeDecision(makeReq("b", "s", 200), /*lastConsumed*/ 100, + /*resolves*/ true, /*isFocusedTarget*/ false); + CHECK(!d.apply); + CHECK(d.consumedGeneration == 100); // marker NOT advanced -> still eligible later +} + +// Rule 3: a NEW request, target, but unresolvable -> DROP silently. Marker advances so it +// is never re-evaluated, but no selection change (assignment_request.h reader requirement). +static void testUnresolvableDroppedSilently() { + const auto d = consumeDecision(makeReq("b", "deleted-sample", 200), + /*lastConsumed*/ 100, /*resolves*/ false, + /*isFocusedTarget*/ true); + CHECK(!d.apply); // no selection change + CHECK(d.consumedGeneration == 200); // consumed-as-seen: never re-evaluated + CHECK(d.sampleId.empty()); // nothing to apply +} + +// Rule 4: a NEW request, target, resolvable -> APPLY selection + advance the marker. +static void testAppliedWhenNewTargetResolvable() { + const auto d = consumeDecision(makeReq("bank-7", "cap-42", 200), + /*lastConsumed*/ 100, /*resolves*/ true, + /*isFocusedTarget*/ true); + CHECK(d.apply); + CHECK(d.bankId == "bank-7"); + CHECK(d.sampleId == "cap-42"); + CHECK(d.consumedGeneration == 200); +} + +// Re-assigning the SAME sample id under a NEW generation must re-apply (the generation is +// the disambiguator; a recapture/re-drop of the same id is a fresh assign, not a no-op). +static void testSameIdNewGenerationReapplies() { + // First consume at gen 100. + const auto first = consumeDecision(makeReq("b", "s", 100), 50, true, true); + CHECK(first.apply); + CHECK(first.consumedGeneration == 100); + // Same id, higher generation, marker now at 100 -> applies again. + const auto second = consumeDecision(makeReq("b", "s", 150), 100, true, true); + CHECK(second.apply); + CHECK(second.sampleId == "s"); + CHECK(second.consumedGeneration == 150); +} + +// A fresh instance (lastConsumed == 0) applies a first assign — the default marker must not +// swallow the first request. +static void testFreshInstanceAppliesFirst() { + const auto d = consumeDecision(makeReq("b", "s", 1), 0, true, true); + CHECK(d.apply); + CHECK(d.consumedGeneration == 1); +} + +int main() { + testParseAbsentAndMalformed(); + testParseValid(); + testParseOverflow(); + testFormatRoundTrip(); + testGenerationChanged(); + testNoRequest(); + testNotNewerNotReapplied(); + testNonTargetStaysEligible(); + testUnresolvableDroppedSilently(); + testAppliedWhenNewTargetResolvable(); + testSameIdNewGenerationReapplies(); + testFreshInstanceAppliesFirst(); + + if (g_fail == 0) std::printf("All tests passed.\n"); + return g_fail ? 1 : 0; +} diff --git a/tests/test_sample_map.cpp b/tests/test_sample_map.cpp index 7763915..6b74e13 100644 --- a/tests/test_sample_map.cpp +++ b/tests/test_sample_map.cpp @@ -979,6 +979,58 @@ static void testComponentStateV4StereoWithZoneOverridesRoundTrip() { !back.map.zones[1].startPoint.has_value()); } +// --- v5 component state: the S8/S9 last-consumed-assignment marker ------------- + +static void testComponentStateV5MarkerRoundTrip() { + // The consumed-assignment generation (S8 reader marker) round-trips through the v5 envelope + // alongside selection + mode + zones. A non-zero, > 32-bit value proves the 8-byte LE field. + ComponentState s; + s.selectionId = "pick"; + s.channelMode = ChannelMode::Stereo; + s.lastConsumedAssignGeneration = 1700000123456LL; // > INT32_MAX + s.map.zones.push_back(zone("z0", 0, 127, /*override=*/std::nullopt)); + const ComponentState back = deserializeComponentState(serializeComponentState(s)); + CHECK(back.lastConsumedAssignGeneration == 1700000123456LL); // marker survives + CHECK(back.channelMode == ChannelMode::Stereo); + CHECK(back.selectionId == "pick"); + CHECK(back.map.zones.size() == 1 && back.map.zones[0].sampleId == "z0"); +} + +static void testComponentStateDefaultMarkerIsZero() { + // A default-constructed state has marker 0 and round-trips 0 — a fresh instance's first + // assign (generation >= 1) must not be swallowed by a non-zero default. + const ComponentState back = deserializeComponentState(serializeComponentState(ComponentState{})); + CHECK(back.lastConsumedAssignGeneration == 0); +} + +static void testComponentStateV4LiftsMarkerToZero() { + // A GENUINE v4 blob (version tag 4: mode byte, then id + zones — NO 8-byte marker) must lift + // with lastConsumedAssignGeneration = 0 and its mode/selection/zones intact. Build it by hand + // (serializeComponentState now emits v5, so we cannot use it to make a v4 blob). This proves + // an already-saved pre-S8/S9 instance restores cleanly and its first assign still applies. + std::vector v4; + v4.push_back(4); v4.push_back(0); v4.push_back(0); v4.push_back(0); // version 4 + v4.push_back(1); // channel mode = stereo + const std::string id = "saved"; + v4.push_back(static_cast(id.size())); v4.push_back(0); v4.push_back(0); v4.push_back(0); + v4.insert(v4.end(), id.begin(), id.end()); + v4.push_back(0); v4.push_back(0); v4.push_back(0); v4.push_back(0); // zone count 0 + const ComponentState back = deserializeComponentState(v4); + CHECK(back.lastConsumedAssignGeneration == 0); // no marker in v4 -> default 0 + CHECK(back.channelMode == ChannelMode::Stereo); // v4 mode byte still honored + CHECK(back.selectionId == "saved"); + CHECK(back.map.zones.empty()); +} + +static void testComponentStateV5TruncatedMarker() { + // A v5 blob truncated inside the 8-byte marker (mode byte present, marker cut short) -> empty, + // mono + marker 0 default holds (bounded read, never throws across the host). + std::vector t{5, 0, 0, 0, 1, 0xAA, 0xBB}; // version 5, mode byte, 2 marker bytes + const ComponentState back = deserializeComponentState(t); + CHECK(back.lastConsumedAssignGeneration == 0); + CHECK(back.selectionId.empty() && back.map.zones.empty()); +} + int main() { testSelectByIdHit(); testSelectEmptyIdIsSilence(); @@ -1049,6 +1101,10 @@ int main() { testComponentStateV1V2LiftToMono(); testComponentStateV4TruncatedModeByte(); testComponentStateV4StereoWithZoneOverridesRoundTrip(); + testComponentStateV5MarkerRoundTrip(); + testComponentStateDefaultMarkerIsZero(); + testComponentStateV4LiftsMarkerToZero(); + testComponentStateV5TruncatedMarker(); if (g_fail == 0) std::printf("sample_map: all tests passed\n"); return g_fail != 0;