From 5fabade90cde611c57e05c2941e8f6ac053e0708 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 26 Jul 2026 16:37:46 -0400 Subject: [PATCH] fix(capture): populate contentHash on captured samples to fix always-prompting remove MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit contentHash was left empty on every capture; hashReferencedElsewhere returns false for empty hashes, so every remove looked like a last reference. Add FNV-1a hashBytes to capture_paths, wire into both commit paths. NOTE: this activates index dedup-by-hash in production — a bit-identical re-capture now collapses onto the existing entry instead of adding a duplicate (spec-intended). --- src/capture.cpp | 30 +++++++++++++++++-- src/capture_paths.cpp | 19 ++++++++++++ src/capture_paths.h | 11 +++++++ src/capture_realtime.cpp | 15 +++++++++- src/realtime_record.cpp | 5 +++- tests/test_capture_paths.cpp | 58 ++++++++++++++++++++++++++++++++++++ 6 files changed, 133 insertions(+), 5 deletions(-) diff --git a/src/capture.cpp b/src/capture.cpp index 85d1b42..2ded583 100644 --- a/src/capture.cpp +++ b/src/capture.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -224,6 +225,21 @@ std::string makeUniqueTag() { return std::to_string(static_cast(now)); } +// Reads the whole file into a byte buffer. Returns an empty vector on any I/O +// failure (the caller then leaves contentHash empty — the safe, confirm-eliciting +// direction for an unreadable file). +std::vector readFileBytes(const std::string& path) { + std::ifstream f(path, std::ios::binary | std::ios::ate); + if (!f) return {}; + const std::streamoff size = f.tellg(); + if (size <= 0) return {}; + std::vector bytes(static_cast(size)); + f.seekg(0); + f.read(reinterpret_cast(bytes.data()), size); + if (!f) return {}; + return bytes; +} + } // namespace CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { @@ -483,9 +499,17 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { s.lengthSeconds = request.endSeconds - request.startSeconds; s.captureTempo = Master_GetTempo(); // BPM at capture time (verified ~4651) s.tier = Tier::Scratch; // captures land in scratch by default - // contentHash left empty for M3: hashing the rendered file is a peaks/M2- - // adjacent concern wired in a later milestone. Empty hashes do NOT dedup, so - // this is safe (bank_model treats "" as non-participating). + // Content hash: FNV-1a over the rendered file bytes so hashReferencedElsewhere + // can identify copies in other banks and suppress the last-reference confirm when + // another bank still holds the same file. Best-effort: an unreadable file leaves + // contentHash empty — the safe, confirm-eliciting direction (bank_model treats + // "" as non-participating in dedup, which is the existing fallback semantics). + { + const std::vector fileBytes = readFileBytes(expectedPath); + if (!fileBytes.empty()) { + s.contentHash = hashBytes(fileBytes.data(), fileBytes.size()); + } + } s.createdTimestamp = static_cast(std::time(nullptr)); result.status = CaptureStatus::Ok; diff --git a/src/capture_paths.cpp b/src/capture_paths.cpp index 8e319c6..2c301dd 100644 --- a/src/capture_paths.cpp +++ b/src/capture_paths.cpp @@ -1,9 +1,28 @@ #include "capture_paths.h" #include +#include +#include namespace reasampler { +std::string hashBytes(const std::uint8_t* data, std::size_t len) { + // FNV-1a 64-bit: deterministic, no dependencies, adequate for dedup identity. + // Constants from the FNV spec (http://www.isthe.com/chongo/tech/comp/fnv/). + constexpr std::uint64_t kOffsetBasis = 14695981039346656037ULL; + constexpr std::uint64_t kPrime = 1099511628211ULL; + std::uint64_t h = kOffsetBasis; + for (std::size_t i = 0; i < len; ++i) { + h ^= static_cast(data[i]); + h *= kPrime; + } + // Format as 16-digit lowercase hex (zero-padded) for a fixed-length string. + char buf[17]; + std::snprintf(buf, sizeof(buf), "%016llx", + static_cast(h)); + return std::string(buf); +} + std::string normalizeSlashes(const std::string& path) { std::string out = path; for (char& c : out) { diff --git a/src/capture_paths.h b/src/capture_paths.h index fcefcbc..42f961a 100644 --- a/src/capture_paths.h +++ b/src/capture_paths.h @@ -12,6 +12,8 @@ // the filesystem. The bank subfolder name is a fixed constant so the same // project always resolves the same bank location (determinism). +#include +#include #include namespace reasampler { @@ -31,6 +33,15 @@ struct BankPaths { std::string fileStem; // (RENDER_PATTERN — REAPER appends the extension) }; +// Computes a deterministic FNV-1a 64-bit content hash over `len` bytes at `data` +// and returns it as a 16-character lowercase hex string. Designed to fill +// Sample::contentHash so the confirm-on-last-reference guardrail +// (BankBook::hashReferencedElsewhere) can distinguish "no other bank holds this +// file" from "another bank holds the same file." An empty buffer returns the bare +// FNV-1a 64-bit offset basis in hex (a stable, non-empty sentinel that two empty +// files would share, but real WAV files are never empty). +std::string hashBytes(const std::uint8_t* data, std::size_t len); + // Normalizes a path to forward slashes and strips any trailing slash. Empty in // -> empty out. Pure string transform (does not consult the filesystem). std::string normalizeSlashes(const std::string& path); diff --git a/src/capture_realtime.cpp b/src/capture_realtime.cpp index 4635c11..0017a11 100644 --- a/src/capture_realtime.cpp +++ b/src/capture_realtime.cpp @@ -78,7 +78,7 @@ #include #include -#include "capture_paths.h" +#include "capture_paths.h" // hashBytes, deriveBankPaths #include "peaks.h" // lastFrameAboveThreshold, AudioSample #include "realtime_record.h" #include "render_settings.h" // autoTrimEndRatio, realtimeRecordWindowEnd @@ -512,6 +512,19 @@ CaptureResult finalizeRecording(RealtimeCaptureState& st) { result.status = CaptureStatus::Ok; result.sample = sampleFromRecordedCapture(cap); + // Content hash: FNV-1a over the (possibly trimmed) bank file bytes so + // hashReferencedElsewhere can identify copies in other banks and suppress the + // last-reference confirm when another bank still holds the same file. + // Best-effort: an unreadable file leaves contentHash empty — the safe, + // confirm-eliciting direction (bank_model treats "" as non-participating). + { + const std::vector fileBytes = readAllBytes(destPath); + if (!fileBytes.empty()) { + result.sample.contentHash = + hashBytes(fileBytes.data(), fileBytes.size()); + } + } + // The recorded file's true length differs from the request range when a tail was // recorded, so the Sample length must reflect the FILE, not the range: // Auto with a trim applied -> the trimmed length trimAutoTailInPlace returned. diff --git a/src/realtime_record.cpp b/src/realtime_record.cpp index 0f2bb55..ad3fc4c 100644 --- a/src/realtime_record.cpp +++ b/src/realtime_record.cpp @@ -52,7 +52,10 @@ Sample sampleFromRecordedCapture(const RecordedCapture& cap) { s.lengthSeconds = cap.endSeconds - cap.startSeconds; s.captureTempo = cap.captureTempo; s.tier = Tier::Scratch; // captures land in scratch by default - // contentHash left empty: empty hashes do not participate in dedup (bank_model). + // contentHash set by the caller (capture_realtime.cpp) after the file is + // finalized and on disk — the hash is over the finished file bytes. Left empty + // here because sampleFromRecordedCapture runs before the file exists (the + // mapping is pure / DAW-free); the shell patches it in after the move+trim. s.createdTimestamp = cap.createdTimestamp; return s; } diff --git a/tests/test_capture_paths.cpp b/tests/test_capture_paths.cpp index 426a0cc..98428d1 100644 --- a/tests/test_capture_paths.cpp +++ b/tests/test_capture_paths.cpp @@ -5,8 +5,10 @@ #include "../src/capture_paths.h" +#include #include #include +#include using namespace reasampler; @@ -293,6 +295,57 @@ static void testTransitionInPlaceSaveIsNoOp() { == ProjectTransition::NoOp); } +// --- hashBytes (FNV-1a content hash) ---------------------------------------- +// +// The fix for the confirm-on-last-reference bug: hashBytes produces a 16-char hex +// string that capture.cpp and capture_realtime.cpp store on Sample::contentHash so +// BankBook::hashReferencedElsewhere can detect copies and suppress the confirm when +// another bank still holds the same file. + +static void testHashBytesOutputFormat() { + // Output is always 16 lowercase hex characters. + const std::uint8_t bytes[] = {0x01, 0x02, 0x03}; + const std::string h = hashBytes(bytes, 3); + CHECK(h.size() == 16); + for (char c : h) { + CHECK((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f')); + } +} + +static void testHashBytesDeterministic() { + // Same input always produces the same output (bit-identical captures get + // the same hash, so hashReferencedElsewhere fires correctly for copies). + const std::uint8_t bytes[] = {0xDE, 0xAD, 0xBE, 0xEF, 0x01}; + CHECK(hashBytes(bytes, 5) == hashBytes(bytes, 5)); +} + +static void testHashBytesDistinct() { + // Different inputs produce different hashes (no accidental dedup of distinct + // files). This covers the "one-bit-flip changes the hash" property. + std::uint8_t a[] = {0x00, 0x00}; + std::uint8_t b[] = {0x00, 0x01}; + CHECK(hashBytes(a, 2) != hashBytes(b, 2)); + + std::uint8_t c[] = {0xFF, 0xFF, 0xFF}; + std::uint8_t d[] = {0xFF, 0xFF, 0xFE}; + CHECK(hashBytes(c, 3) != hashBytes(d, 3)); +} + +static void testHashBytesEmptyBufferIsNonEmpty() { + // An empty buffer returns the FNV-1a offset basis in hex (stable, non-empty + // sentinel) — capturing the contract that even empty inputs yield a 16-char hash. + const std::string h = hashBytes(nullptr, 0); + CHECK(h.size() == 16); +} + +static void testHashBytesLargerBufferDiffersFromSmaller() { + // Padding a buffer with a zero byte must change the hash (order + length + // sensitivity so two differently-sized WAV files don't accidentally collide). + const std::uint8_t short_buf[] = {0xAB, 0xCD}; + const std::uint8_t long_buf[] = {0xAB, 0xCD, 0x00}; + CHECK(hashBytes(short_buf, 2) != hashBytes(long_buf, 3)); +} + int main() { testNormalizeSlashes(); testSanitizeStem(); @@ -318,6 +371,11 @@ int main() { testTransitionTwoUnsavedProjectsSwitchLoads(); testTransitionFirstSaveOfUnsavedRelocatesButPlanNoOps(); testTransitionInPlaceSaveIsNoOp(); + testHashBytesOutputFormat(); + testHashBytesDeterministic(); + testHashBytesDistinct(); + testHashBytesEmptyBufferIsNonEmpty(); + testHashBytesLargerBufferDiffersFromSmaller(); if (g_fail == 0) std::printf("capture_paths: all tests passed\n"); else std::printf("capture_paths: %d CHECK(s) FAILED\n", g_fail);