From a91df760ccc33cbe861d519ae9b3a341f46d34e8 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 2 Aug 2026 06:37:40 -0400 Subject: [PATCH 1/3] capture: name the render source in the exact-bounds refusal, and put its one-frame tolerance under test The tolerance is unchanged and now derived, not assumed: frameCountFor lands in {floor(L), ceil(L)}, so a non-frame-aligned window can never miss by more than a frame. Naming the source is what tells a self-bounding render from a short one. --- docs/VERIFICATION.md | 2 + src/core/capture/CLAUDE.md | 4 +- src/core/capture/render_settings.cpp | 12 +++++ src/core/capture/render_settings.h | 7 +++ src/core/capture/render_window.cpp | 7 +++ src/core/capture/render_window.h | 12 +++++ src/shell/capture/capture.cpp | 29 +++++----- tests/test_render_settings.cpp | 24 +++++++++ tests/test_render_window.cpp | 80 +++++++++++++++++++++++++++- 9 files changed, 157 insertions(+), 20 deletions(-) diff --git a/docs/VERIFICATION.md b/docs/VERIFICATION.md index ac26d70..33f9b8e 100644 --- a/docs/VERIFICATION.md +++ b/docs/VERIFICATION.md @@ -26,6 +26,8 @@ Checks for Θ, Ξ, and Ψ work that no unit test can close. Build **Release**, i - [ ] Same source: track scope × time selection, and track scope × razor — same exact window (`PLAN.md:2136`) - [ ] One razor-union case (two disjoint areas, one track) — lands the requested window, no `ReaSampler capture failed:` line (`PLAN.md:2137`) - [ ] Capture an item whose extent already equals the window — still lands, unchanged (the byte-identity regression floor) (`docs/COMPLETED.md:829`) +- [ ] **Open blocker.** A live capture refused with a 38-frame shortfall (195216 of 195254 at 48 kHz). Set View → time unit to Samples, then over the same range run **track** scope and **item** scope in turn and report the refusal's `Render source:` line plus both frame counts, and whether the media under the range ends before the range does +- [ ] Same range extended ~1 s past all media, track scope — a full-length file with trailing silence means postprocessing is off; a short file means a trailing-silence trim is firing despite `RENDER_NORMALIZE &(4<<16)` ## Names and channels diff --git a/src/core/capture/CLAUDE.md b/src/core/capture/CLAUDE.md index 19df546..15688d8 100644 --- a/src/core/capture/CLAUDE.md +++ b/src/core/capture/CLAUDE.md @@ -51,8 +51,8 @@ Detail specific to these pure modules: - `capture_paths` — the REAPER-free path arithmetic behind offline capture: bank-subfolder + unique-filename derivation (`deriveBankPaths`, forward-slash form, no filesystem touch), the absolute-render-dir vs. project-relative-index-path split (`BankPaths`), the persist-side inverse (`resolveBankFile`, `projectDirOfRpp`), the Save-As bank-relocation plan (`deriveRelocationPlan`), and the GUID-primary project-identity classifier (`classifyProjectTransition` → `NoOp`/`Load`/`SaveAsRelocate`) the persist-poll timer drives. - `capture_name` — the REAPER-free composition of one capture's label + file-stem base from its source-track name(s), a local-calendar discriminator (`MM-DD HHMM`, from the shell's clock read), and an optional batch ordinal. The label and the stem deliberately diverge: the stem still passes through `capture_paths::sanitizeStem` (so a name that sanitizes to nothing files as `capture`), while the label keeps the source name verbatim. Stem uniqueness stays entirely `makeUniqueTag`'s — this module never disambiguates. - `insert_plan` — the REAPER-free logic behind the `insert` shell (M6): computes the `InsertMedia` `mode` bitmask from an `InsertOptions` struct (placement target, tempo-conform ratio, preserve-pitch flag), guaranteeing the &4 stretch-to-time-selection bit is never set and that no tempo bits are set when `conform == None`. -- `render_settings` — the REAPER-free logic behind the capture action family: `SourceMode` → `RENDER_SETTINGS` bit mapping, `P_RAZOREDITS` string parsing + range-union bounds, razor-else-time range inference, the FX-scope bypass plan (`fxBypassPlanFor`), the tail-mode → `RENDER_TAILFLAG`/`RENDER_NORMALIZE`/`RENDER_TRIMEND` mapping (`tailRenderSettingsFor`) and its realtime-window analog (`realtimeRecordWindowEnd`), and the capture-action taxonomy table (`captureActionTable`) `main.cpp` iterates to register the CAPTURE_ITEM/CAPTURE_TRACK family. -- `render_window` — the REAPER-free frame arithmetic behind exact capture bounds: `frameCountFor` (the frame count a project-time window occupies at the project rate — the number the offline backend checks the rendered file against before landing it, so a widened render is refused rather than banked) and `itemExtentPrintsWindow`, the predicate `render_settings::sourceModeForScope` consults to decide whether REAPER's selected-items render source can express a requested window at all. +- `render_settings` — the REAPER-free logic behind the capture action family: `SourceMode` → `RENDER_SETTINGS` bit mapping, `P_RAZOREDITS` string parsing + range-union bounds, razor-else-time range inference, the FX-scope bypass plan (`fxBypassPlanFor`), the tail-mode → `RENDER_TAILFLAG`/`RENDER_NORMALIZE`/`RENDER_TRIMEND` mapping (`tailRenderSettingsFor`) and its realtime-window analog (`realtimeRecordWindowEnd`), the capture-action taxonomy table (`captureActionTable`) `main.cpp` iterates to register the CAPTURE_ITEM/CAPTURE_TRACK family, and `renderSourceLabel` (the source named in the offline backend's bounds refusal). +- `render_window` — the REAPER-free frame arithmetic behind exact capture bounds: `frameCountFor` (the frame count a project-time window occupies at the project rate — the number the offline backend checks the rendered file against before landing it, so a render that printed something other than the window is refused rather than banked), `renderHonoredBounds` (the gate's verdict and the sole home of its one-frame tolerance and the derivation behind it), and `itemExtentPrintsWindow`, the predicate `render_settings::sourceModeForScope` consults to decide whether REAPER's selected-items render source can express a requested window at all. - `track_topology` — the REAPER-free folder arithmetic over a project's flat `I_FOLDERDEPTH` delta list: `directChildIndices` names a folder parent's DIRECT children, the set `shell/capture/render_isolation` silences so a ranged item capture does not print its track's children. Grandchildren are excluded by construction — they reach the parent only through the child that owns them. - `tail_control` — the REAPER-free logic behind the docked `bank_panel`'s tail-mode toggle: the cycle order (None → Auto → Manual → None), the Manual-length clamp/scroll-wheel fine-adjust (`clampManualMs`/`adjustManualMs`, 250 ms/notch, 2000 ms default), the toggle's label text (e.g. "Tail: Manual 2.0s"), and the `TailSetting` JSON round-trip persist stores per-project. diff --git a/src/core/capture/render_settings.cpp b/src/core/capture/render_settings.cpp index 4939940..bfdb258 100644 --- a/src/core/capture/render_settings.cpp +++ b/src/core/capture/render_settings.cpp @@ -100,6 +100,18 @@ RenderSettingsChoice renderSettingsFor(SourceMode mode, double /*wetDry*/) { return c; } +const char* renderSourceLabel(SourceMode mode) { + switch (mode) { + case SourceMode::MasterMix: return "master mix"; + case SourceMode::TimeSelection: return "master mix (time selection)"; + case SourceMode::SelectedTracks: return "selected tracks via master"; + case SourceMode::SelectedItems: return "selected media items"; + case SourceMode::RazorArea: return "razor edits"; + case SourceMode::Realtime: return "realtime record"; + } + return "unknown"; // unreachable for a valid enum; never claim a source +} + SourceMode sourceModeForScope(CaptureScope scope, bool itemExtentIsWindow) { switch (scope) { case CaptureScope::Item: diff --git a/src/core/capture/render_settings.h b/src/core/capture/render_settings.h index e3c120d..847b7fd 100644 --- a/src/core/capture/render_settings.h +++ b/src/core/capture/render_settings.h @@ -96,6 +96,13 @@ struct RenderSettingsChoice { // SelectedItems -> &32|single-file; RazorArea -> &4096|single-file. RenderSettingsChoice renderSettingsFor(SourceMode mode, double wetDry); +// The render source a mode drives, in words. Exists for the offline backend's +// bounds refusal: the two ways a render can miss its window — a source that +// derives its own bounds (selected items, razor edits) versus a time-bounded +// render that came up short — are indistinguishable from a frame count alone, +// and naming the source is what tells them apart in a bug report. +const char* renderSourceLabel(SourceMode mode); + // --- Capture scope: the FX-scope invariant ------------------------------------ // // See src/core/capture/CLAUDE.md for the scope contract. There is NO master diff --git a/src/core/capture/render_window.cpp b/src/core/capture/render_window.cpp index df966cd..d24e436 100644 --- a/src/core/capture/render_window.cpp +++ b/src/core/capture/render_window.cpp @@ -24,6 +24,13 @@ long long frameCountFor(double startSeconds, double endSeconds, int sampleRate) return frames > 0 ? frames : 0; } +bool renderHonoredBounds(long long expectedFrames, long long actualFrames) { + const long long delta = actualFrames > expectedFrames + ? actualFrames - expectedFrames + : expectedFrames - actualFrames; + return delta <= 1; +} + bool itemExtentPrintsWindow(double reqStart, double reqEnd, double itemStart, double itemEnd, int sampleRate) { diff --git a/src/core/capture/render_window.h b/src/core/capture/render_window.h index 60508fd..4702b42 100644 --- a/src/core/capture/render_window.h +++ b/src/core/capture/render_window.h @@ -18,6 +18,18 @@ namespace reasampler::capture { // whether the equality is exact or off by a frame. long long frameCountFor(double startSeconds, double endSeconds, int sampleRate); +// True when a landed render's frame count is consistent with `frameCountFor`'s +// answer for the same window. Tolerates a one-frame difference, and exactly one: +// frameCountFor rounds EACH edge, so it sits within a frame of the window's +// real-valued length (end-start)*rate — and a renderer that floors, ceils or +// rounds that same length sits within a frame of it too, so two integers derived +// that way can never be more than one apart. A window whose edges do not land on +// frame boundaries therefore cannot produce a larger difference; anything larger +// is a render that printed something other than the window asked for, whatever +// the alignment. Widening this past one frame retires the exact-bounds invariant +// rather than relaxing it — do not. +bool renderHonoredBounds(long long expectedFrames, long long actualFrames); + // True when a render bounded by the selected items' own extent // [itemStart, itemEnd) already prints exactly the requested // [reqStart, reqEnd) window — the one case where REAPER's selected-items render diff --git a/src/shell/capture/capture.cpp b/src/shell/capture/capture.cpp index 3999df2..6929ce9 100644 --- a/src/shell/capture/capture.cpp +++ b/src/shell/capture/capture.cpp @@ -35,7 +35,7 @@ #include "core/capture/wav_codec.h" // hashWavContent / collapseToMono — the one WAV/RIFF owner #include "core/util/file_bytes.h" #include "core/capture/render_settings.h" -#include "core/capture/render_window.h" // frameCountFor — the exact-bounds number +#include "core/capture/render_window.h" // frameCountFor / renderHonoredBounds — the exact-bounds gate #define REAPERAPI_MINIMAL #define REAPERAPI_WANT_EnumProjects @@ -499,9 +499,10 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { } // Exact bounds, made structural: with no tail requested the file must contain - // (within a tolerance, see below) the requested window's frames, so a source - // mode that silently widened the render fails loudly here instead of landing as - // a successful capture. Auto and Manual add frames by design and are skipped. + // the requested window's frames (renderHonoredBounds owns the tolerance and the + // reasoning behind it), so a render that printed something other than the window + // fails loudly here instead of landing as a successful capture. Auto and Manual + // add frames by design and are skipped. // (On TailMode::None the landed file is read three times on this path — this gate, // the mono collapse, and stampCaptureSample — plus one rewrite when the collapse // fires; Auto/Manual skip this gate entirely, so they read it twice. A @@ -518,24 +519,20 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { static_cast(layout.sampleRate)) : 0; const long long actualFrames = static_cast(layout.frameCount()); - // frameCountFor is a difference of frame indices, not a rounded duration - // (see render_window.h) — REAPER's own edge-rounding can legitimately land - // one frame off that, so the gate tolerates +/-1 rather than exact equality. - // The defect this refuses is a whole-item widening (seconds of extra audio, - // thousands of frames), which a 1-frame tolerance still catches with - // certainty. Tightening to exact equality needs a DAW pass confirming REAPER - // resolves the window's two edges to frame indices the same way this does. - const long long frameDelta = actualFrames > expectedFrames - ? actualFrames - expectedFrames - : expectedFrames - actualFrames; - if (expectedFrames > 0 && frameDelta > 1) { + if (expectedFrames > 0 && + !renderHonoredBounds(expectedFrames, actualFrames)) { result.status = CaptureStatus::BoundsMismatch; + // Naming the render source is load-bearing, not decoration: a source that + // derives its own bounds and a time-bounded render that came up short + // produce the same frame count, and only one of them is a routing defect. result.message = "Render produced " + std::to_string(actualFrames) + " frames but the requested range is " + std::to_string(expectedFrames) + " at " + std::to_string(layout.sampleRate) + " Hz -- the render did not honor the requested bounds. " - "Requested [" + std::to_string(request.startSeconds) + + "Render source: " + + renderSourceLabel(request.sourceMode) + + ". Requested [" + std::to_string(request.startSeconds) + "s, " + std::to_string(request.endSeconds) + "s) -> frame indices [" + std::to_string(std::llround(request.startSeconds * diff --git a/tests/test_render_settings.cpp b/tests/test_render_settings.cpp index 3db957f..9ec4877 100644 --- a/tests/test_render_settings.cpp +++ b/tests/test_render_settings.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -63,6 +64,28 @@ static void testRealtimeIsUnsupportedOffline() { CHECK(!renderSettingsFor(SourceMode::Realtime, 1.0).supported); } +static void testEveryRenderSourceIsNameable() { + // The bounds refusal quotes this label to say WHICH render source missed the + // window, so every mode must name itself and no two may read alike. + const SourceMode all[] = { + SourceMode::MasterMix, SourceMode::TimeSelection, SourceMode::SelectedTracks, + SourceMode::SelectedItems, SourceMode::RazorArea, SourceMode::Realtime, + }; + for (std::size_t i = 0; i < sizeof(all) / sizeof(all[0]); ++i) { + const char* label = renderSourceLabel(all[i]); + CHECK(label != nullptr && label[0] != '\0'); + CHECK(std::strcmp(label, "unknown") != 0); + for (std::size_t j = i + 1; j < sizeof(all) / sizeof(all[0]); ++j) + CHECK(std::strcmp(label, renderSourceLabel(all[j])) != 0); + } + // The two the refusal must tell apart: a source that derives its own bounds vs + // the time-bounded render. + CHECK(std::strcmp(renderSourceLabel(SourceMode::SelectedItems), + "selected media items") == 0); + CHECK(std::strcmp(renderSourceLabel(SourceMode::SelectedTracks), + "selected tracks via master") == 0); +} + // --- tail: TailMode -> RENDER_* mapping (docs/product/capture-tail.md) -------- static void testTailNoneIsExactBounds() { @@ -395,6 +418,7 @@ int main() { testSelectedItemsSingleFile(); testRazorSingleFile(); testRealtimeIsUnsupportedOffline(); + testEveryRenderSourceIsNameable(); testTailNoneIsExactBounds(); testTailAutoIsSurgicalTrim(); testAutoTrimRatioDerivesFromDb(); diff --git a/tests/test_render_window.cpp b/tests/test_render_window.cpp index 6e7b8c6..1060df1 100644 --- a/tests/test_render_window.cpp +++ b/tests/test_render_window.cpp @@ -1,10 +1,12 @@ // Standalone tests for reasampler::render_window — no REAPER, no framework. // Covers the bounds-equality number (a window's exact frame count at the project -// rate) and the predicate that decides whether REAPER's selected-items render -// source can express a requested window at all. +// rate), the verdict the offline backend refuses a capture on, and the predicate +// that decides whether REAPER's selected-items render source can express a +// requested window at all. #include "../src/core/capture/render_window.h" +#include #include using namespace reasampler::capture; @@ -42,6 +44,74 @@ static void testFrameCountRefusesEmptyInvertedAndUnknownRate() { CHECK(frameCountFor(1.0, 2.0, -1) == 0); // rate nonsensical } +static void testWindowStartingAtExactlyZero() { + CHECK(frameCountFor(0.0, 1.0, 48000) == 48000); + // The window from the reported blocker: it starts at 0 and its end lands a + // quarter of a frame off the grid at 48 kHz. + CHECK(frameCountFor(0.0, 4.067797, 48000) == 195254); +} + +// --- renderHonoredBounds: the gate's verdict --------------------------------- + +static void testNonFrameAlignedWindowAcceptsEveryEdgeConvention() { + // 4.067797 s at 48 kHz is 195254.26 frames — not a frame boundary. A correct + // render lands on 195254, and the neighbours a different edge convention would + // produce are inside the gate. + const long long expected = frameCountFor(0.0, 4.067797, 48000); + CHECK(expected == 195254); + CHECK(renderHonoredBounds(expected, 195254)); + CHECK(renderHonoredBounds(expected, 195255)); + CHECK(renderHonoredBounds(expected, 195253)); + // The shortfall actually reported from the DAW is 38 frames — far outside any + // alignment slack, so it is a render that missed the window, and is refused. + CHECK(!renderHonoredBounds(expected, 195216)); +} + +static void testNonAlignmentCanNeverExceedOneFrame() { + // The provable content of the one-frame tolerance: frameCountFor rounds each + // edge, so it lands within a frame of the window's real length — and so does a + // renderer that floors, ceils or rounds that same length. Sweep both edges over + // every eighth of a frame; no pairing may fall outside the gate. The renderer's + // count is derived from the length here, independently of frameCountFor. + const int rate = 48000; + for (int s = 0; s < 8; ++s) { + for (int e = 0; e < 8; ++e) { + const double start = 3.0 + s / (8.0 * rate); + const double end = 7.5 + e / (8.0 * rate); + const long long expected = frameCountFor(start, end, rate); + const double length = (end - start) * rate; + CHECK(renderHonoredBounds(expected, + static_cast(std::floor(length)))); + CHECK(renderHonoredBounds(expected, + static_cast(std::ceil(length)))); + CHECK(renderHonoredBounds(expected, std::llround(length))); + } + } +} + +static void testWholeItemWideningIsStillRefused() { + // The defect the gate was built for: a 1 s window inside a 30 s item printing + // the whole item. + const long long expected = frameCountFor(5.0, 6.0, 48000); + CHECK(expected == 48000); + CHECK(!renderHonoredBounds(expected, 30 * 48000)); +} + +static void testLargeShortfallIsStillRefused() { + const long long expected = frameCountFor(0.0, 4.067797, 48000); + CHECK(!renderHonoredBounds(expected, 190000)); + // Two frames is the smallest miss outside the tolerance, in both directions — + // the tolerance is one frame and stays one frame. + CHECK(!renderHonoredBounds(expected, expected - 2)); + CHECK(!renderHonoredBounds(expected, expected + 2)); +} + +static void testEmptyRenderIsRefusedAgainstARealWindow() { + // A render that produced nothing is a bounds miss like any other; the backend's + // own "did the file parse" guard is what keeps an unreadable render out of here. + CHECK(!renderHonoredBounds(48000, 0)); +} + // --- itemExtentPrintsWindow: can the selected-items source express this? ----- static void testRangeInsideItemCannotBeExpressed() { @@ -106,6 +176,12 @@ int main() { testFrameCountIsExactNotRounded(); testFrameCountIsADifferenceOfIndicesNotADuration(); testFrameCountRefusesEmptyInvertedAndUnknownRate(); + testWindowStartingAtExactlyZero(); + testNonFrameAlignedWindowAcceptsEveryEdgeConvention(); + testNonAlignmentCanNeverExceedOneFrame(); + testWholeItemWideningIsStillRefused(); + testLargeShortfallIsStillRefused(); + testEmptyRenderIsRefusedAgainstARealWindow(); testRangeInsideItemCannotBeExpressed(); testRangeWiderThanItemCannotBeExpressedEither(); testEachEdgeAloneDisqualifies(); From 2005f90c66eb5de7c7ce6b96b2abe5179efd4c23 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 2 Aug 2026 07:23:30 -0400 Subject: [PATCH 2/3] capture: state the bounds tolerance as empirical, refuse unmeasurable renders, keep refused ones for diagnosis The one-frame bound is not provable for a per-edge renderer; the test now shows where it breaks. Refused renders move out of the bank instead of being deleted, so the DAW experiment has something to read. --- docs/TODO.md | 58 ++++++++++--- docs/VERIFICATION.md | 6 +- src/app/CMakeLists.txt | 1 + src/core/capture/CLAUDE.md | 2 +- src/core/capture/render_settings.cpp | 6 +- src/core/capture/render_settings.h | 7 +- src/core/capture/render_window.h | 24 +++--- src/shell/capture/CLAUDE.md | 1 + src/shell/capture/capture.cpp | 65 +++----------- src/shell/capture/capture.h | 3 +- src/shell/capture/render_bounds_gate.cpp | 95 ++++++++++++++++++++ src/shell/capture/render_bounds_gate.h | 30 +++++++ tests/test_render_settings.cpp | 62 +++++++++---- tests/test_render_window.cpp | 105 +++++++++++++++++------ 14 files changed, 339 insertions(+), 126 deletions(-) create mode 100644 src/shell/capture/render_bounds_gate.cpp create mode 100644 src/shell/capture/render_bounds_gate.h diff --git a/docs/TODO.md b/docs/TODO.md index 5044c2e..2f099fa 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -689,19 +689,53 @@ stereo file today. `Sample::channelCount` matching, the same way an offline dead-center capture does; a true-stereo bake is byte-identical to today's output. -## A 0-byte render can pass every gate and land as `Ok` (pre-existing, not a Ψ-W3 regression) +## A 0-byte render can still pass every gate under Auto/Manual tail (narrowed, not closed) **Context (surfaced by Ψ-W3 review).** `OfflineRenderBackend::capture`'s exists-check -(`capture.cpp:489`) passes for a 0-byte file, and the bounds gate (`:507-546`) only fires -when `expectedFrames > 0` — an invalid/empty layout reads `expectedFrames == 0` and skips -the gate rather than refusing. A 0-byte render can therefore reach `stampCaptureSample` -and land as `CaptureStatus::Ok` with an empty `contentHash` and `channelCount == 0`. +passes for a 0-byte file, and the bounds gate used to fire only when `expectedFrames > 0` +— an invalid/empty layout read `expectedFrames == 0` and skipped the gate rather than +refusing, so a 0-byte render reached `stampCaptureSample` and landed as +`CaptureStatus::Ok` with an empty `contentHash` and `channelCount == 0`. -**Not introduced by Ψ-W3.** The exists-check and the `expectedFrames > 0` guard both -predate this track; Ψ-W3 only added the mono-collapse failure report that sits downstream -of this hole and was careful not to assert bytes it never verified (see -`reportCollapseFailure` in `capture.cpp`). +**Narrowed.** `shell/capture/render_bounds_gate` now refuses an unmeasurable render +(invalid layout, or a layout declaring no sample rate) instead of skipping it. That +covers `TailMode::None` only — the gate does not judge Auto/Manual, which add frames by +design, so a 0-byte render under either of those still lands as `Ok`. The refusal reuses +`CaptureStatus::BoundsMismatch` rather than minting its own status; the earlier note here +preferred a distinct status, and that preference is unresolved, not withdrawn. -**Intended fix.** After the exists-check, also reject a 0-byte file explicitly (its own -status, not folded into `BoundsMismatch`, since a 0-byte file was never bounds-checked at -all) before anything downstream reads it. +**Intended fix.** Reject a 0-byte / unparseable render right after the exists-check, on +every tail mode, before anything downstream reads it. + +## An offline capture can be refused for a short render — root cause open + +**Symptom (live, 2026-08-02).** A capture over [0.000000s, 4.067797s) at 48 kHz was +refused: `Render produced 195216 frames but the requested range is 195254`. 38 frames +short — two orders of magnitude outside the gate's one-frame tolerance, so the tolerance +is not what refused it. + +**Hypothesis A — the render bounds itself to the media it can see.** REAPER's +selected-items render source (`&32`) derives its bounds from the selected items' own +extents (`src/core/capture/CLAUDE.md` §Gotchas — itself an inference from an observed +defect, not a header fact). If a time-bounded selected-tracks render (`&128`) does the +same thing against content extent, a range running past the end of its material comes up +exactly as short as the material is. + +**Hypothesis B — a trailing-silence trim fires anyway.** `TailMode::None` sets +`RENDER_NORMALIZE = &(4<<16)` (disable all postprocessing) and `RENDER_TRIMEND = 0`. If +REAPER trims regardless of that bit, a range whose material decays before its end loses +exactly the decayed frames. + +**Not excluded — the gate itself.** `renderHonoredBounds`' one-frame tolerance is +empirical, not proven (`src/core/capture/render_window.h`): a renderer that resolves the +window's two edges by DIFFERENT conventions can sit two frames from `frameCountFor`'s +answer on a correctly-honored render. That cannot account for 38 frames, so it is not +this refusal — but it means a future one- or two-frame refusal may be ours, which is why +the tolerance was not widened on speculation. Widening it is a precision-invariant +decision, not a bug fix. + +**How it gets decided.** `docs/VERIFICATION.md` §Capture range and bounds, the three +numbered blocker steps: step 1 separates A's `&32` path from the shared `&128` path (and +says how to tell when it failed to), step 2 asks whether the render is short at all, step +3 reads the retained refused render to place the missing frames. Nothing here should be +"fixed" before that comes back. diff --git a/docs/VERIFICATION.md b/docs/VERIFICATION.md index 33f9b8e..6aa2e8a 100644 --- a/docs/VERIFICATION.md +++ b/docs/VERIFICATION.md @@ -26,8 +26,10 @@ Checks for Θ, Ξ, and Ψ work that no unit test can close. Build **Release**, i - [ ] Same source: track scope × time selection, and track scope × razor — same exact window (`PLAN.md:2136`) - [ ] One razor-union case (two disjoint areas, one track) — lands the requested window, no `ReaSampler capture failed:` line (`PLAN.md:2137`) - [ ] Capture an item whose extent already equals the window — still lands, unchanged (the byte-identity regression floor) (`docs/COMPLETED.md:829`) -- [ ] **Open blocker.** A live capture refused with a 38-frame shortfall (195216 of 195254 at 48 kHz). Set View → time unit to Samples, then over the same range run **track** scope and **item** scope in turn and report the refusal's `Render source:` line plus both frame counts, and whether the media under the range ends before the range does -- [ ] Same range extended ~1 s past all media, track scope — a full-length file with trailing silence means postprocessing is off; a short file means a trailing-silence trim is firing despite `RENDER_NORMALIZE &(4<<16)` +- [ ] **Open blocker — root cause unknown; the three steps below are the experiment** (both live hypotheses and what is NOT yet excluded: `docs/TODO.md` §An offline capture can be refused for a short render). A live capture over [0.000000s, 4.067797s) was refused 38 frames short (195216 of 195254 at 48 kHz). Set View → time unit to Samples first +- [ ] **Step 1 — does the item-scope render bound itself to the media?** This only tests anything if item scope actually reaches REAPER's selected-items render, and it does that ONLY when the selected items' extent already equals the requested window (`itemExtentPrintsWindow`, `src/core/capture/render_window.h`); otherwise item scope re-sources through the items' own tracks — the same source track scope uses, so the two runs would test one thing twice. So: snap the time selection to the item's exact start and end, run **item** scope, then **track** scope over the identical range. Report both `Render source:` lines and both frame counts. **If both lines read `selected tracks via master`, the item path was NOT exercised** — the extents did not match; re-snap and repeat before concluding anything +- [ ] **Step 2 — full-length or short?** Extend the same range ~1 s past the end of all media, **track** scope. Landing with the full range (no refusal, the card reads the extended length) rules out BOTH a trailing-silence trim and a content-extent bound at once. A short render does NOT tell them apart: a trim firing despite `RENDER_NORMALIZE &(4<<16)` and a render bounding itself to content extent produce the same count. Report which happened, then run step 3 +- [ ] **Step 3 — where are the missing frames?** A refused render is kept deliberately, not deleted: it is moved to `/reasampler_refused/`. **Follow the path in the refusal line, not this sentence** — if the move itself failed the file stays in the bank folder, unindexed, and the line says which happened. Insert it against the source over the same range and report whether the head aligns. Frames missing from the TAIL with an aligned head fits either a tail trim or a content-extent bound; a head offset fits neither and is a start-position defect. Also report whether the media under the range ends before the range does. Delete `reasampler_refused/` when done — nothing in the bank references it ## Names and channels diff --git a/src/app/CMakeLists.txt b/src/app/CMakeLists.txt index 5bbc5bb..f39c62b 100644 --- a/src/app/CMakeLists.txt +++ b/src/app/CMakeLists.txt @@ -14,6 +14,7 @@ add_library(reaper_reasampler MODULE ${REASAMPLER_SRC_DIR}/shell/capture/scope_resolve.cpp ${REASAMPLER_SRC_DIR}/shell/capture/render_selection.cpp ${REASAMPLER_SRC_DIR}/shell/capture/render_isolation.cpp + ${REASAMPLER_SRC_DIR}/shell/capture/render_bounds_gate.cpp ${REASAMPLER_SRC_DIR}/shell/capture/realtime_lifecycle.cpp ${REASAMPLER_SRC_DIR}/shell/capture/capture_realtime_shell.cpp ${REASAMPLER_SRC_DIR}/shell/capture/capture_realtime_finalize.cpp diff --git a/src/core/capture/CLAUDE.md b/src/core/capture/CLAUDE.md index 15688d8..104a57c 100644 --- a/src/core/capture/CLAUDE.md +++ b/src/core/capture/CLAUDE.md @@ -52,7 +52,7 @@ Detail specific to these pure modules: - `capture_name` — the REAPER-free composition of one capture's label + file-stem base from its source-track name(s), a local-calendar discriminator (`MM-DD HHMM`, from the shell's clock read), and an optional batch ordinal. The label and the stem deliberately diverge: the stem still passes through `capture_paths::sanitizeStem` (so a name that sanitizes to nothing files as `capture`), while the label keeps the source name verbatim. Stem uniqueness stays entirely `makeUniqueTag`'s — this module never disambiguates. - `insert_plan` — the REAPER-free logic behind the `insert` shell (M6): computes the `InsertMedia` `mode` bitmask from an `InsertOptions` struct (placement target, tempo-conform ratio, preserve-pitch flag), guaranteeing the &4 stretch-to-time-selection bit is never set and that no tempo bits are set when `conform == None`. - `render_settings` — the REAPER-free logic behind the capture action family: `SourceMode` → `RENDER_SETTINGS` bit mapping, `P_RAZOREDITS` string parsing + range-union bounds, razor-else-time range inference, the FX-scope bypass plan (`fxBypassPlanFor`), the tail-mode → `RENDER_TAILFLAG`/`RENDER_NORMALIZE`/`RENDER_TRIMEND` mapping (`tailRenderSettingsFor`) and its realtime-window analog (`realtimeRecordWindowEnd`), the capture-action taxonomy table (`captureActionTable`) `main.cpp` iterates to register the CAPTURE_ITEM/CAPTURE_TRACK family, and `renderSourceLabel` (the source named in the offline backend's bounds refusal). -- `render_window` — the REAPER-free frame arithmetic behind exact capture bounds: `frameCountFor` (the frame count a project-time window occupies at the project rate — the number the offline backend checks the rendered file against before landing it, so a render that printed something other than the window is refused rather than banked), `renderHonoredBounds` (the gate's verdict and the sole home of its one-frame tolerance and the derivation behind it), and `itemExtentPrintsWindow`, the predicate `render_settings::sourceModeForScope` consults to decide whether REAPER's selected-items render source can express a requested window at all. +- `render_window` — the REAPER-free frame arithmetic behind exact capture bounds: `frameCountFor` (the frame count a project-time window occupies at the project rate — the number the offline backend checks the rendered file against before landing it, so a render that printed something other than the window is refused rather than banked), `renderHonoredBounds` (the gate's verdict and the sole home of its one-frame tolerance, which is empirical rather than proven — the header states which renderer models it covers and which it does not), and `itemExtentPrintsWindow`, the predicate `render_settings::sourceModeForScope` consults to decide whether REAPER's selected-items render source can express a requested window at all. - `track_topology` — the REAPER-free folder arithmetic over a project's flat `I_FOLDERDEPTH` delta list: `directChildIndices` names a folder parent's DIRECT children, the set `shell/capture/render_isolation` silences so a ranged item capture does not print its track's children. Grandchildren are excluded by construction — they reach the parent only through the child that owns them. - `tail_control` — the REAPER-free logic behind the docked `bank_panel`'s tail-mode toggle: the cycle order (None → Auto → Manual → None), the Manual-length clamp/scroll-wheel fine-adjust (`clampManualMs`/`adjustManualMs`, 250 ms/notch, 2000 ms default), the toggle's label text (e.g. "Tail: Manual 2.0s"), and the `TailSetting` JSON round-trip persist stores per-project. diff --git a/src/core/capture/render_settings.cpp b/src/core/capture/render_settings.cpp index bfdb258..5cec2f2 100644 --- a/src/core/capture/render_settings.cpp +++ b/src/core/capture/render_settings.cpp @@ -102,8 +102,10 @@ RenderSettingsChoice renderSettingsFor(SourceMode mode, double /*wetDry*/) { const char* renderSourceLabel(SourceMode mode) { switch (mode) { - case SourceMode::MasterMix: return "master mix"; - case SourceMode::TimeSelection: return "master mix (time selection)"; + // MasterMix and TimeSelection share this label because they ARE the same + // render — see the header. + case SourceMode::MasterMix: + case SourceMode::TimeSelection: return "master mix"; case SourceMode::SelectedTracks: return "selected tracks via master"; case SourceMode::SelectedItems: return "selected media items"; case SourceMode::RazorArea: return "razor edits"; diff --git a/src/core/capture/render_settings.h b/src/core/capture/render_settings.h index 847b7fd..6115a63 100644 --- a/src/core/capture/render_settings.h +++ b/src/core/capture/render_settings.h @@ -100,7 +100,12 @@ RenderSettingsChoice renderSettingsFor(SourceMode mode, double wetDry); // bounds refusal: the two ways a render can miss its window — a source that // derives its own bounds (selected items, razor edits) versus a time-bounded // render that came up short — are indistinguishable from a frame count alone, -// and naming the source is what tells them apart in a bug report. +// and naming the source is what tells them apart in a bug report. Quoted verbatim +// in docs/VERIFICATION.md, which asks for this exact line back. +// +// MasterMix and TimeSelection deliberately answer the SAME words: they map to the +// same RENDER_SETTINGS value and every capture renders custom-time-bounded, so +// naming them apart would assert a render distinction that does not exist. const char* renderSourceLabel(SourceMode mode); // --- Capture scope: the FX-scope invariant ------------------------------------ diff --git a/src/core/capture/render_window.h b/src/core/capture/render_window.h index 4702b42..a26bfdb 100644 --- a/src/core/capture/render_window.h +++ b/src/core/capture/render_window.h @@ -13,21 +13,21 @@ namespace reasampler::capture { // Returns 0 for a non-positive rate or an empty/inverted window. // // The offline backend compares this against the rendered file's own frame count, so -// exact-bounds failures surface as a refused capture rather than a wrong file. That -// REAPER resolves the two edges the same way is UNVERIFIED — a DAW pass decides -// whether the equality is exact or off by a frame. +// exact-bounds failures surface as a refused capture rather than a wrong file. long long frameCountFor(double startSeconds, double endSeconds, int sampleRate); // True when a landed render's frame count is consistent with `frameCountFor`'s -// answer for the same window. Tolerates a one-frame difference, and exactly one: -// frameCountFor rounds EACH edge, so it sits within a frame of the window's -// real-valued length (end-start)*rate — and a renderer that floors, ceils or -// rounds that same length sits within a frame of it too, so two integers derived -// that way can never be more than one apart. A window whose edges do not land on -// frame boundaries therefore cannot produce a larger difference; anything larger -// is a render that printed something other than the window asked for, whatever -// the alignment. Widening this past one frame retires the exact-bounds invariant -// rather than relaxing it — do not. +// answer for the same window. Tolerates a one-frame difference, and exactly one. +// +// That bound is EMPIRICAL. It is provable only for renderer models that derive the +// count from the window's LENGTH (floor/ceil/round of (end-start)*rate) or resolve +// both edges by the SAME convention; a renderer that resolves the start edge and the +// end edge by DIFFERENT conventions can legitimately sit TWO frames from this answer +// (tests/test_render_window.cpp pins both facts). Which model REAPER uses is +// unverified, so a refusal one or two frames wide may be this gate's fault rather than +// the render's — the open DAW question in docs/VERIFICATION.md §Capture range and +// bounds. Widening past one frame retires the exact-bounds invariant rather than +// relaxing it, and is not a fix to reach for before that question is answered. bool renderHonoredBounds(long long expectedFrames, long long actualFrames); // True when a render bounded by the selected items' own extent diff --git a/src/shell/capture/CLAUDE.md b/src/shell/capture/CLAUDE.md index 94ee474..5fd5f22 100644 --- a/src/shell/capture/CLAUDE.md +++ b/src/shell/capture/CLAUDE.md @@ -52,6 +52,7 @@ detail not covered there: ## Modules - `capture` — two CONCRETE backends with deliberately different lifecycles (no shared interface — the former `ICaptureBackend` was deleted in Q-W3, T4-26: one deriver, zero polymorphic call sites): `OfflineRenderBackend` (deterministic default, synchronous) and `RealtimeRecordBackend` (async begin/tick/abort). Input: `CaptureRequest`. Output: finished file + populated `Sample` handed to `bank_model`. It also owns the two file-side steps both backends share, in this order: `collapseCapturedFileToMono` (the lossless mono collapse, applied to the landed file) and `stampCaptureSample`, which measures the channel count off that same file so the entry and the audio cannot disagree. And `captureNameFor` — the impure local-clock read the entry points call to build a request's label + stem, kept out of the pure `core/capture/capture_name` composition it feeds. +- `render_bounds_gate` (`shell/capture`) — the exact-bounds verdict on a landed offline render and the refusal's file handling, split off `capture.cpp` on the render-vs-judge seam. Refuses a frame count that is not the window's AND a file whose frames cannot be measured at all (an invalid layout used to skip the gate and land with an unknown channel count). Judges `TailMode::None` only — Auto/Manual add frames by design, and an unmeasurable render still lands under those two (`docs/TODO.md`). A refused render is MOVED to `/reasampler_refused/` rather than deleted, so the frames it did print survive for diagnosis while the short-render root cause is open; the bank never sees it either way. - `scope_resolve` (`shell/capture`) — scope/source resolution shared by every capture entry point (Q-W3 hoist out of `main.cpp`): razor-else-time range inference, selected-track/selected-item-owning-track collection with canonical GUIDs, and the M10 provenance-assembly inputs (read BEFORE the FX-bypass guard neutralizes the in-scope chain). Also the one place a source track's NAME is read (`trackName`, via `GetTrackName` — chosen over `P_NAME` because it already answers REAPER's `"Track N"` convention for an unnamed track), landed on `ResolvedSource::trackNames` parallel to `sourceTracks` and composed into the capture's label + stem by the pure `core/capture/capture_name`. - `render_selection` (`shell/capture`) — the transient track selection a selected-tracks render (`&128`) requires, as a stack RAII guard: REAPER prints whatever tracks are selected, so `renderOffline` makes the request's own tracks BE the selection for the render's duration and restores the user's set on every exit path. Engaged ONLY for that source mode, which leaves a stated residual: a `&32` selected-items render still prints whatever ITEMS the user has selected. Live captures are unaffected (that selection is the source), but a recipe replay of a `SelectedItems` capture renders against whatever happens to be selected then — the recipe stores tracks and a range, never item GUIDs, so this guard cannot close it. Filed in `docs/TODO.md`. - `render_isolation` (`shell/capture`) — the transient upstream silencing a ranged ITEM render needs, as a stack RAII guard alongside the two above: the selected-tracks source prints everything flowing INTO the track, so each direct folder child's `B_MAINSEND` and each of the track's receives' `B_MUTE` are cut for the render and restored on every exit path. Direct children only — a grandchild reaches the track through the child that owns it. The child-set walk is pure (`core/capture/track_topology`). diff --git a/src/shell/capture/capture.cpp b/src/shell/capture/capture.cpp index 6929ce9..d5089b4 100644 --- a/src/shell/capture/capture.cpp +++ b/src/shell/capture/capture.cpp @@ -22,7 +22,6 @@ #include "shell/capture/capture.h" #include -#include #include #include #include @@ -35,7 +34,7 @@ #include "core/capture/wav_codec.h" // hashWavContent / collapseToMono — the one WAV/RIFF owner #include "core/util/file_bytes.h" #include "core/capture/render_settings.h" -#include "core/capture/render_window.h" // frameCountFor / renderHonoredBounds — the exact-bounds gate +#include "shell/capture/render_bounds_gate.h" // the exact-bounds verdict on the landed render #define REAPERAPI_MINIMAL #define REAPERAPI_WANT_EnumProjects @@ -498,60 +497,24 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { return result; } - // Exact bounds, made structural: with no tail requested the file must contain - // the requested window's frames (renderHonoredBounds owns the tolerance and the - // reasoning behind it), so a render that printed something other than the window - // fails loudly here instead of landing as a successful capture. Auto and Manual - // add frames by design and are skipped. - // (On TailMode::None the landed file is read three times on this path — this gate, + // Exact bounds, made structural: render_bounds_gate judges the landed file against + // the requested window and owns what becomes of a render that fails. + // (On TailMode::None the landed file is read three times on this path — that gate, // the mono collapse, and stampCaptureSample — plus one rewrite when the collapse - // fires; Auto/Manual skip this gate entirely, so they read it twice. A - // once-per-capture cost on an already-warm file, judged acceptable.) A - // bounded/header-only read is not a clean substitute: parseWavLayout only marks the - // data chunk valid when the buffer holds the chunk's FULL declared body - // (bodyInBounds), so a truncated read would read as invalid here on every real - // capture, not just malformed ones. - if (request.tailMode == TailMode::None) { - const WavLayout layout = - parseWavLayout(util::readFileBytes(expectedPath)); - const long long expectedFrames = layout.valid - ? frameCountFor(request.startSeconds, request.endSeconds, - static_cast(layout.sampleRate)) - : 0; - const long long actualFrames = static_cast(layout.frameCount()); - if (expectedFrames > 0 && - !renderHonoredBounds(expectedFrames, actualFrames)) { - result.status = CaptureStatus::BoundsMismatch; - // Naming the render source is load-bearing, not decoration: a source that - // derives its own bounds and a time-bounded render that came up short - // produce the same frame count, and only one of them is a routing defect. - result.message = "Render produced " + std::to_string(actualFrames) + - " frames but the requested range is " + - std::to_string(expectedFrames) + " at " + - std::to_string(layout.sampleRate) + - " Hz -- the render did not honor the requested bounds. " - "Render source: " + - renderSourceLabel(request.sourceMode) + - ". Requested [" + std::to_string(request.startSeconds) + - "s, " + std::to_string(request.endSeconds) + - "s) -> frame indices [" + - std::to_string(std::llround(request.startSeconds * - layout.sampleRate)) + - ", " + - std::to_string(std::llround(request.endSeconds * - layout.sampleRate)) + - "). Nothing was added to the bank; the render at " + - expectedPath + " was never indexed and has been cleaned up."; - std::error_code ec; - std::filesystem::remove(expectedPath, ec); - return result; - } + // fires; Auto/Manual are not gated, so they read it twice. A once-per-capture cost + // on an already-warm file, judged acceptable.) + const BoundsVerdict bounds = + checkRenderedBounds(expectedPath, projectDir, request); + if (bounds.refused) { + result.status = CaptureStatus::BoundsMismatch; + result.message = bounds.message; + return result; } // Lossless mono collapse, deliberately AFTER the bounds gate: the gate measures // REAPER's own render against the requested window, so nothing of ours may sit - // between the render and that measurement, and a refusal must delete the - // renderer's file rather than one this step had already rewritten. The collapse + // between the render and that measurement, and the file a refusal retains must be + // the renderer's own rather than one this step had already rewritten. The collapse // preserves the frame count, so the two are order-independent in outcome — only // in what each is measuring. const MonoCollapseOutcome collapseOutcome = diff --git a/src/shell/capture/capture.h b/src/shell/capture/capture.h index 37209f7..c52b460 100644 --- a/src/shell/capture/capture.h +++ b/src/shell/capture/capture.h @@ -87,7 +87,8 @@ enum class CaptureStatus { RenderFailed, // the render action ran but produced no output file TransportBusy, // realtime backend: transport already playing/recording — refused MultiTrackSelection, // a selected-tracks render over >1 track — would render N files - BoundsMismatch, // the rendered file's frame count is not the requested window's + BoundsMismatch, // the rendered file's frames are not the requested window's — or + // could not be measured to say (render_bounds_gate) }; struct CaptureResult { diff --git a/src/shell/capture/render_bounds_gate.cpp b/src/shell/capture/render_bounds_gate.cpp new file mode 100644 index 0000000..4b38d8a --- /dev/null +++ b/src/shell/capture/render_bounds_gate.cpp @@ -0,0 +1,95 @@ +// render_bounds_gate.cpp — see the header. + +#include "shell/capture/render_bounds_gate.h" + +#include +#include +#include +#include + +#include "core/capture/render_settings.h" +#include "core/capture/render_window.h" +#include "core/capture/wav_codec.h" +#include "core/util/file_bytes.h" + +namespace reasampler::capture { + +namespace { + +// Deliberately a sibling of the bank folder, never inside it: prune enumerates the bank +// directory, and a refused render must be reachable by the person debugging it and by +// nothing else. +constexpr const char* kRefusedSubfolder = "reasampler_refused"; + +// Moves a refused render out of the bank and returns the sentence naming where it went. +// A failed move leaves the file where the renderer wrote it and says so — never a path +// that does not exist. +std::string retainRefusedRender(const std::string& renderedPath, + const std::string& projectDir) { + namespace fs = std::filesystem; + std::error_code ec; + const std::string dir = projectDir + "/" + kRefusedSubfolder; + fs::create_directories(dir, ec); + if (!ec) { + const std::string dest = + dir + "/" + fs::path(renderedPath).filename().string(); + fs::rename(renderedPath, dest, ec); + if (!ec) + return " Nothing was added to the bank. The refused render was KEPT for " + "diagnosis at " + dest + " -- outside the bank, indexed by nothing; " + "delete it when done."; + } + return " Nothing was added to the bank. The refused render was kept for diagnosis " + "but could not be moved out of the bank folder; it is still at " + + renderedPath + ", indexed by nothing. Delete it when done."; +} + +} // namespace + +BoundsVerdict checkRenderedBounds(const std::string& renderedPath, + const std::string& projectDir, + const CaptureRequest& request) { + BoundsVerdict v; + if (request.tailMode != TailMode::None) return v; + + // Whole-file read, not a bounded/header-only one: parseWavLayout marks the data + // chunk valid only when the buffer holds its FULL declared body, so a truncated + // read would read as invalid on every real capture — and invalid refuses here. + const WavLayout layout = parseWavLayout(util::readFileBytes(renderedPath)); + + // Why the refusal names the render source at all: render_settings.h's renderSourceLabel. + const std::string source = + std::string(" Render source: ") + renderSourceLabel(request.sourceMode) + "."; + + // An unmeasurable render used to SKIP this gate and land in the bank with an + // unknown channel count. It is a refusal now: exact bounds cannot be asserted over + // a file whose frames were never counted. + if (!layout.valid || layout.sampleRate == 0) { + v.refused = true; + v.message = "Render at " + renderedPath + " could not be measured -- its WAV " + "header did not parse, or declared no sample rate -- so the frames " + "it holds were never checked against the requested range." + source + + retainRefusedRender(renderedPath, projectDir); + return v; + } + + const int rate = static_cast(layout.sampleRate); + const long long expectedFrames = + frameCountFor(request.startSeconds, request.endSeconds, rate); + const long long actualFrames = static_cast(layout.frameCount()); + if (renderHonoredBounds(expectedFrames, actualFrames)) return v; + + v.refused = true; + v.message = "Render produced " + std::to_string(actualFrames) + + " frames but the requested range is " + std::to_string(expectedFrames) + + " at " + std::to_string(rate) + + " Hz -- the render did not honor the requested bounds." + source + + " Requested [" + std::to_string(request.startSeconds) + "s, " + + std::to_string(request.endSeconds) + "s) -> frame indices [" + + std::to_string(std::llround(request.startSeconds * rate)) + ", " + + std::to_string(std::llround(request.endSeconds * rate)) + ")." + + retainRefusedRender(renderedPath, projectDir); + return v; +} + +} // namespace reasampler::capture diff --git a/src/shell/capture/render_bounds_gate.h b/src/shell/capture/render_bounds_gate.h new file mode 100644 index 0000000..18d0e59 --- /dev/null +++ b/src/shell/capture/render_bounds_gate.h @@ -0,0 +1,30 @@ +#pragma once +// render_bounds_gate — the exact-bounds verdict on a landed offline render, and what +// happens to a render that fails it. Split out of capture.cpp on the responsibility +// seam: that TU drives the render, this one judges the file it produced. +// No REAPER types — pure core plus the filesystem. + +#include + +#include "shell/capture/capture.h" + +namespace reasampler::capture { + +// A refused render is MOVED out of the bank, not deleted: while the root cause of a +// short render is open (docs/TODO.md), the frames it did print are the evidence — and +// nothing may index a file the bank never accepted. +struct BoundsVerdict { + bool refused = false; + std::string message; // console text; meaningful only when refused +}; + +// Judges `renderedPath` against `request`'s window. Refuses on two counts: the file's +// frame count is not the window's (render_window::renderHonoredBounds owns the +// tolerance and its limits), or the file cannot be measured at all — an unmeasured +// render is not a verified one. TailMode::Auto/Manual add frames by design and are +// never judged here. `projectDir` is where a refused render is parked. +BoundsVerdict checkRenderedBounds(const std::string& renderedPath, + const std::string& projectDir, + const CaptureRequest& request); + +} // namespace reasampler::capture diff --git a/tests/test_render_settings.cpp b/tests/test_render_settings.cpp index 9ec4877..e524eb3 100644 --- a/tests/test_render_settings.cpp +++ b/tests/test_render_settings.cpp @@ -64,26 +64,49 @@ static void testRealtimeIsUnsupportedOffline() { CHECK(!renderSettingsFor(SourceMode::Realtime, 1.0).supported); } -static void testEveryRenderSourceIsNameable() { - // The bounds refusal quotes this label to say WHICH render source missed the - // window, so every mode must name itself and no two may read alike. - const SourceMode all[] = { - SourceMode::MasterMix, SourceMode::TimeSelection, SourceMode::SelectedTracks, - SourceMode::SelectedItems, SourceMode::RazorArea, SourceMode::Realtime, - }; - for (std::size_t i = 0; i < sizeof(all) / sizeof(all[0]); ++i) { - const char* label = renderSourceLabel(all[i]); - CHECK(label != nullptr && label[0] != '\0'); - CHECK(std::strcmp(label, "unknown") != 0); - for (std::size_t j = i + 1; j < sizeof(all) / sizeof(all[0]); ++j) - CHECK(std::strcmp(label, renderSourceLabel(all[j])) != 0); - } - // The two the refusal must tell apart: a source that derives its own bounds vs - // the time-bounded render. - CHECK(std::strcmp(renderSourceLabel(SourceMode::SelectedItems), - "selected media items") == 0); +static void testEveryRenderSourceLabelIsPinnedVerbatim() { + // docs/VERIFICATION.md asks Daniel to report the refusal's `Render source:` line + // back verbatim, so every label is pinned to its literal — a typo in any of them + // breaks the report that quotes it, and only a literal catches that. + CHECK(std::strcmp(renderSourceLabel(SourceMode::MasterMix), "master mix") == 0); + CHECK(std::strcmp(renderSourceLabel(SourceMode::TimeSelection), "master mix") == 0); CHECK(std::strcmp(renderSourceLabel(SourceMode::SelectedTracks), "selected tracks via master") == 0); + CHECK(std::strcmp(renderSourceLabel(SourceMode::SelectedItems), + "selected media items") == 0); + CHECK(std::strcmp(renderSourceLabel(SourceMode::RazorArea), "razor edits") == 0); + CHECK(std::strcmp(renderSourceLabel(SourceMode::Realtime), "realtime record") == 0); +} + +static void testLabelsSeparateExactlyWhatTheRenderSeparates() { + // The labels partition the offline modes the way RENDER_SETTINGS does, and no + // finer: same bits => same words (MasterMix/TimeSelection both render the master + // mix, custom-time-bounded), different bits => different words. Naming two modes + // apart that render identically would put a distinction in a bug report that does + // not exist in the render. + const SourceMode offline[] = { + SourceMode::MasterMix, SourceMode::TimeSelection, SourceMode::SelectedTracks, + SourceMode::SelectedItems, SourceMode::RazorArea, + }; + constexpr std::size_t n = sizeof(offline) / sizeof(offline[0]); + for (std::size_t i = 0; i < n; ++i) { + const char* label = renderSourceLabel(offline[i]); + CHECK(label != nullptr && label[0] != '\0'); + CHECK(std::strcmp(label, "unknown") != 0); + CHECK(renderSettingsFor(offline[i], 1.0).supported); + for (std::size_t j = i + 1; j < n; ++j) { + const bool sameRender = renderSettingsFor(offline[i], 1.0).settings == + renderSettingsFor(offline[j], 1.0).settings; + const bool sameLabel = + std::strcmp(label, renderSourceLabel(offline[j])) == 0; + CHECK(sameRender == sameLabel); + } + } + // Realtime is not an offline render source at all, so it names its own mechanism + // rather than a RENDER_SETTINGS value — outside the partition above by design. + CHECK(!renderSettingsFor(SourceMode::Realtime, 1.0).supported); + CHECK(std::strcmp(renderSourceLabel(SourceMode::Realtime), + renderSourceLabel(SourceMode::MasterMix)) != 0); } // --- tail: TailMode -> RENDER_* mapping (docs/product/capture-tail.md) -------- @@ -418,7 +441,8 @@ int main() { testSelectedItemsSingleFile(); testRazorSingleFile(); testRealtimeIsUnsupportedOffline(); - testEveryRenderSourceIsNameable(); + testEveryRenderSourceLabelIsPinnedVerbatim(); + testLabelsSeparateExactlyWhatTheRenderSeparates(); testTailNoneIsExactBounds(); testTailAutoIsSurgicalTrim(); testAutoTrimRatioDerivesFromDb(); diff --git a/tests/test_render_window.cpp b/tests/test_render_window.cpp index 1060df1..ee7cbc1 100644 --- a/tests/test_render_window.cpp +++ b/tests/test_render_window.cpp @@ -53,10 +53,9 @@ static void testWindowStartingAtExactlyZero() { // --- renderHonoredBounds: the gate's verdict --------------------------------- -static void testNonFrameAlignedWindowAcceptsEveryEdgeConvention() { +static void testNonFrameAlignedWindowAcceptsItsAdjacentCounts() { // 4.067797 s at 48 kHz is 195254.26 frames — not a frame boundary. A correct - // render lands on 195254, and the neighbours a different edge convention would - // produce are inside the gate. + // render lands on 195254, and both adjacent counts are inside the gate. const long long expected = frameCountFor(0.0, 4.067797, 48000); CHECK(expected == 195254); CHECK(renderHonoredBounds(expected, 195254)); @@ -67,28 +66,82 @@ static void testNonFrameAlignedWindowAcceptsEveryEdgeConvention() { CHECK(!renderHonoredBounds(expected, 195216)); } -static void testNonAlignmentCanNeverExceedOneFrame() { - // The provable content of the one-frame tolerance: frameCountFor rounds each - // edge, so it lands within a frame of the window's real length — and so does a - // renderer that floors, ceils or rounds that same length. Sweep both edges over - // every eighth of a frame; no pairing may fall outside the gate. The renderer's - // count is derived from the length here, independently of frameCountFor. - const int rate = 48000; - for (int s = 0; s < 8; ++s) { - for (int e = 0; e < 8; ++e) { - const double start = 3.0 + s / (8.0 * rate); - const double end = 7.5 + e / (8.0 * rate); - const long long expected = frameCountFor(start, end, rate); - const double length = (end - start) * rate; - CHECK(renderHonoredBounds(expected, - static_cast(std::floor(length)))); - CHECK(renderHonoredBounds(expected, - static_cast(std::ceil(length)))); - CHECK(renderHonoredBounds(expected, std::llround(length))); +static void testLengthDerivedAndSameConventionRenderersStayWithinOneFrame() { + // What the one-frame tolerance is actually good for. Two families of renderer are + // inside it at every offset swept here: one that derives its count from the + // window's LENGTH (floor/ceil/round of (end-start)*rate), and one that resolves + // each EDGE to a frame using the SAME convention on both edges. Every count below + // is computed from the window, never from frameCountFor, so this compares two + // derivations rather than restating one. Round-both-edges is omitted deliberately: + // that IS frameCountFor's own convention, so asserting it would be tautological. + // + // 8192 is a power of two, so an eighth of a frame is exact in double there and the + // .5 rounding ties are really hit; at 48000/44100 (the shipping rates) they are + // only approached, which is why all three are swept. + struct Window { double start; double end; }; + const int rates[] = {48000, 44100, 8192}; + const Window windows[] = {{3.0, 7.5}, {0.0, 4.067797}, {10.25, 10.75}}; + for (int rate : rates) { + for (const Window& w : windows) { + for (int s = 0; s < 8; ++s) { + for (int e = 0; e < 8; ++e) { + const double start = w.start + s / (8.0 * rate); + const double end = w.end + e / (8.0 * rate); + const long long expected = frameCountFor(start, end, rate); + + const double length = (end - start) * rate; + CHECK(renderHonoredBounds(expected, + static_cast(std::floor(length)))); + CHECK(renderHonoredBounds(expected, + static_cast(std::ceil(length)))); + CHECK(renderHonoredBounds(expected, std::llround(length))); + + const double startFrames = start * rate; + const double endFrames = end * rate; + CHECK(renderHonoredBounds( + expected, static_cast(std::floor(endFrames) - + std::floor(startFrames)))); + CHECK(renderHonoredBounds( + expected, static_cast(std::ceil(endFrames) - + std::ceil(startFrames)))); + } + } } } } +static void testMixedEdgeConventionsCanMissByTwoAndAreRefused() { + // The hole in that bound, stated rather than hidden. A renderer that resolves the + // two edges by DIFFERENT conventions lands two frames from frameCountFor's answer + // whenever the start sits past mid-frame and the end before it (resolved outward), + // or the mirror image (resolved inward). The gate refuses both — correctly if + // REAPER derives its count from the window's length, wrongly if it resolves edges + // this way. No unit test can settle which; see render_window.h. + const int rate = 8192; // power of two: the eighth-frame offsets below are exact + + // Outward: start .625 into a frame, end .375 into one. + const double start = 10.25 + 5.0 / (8.0 * rate); + const double end = 10.75 + 3.0 / (8.0 * rate); + CHECK(start * rate == 83968.625); // the premise, not an outcome — pinned so a + CHECK(end * rate == 88064.375); // representability slip can't fake the result + const long long expected = frameCountFor(start, end, rate); + CHECK(expected == 4095); + const long long outward = static_cast(std::ceil(end * rate) - + std::floor(start * rate)); + CHECK(outward == 4097); + CHECK(!renderHonoredBounds(expected, outward)); + + // Inward, mirrored fractions. + const double start2 = 10.25 + 3.0 / (8.0 * rate); + const double end2 = 10.75 + 5.0 / (8.0 * rate); + const long long expected2 = frameCountFor(start2, end2, rate); + CHECK(expected2 == 4097); + const long long inward = static_cast(std::floor(end2 * rate) - + std::ceil(start2 * rate)); + CHECK(inward == 4095); + CHECK(!renderHonoredBounds(expected2, inward)); +} + static void testWholeItemWideningIsStillRefused() { // The defect the gate was built for: a 1 s window inside a 30 s item printing // the whole item. @@ -107,8 +160,9 @@ static void testLargeShortfallIsStillRefused() { } static void testEmptyRenderIsRefusedAgainstARealWindow() { - // A render that produced nothing is a bounds miss like any other; the backend's - // own "did the file parse" guard is what keeps an unreadable render out of here. + // A render that produced nothing is a bounds miss like any other. A render whose + // frames could not be MEASURED never reaches this predicate — shell/capture/ + // render_bounds_gate refuses it before the comparison. CHECK(!renderHonoredBounds(48000, 0)); } @@ -177,8 +231,9 @@ int main() { testFrameCountIsADifferenceOfIndicesNotADuration(); testFrameCountRefusesEmptyInvertedAndUnknownRate(); testWindowStartingAtExactlyZero(); - testNonFrameAlignedWindowAcceptsEveryEdgeConvention(); - testNonAlignmentCanNeverExceedOneFrame(); + testNonFrameAlignedWindowAcceptsItsAdjacentCounts(); + testLengthDerivedAndSameConventionRenderersStayWithinOneFrame(); + testMixedEdgeConventionsCanMissByTwoAndAreRefused(); testWholeItemWideningIsStillRefused(); testLargeShortfallIsStillRefused(); testEmptyRenderIsRefusedAgainstARealWindow(); From 0511d16d4f66deb88d3d0762bcbac7084bbcfb31 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 2 Aug 2026 08:00:12 -0400 Subject: [PATCH 3/3] capture: close batch-quarantine silence, 0-byte asymmetry, and round-two doc overclaims Batch captures now name the retained-render folder once instead of nothing; Auto/Manual tail modes refuse a 0-byte render like None does; VERIFICATION.md steps 1-3 no longer invite a false conclusion; docs/comments no longer overclaim. --- docs/COMPLETED.md | 3 +- docs/PLAN.md | 4 +- docs/TODO.md | 51 +++++++++++++++++++----- docs/VERIFICATION.md | 6 +-- src/core/capture/render_window.cpp | 5 ++- src/shell/capture/CLAUDE.md | 2 +- src/shell/capture/capture.cpp | 12 ++++++ src/shell/capture/capture_batch.cpp | 31 +++++++++++++- src/shell/capture/render_bounds_gate.cpp | 28 +++++++++++-- src/shell/capture/render_bounds_gate.h | 11 +++++ 10 files changed, 127 insertions(+), 26 deletions(-) diff --git a/docs/COMPLETED.md b/docs/COMPLETED.md index 0adc480..e50d580 100644 --- a/docs/COMPLETED.md +++ b/docs/COMPLETED.md @@ -831,7 +831,8 @@ code, which makes the byte-identity regression floor structural rather than hope and makes the fix cheap to revert if the underlying inference proves wrong. Also added: a transient isolation guard cutting `B_MAINSEND` on direct folder children and muting receives so an item capture stays true to item scope, and a post-render frame-count -gate (±1 tolerance, tail-None only) that refuses and self-cleans a widened render. New +gate (±1 tolerance, tail-None only) that refuses a widened render and retains it outside +the bank for diagnosis rather than deleting it. New modules `core/capture/render_window`, `core/capture/track_topology`, `shell/capture/render_selection`, `shell/capture/render_isolation`. diff --git a/docs/PLAN.md b/docs/PLAN.md index b9c02a7..e183a3e 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -2128,8 +2128,8 @@ full-extent case runs literally unchanged code, keeping the byte-identity regres floor structural and the fix cheap to revert if the override inference proves wrong. Also landed: a transient isolation guard (cutting `B_MAINSEND` on direct folder children, muting receives) so an item capture stays true to item scope, and a -post-render frame-count gate (±1 tolerance, tail-None only) that refuses and self-cleans -a widened render. New modules `core/capture/render_window`, `core/capture/track_topology`, +post-render frame-count gate (±1 tolerance, tail-None only) that refuses a widened +render and retains it outside the bank for diagnosis rather than deleting it. New modules `core/capture/render_window`, `core/capture/track_topology`, `shell/capture/render_selection`, `shell/capture/render_isolation`. The whole fix rests on the unverified inference that REAPER's selected-tracks render source overrides custom time bounds — Ψ-W3-T1 (below) now also depends on it. **DAW-verification diff --git a/docs/TODO.md b/docs/TODO.md index 2f099fa..3ef3058 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -689,7 +689,7 @@ stereo file today. `Sample::channelCount` matching, the same way an offline dead-center capture does; a true-stereo bake is byte-identical to today's output. -## A 0-byte render can still pass every gate under Auto/Manual tail (narrowed, not closed) +## A 0-byte render can still pass every gate under Auto/Manual tail (closed) **Context (surfaced by Ψ-W3 review).** `OfflineRenderBackend::capture`'s exists-check passes for a 0-byte file, and the bounds gate used to fire only when `expectedFrames > 0` @@ -697,22 +697,25 @@ passes for a 0-byte file, and the bounds gate used to fire only when `expectedFr refusing, so a 0-byte render reached `stampCaptureSample` and landed as `CaptureStatus::Ok` with an empty `contentHash` and `channelCount == 0`. -**Narrowed.** `shell/capture/render_bounds_gate` now refuses an unmeasurable render -(invalid layout, or a layout declaring no sample rate) instead of skipping it. That -covers `TailMode::None` only — the gate does not judge Auto/Manual, which add frames by -design, so a 0-byte render under either of those still lands as `Ok`. The refusal reuses -`CaptureStatus::BoundsMismatch` rather than minting its own status; the earlier note here -preferred a distinct status, and that preference is unresolved, not withdrawn. +**Narrowed, then reopened as an asymmetry.** `shell/capture/render_bounds_gate` was +first changed to refuse an unmeasurable render (invalid layout, or a layout declaring no +sample rate) instead of skipping it — but that gate only ever judges `TailMode::None`, +so a 0-byte render under Auto/Manual still landed as `Ok`, while `None` now refused and +quarantined the identical file. The two tail modes disagreed on a defect neither should +accept. -**Intended fix.** Reject a 0-byte / unparseable render right after the exists-check, on -every tail mode, before anything downstream reads it. +**Closed.** `capture.cpp` now checks `checkRenderedFileNotEmpty` right after the +exists-check, on every tail mode, before the `TailMode::None`-only bounds gate runs — +a 0-byte render is refused and quarantined identically regardless of tail mode. The +refusal reuses `CaptureStatus::BoundsMismatch` rather than minting its own status; the +earlier note here preferred a distinct status, and that preference is unresolved, not +withdrawn. ## An offline capture can be refused for a short render — root cause open **Symptom (live, 2026-08-02).** A capture over [0.000000s, 4.067797s) at 48 kHz was refused: `Render produced 195216 frames but the requested range is 195254`. 38 frames -short — two orders of magnitude outside the gate's one-frame tolerance, so the tolerance -is not what refused it. +short — 38x the gate's one-frame tolerance, so the tolerance is not what refused it. **Hypothesis A — the render bounds itself to the media it can see.** REAPER's selected-items render source (`&32`) derives its bounds from the selected items' own @@ -739,3 +742,29 @@ numbered blocker steps: step 1 separates A's `&32` path from the shared `&128` p says how to tell when it failed to), step 2 asks whether the render is short at all, step 3 reads the retained refused render to place the missing frames. Nothing here should be "fixed" before that comes back. + +## Split `render_bounds_gate` on the verdict/message vs. filesystem seam + +**Context (Ψ-W3 round-two review).** `render_bounds_gate.cpp` mixes pure verdict +composition (frame-count comparison, message text) with filesystem I/O +(`retainRefusedRender`'s `fs::create_directories`/`fs::rename`) in one shell TU. The +verdict half has no REAPER dependency and no filesystem dependency either — it could be +`core/capture`, unit-tested directly instead of only through the pure `render_window` +functions it calls. The reviewer's suggested split: verdict + message composition pure +and testable in `core/capture`, leaving only `retainRefusedRender` (and the two thin +`checkRendered*` entry points that call it) in `shell/capture`. + +**Why deferred.** Out of scope for the dispatch that surfaced it — a structural split, +not the bug fix in front of it. + +**Filed also because it's already slightly wrong today.** `render_bounds_gate.cpp` +touches no REAPER API (it is `` + the pure `core/capture` modules only), so +`src/shell/capture/CLAUDE.md`'s "this directory is the REAPER API surface only" scope +line no longer describes it — one more small argument for eventually moving the +REAPER-free half to `core/capture`, separate from the untested-filesystem-code gap +above. + +**Done looks like.** `core/capture` owns a pure `checkRenderedBoundsVerdict`-shaped +function under a `_tests` target with no REAPER, no VST3 SDK, and no +filesystem includes; `shell/capture/render_bounds_gate` shrinks to the file-move and +the two callers' plumbing. diff --git a/docs/VERIFICATION.md b/docs/VERIFICATION.md index 6aa2e8a..c8e8bc0 100644 --- a/docs/VERIFICATION.md +++ b/docs/VERIFICATION.md @@ -27,9 +27,9 @@ Checks for Θ, Ξ, and Ψ work that no unit test can close. Build **Release**, i - [ ] One razor-union case (two disjoint areas, one track) — lands the requested window, no `ReaSampler capture failed:` line (`PLAN.md:2137`) - [ ] Capture an item whose extent already equals the window — still lands, unchanged (the byte-identity regression floor) (`docs/COMPLETED.md:829`) - [ ] **Open blocker — root cause unknown; the three steps below are the experiment** (both live hypotheses and what is NOT yet excluded: `docs/TODO.md` §An offline capture can be refused for a short render). A live capture over [0.000000s, 4.067797s) was refused 38 frames short (195216 of 195254 at 48 kHz). Set View → time unit to Samples first -- [ ] **Step 1 — does the item-scope render bound itself to the media?** This only tests anything if item scope actually reaches REAPER's selected-items render, and it does that ONLY when the selected items' extent already equals the requested window (`itemExtentPrintsWindow`, `src/core/capture/render_window.h`); otherwise item scope re-sources through the items' own tracks — the same source track scope uses, so the two runs would test one thing twice. So: snap the time selection to the item's exact start and end, run **item** scope, then **track** scope over the identical range. Report both `Render source:` lines and both frame counts. **If both lines read `selected tracks via master`, the item path was NOT exercised** — the extents did not match; re-snap and repeat before concluding anything -- [ ] **Step 2 — full-length or short?** Extend the same range ~1 s past the end of all media, **track** scope. Landing with the full range (no refusal, the card reads the extended length) rules out BOTH a trailing-silence trim and a content-extent bound at once. A short render does NOT tell them apart: a trim firing despite `RENDER_NORMALIZE &(4<<16)` and a render bounding itself to content extent produce the same count. Report which happened, then run step 3 -- [ ] **Step 3 — where are the missing frames?** A refused render is kept deliberately, not deleted: it is moved to `/reasampler_refused/`. **Follow the path in the refusal line, not this sentence** — if the move itself failed the file stays in the bank folder, unindexed, and the line says which happened. Insert it against the source over the same range and report whether the head aligns. Frames missing from the TAIL with an aligned head fits either a tail trim or a content-extent bound; a head offset fits neither and is a start-position defect. Also report whether the media under the range ends before the range does. Delete `reasampler_refused/` when done — nothing in the bank references it +- [ ] **Step 1 — does the shortfall follow the render source?** This only tests anything if item scope actually reaches REAPER's selected-items render, and it does that ONLY when the selected items' extent already equals the requested window (`itemExtentPrintsWindow`, `src/core/capture/render_window.h`); otherwise item scope re-sources through the items' own tracks — the same source track scope uses, so the two runs would test one thing twice. So: snap the time selection to the item's exact start and end, run **item** scope, then **track** scope over the identical range. Report both `Render source:` lines and both frame counts. **If both lines read `selected tracks via master`, the item path was NOT exercised** — the extents did not match; re-snap and repeat before concluding anything. **A landing `&32` run here is not evidence `&32` honours custom bounds** — at a window snapped to the item's own extent, a render that honours the window and one that bounds itself to media content print IDENTICAL frames, so this step cannot tell those two apart; it only tells you which render source is in play. **If neither run refuses at this snapped range, the blocker did not reproduce here** — this range does not recreate the original refusal, which ran past the end of its media; move to step 2, which does +- [ ] **Step 2 — full-length or short?** Extend the same range ~1 s past the end of all media, **track** scope. Landing with the full range (no refusal, the card reads the extended length) rules out BOTH a trailing-silence trim and a content-extent bound at once — but it also means there is no refused render for step 3 to read; re-run the ORIGINAL refusing range ([0.000000s, 4.067797s), track scope) to produce one before continuing. A short render does NOT tell the two hypotheses apart: a trim firing despite `RENDER_NORMALIZE &(4<<16)` and a render bounding itself to content extent produce the same count — and that render IS the one step 3 reads. Report which happened, then run step 3 +- [ ] **Step 3 — where are the missing frames?** Reads the short render from step 2 (or, if step 2 landed, the fresh refused render from re-running the original range per step 2's note) — not anything step 1 may have left behind, since a correctly-snapped step 1 should not have refused at all. A refused render is kept deliberately, not deleted: it is moved to `/reasampler_refused/`. **Follow the path in the refusal line, not this sentence** — if the move itself failed the file stays in the bank folder, unindexed, and the line says which happened. Filenames carry a timestamp/counter but no scope marker, so if more than one file has landed in `reasampler_refused/` by now, the one from step 2 is the most recently written one — or empty the folder before running step 2 so there is only one candidate. Insert it against the source over the same range and report whether the head aligns. Frames missing from the TAIL with an aligned head fits either a tail trim or a content-extent bound; a head offset fits neither and is a start-position defect. Also report whether the media under the range ends before the range does. Delete `reasampler_refused/` when done — nothing in the bank references it ## Names and channels diff --git a/src/core/capture/render_window.cpp b/src/core/capture/render_window.cpp index d24e436..e32e642 100644 --- a/src/core/capture/render_window.cpp +++ b/src/core/capture/render_window.cpp @@ -8,8 +8,9 @@ namespace reasampler::capture { namespace { -// Round-to-nearest, so a position that sits mid-frame maps to the frame a render -// of it prints rather than to the frame below it. +// Round-to-nearest: the convention THIS module measures a window by, so a mid-frame +// position maps to the closer frame boundary rather than always down. Not a claim +// about how any renderer resolves that position -- see the header's caveat. long long frameIndexAt(double seconds, int sampleRate) { return std::llround(seconds * static_cast(sampleRate)); } diff --git a/src/shell/capture/CLAUDE.md b/src/shell/capture/CLAUDE.md index 5fd5f22..37bbea9 100644 --- a/src/shell/capture/CLAUDE.md +++ b/src/shell/capture/CLAUDE.md @@ -52,7 +52,7 @@ detail not covered there: ## Modules - `capture` — two CONCRETE backends with deliberately different lifecycles (no shared interface — the former `ICaptureBackend` was deleted in Q-W3, T4-26: one deriver, zero polymorphic call sites): `OfflineRenderBackend` (deterministic default, synchronous) and `RealtimeRecordBackend` (async begin/tick/abort). Input: `CaptureRequest`. Output: finished file + populated `Sample` handed to `bank_model`. It also owns the two file-side steps both backends share, in this order: `collapseCapturedFileToMono` (the lossless mono collapse, applied to the landed file) and `stampCaptureSample`, which measures the channel count off that same file so the entry and the audio cannot disagree. And `captureNameFor` — the impure local-clock read the entry points call to build a request's label + stem, kept out of the pure `core/capture/capture_name` composition it feeds. -- `render_bounds_gate` (`shell/capture`) — the exact-bounds verdict on a landed offline render and the refusal's file handling, split off `capture.cpp` on the render-vs-judge seam. Refuses a frame count that is not the window's AND a file whose frames cannot be measured at all (an invalid layout used to skip the gate and land with an unknown channel count). Judges `TailMode::None` only — Auto/Manual add frames by design, and an unmeasurable render still lands under those two (`docs/TODO.md`). A refused render is MOVED to `/reasampler_refused/` rather than deleted, so the frames it did print survive for diagnosis while the short-render root cause is open; the bank never sees it either way. +- `render_bounds_gate` (`shell/capture`) — the exact-bounds verdict on a landed offline render and the refusal's file handling, split off `capture.cpp` on the render-vs-judge seam. Refuses a frame count that is not the window's AND a file whose frames cannot be measured at all (an invalid layout used to skip the gate and land with an unknown channel count). Judges `TailMode::None` only — Auto/Manual add frames by design, and an unmeasurable render still lands under those two (`docs/TODO.md`). A refused render is MOVED to `/reasampler_refused/` rather than deleted, so the frames it did print survive for diagnosis while the short-render root cause is open; the bank never INDEXES it either way — but a failed move leaves the file sitting unindexed in the bank folder itself, not `reasampler_refused/` (the console message says which happened). - `scope_resolve` (`shell/capture`) — scope/source resolution shared by every capture entry point (Q-W3 hoist out of `main.cpp`): razor-else-time range inference, selected-track/selected-item-owning-track collection with canonical GUIDs, and the M10 provenance-assembly inputs (read BEFORE the FX-bypass guard neutralizes the in-scope chain). Also the one place a source track's NAME is read (`trackName`, via `GetTrackName` — chosen over `P_NAME` because it already answers REAPER's `"Track N"` convention for an unnamed track), landed on `ResolvedSource::trackNames` parallel to `sourceTracks` and composed into the capture's label + stem by the pure `core/capture/capture_name`. - `render_selection` (`shell/capture`) — the transient track selection a selected-tracks render (`&128`) requires, as a stack RAII guard: REAPER prints whatever tracks are selected, so `renderOffline` makes the request's own tracks BE the selection for the render's duration and restores the user's set on every exit path. Engaged ONLY for that source mode, which leaves a stated residual: a `&32` selected-items render still prints whatever ITEMS the user has selected. Live captures are unaffected (that selection is the source), but a recipe replay of a `SelectedItems` capture renders against whatever happens to be selected then — the recipe stores tracks and a range, never item GUIDs, so this guard cannot close it. Filed in `docs/TODO.md`. - `render_isolation` (`shell/capture`) — the transient upstream silencing a ranged ITEM render needs, as a stack RAII guard alongside the two above: the selected-tracks source prints everything flowing INTO the track, so each direct folder child's `B_MAINSEND` and each of the track's receives' `B_MUTE` are cut for the render and restored on every exit path. Direct children only — a grandchild reaches the track through the child that owns it. The child-set walk is pure (`core/capture/track_topology`). diff --git a/src/shell/capture/capture.cpp b/src/shell/capture/capture.cpp index d5089b4..6e7355a 100644 --- a/src/shell/capture/capture.cpp +++ b/src/shell/capture/capture.cpp @@ -497,6 +497,18 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { return result; } + // A 0-byte render is refused on every tail mode, ahead of and independent from the + // TailMode::None-only bounds gate below — Auto/Manual add frames by design but never + // legitimately produce zero (docs/TODO.md "0-byte render" entry: before this check, + // Auto/Manual landed an empty file as CaptureStatus::Ok with channelCount == 0). + const BoundsVerdict emptyVerdict = + checkRenderedFileNotEmpty(expectedPath, projectDir); + if (emptyVerdict.refused) { + result.status = CaptureStatus::BoundsMismatch; + result.message = emptyVerdict.message; + return result; + } + // Exact bounds, made structural: render_bounds_gate judges the landed file against // the requested window and owns what becomes of a render that fails. // (On TailMode::None the landed file is read three times on this path — that gate, diff --git a/src/shell/capture/capture_batch.cpp b/src/shell/capture/capture_batch.cpp index 31714af..97f79fe 100644 --- a/src/shell/capture/capture_batch.cpp +++ b/src/shell/capture/capture_batch.cpp @@ -16,11 +16,13 @@ #include "shell/panel/panel_bank_ops.h" // bankPanelSelectedSampleIds / SourceBankId #include "shell/panel/panel_input.h" // bankPanelRefresh #include "core/capture/batch_capture.h" // planCaptureUnits / BatchOutcome +#include "core/capture/capture_paths.h" // projectDirOfRpp #include "core/model/bank_book.h" // BankBook / Bank #include "core/model/provenance.h" // recipe parse/build, fingerprint #include "shell/persist/session.h" // ReaSamplerSession #include "shell/capture/capture_orchestrator.h" // captureAndIndexOne / renderOffline #include "shell/capture/provenance_shell.h" // fxChainIdentity* / trackByGuid +#include "shell/capture/render_bounds_gate.h" // refusedRenderFolder #include "shell/capture/scope_resolve.h" // ResolvedSource #include "shell/capture/track_guid.h" // guidString @@ -28,6 +30,7 @@ #define REAPERAPI_MINIMAL #define REAPERAPI_WANT_CountSelectedMediaItems +#define REAPERAPI_WANT_EnumProjects #define REAPERAPI_WANT_GetSelectedMediaItem #define REAPERAPI_WANT_GetMediaItem_Track #define REAPERAPI_WANT_GetMediaItemInfo_Value @@ -95,6 +98,24 @@ private: std::vector selected_; }; +// Names where refused renders were retained, once, when the batch quarantined at +// least one (BoundsMismatch failures only -- a render that never produced a file has +// nothing to retain). Without this, a batch's per-unit failure detail (which DOES name +// the destination, same as a single capture's console line) never reaches the console +// at all -- the batch summary reports ordinals only. +std::string withQuarantineNote(std::string line, int quarantinedCount) { + if (quarantinedCount <= 0) return line; + std::vector buf(4096, '\0'); + EnumProjects(-1, buf.data(), static_cast(buf.size())); + const std::string dir = projectDirOfRpp(std::string(buf.data())); + if (dir.empty()) return line; // unreachable: a quarantine implies a saved project + line += " " + std::to_string(quarantinedCount) + " refused render" + + (quarantinedCount == 1 ? " was" : "s were") + " retained for diagnosis, " + "normally at " + refusedRenderFolder(dir) + " (delete when done) -- one " + "whose move there failed instead stays in the bank folder, unindexed."; + return line; +} + // Deselect-all then select-one so the offline render's &32 bit captures exactly // this item. Called inside ItemSelectionGuard, which restores the original selection. void selectOnlyItem(MediaItem* item) @@ -196,6 +217,7 @@ void RunBatchCaptureItems(ReaSamplerSession& session) BatchOutcome outcome; bool anyAdded = false; + int quarantined = 0; // BoundsMismatch failures, each of which retained a file { // selGuard restores the original item selection on every exit path. ItemSelectionGuard selGuard; @@ -232,6 +254,7 @@ void RunBatchCaptureItems(ReaSamplerSession& session) const bool ok = (res.status == CaptureStatus::Ok); outcome.record(unit.ordinal, ok, ok ? std::string{} : res.message); if (ok) anyAdded = true; + else if (res.status == CaptureStatus::BoundsMismatch) ++quarantined; } } // selGuard restores the original selection here, on every path @@ -243,7 +266,8 @@ void RunBatchCaptureItems(ReaSamplerSession& session) session.saveToActiveProject(); } - ShowConsoleMsg((outcome.summaryLine("item") + "\n").c_str()); + ShowConsoleMsg((withQuarantineNote(outcome.summaryLine("item"), quarantined) + + "\n").c_str()); } // One sample per razor area, track scope over that area's own range. Track scope @@ -267,6 +291,7 @@ void RunBatchCaptureRazor(ReaSamplerSession& session) BatchOutcome outcome; bool anyAdded = false; + int quarantined = 0; // BoundsMismatch failures, each of which retained a file { // selGuard restores the original track selection on every exit path. TrackSelectionGuard selGuard; @@ -298,6 +323,7 @@ void RunBatchCaptureRazor(ReaSamplerSession& session) const bool ok = (res.status == CaptureStatus::Ok); outcome.record(unit.ordinal, ok, ok ? std::string{} : res.message); if (ok) anyAdded = true; + else if (res.status == CaptureStatus::BoundsMismatch) ++quarantined; } } // selGuard restores the original track selection here, on every path @@ -307,7 +333,8 @@ void RunBatchCaptureRazor(ReaSamplerSession& session) session.saveToActiveProject(); } - ShowConsoleMsg((outcome.summaryLine("razor area") + "\n").c_str()); + ShowConsoleMsg((withQuarantineNote(outcome.summaryLine("razor area"), quarantined) + + "\n").c_str()); } // Regenerates a provenanced sample's file from its recorded source's current state diff --git a/src/shell/capture/render_bounds_gate.cpp b/src/shell/capture/render_bounds_gate.cpp index 4b38d8a..9fffa26 100644 --- a/src/shell/capture/render_bounds_gate.cpp +++ b/src/shell/capture/render_bounds_gate.cpp @@ -3,6 +3,7 @@ #include "shell/capture/render_bounds_gate.h" #include +#include #include #include #include @@ -46,6 +47,24 @@ std::string retainRefusedRender(const std::string& renderedPath, } // namespace +std::string refusedRenderFolder(const std::string& projectDir) { + return projectDir + "/" + kRefusedSubfolder; +} + +BoundsVerdict checkRenderedFileNotEmpty(const std::string& renderedPath, + const std::string& projectDir) { + BoundsVerdict v; + std::error_code ec; + const std::uintmax_t size = std::filesystem::file_size(renderedPath, ec); + if (ec || size != 0) return v; // stat failure isn't this check's job — leave it be + + v.refused = true; + v.message = "Render at " + renderedPath + " is 0 bytes -- REAPER produced an empty " + "file, so there is nothing to check the requested range against." + + retainRefusedRender(renderedPath, projectDir); + return v; +} + BoundsVerdict checkRenderedBounds(const std::string& renderedPath, const std::string& projectDir, const CaptureRequest& request) { @@ -66,10 +85,11 @@ BoundsVerdict checkRenderedBounds(const std::string& renderedPath, // a file whose frames were never counted. if (!layout.valid || layout.sampleRate == 0) { v.refused = true; - v.message = "Render at " + renderedPath + " could not be measured -- its WAV " - "header did not parse, or declared no sample rate -- so the frames " - "it holds were never checked against the requested range." + source + - retainRefusedRender(renderedPath, projectDir); + v.message = "Render at " + renderedPath + " could not be measured -- it could " + "not be read (locked, missing, or a permissions error), its WAV " + "header did not parse, or it declared no sample rate -- so the " + "frames it holds were never checked against the requested range." + + source + retainRefusedRender(renderedPath, projectDir); return v; } diff --git a/src/shell/capture/render_bounds_gate.h b/src/shell/capture/render_bounds_gate.h index 18d0e59..27a4eb1 100644 --- a/src/shell/capture/render_bounds_gate.h +++ b/src/shell/capture/render_bounds_gate.h @@ -27,4 +27,15 @@ BoundsVerdict checkRenderedBounds(const std::string& renderedPath, const std::string& projectDir, const CaptureRequest& request); +// A 0-byte render is refused on every tail mode (Auto/Manual add frames by design but +// never legitimately produce zero), independent of and ahead of the TailMode::None-only +// gate above, which does not run on Auto/Manual at all. +BoundsVerdict checkRenderedFileNotEmpty(const std::string& renderedPath, + const std::string& projectDir); + +// Where a refused render is retained -- exposed so a multi-unit caller (batch capture) +// can name the folder once without duplicating the subfolder name `checkRenderedBounds` +// and `checkRenderedFileNotEmpty` already use internally. +std::string refusedRenderFolder(const std::string& projectDir); + } // namespace reasampler::capture