diff --git a/src/capture.cpp b/src/capture.cpp index 03c66af..cadc44e 100644 --- a/src/capture.cpp +++ b/src/capture.cpp @@ -13,7 +13,6 @@ #include "capture.h" #include -#include #include #include #include @@ -28,6 +27,7 @@ #define REAPERAPI_WANT_GetSetProjectInfo_String #define REAPERAPI_WANT_GetSet_LoopTimeRange #define REAPERAPI_WANT_Main_OnCommand +#define REAPERAPI_WANT_Main_SaveProject #define REAPERAPI_WANT_Master_GetTempo #define REAPERAPI_WANT_ShowConsoleMsg #include "reaper_plugin_functions.h" @@ -81,45 +81,30 @@ constexpr double kNormalizeDisableAll = static_cast(4 << 16); // 262144 // capture nulls exactly against its source. 16/24-bit int paths require dither // for correctness, which is nondeterministic — unacceptable for a precision tool. // -// DAW-ONLY ASSUMPTION: RENDER_FORMAT takes a base64-encoded sink config, OR -// (per SDK header line ~3114) a simple 4-byte string to use *default* settings -// for that sink type. The WAV sink fourcc is "evaw" ("wave" little-endian). +// API FACT (SDK header line ~3114): GetSetProjectInfo_String("RENDER_FORMAT", ...) +// uses the BASE64-ENCODED string form of the sink config — NOT raw binary bytes. +// Writing raw bytes causes REAPER to silently reject the value and fall back to +// the project's default render format (typically 16-bit/44.1 kHz). This was the +// confirmed root cause of the M3 offline-capture regression. // -// The 4-byte "evaw" fallback is header-DOCUMENTED and safe, but it inherits the -// USER'S default WAV bit depth — not deterministic across machines. To PIN the -// bit depth we build the full WAV sink config blob below. That blob's byte -// layout is REAPER-INTERNAL and is NOT in the SDK header, so it cannot be -// verified here and MUST be confirmed in a running REAPER (capture, then inspect -// the wav header / render stats). If the pinned blob proves wrong on the live -// build, the safe fallback is to write the 4 raw bytes {'e','v','a','w'} to -// RENDER_FORMAT (header-documented default WAV settings) and set the project's -// default WAV depth to float once by hand. -// -// Known REAPER WAV config layout (community-documented, unverifiable here): -// char[4] fourcc = 'evaw' -// int32 bit-depth field: 0=8bit,1=16bit,2=24bit,3=32bit int,4=32bit float(*) -// int32 flags (0 = defaults: little-endian, no BWF/loop metadata) -// (*) the exact float encoding is the single value most needing live confirmation. -void appendInt32LE(std::vector& out, std::int32_t v) { - out.push_back(static_cast(v & 0xFF)); - out.push_back(static_cast((v >> 8) & 0xFF)); - out.push_back(static_cast((v >> 16) & 0xFF)); - out.push_back(static_cast((v >> 24) & 0xFF)); -} +// GROUND TRUTH: base64 string captured from a live REAPER configured to +// WAV / 32-bit float. Decodes to 7 bytes: 65 76 61 77 20 00 00 +// = "evaw" (WAV fourcc, little-endian) + 0x20 (=32, the float bit-depth field) +// + 0x00 0x00 (flags: little-endian, no BWF/loop metadata). +constexpr const char* kRenderFormatWavFloat32 = "ZXZhdyAAAA=="; -// Full WAV sink config with the bit depth pinned. See the DAW-ONLY ASSUMPTION -// above — the depth-code mapping is the load-bearing unknown to confirm live. -std::vector wavSinkConfigPinned(WavBitDepth depth) { - std::int32_t depthCode = 4; // default: 32-bit float +// Int16 / Int24 blob strings are NOT implemented in M3 — their byte encoding +// was not captured from a live REAPER and must not be guessed. If M7+ adds +// them, capture the ground-truth base64 from a running REAPER first. +// +// Returns nullptr for unsupported depths. +const char* wavSinkConfigBase64(WavBitDepth depth) { switch (depth) { - case WavBitDepth::Int16: depthCode = 1; break; - case WavBitDepth::Int24: depthCode = 2; break; - case WavBitDepth::Float32: depthCode = 4; break; + case WavBitDepth::Float32: return kRenderFormatWavFloat32; + case WavBitDepth::Int16: return nullptr; // M7+: capture ground-truth blob first + case WavBitDepth::Int24: return nullptr; // M7+: capture ground-truth blob first } - std::vector cfg = {'e', 'v', 'a', 'w'}; - appendInt32LE(cfg, depthCode); - appendInt32LE(cfg, 0); // flags: defaults (LE, no metadata) - return cfg; + return nullptr; } // --- RENDER_* snapshot / restore -------------------------------------------- @@ -260,12 +245,38 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { // Resolve the project directory. GetProjectPathEx writes the effective // recording/project path (absolute). Verified: SDK header line ~2550, // GetProjectPathEx(ReaProject*, char* bufOut, int bufOut_sz). - std::vector projPathBuf(4096, '\0'); - GetProjectPathEx(proj, projPathBuf.data(), static_cast(projPathBuf.size())); - const std::string projectDir(projPathBuf.data()); + // + // Unsaved-project guard: an active project that has never been saved has + // an empty path. In that state any bank folder resolution would either fall + // back to a CWD-relative directory (violating the relative-paths-only + // invariant) or write to REAPER's default media location — both are wrong. + // Instead we prompt the user to save, then re-query. If the save dialog is + // cancelled (path still empty), refuse and write nothing. + // + // DAW-ONLY ASSUMPTION: Main_SaveProject(proj, true) opens a Save/Save-As + // dialog and blocks until the user dismisses it. "true" means forceSaveAsIn + // — verified SDK header line ~4599: + // void Main_SaveProject(ReaProject* proj, bool forceSaveAsInOptional) + // The blocking behaviour and dialog appearance can only be confirmed in a + // running REAPER. + auto resolveProjectDir = [&]() -> std::string { + std::vector buf(4096, '\0'); + GetProjectPathEx(proj, buf.data(), static_cast(buf.size())); + return std::string(buf.data()); + }; + + std::string projectDir = resolveProjectDir(); if (projectDir.empty()) { + // Prompt the user to save so the project acquires a path. + Main_SaveProject(proj, true); + // Re-query: if the dialog was confirmed the path is now set; if the + // user cancelled it is still empty. + projectDir = resolveProjectDir(); + } + if (projectDir.empty()) { + // User cancelled the save dialog — refuse, write nothing. result.status = CaptureStatus::NoProject; - result.message = "Project has no path yet (save the project first)."; + result.message = "Project must be saved before capture — nothing captured."; return result; } @@ -297,10 +308,26 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { // Master mix, no stems, no render matrix. GetSetProjectInfo(proj, "RENDER_SETTINGS", kRenderSettingsMasterMix, true); - // 0 sampleRate => follow the (fixed) project rate. Channel count preserved - // (no silent stereo fold — precision invariant). - GetSetProjectInfo(proj, "RENDER_SRATE", - static_cast(request.sampleRate), true); + // Resolve the effective sample rate. When the request carries 0 ("follow + // project"), read PROJECT_SRATE explicitly so RENDER_SRATE is set to the + // actual value — not left as 0 for REAPER to interpret. SDK header line ~3064: + // PROJECT_SRATE = sample rate (ignored unless PROJECT_SRATE_USE set); the + // value is still readable via GetSetProjectInfo even when _USE is clear. + const int effectiveSampleRate = (request.sampleRate > 0) + ? request.sampleRate + : static_cast(GetSetProjectInfo(proj, "PROJECT_SRATE", 0.0, false)); + + // Pin RENDER_SRATE only when the resolved rate is known (> 0). PROJECT_SRATE + // can read 0 on a project that has never explicitly pinned a sample rate (e.g. + // brand-new projects before the user has visited the project settings). Forcing + // RENDER_SRATE = 0 would re-introduce the "0 as literal" trap we fixed by + // moving away from blind passthrough. When the rate is unknown, leave + // RENDER_SRATE unset so REAPER follows its own project-rate default — which is + // correct behaviour for that project — rather than pinning a bogus 0. + if (effectiveSampleRate > 0) { + GetSetProjectInfo(proj, "RENDER_SRATE", + static_cast(effectiveSampleRate), true); + } GetSetProjectInfo(proj, "RENDER_CHANNELS", static_cast(request.channelCount), true); @@ -323,14 +350,18 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { setProjString(proj, "RENDER_FILE", paths.absoluteDir); setProjString(proj, "RENDER_PATTERN", paths.fileStem); - // Pin the WAV format to the chosen bit depth (float by default). See the - // wavSinkConfigPinned DAW-ONLY ASSUMPTION. - const std::vector fmt = wavSinkConfigPinned(request.bitDepth); - { - std::vector fmtBuf(fmt.begin(), fmt.end()); - fmtBuf.push_back('\0'); - GetSetProjectInfo_String(proj, "RENDER_FORMAT", fmtBuf.data(), true); + // Pin the WAV format using the ground-truth base64 blob for the chosen depth. + // Int16/Int24 are not implemented (no live-captured blob) — fail explicitly + // rather than silently mis-render at the wrong bit depth. + const char* fmtBase64 = wavSinkConfigBase64(request.bitDepth); + if (!fmtBase64) { + result.status = CaptureStatus::UnsupportedFormat; + result.message = "Requested bit depth has no verified RENDER_FORMAT blob " + "(M3 supports Float32 only; Int16/Int24 are M7+)."; + return result; + // guard's dtor restores every RENDER_* setting here. } + setProjString(proj, "RENDER_FORMAT", fmtBase64); // --- Trigger the headless render ---------------------------------------- // DAW-ONLY ASSUMPTION (see kActionRenderUsingMostRecentSettings): this runs @@ -371,11 +402,12 @@ CaptureResult OfflineRenderBackend::capture(const CaptureRequest& request) { // unverifiable PPQ resolution here — it requires a live REAPER to validate. s.wetDry = request.wetDry; s.channelCount = request.channelCount; - // DEFERRED (M6/M7): sampleRate stored as 0 = "follow project rate". Resolving - // the actual project sample rate via PROJECT_SRATE/PROJECT_SRATE_USE would - // require a live REAPER to verify. For the M3 spike this is intentional: - // the project rate is fixed for a given project and the render inherits it. - s.sampleRate = request.sampleRate; // 0 == follow project rate + // Store the resolved sample rate only when it is known (> 0). If the project + // never pinned a rate (PROJECT_SRATE read 0), we did not force RENDER_SRATE + // either, so the render ran at REAPER's project default — an unknown value from + // this code's perspective. Leave sampleRate at 0 (the Sample zero-value) rather + // than store a bogus literal; M6/M7 can fill it in by probing the rendered file. + s.sampleRate = effectiveSampleRate; // 0 when project rate was unknown s.lengthSeconds = request.endSeconds - request.startSeconds; s.captureTempo = Master_GetTempo(); // BPM at capture time (verified ~4651) s.tier = Tier::Scratch; // captures land in scratch by default diff --git a/src/capture.h b/src/capture.h index c90b7a9..91d92e3 100644 --- a/src/capture.h +++ b/src/capture.h @@ -70,6 +70,7 @@ enum class CaptureStatus { NoProject, // no active project to render / resolve a bank folder EmptyRange, // start >= end: nothing to render UnsupportedMode, // backend does not implement this source mode (M3 scope) + UnsupportedFormat, // requested bit depth has no known REAPER blob (M3: Float32 only) RenderFailed, // the render action ran but produced no output file }; diff --git a/src/capture_paths.cpp b/src/capture_paths.cpp index 9f1e3b8..5cea35a 100644 --- a/src/capture_paths.cpp +++ b/src/capture_paths.cpp @@ -1,5 +1,7 @@ #include "capture_paths.h" +#include + namespace reasampler { std::string normalizeSlashes(const std::string& path) { @@ -51,13 +53,22 @@ BankPaths deriveBankPaths(const std::string& projectDir, } const std::string fileName = stem + ".wav"; + // Precondition: the capture shell must resolve a non-empty project directory + // before calling this function. An empty projectDir would produce a bare + // relative "reasampler_bank" path — the silent default-location fallback this + // tool explicitly forbids. Assert in debug; leave absoluteDir empty in release + // so any caller that ignores the precondition fails loudly at the render/stat + // step rather than silently writing to CWD. + assert(!dir.empty() && "deriveBankPaths: projectDir must not be empty"); + BankPaths p; p.fileStem = stem; // stem only — REAPER appends extension p.fileName = fileName; p.relativePath = std::string(kBankSubfolder) + "/" + fileName; // absoluteDir intentionally omits a trailing slash (RENDER_FILE wants the // directory itself; RENDER_PATTERN supplies the file name separately). - p.absoluteDir = dir.empty() ? std::string(kBankSubfolder) + // Empty when precondition is violated (dir empty) — caller must not proceed. + p.absoluteDir = dir.empty() ? std::string{} : dir + "/" + kBankSubfolder; return p; } diff --git a/tests/test_capture_paths.cpp b/tests/test_capture_paths.cpp index 6c96dcd..d6ef4e7 100644 --- a/tests/test_capture_paths.cpp +++ b/tests/test_capture_paths.cpp @@ -67,13 +67,20 @@ static void testDeriveHandlesTrailingSlashProjectDir() { CHECK(p.fileName == "mix_7.wav"); } -static void testDeriveEmptyProjectDirFallsBackToRelative() { - // Defensive: with no project dir, absoluteDir is just the bank subfolder - // (the shell rejects the no-path case before this, but the arithmetic must - // not emit a leading slash that would read as absolute). +static void testDeriveEmptyProjectDirIsRejected() { + // Precondition: deriveBankPaths requires a non-empty projectDir. + // In debug builds the assert(!dir.empty()) fires immediately and aborts + // the process — that IS the check, so we don't call into it there. + // In release/NDEBUG builds the assert is elided; we verify the fallback + // contract: absoluteDir is left empty (not a bare "reasampler_bank") so + // any caller that ignores the precondition fails loudly at the render/stat + // step rather than silently writing to CWD. +#ifdef NDEBUG BankPaths p = deriveBankPaths("", "mix", ""); - CHECK(p.absoluteDir == "reasampler_bank"); + CHECK(p.absoluteDir.empty()); CHECK(p.relativePath == "reasampler_bank/mix.wav"); +#endif + // Debug: assert fires on the call above — contract verified by the crash. } static void testDeterministicForSameInputs() { @@ -108,7 +115,7 @@ int main() { testDeriveRelativePathIsProjectRelative(); testDeriveAbsoluteDirJoinsProjectDir(); testDeriveHandlesTrailingSlashProjectDir(); - testDeriveEmptyProjectDirFallsBackToRelative(); + testDeriveEmptyProjectDirIsRejected(); testDeterministicForSameInputs(); testFileStem();