Fix eight review findings on the render-bounds diagnostics
Corrects a false comment example, fixes two tests that couldn't detect their own regressions, adds two more read-back checkpoints around Main_OnCommand so a drift report self-locates, guards a spurious zero-vs-zero coincidence match, and softens two sentences that overclaimed cause or defect.
This commit is contained in:
@@ -70,7 +70,11 @@ std::string describeBoundsDrift(double reqStart, double reqEnd,
|
|||||||
// straight back, so anything but the same bits is a value REAPER changed.
|
// straight back, so anything but the same bits is a value REAPER changed.
|
||||||
if (storedStart == reqStart && storedEnd == reqEnd) return {};
|
if (storedStart == reqStart && storedEnd == reqEnd) return {};
|
||||||
|
|
||||||
std::string s = "REAPER did not keep the render bounds it was handed -- asked for [" +
|
// Says only that the two differ, not why -- a legitimate clamp (negative start,
|
||||||
|
// end past project end) reads back differently for the same reason a precision
|
||||||
|
// defect would, and this sentence cannot tell those apart.
|
||||||
|
std::string s = "REAPER read back different render bounds than it was handed -- "
|
||||||
|
"asked for [" +
|
||||||
exactly(reqStart) + "s, " + exactly(reqEnd) + "s), read back [" +
|
exactly(reqStart) + "s, " + exactly(reqEnd) + "s), read back [" +
|
||||||
exactly(storedStart) + "s, " + exactly(storedEnd) + "s).";
|
exactly(storedStart) + "s, " + exactly(storedEnd) + "s).";
|
||||||
if (sampleRate > 0) {
|
if (sampleRate > 0) {
|
||||||
|
|||||||
@@ -2,7 +2,9 @@
|
|||||||
// render_window — pure frame arithmetic for a capture's requested window: the
|
// render_window — pure frame arithmetic for a capture's requested window: the
|
||||||
// frame count a project-time range occupies, whether a render whose bounds come
|
// frame count a project-time range occupies, whether a render whose bounds come
|
||||||
// from the selected items' own extent already prints that window, and the two
|
// from the selected items' own extent already prints that window, and the two
|
||||||
// diagnostics that say where a short render lost its frames.
|
// diagnostics that bound a short render without locating it: whether the stored
|
||||||
|
// RENDER_* bounds round-tripped, and whether the shortfall matches a millisecond-
|
||||||
|
// floor coincidence.
|
||||||
// NO REAPER types; unit-tested by tests/test_render_window.cpp.
|
// NO REAPER types; unit-tested by tests/test_render_window.cpp.
|
||||||
|
|
||||||
#include <string>
|
#include <string>
|
||||||
@@ -54,7 +56,7 @@ bool itemExtentPrintsWindow(double reqStart, double reqEnd,
|
|||||||
// A COINCIDENCE OF COUNTS, not a claim about how anything resolved the end: nothing
|
// A COINCIDENCE OF COUNTS, not a claim about how anything resolved the end: nothing
|
||||||
// renders from this number and no capture path asks for it. Whole-millisecond values
|
// renders from this number and no capture path asks for it. Whole-millisecond values
|
||||||
// are recognized within a nanosecond, because a decimal millisecond is not always one
|
// are recognized within a nanosecond, because a decimal millisecond is not always one
|
||||||
// in binary (0.029 * 1000 lands just below 29) and a bare floor would drop a
|
// in binary (1.007 * 1000 lands just below 1007) and a bare floor would drop a
|
||||||
// millisecond from a window already on the grid. A nanosecond is far under one frame
|
// millisecond from a window already on the grid. A nanosecond is far under one frame
|
||||||
// at any rate we render, so a real sub-millisecond remainder still floors.
|
// at any rate we render, so a real sub-millisecond remainder still floors.
|
||||||
long long msFlooredEndFrameCount(double startSeconds, double endSeconds,
|
long long msFlooredEndFrameCount(double startSeconds, double endSeconds,
|
||||||
|
|||||||
@@ -463,14 +463,18 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) {
|
|||||||
|
|
||||||
// Silent unless a bound came back changed. Fires on EVERY tail mode on purpose:
|
// Silent unless a bound came back changed. Fires on EVERY tail mode on purpose:
|
||||||
// only None is judged against its window after the render, so this is the sole
|
// only None is judged against its window after the render, so this is the sole
|
||||||
// signal an Auto/Manual capture was shortened before it ever started.
|
// signal an Auto/Manual capture was shortened before it ever started. Named by
|
||||||
{
|
// checkpoint so a DAW observation is self-locating: three reads bracket the two
|
||||||
|
// places REAPER could quantize — the store, and the render itself.
|
||||||
|
auto reportDrift = [&](const char* checkpoint, double atStart, double atEnd) {
|
||||||
const std::string drift =
|
const std::string drift =
|
||||||
describeBoundsDrift(request.startSeconds, request.endSeconds,
|
describeBoundsDrift(request.startSeconds, request.endSeconds,
|
||||||
storedStart, storedEnd, effectiveSampleRate);
|
atStart, atEnd, effectiveSampleRate);
|
||||||
if (!drift.empty())
|
if (!drift.empty())
|
||||||
ShowConsoleMsg(("ReaSampler capture: " + drift + "\n").c_str());
|
ShowConsoleMsg(("ReaSampler capture (" + std::string(checkpoint) + "): " +
|
||||||
}
|
drift + "\n").c_str());
|
||||||
|
};
|
||||||
|
reportDrift("at store", storedStart, storedEnd);
|
||||||
|
|
||||||
GetSetProjectInfo(proj, "RENDER_CHANNELS",
|
GetSetProjectInfo(proj, "RENDER_CHANNELS",
|
||||||
static_cast<double>(request.channelCount), true);
|
static_cast<double>(request.channelCount), true);
|
||||||
@@ -504,8 +508,22 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) {
|
|||||||
}
|
}
|
||||||
setProjString(proj, "RENDER_FORMAT", fmtBase64);
|
setProjString(proj, "RENDER_FORMAT", fmtBase64);
|
||||||
|
|
||||||
|
// Read again right here: a mismatch against the store-time read-back above
|
||||||
|
// means something between the two writes and this line moved the bounds,
|
||||||
|
// before the render ever ran.
|
||||||
|
reportDrift("before render",
|
||||||
|
GetSetProjectInfo(proj, "RENDER_STARTPOS", 0.0, false),
|
||||||
|
GetSetProjectInfo(proj, "RENDER_ENDPOS", 0.0, false));
|
||||||
|
|
||||||
Main_OnCommand(kActionRenderUsingMostRecentSettings, 0);
|
Main_OnCommand(kActionRenderUsingMostRecentSettings, 0);
|
||||||
|
|
||||||
|
// And once more here, while the guard above is still live and before it restores
|
||||||
|
// anything: only the gap between this read and the one immediately above can be
|
||||||
|
// the render itself.
|
||||||
|
reportDrift("after render",
|
||||||
|
GetSetProjectInfo(proj, "RENDER_STARTPOS", 0.0, false),
|
||||||
|
GetSetProjectInfo(proj, "RENDER_ENDPOS", 0.0, false));
|
||||||
|
|
||||||
// Main_OnCommand returns void, so a failed render is silent — stat the
|
// Main_OnCommand returns void, so a failed render is silent — stat the
|
||||||
// expected output path to detect it.
|
// expected output path to detect it.
|
||||||
const std::string expectedPath = paths.absoluteDir + "/" + paths.fileName;
|
const std::string expectedPath = paths.absoluteDir + "/" + paths.fileName;
|
||||||
|
|||||||
@@ -369,6 +369,11 @@ RealtimeRecordBackend::begin(const CaptureRequest& request,
|
|||||||
|
|
||||||
// recordWindowEnd extends past the range end for a tail mode so the
|
// recordWindowEnd extends past the range end for a tail mode so the
|
||||||
// transport captures the decay; cursor + time selection are restored by restore().
|
// transport captures the decay; cursor + time selection are restored by restore().
|
||||||
|
// `[verify — DAW]` whether rs/re come back changed on this isSet=true call: the SDK
|
||||||
|
// header names both `double*` but documents no read-back semantics for either
|
||||||
|
// direction, and nothing here reads rs/re again after the call to notice. Lower
|
||||||
|
// stakes than the offline RENDER_* store: completion is driven by the play cursor
|
||||||
|
// reaching the range end (tick(), below), not by re-reading this pair.
|
||||||
double rs = request.startSeconds, re = st->recordWindowEnd_;
|
double rs = request.startSeconds, re = st->recordWindowEnd_;
|
||||||
GetSet_LoopTimeRange(true, false, &rs, &re, false);
|
GetSet_LoopTimeRange(true, false, &rs, &re, false);
|
||||||
SetEditCurPos(request.startSeconds, false, false);
|
SetEditCurPos(request.startSeconds, false, false);
|
||||||
|
|||||||
@@ -100,12 +100,17 @@ BoundsVerdict checkRenderedBounds(const std::string& renderedPath,
|
|||||||
if (renderHonoredBounds(expectedFrames, actualFrames)) return v;
|
if (renderHonoredBounds(expectedFrames, actualFrames)) return v;
|
||||||
|
|
||||||
// Says whether this shortfall has the one shape two live short renders already
|
// Says whether this shortfall has the one shape two live short renders already
|
||||||
// matched to the frame, so every refusal from here on adds to (or breaks) that
|
// matched to the frame: the END alone floored to the millisecond. Checked against
|
||||||
// evidence instead of needing the arithmetic done by hand. A count coincidence
|
// the END only -- a refusal whose START is also off-grid and independently floored
|
||||||
// only — it does not establish how the render resolved anything.
|
// would not match this shape, and this note's silence on that refusal is this
|
||||||
|
// check not covering it, not the coincidence breaking. Excludes 0, which every
|
||||||
|
// sub-millisecond window (a legitimate day-one capture) also floors to, and which
|
||||||
|
// would otherwise match a render that produced nothing. A count coincidence only —
|
||||||
|
// it does not establish how the render resolved anything.
|
||||||
|
const long long msFlooredEnd =
|
||||||
|
msFlooredEndFrameCount(request.startSeconds, request.endSeconds, rate);
|
||||||
const std::string msNote =
|
const std::string msNote =
|
||||||
actualFrames == msFlooredEndFrameCount(request.startSeconds,
|
(msFlooredEnd > 0 && actualFrames == msFlooredEnd)
|
||||||
request.endSeconds, rate)
|
|
||||||
? " Those are exactly the frames this window holds with its end floored to"
|
? " Those are exactly the frames this window holds with its end floored to"
|
||||||
" the millisecond -- a match on the count, not a measured cause."
|
" the millisecond -- a match on the count, not a measured cause."
|
||||||
: std::string();
|
: std::string();
|
||||||
|
|||||||
@@ -278,11 +278,16 @@ static void testWindowAlreadyOnTheMillisecondGridLosesNothing() {
|
|||||||
|
|
||||||
static void testOneFrameOfRemainderStillFloors() {
|
static void testOneFrameOfRemainderStillFloors() {
|
||||||
// The whole-millisecond tolerance must sit far below a frame, or it would swallow
|
// The whole-millisecond tolerance must sit far below a frame, or it would swallow
|
||||||
// the very remainder this diagnostic exists to find. One frame at 48 kHz is 20.8 us
|
// the very remainder this diagnostic exists to find. A remainder JUST BELOW a
|
||||||
// — four orders of magnitude above the nanosecond tolerance.
|
// millisecond boundary is the discriminating case: one frame short of 1.0 s is
|
||||||
|
// 999.979166 ms, only ~0.0208 ms off the next whole millisecond. The shipped
|
||||||
|
// nanosecond tolerance still floors it down; a tolerance any wider than ~0.021 ms
|
||||||
|
// would snap it up to the millisecond instead and this test would then see 48000,
|
||||||
|
// not 47952 — which is what would fail if the tolerance regressed to something
|
||||||
|
// that wide.
|
||||||
const double oneFrame = 1.0 / 48000.0;
|
const double oneFrame = 1.0 / 48000.0;
|
||||||
CHECK(frameCountFor(0.0, 1.0 + oneFrame, 48000) == 48001);
|
CHECK(frameCountFor(0.0, 1.0 - oneFrame, 48000) == 47999);
|
||||||
CHECK(msFlooredEndFrameCount(0.0, 1.0 + oneFrame, 48000) == 48000);
|
CHECK(msFlooredEndFrameCount(0.0, 1.0 - oneFrame, 48000) == 47952);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void testMillisecondFloorAt44100WhereAMillisecondIsNotWholeFrames() {
|
static void testMillisecondFloorAt44100WhereAMillisecondIsNotWholeFrames() {
|
||||||
@@ -333,12 +338,22 @@ static void testADriftedEndNamesBothWindowsAndBothCounts() {
|
|||||||
static void testTheReportPrintsEnoughDigitsToShowTheDrift() {
|
static void testTheReportPrintsEnoughDigitsToShowTheDrift() {
|
||||||
// A report whose two numbers print identically is evidence of nothing. Two ends a
|
// A report whose two numbers print identically is evidence of nothing. Two ends a
|
||||||
// single ULP apart — far under the sixth decimal a shorter rendering would stop at
|
// single ULP apart — far under the sixth decimal a shorter rendering would stop at
|
||||||
// — must still read as two different numbers.
|
// — must still read as two different numbers. Pinned as the actual %.17g literals
|
||||||
|
// (not the needle the two ends share, "s)", which occurs at every precision and so
|
||||||
|
// proves nothing): a report that regressed to a shorter format like %.6g would
|
||||||
|
// print the same six significant digits for both ends, and these two `contains`
|
||||||
|
// checks would then fail.
|
||||||
const double asked = 4.067797;
|
const double asked = 4.067797;
|
||||||
const double stored = std::nextafter(asked, 5.0);
|
const double stored = std::nextafter(asked, 5.0);
|
||||||
|
char askedBuf[32], storedBuf[32];
|
||||||
|
std::snprintf(askedBuf, sizeof(askedBuf), "%.17g", asked);
|
||||||
|
std::snprintf(storedBuf, sizeof(storedBuf), "%.17g", stored);
|
||||||
|
CHECK(std::string(askedBuf) != std::string(storedBuf));
|
||||||
|
|
||||||
const std::string s = describeBoundsDrift(0.0, asked, 0.0, stored, 48000);
|
const std::string s = describeBoundsDrift(0.0, asked, 0.0, stored, 48000);
|
||||||
CHECK(!s.empty());
|
CHECK(!s.empty());
|
||||||
CHECK(!contains(s, "4.067797s, read back [0s, 4.067797s)"));
|
CHECK(contains(s, askedBuf));
|
||||||
|
CHECK(contains(s, storedBuf));
|
||||||
}
|
}
|
||||||
|
|
||||||
static void testADriftedStartIsCaughtToo() {
|
static void testADriftedStartIsCaughtToo() {
|
||||||
|
|||||||
Reference in New Issue
Block a user