From 7d0fcc49acfd4b6c515eaecd5d130cdb33f9fb1c Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 26 Jul 2026 18:03:20 -0400 Subject: [PATCH] fix(m10): item-scope take-FX fingerprint; Windows path case-fold; comments Item scope now reads the active take's FX chain via TakeFX_* (new fxChainIdentityForItems helper), not the owning track's chain. normalizeSlashes lowercases on _WIN32 for detectParent. Stale comments corrected; case-fold tests added. --- src/bank_model.h | 5 +-- src/capture_paths.cpp | 8 +++++ src/capture_paths.h | 4 +++ src/main.cpp | 64 ++++++++++++++++++++++++++---------- src/provenance.h | 16 +++++---- src/provenance_shell.cpp | 39 ++++++++++++++++++++++ src/provenance_shell.h | 20 +++++++---- tests/test_capture_paths.cpp | 59 ++++++++++++++++++++++++++++++--- 8 files changed, 177 insertions(+), 38 deletions(-) diff --git a/src/bank_model.h b/src/bank_model.h index 4af7b5f..2bb59ad 100644 --- a/src/bank_model.h +++ b/src/bank_model.h @@ -44,8 +44,9 @@ struct SourceRange { }; // Present only when a sample was resampled FROM another sample. Carries the -// parent's id and the FX-chain snapshot string captured at resample time, so the -// null-test / re-capture-from-source action (M10) can reconstruct the chain. +// parent's id and the FX-chain snapshot string (a thin drift fingerprint, NOT a +// restorable chunk) captured at resample time; the re-capture-from-source action +// (M10) uses it to detect chain drift and replay the original capture request. struct Provenance { std::string parentSampleId; std::string fxChainSnapshot; diff --git a/src/capture_paths.cpp b/src/capture_paths.cpp index f328962..c512ded 100644 --- a/src/capture_paths.cpp +++ b/src/capture_paths.cpp @@ -1,6 +1,7 @@ #include "capture_paths.h" #include +#include #include #include #include // std::memcmp @@ -124,6 +125,13 @@ std::string normalizeSlashes(const std::string& path) { if (out.size() > 1 && out.back() == '/') { out.pop_back(); } +#ifdef _WIN32 + // Windows paths are case-insensitive. Fold to lowercase so that two paths + // that differ only in drive-letter or component casing compare equal (e.g. + // "C:/Foo/BAR.wav" == "c:/foo/bar.wav"). On macOS/Linux, exact case is + // preserved (the filesystem is case-sensitive; folding would be wrong). + for (char& c : out) c = static_cast(std::tolower(static_cast(c))); +#endif return out; } diff --git a/src/capture_paths.h b/src/capture_paths.h index 3286287..dee685d 100644 --- a/src/capture_paths.h +++ b/src/capture_paths.h @@ -68,6 +68,10 @@ std::string hashWavContent(const std::vector& bytes); // Normalizes a path to forward slashes and strips any trailing slash. Empty in // -> empty out. Pure string transform (does not consult the filesystem). +// Platform case rule: on Windows (_WIN32) the result is also lowercased so that +// paths differing only in drive-letter or component casing compare equal (Windows +// paths are case-insensitive). On macOS/Linux the case is preserved exactly (those +// filesystems are case-sensitive). std::string normalizeSlashes(const std::string& path); // Sanitizes a caller-supplied base name into a filesystem-safe stem: keeps diff --git a/src/main.cpp b/src/main.cpp index a117960..16c2793 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -438,8 +438,8 @@ static reasampler::ProvenanceScope provenanceScopeFor(reasampler::CaptureScope s // fingerprint (P1=a) — scope + source mode + exact range + tail + rate + channels + // source track GUIDs + the in-scope source FX-chain identity — so "re-capture from // source" can replay the request and report drift. NEVER a serialized chain to -// restore. Item scope folds an empty FX identity (take/item FX are not enumerable -// via TrackFX_*); the drift signal then keys on scope+range, which is honest. +// restore. Item scope reads the active take's TakeFX chain (via TakeFX_*) per +// selected item, combined in item order; Track scope reads the track FX chain. static std::optional buildCaptureProvenance( const reasampler::CaptureRequest& req, reasampler::CaptureScope scope, @@ -472,15 +472,27 @@ static std::optional buildCaptureProvenance( recipe.sampleRate = req.sampleRate; recipe.channelCount = req.channelCount; recipe.trackGuids = req.trackGuids; - // The in-scope FX-chain identity is the per-track chains combined in track order - // (Track scope), length-prefixed so distinct partitions never collide. Item scope - // has no readable take-FX chain, so each track folds to an empty identity and the - // combined result stays stable/honest (drift then keys on scope + range). - std::vector perTrack; - perTrack.reserve(src.sourceTracks.size()); - for (MediaTrack* tr : src.sourceTracks) - perTrack.push_back(reasampler::fxChainIdentityForTrack(tr)); - recipe.fxChainIdentity = reasampler::combineChainIdentities(perTrack); + // The in-scope FX-chain identity: + // Track scope — per-track chains combined in track order (TrackFX_*). + // Item scope — per-item active-take chains combined in item order (TakeFX_*); + // the owning track's FX chain is OUT OF SCOPE for an item capture and must + // not be fingerprinted here (it is bypassed during render, not heard). + if (scope == reasampler::CaptureScope::Item) { + const int n = CountSelectedMediaItems(nullptr); + std::vector items; + items.reserve(static_cast(n < 0 ? 0 : n)); + for (int i = 0; i < n; ++i) { + MediaItem* it = GetSelectedMediaItem(nullptr, i); + if (it) items.push_back(it); + } + recipe.fxChainIdentity = reasampler::fxChainIdentityForItems(items); + } else { + std::vector perTrack; + perTrack.reserve(src.sourceTracks.size()); + for (MediaTrack* tr : src.sourceTracks) + perTrack.push_back(reasampler::fxChainIdentityForTrack(tr)); + recipe.fxChainIdentity = reasampler::combineChainIdentities(perTrack); + } reasampler::Provenance prov; prov.parentSampleId = *parentId; @@ -883,12 +895,26 @@ static void RunRecaptureFromSource() req.trackGuids = recipe->trackGuids; // Read the CURRENT source FX-chain identity BEFORE the render bypasses it, to - // compare against the recorded identity for drift reporting. - std::vector perTrackNow; - perTrackNow.reserve(sourceTracks.size()); - for (MediaTrack* tr : sourceTracks) - perTrackNow.push_back(reasampler::fxChainIdentityForTrack(tr)); - const std::string currentIdentity = reasampler::combineChainIdentities(perTrackNow); + // compare against the recorded identity for drift reporting. Mirror the same + // scope split as buildCaptureProvenance: item scope reads take FX via TakeFX_*; + // track scope reads the track FX chain via TrackFX_*. + std::string currentIdentity; + if (scope == reasampler::CaptureScope::Item) { + const int n = CountSelectedMediaItems(nullptr); + std::vector items; + items.reserve(static_cast(n < 0 ? 0 : n)); + for (int i = 0; i < n; ++i) { + MediaItem* it = GetSelectedMediaItem(nullptr, i); + if (it) items.push_back(it); + } + currentIdentity = reasampler::fxChainIdentityForItems(items); + } else { + std::vector perTrackNow; + perTrackNow.reserve(sourceTracks.size()); + for (MediaTrack* tr : sourceTracks) + perTrackNow.push_back(reasampler::fxChainIdentityForTrack(tr)); + currentIdentity = reasampler::combineChainIdentities(perTrackNow); + } const bool drifted = (currentIdentity != recipe->fxChainIdentity); // Render (bank-only; renderOffline never touches the timeline). @@ -918,6 +944,10 @@ static void RunRecaptureFromSource() updated.captureTempo = res.sample.captureTempo; updated.trackGuids = res.sample.trackGuids; updated.createdTimestamp = res.sample.createdTimestamp; + // NOTE: levels, clipped, and lengthBeats are carried from the original (via the + // *orig copy above) because the offline backend does not populate them today + // (res.sample leaves them at defaults). If a later milestone populates these + // fields at capture time, refresh them here from res.sample instead. reasampler::Provenance prov; prov.parentSampleId = recordedParentId; prov.fxChainSnapshot = reasampler::buildFingerprint(refreshed); diff --git a/src/provenance.h b/src/provenance.h index 810fa37..d8f5a66 100644 --- a/src/provenance.h +++ b/src/provenance.h @@ -46,13 +46,14 @@ enum class ProvenanceScope { Track, }; -// One FX-chain entry as the shell reads it from REAPER (TrackFX_GetFXName / -// TrackFX_GetFXGUID / TrackFX_GetEnabled). Plain data — the shell fills it, the +// One FX-chain entry as the shell reads it from REAPER. For Track scope the shell +// uses TrackFX_GetFXName/GetFXGUID/GetEnabled; for Item scope it uses the TakeFX_* +// equivalents over the active take's FX chain. Plain data — the shell fills it, the // pure fold turns the vector into the identity string. struct FxIdentityEntry { - std::string name; // TrackFX_GetFXName - std::string guid; // TrackFX_GetFXGUID -> guidToString (per-instance identity) - bool enabled; // TrackFX_GetEnabled + std::string name; // TrackFX_GetFXName / TakeFX_GetFXName + std::string guid; // TrackFX_GetFXGUID / TakeFX_GetFXGUID -> guidToString + bool enabled; // TrackFX_GetEnabled / TakeFX_GetEnabled }; // The recorded capture recipe + source FX-chain identity — the thin fingerprint. @@ -138,8 +139,9 @@ struct BankFileRef { // // Returns the parent sample id, or nullopt when the capture is not a genuine // resample-from-sample. Comparison is exact path identity; the caller normalizes -// both sides identically so a slash/case difference never spuriously matches or -// misses (case handling is the caller's normalization contract, not decided here). +// both sides identically via normalizeSlashes (which lowercases on Windows) so a +// slash/case difference never spuriously matches or misses. On Windows both sides +// are lowercased before they reach here; on macOS/Linux they are case-exact. std::optional detectParent( const std::vector& sourceItemFiles, const std::vector& bankFiles); diff --git a/src/provenance_shell.cpp b/src/provenance_shell.cpp index fdda40f..111ae6c 100644 --- a/src/provenance_shell.cpp +++ b/src/provenance_shell.cpp @@ -8,6 +8,10 @@ // * TrackFX_GetFXName(MediaTrack*, int, char*, int) -> bool (~7356) // * TrackFX_GetFXGUID(MediaTrack*, int) -> GUID* (~7348) // * TrackFX_GetEnabled(MediaTrack*, int) -> bool (~7291) +// * TakeFX_GetCount(MediaItem_Take*) (~6710) +// * TakeFX_GetFXName(MediaItem_Take*, int, char*, int) -> bool (~6758) +// * TakeFX_GetFXGUID(MediaItem_Take*, int) -> GUID* (~6750) +// * TakeFX_GetEnabled(MediaItem_Take*, int) -> bool (~6718) // * CountSelectedMediaItems / GetSelectedMediaItem (selection reads) // * GetActiveTake(MediaItem*) -> MediaItem_Take* (active take) // * GetMediaItemTake_Source(MediaItem_Take*) -> PCM_source* (~2053) @@ -28,6 +32,10 @@ #define REAPERAPI_WANT_TrackFX_GetFXName #define REAPERAPI_WANT_TrackFX_GetFXGUID #define REAPERAPI_WANT_TrackFX_GetEnabled +#define REAPERAPI_WANT_TakeFX_GetCount +#define REAPERAPI_WANT_TakeFX_GetFXName +#define REAPERAPI_WANT_TakeFX_GetFXGUID +#define REAPERAPI_WANT_TakeFX_GetEnabled #define REAPERAPI_WANT_CountSelectedMediaItems #define REAPERAPI_WANT_GetSelectedMediaItem #define REAPERAPI_WANT_CountTrackMediaItems @@ -67,6 +75,37 @@ std::string fxChainIdentityForTrack(MediaTrack* tr) { return fxChainIdentity(rows); } +std::string fxChainIdentityForItems(const std::vector& items) { + // For Item scope the in-scope chain is each item's active take's FX chain, NOT + // the owning track's FX chain (the track chain is out-of-scope and is bypassed + // during render). TakeFX_* is the correct family here. + std::vector perItem; + perItem.reserve(items.size()); + for (MediaItem* it : items) { + if (!it) { perItem.push_back(fxChainIdentity({})); continue; } + MediaItem_Take* take = GetActiveTake(it); + if (!take) { perItem.push_back(fxChainIdentity({})); continue; } + std::vector rows; + const int n = TakeFX_GetCount(take); + rows.reserve(static_cast(n < 0 ? 0 : n)); + for (int i = 0; i < n; ++i) { + FxIdentityEntry e; + char nameBuf[512] = {0}; + if (TakeFX_GetFXName(take, i, nameBuf, static_cast(sizeof(nameBuf)))) + e.name = nameBuf; + if (GUID* g = TakeFX_GetFXGUID(take, i)) { + char gb[64] = {0}; + guidToString(g, gb); + e.guid = gb; + } + e.enabled = TakeFX_GetEnabled(take, i); + rows.push_back(std::move(e)); + } + perItem.push_back(fxChainIdentity(rows)); + } + return combineChainIdentities(perItem); +} + namespace { // The active take source file of one item, normalized. Empty if unresolvable. std::string itemSourceFile(MediaItem* it) { diff --git a/src/provenance_shell.h b/src/provenance_shell.h index 8054c28..e53888b 100644 --- a/src/provenance_shell.h +++ b/src/provenance_shell.h @@ -22,20 +22,26 @@ #include "provenance.h" class MediaTrack; +class MediaItem; namespace reasampler { class BankBook; -// The in-scope FX-chain identity of a source track, folded to the pure -// provenance string. For TRACK scope this is the track's own FX chain; for ITEM -// scope the take/item FX are the in-scope chain — but item/take FX are not -// enumerable via the TrackFX_* family, so an item-scope capture folds an EMPTY -// chain identity (the drift signal then keys on scope + range only, which is -// honest: we do not claim to fingerprint take FX we cannot read). TRACK scope reads -// TrackFX_GetCount / GetFXName / GetFXGUID / GetEnabled in chain order. +// The in-scope FX-chain identity of a source track (Track scope), folded to the +// pure provenance string. Reads the track's own FX chain via TrackFX_GetCount / +// TrackFX_GetFXName / TrackFX_GetFXGUID / TrackFX_GetEnabled in chain order. std::string fxChainIdentityForTrack(MediaTrack* tr); +// The in-scope FX-chain identity for Item scope: enumerates each item's active +// take FX chain via TakeFX_GetCount / TakeFX_GetFXName / TakeFX_GetFXGUID / +// TakeFX_GetEnabled, in item order then FX order, combined with +// combineChainIdentities so distinct per-item partitions never collide. Returns +// the combined identity string (empty combined identity for a no-FX or no-item +// set). The items vector is the same source-item set the shell collected for the +// item-scope capture (selected items whose owning tracks were also collected). +std::string fxChainIdentityForItems(const std::vector& items); + // Reads the media-file path of every SELECTED media item's active take source // (GetMediaItemTake_Source -> GetMediaSourceFileName), normalized to forward-slash. // Unresolvable items (no take / no source / empty name) are omitted — never an diff --git a/tests/test_capture_paths.cpp b/tests/test_capture_paths.cpp index 301e8eb..4796893 100644 --- a/tests/test_capture_paths.cpp +++ b/tests/test_capture_paths.cpp @@ -18,12 +18,40 @@ static int g_fail = 0; std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) static void testNormalizeSlashes() { +#ifdef _WIN32 + // On Windows paths are lowercased for case-insensitive comparison. + CHECK(normalizeSlashes("C:\\a\\b") == "c:/a/b"); + CHECK(normalizeSlashes("a/b/c") == "a/b/c"); + CHECK(normalizeSlashes("a/b/") == "a/b"); // trailing slash stripped + CHECK(normalizeSlashes("a\\b\\") == "a/b"); // backslash + trailing + CHECK(normalizeSlashes("/") == "/"); // lone root preserved + CHECK(normalizeSlashes("") == ""); // empty stays empty +#else CHECK(normalizeSlashes("C:\\a\\b") == "C:/a/b"); CHECK(normalizeSlashes("a/b/c") == "a/b/c"); - CHECK(normalizeSlashes("a/b/") == "a/b"); // trailing slash stripped - CHECK(normalizeSlashes("a\\b\\") == "a/b"); // backslash + trailing - CHECK(normalizeSlashes("/") == "/"); // lone root preserved - CHECK(normalizeSlashes("") == ""); // empty stays empty + CHECK(normalizeSlashes("a/b/") == "a/b"); + CHECK(normalizeSlashes("a\\b\\") == "a/b"); + CHECK(normalizeSlashes("/") == "/"); + CHECK(normalizeSlashes("") == ""); +#endif +} + +// Windows case-folding: paths differing only in casing must compare equal after +// normalizeSlashes, since Windows paths are case-insensitive. On non-Windows the +// function is case-preserving (filesystem is case-sensitive). +static void testNormalizeSlashesCaseFolding() { +#ifdef _WIN32 + // Drive letter and component casing differences are neutralized. + CHECK(normalizeSlashes("C:/Foo/BAR.wav") == normalizeSlashes("c:/foo/bar.wav")); + CHECK(normalizeSlashes("C:/Foo/BAR.wav") == "c:/foo/bar.wav"); + // Mixed-case input produces consistently lowercase output. + CHECK(normalizeSlashes("C:\\Users\\Daniel\\Proj\\File.WAV") + == "c:/users/daniel/proj/file.wav"); +#else + // Non-Windows: case is preserved exactly (case-sensitive filesystem). + CHECK(normalizeSlashes("C:/Foo/BAR.wav") != normalizeSlashes("c:/foo/bar.wav")); + CHECK(normalizeSlashes("C:/Foo/BAR.wav") == "C:/Foo/BAR.wav"); +#endif } static void testSanitizeStem() { @@ -57,7 +85,12 @@ static void testDeriveRelativePathIsProjectRelative() { static void testDeriveAbsoluteDirJoinsProjectDir() { BankPaths p = deriveBankPaths("C:\\Users\\d\\proj", "kick", ""); // Backslashes normalized; bank subfolder appended; no trailing slash. + // On Windows the drive-letter + components are lowercased by normalizeSlashes. +#ifdef _WIN32 + CHECK(p.absoluteDir == "c:/users/d/proj/reasampler_bank"); +#else CHECK(p.absoluteDir == "C:/Users/d/proj/reasampler_bank"); +#endif // No unique tag -> stem has no trailing "_". CHECK(p.fileName == "kick.wav"); CHECK(p.relativePath == "reasampler_bank/kick.wav"); @@ -116,7 +149,15 @@ static void testFileStem() { static void testResolveBankFileAgainstProjectDir() { // A relative index entry resolves to /, forward- - // slashed, regardless of the input slash style. + // slashed, regardless of the input slash style. On Windows the result is also + // lowercased (Windows paths are case-insensitive; normalizeSlashes folds them). +#ifdef _WIN32 + CHECK(resolveBankFile("C:\\Users\\d\\proj", "reasampler_bank/kick.wav") + == "c:/users/d/proj/reasampler_bank/kick.wav"); + // Backslashes in the stored relative path are normalized on resolution. + CHECK(resolveBankFile("C:/p", "reasampler_bank\\a.wav") + == "c:/p/reasampler_bank/a.wav"); +#else CHECK(resolveBankFile("C:\\Users\\d\\proj", "reasampler_bank/kick.wav") == "C:/Users/d/proj/reasampler_bank/kick.wav"); CHECK(resolveBankFile("/home/d/proj", "reasampler_bank/mix.wav") @@ -127,6 +168,7 @@ static void testResolveBankFileAgainstProjectDir() { // Backslashes in the stored relative path are normalized on resolution. CHECK(resolveBankFile("/p", "reasampler_bank\\a.wav") == "/p/reasampler_bank/a.wav"); +#endif } static void testResolveBankFileRejectsEmptyInputs() { @@ -159,10 +201,16 @@ static void testResolveAgainstNewProjectDirAfterSaveAs() { static void testRelocationPlanForSaveAs() { // Save-As to a different directory: relocation is needed; both bank dirs are // /reasampler_bank, forward-slashed, no trailing slash. + // On Windows the drive-letter and path components are lowercased. BankRelocation r = deriveRelocationPlan("C:\\old\\proj", "C:/new/proj"); CHECK(r.needed); +#ifdef _WIN32 + CHECK(r.oldBankDir == "c:/old/proj/reasampler_bank"); + CHECK(r.newBankDir == "c:/new/proj/reasampler_bank"); +#else CHECK(r.oldBankDir == "C:/old/proj/reasampler_bank"); CHECK(r.newBankDir == "C:/new/proj/reasampler_bank"); +#endif } static void testRelocationPlanNotNeededForSaveInPlace() { @@ -501,6 +549,7 @@ static void testHashWavContentDomainSeparationFromWholeFile() { int main() { testNormalizeSlashes(); + testNormalizeSlashesCaseFolding(); testSanitizeStem(); testDeriveRelativePathIsProjectRelative(); testDeriveAbsoluteDirJoinsProjectDir();