Ψ-W3 remediation: fix the verify doc's wrong render source, add missing content checks, soften unverified claims
Corrected the Render-dialog source name the refusal's evidence depends on, added a by-ear content check and a file-size channel proxy, and stopped two comments from overclaiming.
This commit is contained in:
+27
-5
@@ -527,11 +527,16 @@ message where a correct summed file used to land.
|
||||
|
||||
**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 summed
|
||||
file", the refusal is over-strict for the TRACK scope and should be narrowed back; the
|
||||
ITEM-scope half stays regardless, since a per-item-track render is not the sum the user
|
||||
asked for either way. If it comes back "one file per track", nothing to do and the
|
||||
inference is retired into fact.
|
||||
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.** Narrowing the refusal must keep the ITEM scope
|
||||
refusing, must keep `renderOffline` the single seam (so a recipe replay cannot diverge
|
||||
@@ -645,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.
|
||||
|
||||
@@ -30,9 +30,15 @@ Read off:
|
||||
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.
|
||||
|
||||
Repeat with a **razor area** on `A` over the same 10–12 s span and no time selection: same
|
||||
three readings.
|
||||
- **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.
|
||||
@@ -69,7 +75,10 @@ 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 (stems)* — the dialog wording for `&128`.
|
||||
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
|
||||
@@ -122,16 +131,24 @@ 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). Read off:
|
||||
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:
|
||||
|
||||
- The panel card reports **1 channel**.
|
||||
- The console shows **no** `the lossless mono collapse ... landed intact, as captured.`
|
||||
line.
|
||||
- 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`, `testCollapseOutcomesReportDistinctly`), and its console line
|
||||
has never been seen in a running REAPER. If you ever do see it, the capture in question is
|
||||
intact and correctly measured — only the size win was lost.
|
||||
(`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).
|
||||
|
||||
@@ -117,6 +117,11 @@ bool isMultiTrackStemRender(SourceMode mode, int sourceTrackCount) {
|
||||
}
|
||||
|
||||
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 "
|
||||
|
||||
@@ -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 <string>
|
||||
|
||||
@@ -129,7 +129,11 @@ enum class MonoCollapseOutcome {
|
||||
|
||||
// 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.
|
||||
// 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) -----------------------------------------
|
||||
|
||||
@@ -231,23 +231,28 @@ CaptureName captureNameFor(const std::vector<std::string>& sourceNames,
|
||||
|
||||
namespace {
|
||||
|
||||
// One console line per genuine collapse failure. The capture itself is intact and was
|
||||
// measured after this step, so the failure costs only the size win — but silence here is
|
||||
// what made a failed rewrite read exactly like a legitimately stereo capture.
|
||||
void reportCollapseFailure(const std::string& absolutePath, const char* what) {
|
||||
ShowConsoleMsg(("ReaSampler capture: the lossless mono collapse " + std::string(what) +
|
||||
" -- " + absolutePath +
|
||||
" landed intact, as captured.\n").c_str());
|
||||
// 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) {
|
||||
MonoCollapseOutcome collapseCapturedFileToMono(const std::string& absolutePath,
|
||||
const char* consoleLabel) {
|
||||
const std::vector<std::uint8_t> bytes = util::readFileBytes(absolutePath);
|
||||
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");
|
||||
reportCollapseFailure(absolutePath, "could not read the captured file", consoleLabel);
|
||||
return MonoCollapseOutcome::Failed;
|
||||
}
|
||||
|
||||
@@ -265,7 +270,7 @@ MonoCollapseOutcome collapseCapturedFileToMono(const std::string& absolutePath)
|
||||
{
|
||||
std::ofstream out(tempPath, std::ios::binary | std::ios::trunc);
|
||||
if (!out) {
|
||||
reportCollapseFailure(absolutePath, "could not open its temporary file");
|
||||
reportCollapseFailure(absolutePath, "could not open its temporary file", consoleLabel);
|
||||
return MonoCollapseOutcome::Failed;
|
||||
}
|
||||
out.write(reinterpret_cast<const char*>(collapse.bytes.data()),
|
||||
@@ -275,7 +280,7 @@ MonoCollapseOutcome collapseCapturedFileToMono(const std::string& absolutePath)
|
||||
if (!wroteOk) {
|
||||
std::error_code ec;
|
||||
std::filesystem::remove(tempPath, ec);
|
||||
reportCollapseFailure(absolutePath, "could not write the rebuilt file");
|
||||
reportCollapseFailure(absolutePath, "could not write the rebuilt file", consoleLabel);
|
||||
return MonoCollapseOutcome::Failed;
|
||||
}
|
||||
}
|
||||
@@ -283,7 +288,7 @@ MonoCollapseOutcome collapseCapturedFileToMono(const std::string& absolutePath)
|
||||
std::filesystem::rename(tempPath, absolutePath, ec);
|
||||
if (ec) {
|
||||
std::filesystem::remove(tempPath, ec); // don't leave litter on a failed rename
|
||||
reportCollapseFailure(absolutePath, "could not replace the captured file");
|
||||
reportCollapseFailure(absolutePath, "could not replace the captured file", consoleLabel);
|
||||
return MonoCollapseOutcome::Failed;
|
||||
}
|
||||
return MonoCollapseOutcome::Collapsed;
|
||||
@@ -497,8 +502,9 @@ 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 landed file is read three times on this path — this gate, the mono collapse,
|
||||
// and stampCaptureSample — plus one rewrite when the collapse fires; a
|
||||
// (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
|
||||
|
||||
@@ -80,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
|
||||
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
|
||||
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 {
|
||||
@@ -130,7 +130,10 @@ CaptureName captureNameFor(const std::vector<std::string>& sourceNames,
|
||||
// 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.
|
||||
MonoCollapseOutcome collapseCapturedFileToMono(const std::string& absolutePath);
|
||||
// `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
|
||||
|
||||
@@ -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 MonoCollapseOutcome collapseOutcome = collapseCapturedFileToMono(destPath);
|
||||
const MonoCollapseOutcome collapseOutcome =
|
||||
collapseCapturedFileToMono(destPath, "ReaSampler realtime capture");
|
||||
|
||||
// Pure recorded-capture -> Sample mapping (identity, bounds echo, tier).
|
||||
RecordedCapture cap;
|
||||
|
||||
@@ -32,7 +32,8 @@
|
||||
// it never sums back into the master — no feedback, no monitoring double).
|
||||
// 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 (that render source cannot express a sum; see shell/capture/CLAUDE.md).
|
||||
// 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
|
||||
|
||||
@@ -297,6 +297,22 @@ static void testRefusalMessagesAreSiblingsWithDistinctExits() {
|
||||
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) -----------------
|
||||
|
||||
static void testRangeInference() {
|
||||
@@ -396,6 +412,7 @@ int main() {
|
||||
testRangedItemScopeRendersTimeBounded();
|
||||
testMultiTrackStemRenderIsNamedForRefusal();
|
||||
testRefusalMessagesAreSiblingsWithDistinctExits();
|
||||
testRefusalMessagesMatchGoldenLiterals();
|
||||
testRangeInference();
|
||||
testItemScopeBypassesEverythingButTake();
|
||||
testTrackScopeKeepsSelfBypassesAncestorsAndMaster();
|
||||
|
||||
@@ -779,7 +779,9 @@ static void testCollapsePreservesQuietNaNBitPattern() {
|
||||
// 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.
|
||||
static void testCollapseOutcomesReportDistinctly() {
|
||||
// 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);
|
||||
@@ -828,7 +830,7 @@ int main() {
|
||||
testCollapseDeclinesOnUnparseableBytes();
|
||||
testCollapseChangesContentHash();
|
||||
testCollapsePreservesQuietNaNBitPattern();
|
||||
testCollapseOutcomesReportDistinctly();
|
||||
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);
|
||||
|
||||
Reference in New Issue
Block a user