diff --git a/docs/TODO.md b/docs/TODO.md index 3d8e58e..fe32b04 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -511,39 +511,45 @@ registration and `ingestHandleSectionCommand` move into it and gain unit coverag `ingest.cpp`'s own `command_id`+`gaccel` registration collapses onto `action_registry::registerAction` where the shapes match. -## A multi-track TRACK capture renders one file per track and lands one of them +## The `&128` multi-track output shape is still DAW-unobserved, and a refusal now rests on it -**Context (surfaced by Ψ-W1-T1, capture-range-exactness).** Track scope has always -rendered through `RENDER_SETTINGS &128` ("selected tracks via master"), and the -ranged item capture now joins it there. The SDK header (~3041) documents the -single-file bit `&(4<<16)` for "rendering selected items or razor edits" only, so it -does not apply to `&128`: the reading is that N selected tracks produce N files. -`capture.cpp` sets `RENDER_PATTERN` to one literal stem and detects success by -`std::filesystem::exists`, so N stems collapse onto one name and whichever file -survives lands as a successful capture carrying one track's audio. +**Context.** The multi-track TRACK capture no longer lands one track's audio under an +`Ok`: `renderOffline` refuses every selected-tracks render covering more than one track, +both scopes, naming the way out (`render_settings::isMultiTrackStemRender` / +`multiTrackRefusalMessage`). What did NOT change is the evidence: the per-track-output +reading of `&128` is still INFERRED from the SDK header documenting the single-file bit +`&(4<<16)` for item/razor sources only. It has never been observed in a DAW. -**The wart.** `renderOffline` refuses this shape for the ranged ITEM capture -(`render_settings::isMultiTrackRangedItemRender`) because that path was newly routed -into it. Track scope with two or more tracks selected has the same exposure and is -deliberately untouched — changing a shipped action from "produces a file" to -"refuses" is a behavioral-contract change, and it was out of that track's surface. +**The wart.** The refusal is therefore as unverified as the defect it closes. If REAPER +in fact sums a multi-track `&128` render into the single literal `RENDER_PATTERN`, the +refusal costs a working capture — a user who selects two tracks and captures gets a +message where a correct summed file used to land. -**Intended fix.** Not proposed. Three shapes exist and the choice is a product call, -not a mechanical one: refuse (matching the item path), render each track and land N -bank entries (that is batch capture's meaning, not this action's), or sum the -selected tracks into one file (needs a summing render source `&128` does not offer). +**Intended fix.** Run the observation in `docs/verify-track-scope-multitrack.md` §3 (a +hand-driven Render dialog, source "selected tracks via master", one literal filename, two +tracks selected — then count the files REAPER writes). If it comes back "one file per +track", nothing to do and the inference is retired into fact. If it comes back "one +summed file", the refusal is over-strict for the TRACK scope and should be narrowed back +— and the ITEM-scope half is then an OPEN question, not settled: a full-extent item +capture already sums a multi-track item selection via `&32|single-file` +(`tests/test_render_settings.cpp:262`), so if `&128` also sums, a ranged item capture +routed through it sums too, and keeping the item refusal in that branch would make item +scope inconsistent with itself across the range boundary (full-extent sums, ranged +refuses, same scope). Whether that inconsistency is acceptable or the item refusal should +narrow too needs its own look at that point — not decided here. -**The constraint the fix MUST handle.** The per-track-output reading of `&128` is -INFERRED from the header's single-file wording, never observed in a DAW. Verify it -first — including what REAPER actually writes when N stems share one literal -`RENDER_PATTERN` — because the answer decides whether this is a defect at all. +**The constraint the fix MUST handle.** Narrowing the refusal must keep the ITEM scope +refusing, must keep `renderOffline` the single seam (so a recipe replay cannot diverge +from a fresh capture), and must not re-open the collapse for any caller that reaches +`&128` later — the predicate is keyed on the render source precisely so new callers +inherit it. -**Priority / risk.** Unknown until the DAW check above runs. If the reading is right, -a silently-wrong capture on an ordinary two-track selection; if wrong, nothing. +**Priority / risk.** Low and bounded either way: the current behavior refuses rather than +lands wrong audio, so the cost of being wrong here is a refused capture, not a bad one. -**Done looks like.** The `&128` multi-track output shape is DAW-confirmed, and track -scope either produces defined correct output for a multi-track selection or refuses -it with a message naming the reason. +**Done looks like.** The `&128` multi-track output shape is DAW-observed and written into +`src/shell/capture/CLAUDE.md` as fact rather than inference, and the refusal is either +kept as-is or narrowed to the item scope with that observation cited. ## A `SelectedItems` recipe replays against whatever items are selected then @@ -644,3 +650,20 @@ stereo file today. **Done looks like.** A dead-center instrument bake lands as a 1-channel file with `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) + +**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`. + +**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`). + +**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. diff --git a/docs/verify-track-scope-multitrack.md b/docs/verify-track-scope-multitrack.md new file mode 100644 index 0000000..3ef819f --- /dev/null +++ b/docs/verify-track-scope-multitrack.md @@ -0,0 +1,154 @@ +# DAW verification — track-scope capture over a multi-track selection + +What a DAW pass must establish for the multi-track track capture, and the exact numbers +or strings to read off. Nothing below can be closed by a unit test: every item depends on +what REAPER actually does with a render request. + +**Build to use.** Release, installed into `UserPlugins/`, REAPER restarted — extensions +load at startup only. Set the docked panel's tail toggle to **None** before every cell; +Auto adds an 8 s window and Manual a fixed one, and both would invalidate the frame-count +readings. + +**Project to use.** One saved project, project sample rate pinned to 48000. Two audio +tracks, `A` and `B`, each holding one item at least 30 s long, with *audibly different* +content (a tone on `A`, a drum loop on `B`). One folder track `F` with `A` and `B` as its +children, used only in §5. + +--- + +## 1. The regression floor — single-track track capture is unchanged + +Select **track `A` only**. Make a time selection from **10.000 s to 12.000 s**. Run +*ReaSampler: capture selected track(s)*. + +Read off: + +- A file appears in the project's bank folder, and one new card appears on the panel. +- The card's length reads **2.000 s**; its frame count is **96000** (`round(12.0 × 48000) + − round(10.0 × 48000)`). The backend refuses the capture with `BoundsMismatch` if the + render is more than one frame off that, so a landed capture already proves the number + to ±1 — what you are confirming here is that it landed at all. +- The REAPER console shows **no** `ReaSampler capture failed:` line. +- Track `A` is still the only selected track afterwards. +- **Content, not just length.** Listen to the landed file. `A` and `B` carry *audibly + different* content by the project setup above (tone vs. drum loop), so this is a by-ear + check, not a null test: the capture must be the tone alone, with **no** drum-loop bleed. + Expected: pure tone, matching `A` soloed. Failing: any trace of `B`'s drum loop audible + in the file. This is not a tautology check — the SDK header's own `RENDER_SETTINGS` line + admits a second reading, `(&(1|2)==0)=master mix`, under which a single-track track + capture could render the **whole master mix** (both `A` and `B`) rather than `A` alone; + drum-loop bleed here is exactly what that misreading would produce, and this is the + cheapest place in the whole doc to catch it. + +**This is the byte-identical floor.** If either cell now refuses, the change is wrong — +the refusal must fire only above one track. + +## 2. The defect cell — two selected tracks now refuse + +Select **`A` and `B` together**. Time selection 10.000–12.000 s. Run *capture selected +track(s)*. + +Read off: + +- The console prints exactly: + `ReaSampler capture failed: A track capture renders the selected tracks through the + master, and more than one track cannot land as a single file. Capture one track at a + time, or route them into a folder/bus track and capture that (a folder's own output is + its children summed).` +- **No** new card on the panel, and **no** new `.wav` in the bank folder (check the folder + directly — a stray file with nothing indexing it would mean the refusal fired too late). +- `A` and `B` are both still selected, both still unmuted, and neither track's fader, pan, + or FX-bypass state changed. The refusal returns before any guard is constructed, so + there should be nothing to restore — this reading is what confirms that. + +Repeat with a **razor area spanning both tracks** and no time selection: identical +readings. Note that the track *selection* is what the refusal counts — a razor over two +tracks with only `A` selected is a one-track capture and must still succeed (§1). + +## 3. The decisive observation — what `&128` actually writes + +**This is the one that retires an inference, and it is the reason `docs/TODO.md` still +carries an entry.** The refusal in §2 rests on reading the SDK header's single-file bit +`&(4<<16)` as applying to item/razor sources only, never to `&128` — so N selected tracks +are believed to produce N files. That has never been observed. + +Drive REAPER's own Render dialog by hand, with the extension out of the loop: + +1. Select `A` and `B`. +2. File → Render. **Source:** *Selected tracks via master* — the dialog wording for `&128` + (SDK header ~3041). Do **not** pick *Stems (selected tracks)* — that is `&2`, a + different source bit that unambiguously writes one file per track and would confirm + nothing about `&128`. + **Bounds:** *Custom time range*, 10.000 to 12.000 s. +3. **File name:** a literal stem with **no wildcards at all** — e.g. `stemprobe`. Clear + `$track` / `$item` / anything else from the pattern; the extension writes exactly one + literal stem, so the probe must too. +4. Render to an empty scratch folder. + +Read off — **the file count in that folder**: + +- **Two files** (however REAPER disambiguated them, or one file that visibly got + overwritten): the inference holds, the §2 refusal is correct, and the `docs/TODO.md` + entry can be closed by writing this observation into `src/shell/capture/CLAUDE.md` as + fact. +- **One file containing `A` and `B` summed** (confirm by ear, or by nulling it against a + master render of the same range with only `A` and `B` unmuted): the inference is wrong, + the §2 refusal costs a working capture, and the track-scope half should be narrowed back + per the `docs/TODO.md` entry. The item-scope half stays either way. + +Also record **what REAPER named the files** — that decides whether a future correct +multi-track capture could ever be built on this source at all. + +## 4. Recapture replays the same answer + +Take a **single-track** track capture that carries provenance (capture a range on `A` +whose source item is itself a bank sample, so `detectParent` fires), select its card, and +run *re-capture from source*. It must regenerate — same audio, same 96000 frames. + +Then construct the multi-track case: a recorded recipe whose `trackGuids` names two +tracks. The reachable way to get one is to have captured it before this change; if no such +entry exists in any project, record that this cell was **not exercised** rather than +inventing one. When it is exercised, read off: + +- `ReaSampler re-capture failed:` followed by the **same** message text as §2. +- The bank entry is untouched — same file, same hash, same card. + +## 5. The way out actually works + +Route `A` and `B` into folder `F`. Select **`F` only**, time selection 10.000–12.000 s, +capture track scope. + +Read off: one card, 2.000 s, and the audio contains **both** `A` and `B`. This is what the +refusal message tells the user to do, so it has to be true. + +## 6. Realtime still accepts a multi-track selection + +Select `A` and `B`. Run *ReaSampler: capture selected track(s) in realtime* over the same +range. Read off: **one** card, and its audio contains both tracks. Realtime taps each +source track with a send into one temp track, so it sums where the offline render cannot — +the divergence from §2 is deliberate and this cell is what confirms it is real. + +## 7. Mono collapse — what is and is not reachable + +Capture a range on a track whose content is dead-center (a mono source panned center, or +a duplicated-channel file), using time selection **10.000 s to 12.000 s** (2.000 s, 96000 +frames at 48000 Hz — the §1 convention, so the resulting file size is exact). The panel has +no channel-count readout anywhere (`Sample::channelCount` is not drawn by +`src/shell/panel/panel_render.cpp`), so read the proxy instead: + +- Check the landed `.wav`'s size on disk (Explorer → Properties, or a directory listing). A + successful collapse is the extension's own rebuild — canonical 44-byte header + 96000 × + 4 bytes = **384,044 bytes**. A file near double that (~768,044 bytes, plus whatever + REAPER's own render adds for `bext`/metadata chunks) means the collapse did not fire — + recheck the source is genuinely dead-center before treating this as a defect. +- The console shows **no** `the lossless mono collapse ... already reached the bank; only + the size win from the collapse was lost.` line. + +**Not DAW-reachable:** the collapse's *failure* branch. It fires only if the captured file +cannot be read, or its temporary rewrite cannot be written or renamed, inside the same +call that just rendered the file — there is no manual way to inject that fault between the +render and the rename. The branch is covered only at its reporting seam +(`tests/test_wav_codec.cpp`, `testCollapseOutcomeSuffixesAreDistinctStrings`), and its console line +has never been seen in a running REAPER. If you ever do see it, the render already reached +the bank — the report only tells you the collapse's size win was lost, not that the bytes +were verified (see `docs/TODO.md`'s 0-byte-render entry). diff --git a/src/core/capture/render_settings.cpp b/src/core/capture/render_settings.cpp index 72e87be..4939940 100644 --- a/src/core/capture/render_settings.cpp +++ b/src/core/capture/render_settings.cpp @@ -112,11 +112,32 @@ SourceMode sourceModeForScope(CaptureScope scope, bool itemExtentIsWindow) { return SourceMode::SelectedTracks; } -bool isMultiTrackRangedItemRender(CaptureScope scope, SourceMode mode, - int sourceTrackCount) { - return scope == CaptureScope::Item - && mode == SourceMode::SelectedTracks - && sourceTrackCount > 1; +bool isMultiTrackStemRender(SourceMode mode, int sourceTrackCount) { + return mode == SourceMode::SelectedTracks && sourceTrackCount > 1; +} + +std::string multiTrackRefusalMessage(CaptureScope scope) { + // Deliberately does not name realtime capture as a way out, though it is the one + // action that sums correctly here: realtime is non-deterministic (hardware/performed + // FX, no bit-identical-repeats guarantee), so pointing an offline refusal at it would + // trade one invariant for another rather than just naming a substitute. A stated + // choice, not an oversight. + switch (scope) { + case CaptureScope::Item: + return "This range is narrower than the selected items, so it renders " + "through their tracks -- and those items span more than one track, " + "which this shape cannot land as a single file. Capture one track's " + "items at a time, or make the range match the items' extent."; + case CaptureScope::Track: + return "A track capture renders the selected tracks through the master, " + "and more than one track cannot land as a single file. Capture one " + "track at a time, or route them into a folder/bus track and capture " + "that (a folder's own output is its children summed)."; + } + // Unreachable for a valid enum; a refusal with no way out is still better than a + // silent one, so fail closed to the scope-agnostic half of the message. + return "This selection spans more than one track, which cannot land as a single " + "file. Capture one track at a time."; } RangeSource inferRangeSource(bool hasRazorArea) { diff --git a/src/core/capture/render_settings.h b/src/core/capture/render_settings.h index c871a54..e3c120d 100644 --- a/src/core/capture/render_settings.h +++ b/src/core/capture/render_settings.h @@ -1,8 +1,9 @@ #pragma once // render_settings — the REAPER-free logic behind the capture action family: // sourceMode -> RENDER_SETTINGS bits, P_RAZOREDITS parsing + range union, -// razor-else-time inference, the FX-scope bypass plan, and the capture-action -// table main.cpp iterates. Bit MEANINGS below are transcribed verbatim from +// razor-else-time inference, the FX-scope bypass plan, the capture-action +// table main.cpp iterates, and the multi-track-stem refusal + its user-facing +// message text. Bit MEANINGS below are transcribed verbatim from // reaper_plugin_functions.h; the CHOICE of which bits each mode sets is tested. #include @@ -124,19 +125,23 @@ enum class CaptureScope { // track ITSELF is deliberately not isolated; see src/shell/capture/CLAUDE.md. SourceMode sourceModeForScope(CaptureScope scope, bool itemExtentIsWindow); -// True for the one render shape that cannot land as a single capture: an item-scope -// capture re-sourced to the selected-tracks render (its window is not the item -// extent) whose selected items span more than one track. That source is read as -// rendering one file per selected track — the single-file bit is documented for -// item/razor sources only (SDK header ~3041), which is the whole basis for the -// reading and is DAW-unverified. If it holds, N tracks collapse N stems onto one -// literal render pattern and whichever file survived would land as a successful -// capture carrying one track's audio. The caller refuses instead. +// True for the one render shape that cannot land as a single capture: a selected-tracks +// render covering more than one track — a ranged item capture whose items span several +// tracks, or any multi-track track capture. That source is read as rendering one file +// per selected track — the single-file bit is documented for item/razor sources only +// (SDK header ~3041), which is the whole basis for the reading and is DAW-unverified. +// If it holds, N tracks collapse N stems onto one literal render pattern and whichever +// file survived would land as a successful capture carrying one track's audio. The +// caller refuses instead. // -// Track scope is deliberately NOT covered here even though it renders through the -// same source — see src/shell/capture/CLAUDE.md §Gotchas. -bool isMultiTrackRangedItemRender(CaptureScope scope, SourceMode mode, - int sourceTrackCount); +// Scope is deliberately NOT a parameter: the exposure comes from the render SOURCE, +// which both scopes reach. +bool isMultiTrackStemRender(SourceMode mode, int sourceTrackCount); + +// The refusal text for the shape above. Keyed on scope because only the way OUT differs: +// an item capture can also widen its range to the items' own extent, which a track +// capture has no analog for. Kept beside the predicate so the two read as siblings. +std::string multiTrackRefusalMessage(CaptureScope scope); // --- Range inference: razor-else-time (orthogonal to scope) ------------------- // diff --git a/src/core/capture/wav_codec.cpp b/src/core/capture/wav_codec.cpp index eb4b1ff..c0b5d8a 100644 --- a/src/core/capture/wav_codec.cpp +++ b/src/core/capture/wav_codec.cpp @@ -300,6 +300,16 @@ MonoCollapse collapseToMono(const std::vector& bytes) { return out; } +std::string monoCollapseSuffix(MonoCollapseOutcome outcome) { + switch (outcome) { + case MonoCollapseOutcome::Declined: return {}; + case MonoCollapseOutcome::Collapsed: return " (collapsed to mono)"; + case MonoCollapseOutcome::Failed: + return " (mono collapse failed -- left as captured)"; + } + return {}; // unreachable for a valid enum; claim nothing rather than a wrong outcome +} + std::string hashBytes(const std::uint8_t* data, std::size_t len) { // FNV-1a 64-bit: deterministic, no dependencies, adequate for dedup identity. std::uint64_t h = kFnvOffsetBasis; diff --git a/src/core/capture/wav_codec.h b/src/core/capture/wav_codec.h index 9fad071..88ab15d 100644 --- a/src/core/capture/wav_codec.h +++ b/src/core/capture/wav_codec.h @@ -117,6 +117,25 @@ struct MonoCollapse { // including the bext/source-position consequence beyond hashing. MonoCollapse collapseToMono(const std::vector& bytes); +// How applying the collapse to a captured FILE ended. `Declined` is collapseToMono's own +// "nothing to do"; `Failed` is a read that never happened or a warranted rewrite that did +// not land. The capture is intact and correctly measured in every case — only the report +// tells them apart, which is why the two must not share one value. +enum class MonoCollapseOutcome { + Declined, + Collapsed, + Failed, +}; + +// The capture message's collapse suffix — empty for Declined, so a capture that had +// nothing to collapse reads exactly as it did before the collapse existed. Shared by +// both backends so one outcome cannot be reported two ways. NOT user-observable on its +// own: CaptureResult::message on a successful capture is never printed by any caller, so +// the Collapsed/Failed text this returns reaches no one today — the one observable +// channel for a genuine Failed outcome is the backends' own reportCollapseFailure +// console line. +std::string monoCollapseSuffix(MonoCollapseOutcome outcome); + // --- Content identity (dedup hashes) ----------------------------------------- // Deterministic FNV-1a 64-bit content hash over `len` bytes, as 16-char lowercase diff --git a/src/shell/capture/CLAUDE.md b/src/shell/capture/CLAUDE.md index ec66da9..94ee474 100644 --- a/src/shell/capture/CLAUDE.md +++ b/src/shell/capture/CLAUDE.md @@ -74,11 +74,17 @@ detail not covered there: each other (the former `ICaptureBackend` was removed) — do not reintroduce one without a real second polymorphic call site. - **The selected-tracks render (`&128`) is read as emitting one file per selected - track** — the single-file bit is documented for item/razor sources only (SDK header - ~3041), and that is the whole basis for the reading; it is DAW-unverified. If it - holds, then since `RENDER_PATTERN` is one literal stem and success is a file-exists - check, N tracks would land one track's audio as a successful capture. - `renderOffline` refuses that shape for the RANGED ITEM capture only - (`render_settings::isMultiTrackRangedItemRender`). Track scope renders through the - same source with the same exposure and is deliberately untouched here — filed in - `docs/TODO.md`. + track** — the single-file bit `&(4<<16)` is documented for item/razor sources only + (SDK header ~3041), and that is the whole basis for the reading; it is DAW-unverified. + If it holds, then since `RENDER_PATTERN` is one literal stem and success is a + file-exists check, N tracks would land one track's audio as a successful capture. + `renderOffline` refuses EVERY multi-track render through that source + (`render_settings::isMultiTrackStemRender`) — the ranged item capture and the plain + track capture alike, each with its own way out + (`render_settings::multiTrackRefusalMessage`). The refusal is keyed on the render + SOURCE and not on the scope, so a future caller that reaches `&128` inherits it. + Re-opening a multi-track track capture needs the DAW check in + `docs/verify-track-scope-multitrack.md` to come back the other way first. +- **Realtime is the one capture path that accepts a multi-track selection**, and it is + correct to: its per-source-track sends sum in the one temp track, which is a real mix + rather than a stem collapse. The offline refusal above does not apply to it. diff --git a/src/shell/capture/capture.cpp b/src/shell/capture/capture.cpp index 12f72f1..3999df2 100644 --- a/src/shell/capture/capture.cpp +++ b/src/shell/capture/capture.cpp @@ -45,6 +45,7 @@ #define REAPERAPI_WANT_Main_OnCommand #define REAPERAPI_WANT_Main_SaveProject #define REAPERAPI_WANT_Master_GetTempo +#define REAPERAPI_WANT_ShowConsoleMsg #define REAPERAPI_WANT_TimeMap_GetTimeSigAtTime #include "reaper_plugin_functions.h" @@ -228,12 +229,35 @@ CaptureName captureNameFor(const std::vector& sourceNames, return composeCaptureName(in); } -bool collapseCapturedFileToMono(const std::string& absolutePath) { +namespace { + +// One console line per genuine collapse failure — silence here is what made a failed +// rewrite read exactly like a legitimately stereo capture. Deliberately does NOT claim +// the captured bytes are intact: a 0-byte render can reach this branch too (it passes the +// exists/bounds gates upstream; see docs/TODO.md), and this path never verified the bytes +// it's reporting on. +void reportCollapseFailure(const std::string& absolutePath, const char* what, + const char* consoleLabel) { + ShowConsoleMsg((std::string(consoleLabel) + ": the lossless mono collapse " + + std::string(what) + " -- " + absolutePath + + " already reached the bank; only the size win from the collapse " + "was lost.\n").c_str()); +} + +} // namespace + +MonoCollapseOutcome collapseCapturedFileToMono(const std::string& absolutePath, + const char* consoleLabel) { const std::vector bytes = util::readFileBytes(absolutePath); - if (bytes.empty()) return false; + if (bytes.empty()) { + // Failed, not Declined: the read that would have decided never happened, so + // "the channels differ" is a claim this path cannot make. + reportCollapseFailure(absolutePath, "could not read the captured file", consoleLabel); + return MonoCollapseOutcome::Failed; + } const MonoCollapse collapse = collapseToMono(bytes); - if (!collapse.collapsed) return false; + if (!collapse.collapsed) return MonoCollapseOutcome::Declined; // Sibling temp + rename, NOT an in-place truncating write: this runs unconditionally // on the deterministic offline path (which never reopened its render for write before @@ -245,7 +269,10 @@ bool collapseCapturedFileToMono(const std::string& absolutePath) { const std::string tempPath = absolutePath + ".moncollapse.tmp"; { std::ofstream out(tempPath, std::ios::binary | std::ios::trunc); - if (!out) return false; + if (!out) { + reportCollapseFailure(absolutePath, "could not open its temporary file", consoleLabel); + return MonoCollapseOutcome::Failed; + } out.write(reinterpret_cast(collapse.bytes.data()), static_cast(collapse.bytes.size())); const bool wroteOk = static_cast(out); @@ -253,16 +280,18 @@ bool collapseCapturedFileToMono(const std::string& absolutePath) { if (!wroteOk) { std::error_code ec; std::filesystem::remove(tempPath, ec); - return false; + reportCollapseFailure(absolutePath, "could not write the rebuilt file", consoleLabel); + return MonoCollapseOutcome::Failed; } } std::error_code ec; std::filesystem::rename(tempPath, absolutePath, ec); if (ec) { std::filesystem::remove(tempPath, ec); // don't leave litter on a failed rename - return false; + reportCollapseFailure(absolutePath, "could not replace the captured file", consoleLabel); + return MonoCollapseOutcome::Failed; } - return true; + return MonoCollapseOutcome::Collapsed; } void stampCaptureSample(Sample& s, const CaptureRequest& req, @@ -473,11 +502,14 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { // (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 file is read again by stampCaptureSample below; the duplicate read is a - // once-per-capture cost on an already-warm file.) 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. + // (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 + // 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)); @@ -525,7 +557,8 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { // renderer's file 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 bool collapsedToMono = collapseCapturedFileToMono(expectedPath); + const MonoCollapseOutcome collapseOutcome = + collapseCapturedFileToMono(expectedPath); // Record the request's own bounds (exact) rather than re-measuring the file. Sample s; @@ -553,8 +586,7 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { result.message = "Captured [" + std::to_string(request.startSeconds) + "s, " + std::to_string(request.endSeconds) + "s] -> " + - paths.relativePath + - (collapsedToMono ? " (collapsed to mono)" : ""); + paths.relativePath + monoCollapseSuffix(collapseOutcome); return result; } diff --git a/src/shell/capture/capture.h b/src/shell/capture/capture.h index 71546d2..37209f7 100644 --- a/src/shell/capture/capture.h +++ b/src/shell/capture/capture.h @@ -13,6 +13,7 @@ #include "core/model/bank_model.h" #include "core/capture/capture_name.h" // CaptureName — label + file-stem base #include "core/capture/render_settings.h" // TailMode — the three-state tail contract +#include "core/capture/wav_codec.h" // MonoCollapseOutcome — the collapse's report // Forward-declared, never dereferenced here — only the REAPER-facing .cpp touches these. class MediaTrack; @@ -79,14 +80,14 @@ struct CaptureRequest { // Every failure is an explicit code, never a thrown exception across the REAPER boundary. enum class CaptureStatus { Ok, - NoProject, // no active project to render / resolve a bank folder - EmptyRange, // start >= end: nothing to render - UnsupportedMode, // backend does not implement this source mode - UnsupportedFormat, // requested bit depth has no known REAPER blob (Float32 only) - RenderFailed, // the render action ran but produced no output file - TransportBusy, // realtime backend: transport already playing/recording — refused - MultiTrackRange, // a ranged item capture whose items span >1 track — would render N files - BoundsMismatch, // the rendered file's frame count is not the requested window's + NoProject, // no active project to render / resolve a bank folder + EmptyRange, // start >= end: nothing to render + UnsupportedMode, // backend does not implement this source mode + UnsupportedFormat, // requested bit depth has no known REAPER blob (Float32 only) + 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 }; struct CaptureResult { @@ -126,9 +127,13 @@ CaptureName captureNameFor(const std::vector& sourceNames, // bit-identical (the pure `collapseToMono` decides). Every other file is left // untouched, byte for byte, so the not-collapsed path is exactly what the backend // produced. Must run BEFORE stampCaptureSample, which measures the landed file. -// Returns whether the file was actually rewritten (collapsed AND the write landed) — -// callers use it to make the collapse observable in the reported CaptureResult. -bool collapseCapturedFileToMono(const std::string& absolutePath); +// A Failed outcome is ALSO logged to the console here, because a successful capture's +// CaptureResult::message is not printed by any caller — the return value alone would +// leave a genuine I/O failure indistinguishable from a legitimately stereo capture. +// `consoleLabel` matches each caller's own console-prefix convention (offline: +// "ReaSampler capture"; realtime: "ReaSampler realtime capture"). +MonoCollapseOutcome collapseCapturedFileToMono(const std::string& absolutePath, + const char* consoleLabel = "ReaSampler capture"); // Stamps the metadata shared by both backends onto `s`: trackGuids (echoed from the // request) + channelCount (measured from the produced file's `fmt`; 0/unknown as the diff --git a/src/shell/capture/capture_orchestrator.cpp b/src/shell/capture/capture_orchestrator.cpp index e12450e..09ca4c7 100644 --- a/src/shell/capture/capture_orchestrator.cpp +++ b/src/shell/capture/capture_orchestrator.cpp @@ -184,17 +184,14 @@ CaptureResult renderOffline(CaptureScope scope, { // Refused BEFORE anything is touched, so the refusal path has nothing to // restore. This is the seam BOTH a fresh capture and a recipe replay cross, so - // neither can land the multi-stem render the predicate names. - if (isMultiTrackRangedItemRender(scope, req.sourceMode, - static_cast(sourceTracks.size()))) + // neither can land the multi-stem render the predicate names — and both scopes + // reach it, so a track capture and a ranged item capture refuse alike. + if (isMultiTrackStemRender(req.sourceMode, + static_cast(sourceTracks.size()))) { CaptureResult refused; - refused.status = CaptureStatus::MultiTrackRange; - refused.message = - "This range is narrower than the selected items, so it renders through " - "their tracks -- and those items span more than one track, which this " - "shape cannot land as a single file. Capture one track's items at a " - "time, or make the range match the items' extent."; + refused.status = CaptureStatus::MultiTrackSelection; + refused.message = multiTrackRefusalMessage(scope); return refused; } diff --git a/src/shell/capture/capture_realtime_finalize.cpp b/src/shell/capture/capture_realtime_finalize.cpp index b3a437f..2cc68ec 100644 --- a/src/shell/capture/capture_realtime_finalize.cpp +++ b/src/shell/capture/capture_realtime_finalize.cpp @@ -186,7 +186,8 @@ CaptureResult finalizeRecording(ReaProject* proj, MediaTrack* temp, // Channel-domain rewrite, after the frame-domain trim so it acts on the final // frame set; it preserves the frame count, so the trimmed length above still holds. - const bool collapsedToMono = collapseCapturedFileToMono(destPath); + const MonoCollapseOutcome collapseOutcome = + collapseCapturedFileToMono(destPath, "ReaSampler realtime capture"); // Pure recorded-capture -> Sample mapping (identity, bounds echo, tier). RecordedCapture cap; @@ -225,8 +226,7 @@ CaptureResult finalizeRecording(ReaProject* proj, MediaTrack* temp, std::to_string(request.startSeconds) + "s, " + std::to_string(request.endSeconds) + "s] (recorded " + std::to_string(result.sample.lengthSeconds) + "s) -> " + - paths.relativePath + - (collapsedToMono ? " (collapsed to mono)" : ""); + paths.relativePath + monoCollapseSuffix(collapseOutcome); return result; } diff --git a/src/shell/capture/capture_realtime_shell.cpp b/src/shell/capture/capture_realtime_shell.cpp index e6ae9b2..41a307b 100644 --- a/src/shell/capture/capture_realtime_shell.cpp +++ b/src/shell/capture/capture_realtime_shell.cpp @@ -30,8 +30,10 @@ // TAP: the hidden temp track receives a send FROM each selected source track // (CreateTrackSend(source, temp)) and records its own output (B_MAINSEND=0, so // it never sums back into the master — no feedback, no monitoring double). -// Multiple selected tracks sum in the one temp track, matching how offline -// track scope handles a multi-track selection. +// Multiple selected tracks sum in the one temp track — a real mix, which is why +// realtime accepts a multi-track selection where the offline track scope refuses +// it (the offline render source is read as emitting one file per track, DAW- +// unverified — see src/shell/capture/CLAUDE.md §Gotchas). // // Why this needs no FxBypassGuard: CreateTrackSend defaults to I_SENDMODE=0 // (post-fader), which taps the source track after its own FX/fader/pan — its diff --git a/tests/test_render_settings.cpp b/tests/test_render_settings.cpp index 9143197..3db957f 100644 --- a/tests/test_render_settings.cpp +++ b/tests/test_render_settings.cpp @@ -243,40 +243,74 @@ static void testRangedItemScopeRendersTimeBounded() { CHECK(fxBypassPlanFor(CaptureScope::Item).bypassSelfFx); } -static void testMultiTrackRangedItemRenderIsNamedForRefusal() { - // The one shape that cannot land: a ranged item capture (item scope re-sourced to - // the selected-tracks render) whose items span more than one track. That source - // is INFERRED to render one file per track with no single-file bit available - // (unverified; see src/shell/capture/CLAUDE.md §Gotchas for what that inference - // rests on), so N stems would collapse onto one render pattern and one track's - // audio would land as a successful capture. - CHECK(isMultiTrackRangedItemRender(CaptureScope::Item, - SourceMode::SelectedTracks, 2)); - CHECK(isMultiTrackRangedItemRender(CaptureScope::Item, - SourceMode::SelectedTracks, 7)); +static void testMultiTrackStemRenderIsNamedForRefusal() { + // The one shape that cannot land: a selected-tracks render over more than one + // track. That source is INFERRED to render one file per track with no single-file + // bit available (unverified; see src/shell/capture/CLAUDE.md §Gotchas for what that + // inference rests on), so N stems would collapse onto one render pattern and one + // track's audio would land as a successful capture. + CHECK(isMultiTrackStemRender(SourceMode::SelectedTracks, 2)); + CHECK(isMultiTrackStemRender(SourceMode::SelectedTracks, 7)); - // One track is the whole point of the re-source — it must still render. - CHECK(!isMultiTrackRangedItemRender(CaptureScope::Item, - SourceMode::SelectedTracks, 1)); - CHECK(!isMultiTrackRangedItemRender(CaptureScope::Item, - SourceMode::SelectedTracks, 0)); + // One track is the common case for BOTH scopes — it must still render. This is the + // regression floor for the plain single-track track capture. + CHECK(!isMultiTrackStemRender(SourceMode::SelectedTracks, 1)); + CHECK(!isMultiTrackStemRender(SourceMode::SelectedTracks, 0)); // A full-extent item capture keeps the selected-items source, whose single-file // bit already sums a multi-track item selection into one file. - CHECK(!isMultiTrackRangedItemRender(CaptureScope::Item, - SourceMode::SelectedItems, 3)); + CHECK(!isMultiTrackStemRender(SourceMode::SelectedItems, 3)); + // Razor's single-file bit does the same; master mix is one file by definition. + CHECK(!isMultiTrackStemRender(SourceMode::RazorArea, 3)); + CHECK(!isMultiTrackStemRender(SourceMode::MasterMix, 3)); + // Realtime never reaches the offline render at all (sends sum into one temp track). + CHECK(!isMultiTrackStemRender(SourceMode::Realtime, 3)); - // Track scope renders through the same source and is deliberately untouched here - // — that per-track output predates the item re-source and is filed in docs/TODO.md. - CHECK(!isMultiTrackRangedItemRender(CaptureScope::Track, - SourceMode::SelectedTracks, 3)); + // The predicate is reachable from the mappings it guards: BOTH the ranged item + // capture's source mode and the track scope's resolve to the one it names, while a + // full-extent item capture does not. + CHECK(isMultiTrackStemRender(sourceModeForScope(CaptureScope::Item, false), 2)); + CHECK(isMultiTrackStemRender(sourceModeForScope(CaptureScope::Track, false), 2)); + CHECK(isMultiTrackStemRender(sourceModeForScope(CaptureScope::Track, true), 2)); + CHECK(!isMultiTrackStemRender(sourceModeForScope(CaptureScope::Item, true), 2)); +} - // The predicate is reachable from the mapping it guards: the ranged item capture's - // own source mode is the one it names. - CHECK(isMultiTrackRangedItemRender(CaptureScope::Item, - sourceModeForScope(CaptureScope::Item, false), 2)); - CHECK(!isMultiTrackRangedItemRender(CaptureScope::Item, - sourceModeForScope(CaptureScope::Item, true), 2)); +static void testRefusalMessagesAreSiblingsWithDistinctExits() { + const std::string item = multiTrackRefusalMessage(CaptureScope::Item); + const std::string track = multiTrackRefusalMessage(CaptureScope::Track); + + // Both name the same reason — a shape that cannot land as one file — so a user who + // hits the mistake in either scope reads one story, not two. + CHECK(item.find("single file") != std::string::npos); + CHECK(track.find("single file") != std::string::npos); + + // And both name a way out. The shared one is "one track at a time"; each scope then + // adds the exit only it has (widen the range / capture the folder). + CHECK(item.find("one track's items at a time") != std::string::npos); + CHECK(item.find("range match the items' extent") != std::string::npos); + CHECK(track.find("one track at a time") != std::string::npos); + CHECK(track.find("folder") != std::string::npos); + + // Distinct texts: the track message must not be the item message's wording about + // items and ranges, which would misdescribe what the user actually did. + CHECK(item != track); + CHECK(track.find("selected items") == std::string::npos); +} + +// Golden literals: docs/verify-track-scope-multitrack.md §2 quotes the track message as +// an exact console match. A substring check alone leaves that doc free to drift from +// whatever ships, so pin both strings byte-for-byte here. +static void testRefusalMessagesMatchGoldenLiterals() { + CHECK(multiTrackRefusalMessage(CaptureScope::Item) == + "This range is narrower than the selected items, so it renders " + "through their tracks -- and those items span more than one track, " + "which this shape cannot land as a single file. Capture one track's " + "items at a time, or make the range match the items' extent."); + CHECK(multiTrackRefusalMessage(CaptureScope::Track) == + "A track capture renders the selected tracks through the master, " + "and more than one track cannot land as a single file. Capture one " + "track at a time, or route them into a folder/bus track and capture " + "that (a folder's own output is its children summed)."); } // --- inferRangeSource: razor-else-time (orthogonal to scope) ----------------- @@ -376,7 +410,9 @@ int main() { testRazorUnionBounds(); testScopeSourceModes(); testRangedItemScopeRendersTimeBounded(); - testMultiTrackRangedItemRenderIsNamedForRefusal(); + testMultiTrackStemRenderIsNamedForRefusal(); + testRefusalMessagesAreSiblingsWithDistinctExits(); + testRefusalMessagesMatchGoldenLiterals(); testRangeInference(); testItemScopeBypassesEverythingButTake(); testTrackScopeKeepsSelfBypassesAncestorsAndMaster(); diff --git a/tests/test_wav_codec.cpp b/tests/test_wav_codec.cpp index c3d8204..ec39a03 100644 --- a/tests/test_wav_codec.cpp +++ b/tests/test_wav_codec.cpp @@ -775,6 +775,26 @@ static void testCollapsePreservesQuietNaNBitPattern() { if (!pcm.empty()) CHECK(bitsFromFloat(pcm[0]) == kQuietNaNBits); } +// The file-side collapse has three outcomes, and a genuine I/O failure once reported +// identically to "the channels differ" — a stereo file landing under a plain Ok. The +// report is where they must part: Declined stays silent (a capture with nothing to +// collapse reads as it always did), and the other two say different things. +// Pins the three suffix STRINGS, not user-observable behavior — see wav_codec.h's +// monoCollapseSuffix doc: this text reaches no one on a successful capture. +static void testCollapseOutcomeSuffixesAreDistinctStrings() { + const std::string declined = monoCollapseSuffix(MonoCollapseOutcome::Declined); + const std::string collapsed = monoCollapseSuffix(MonoCollapseOutcome::Collapsed); + const std::string failed = monoCollapseSuffix(MonoCollapseOutcome::Failed); + + CHECK(declined.empty()); + CHECK(!collapsed.empty()); + CHECK(!failed.empty()); + CHECK(collapsed != failed); + // The failure must read as a failure, not as a quieter success. + CHECK(failed.find("failed") != std::string::npos); + CHECK(collapsed.find("failed") == std::string::npos); +} + int main() { testParseCanonicalStereo(); testParseMonoAndLeadingChunk(); @@ -810,6 +830,7 @@ int main() { testCollapseDeclinesOnUnparseableBytes(); testCollapseChangesContentHash(); testCollapsePreservesQuietNaNBitPattern(); + testCollapseOutcomeSuffixesAreDistinctStrings(); if (g_fail == 0) std::printf("wav_codec: all tests passed\n"); else std::printf("wav_codec: %d CHECK(s) FAILED\n", g_fail);