From 8bc5aa1257a6492c7c3076af792328fe45e1d7e8 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Wed, 29 Jul 2026 11:28:55 -0400 Subject: [PATCH] Q-W3 review follow-ups: golden hash literal test, CLAUDE.md wav_codec bullet, dead RecordedCapture field comment, makeUniqueTag residual note --- CLAUDE.md | 2 +- src/core/capture/capture_realtime.h | 10 ++++++++++ src/shell/capture/capture.cpp | 6 ++++++ tests/test_wav_codec.cpp | 20 +++++++++++++++++++- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2b8312f..4db0389 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,7 +64,7 @@ There is no hot-reload. Copy the built binary into REAPER's `UserPlugins/` folde - `bank_book` — multi-bank registry: an ordered set of banks each wrapping a `BankIndex`. **Pool privileges (un-deletable/un-renamable/un-evacuable, never zero banks) enforced in-model.** Owns create/rename/reorder/delete of named banks, active-bank id, index-only move/copy/remove of a sample between banks, and JSON round-trip. - `owned_manifest` — the set of project-relative files the capture path itself created, persisted under the `"owned_files"` ext-state key, so the prune path can distinguish the bank system's own orphans from hand-dropped files. - `app_version` — REAPER-free version/channel identity: CMake-sourced semver constant, ext-state stamp value, and the full set of channel-derived identity accessors. All channel strings derive from one `REASAMPLER_CHANNEL_IS_BETA` bit; no scattered `#ifdef`s in the shells. -- `wav_trim` — 32-bit-float WAV parse + header-aware truncate plan for the realtime tail's PCM decay-scan trim. +- `wav_codec` — chunk walker + layout parse + float32 build + size-field patch + content hashes; the single pure RIFF/WAV owner. (`wav_trim` is now a transitional forwarding alias onto `wav_codec`, kept only so the Q-W2v TUs it feeds compile untouched; retire it once that wave lands.) - `provenance` — capture-recipe fingerprint: build/encode/compare a `rsprov1` fingerprint of scope, range, tail, rate/channels, track GUIDs, and FX-chain identity. **A thin reproducibility fingerprint — NOT a serialized chain to restore.** - `prune_reconcile` — pure prune core: `pruneOrphans(present, referenced, owned)` computes `(owned ∩ present) − referenced`; the safety-critical "which files are orphans" decision, filesystem-free and hard-tested before any I/O exists. Gains `mergeReferenced(bankRefs, liveInstanceHeldPaths)` (pS-usage) — unions live instance holds into the prune referenced-set so the pure orphan computation includes them. - `prune_button` — pure layout/hit-test for the `bank_panel` footer Prune button. diff --git a/src/core/capture/capture_realtime.h b/src/core/capture/capture_realtime.h index b1da9c9..d497e8b 100644 --- a/src/core/capture/capture_realtime.h +++ b/src/core/capture/capture_realtime.h @@ -114,6 +114,16 @@ struct RecordedCapture { std::vector trackGuids; int channelCount = 0; + + // TEST-ONLY / dead in production (Q-W3 review follow-up): the shell no longer + // populates these five fields before calling sampleFromRecordedCapture — the + // finalize path (capture_realtime_finalize.cpp) leaves them at their defaults + // and instead calls the shared stampCaptureSample(result.sample, ...) right + // after, which writes Sample::sampleRate/captureTempo/captureTimeSigNum/ + // captureTimeSigDenom/createdTimestamp directly, overwriting whatever + // sampleFromRecordedCapture set from these. Kept (not deleted) because the pure + // unit tests still construct/assert them directly; removing the fields is a + // struct-shape decision out of scope here. int sampleRate = 0; // 0 when the project rate was unknown (as offline) double captureTempo = 0.0; // BPM at capture time (shell reads Master_GetTempo) // Time signature at capture start (L7 F1; shell reads TimeMap_GetTimeSigAtTime). diff --git a/src/shell/capture/capture.cpp b/src/shell/capture/capture.cpp index 377f74a..b68fd8a 100644 --- a/src/shell/capture/capture.cpp +++ b/src/shell/capture/capture.cpp @@ -239,6 +239,12 @@ std::string makeUniqueTag(const std::string& prefix) { // session distinct regardless of timing. NOTE: the tag varies the file NAME, // not the audio bytes — bit-identical-repeat is about identical *content* for // identical requests; two deliberate captures naturally live in two files. + // RESIDUAL (Q-W3 review follow-up): the counter is per-process, starting over + // at 0 on every REAPER launch/extension reload, so two separate REAPER + // instances (or a reload mid-session) can still mint the same timestamp+counter + // pair in the same wall-clock second — a same-second cross-process collision + // remains theoretically possible. Scoped to per-session deliberately: this fix + // targets the reachable-in-practice single-process batch-capture case above. static std::atomic counter{0}; const std::time_t now = std::time(nullptr); return prefix + std::to_string(static_cast(now)) + "-" + diff --git a/tests/test_wav_codec.cpp b/tests/test_wav_codec.cpp index f3038e6..b768e71 100644 --- a/tests/test_wav_codec.cpp +++ b/tests/test_wav_codec.cpp @@ -9,7 +9,9 @@ // extraction (whole / tail window / clamp / out-of-range); truncate plan (kept pcm = {0.0, 0.5, -0.25, 1.0, -1.0, 0.125}; + auto wav = buildFloat32Wav(2, 48000, 3, pcm); + CHECK(hashWavContent(wav) == "7ccf298c166a670a"); + CHECK(hashBytes(wav.data(), wav.size()) == "68d8a193c958fd44"); +} + int main() { testParseCanonicalStereo(); testParseMonoAndLeadingChunk(); @@ -600,6 +617,7 @@ int main() { testHashWavContentListMetaSkipped(); testHashWavContentDomainSeparationFromWholeFile(); testHashMatchesBuildOutput(); + testGoldenHashLiterals(); if (g_fail == 0) std::printf("wav_codec: all tests passed\n"); else std::printf("wav_codec: %d CHECK(s) FAILED\n", g_fail);