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();