fix: stop dual output recording two canvases to one file

[summeroff]

Summary

Dual output starts a recording per canvas. Both derive their filename from the same global Output/FilenameFormatting default — "%CCYY-%MM-%DD %hh-%mm-%ss", one-second granularity, no per-canvas component — so two recordings started together resolve to the same absolute path and two ffmpeg_muxer instances open one file.

libobs tolerated that silently until the Windows nofollow open tightened its share mask (obs-studio#758), when the second open began failing with EACCES and took down 27 tests. obs-studio#760 relaxed the mask again, so CI is green — but green only means the collision is silent again. The two muxers are still interleaving into one file.

Why the existing guard doesn't catch it

FindBestFilename was purely os_file_exists()-based. ffmpeg_muxer creates the file asynchronously after the start signal, so when the second Start() resolves its path the first file does not exist yet and the name is handed out twice.

The per-canvas WIDTHxHEIGHT-NN suffix that used to make this impossible was removed in b3175ad (#1675) as a "rand hack". That PR did thread the canvas into filename generation, but the canvas is only read for the opt-in %FPS/%CRES/%ORES/%VF tokens — the default pattern contains none of them, so it has no effect. (And it would not have been sufficient anyway: two canvases at the same resolution produce identical %CRES.)

Changes

1. Correctness — an in-memory claim. Each output records the path it claimed; FindBestFilename skips names another live output holds, found via IFileOutput::Manager::for_each. The claim lives on the output object rather than in a process-wide registry so it cannot outlive its owner: once the object leaves the manager it is invisible, making a missed release harmless instead of a permanent phantom that would bump every later recording to (2), (3), … for the rest of the session. Released from the libobs stop signal and from the obs_output_start() failure path (which synthesises its own stop), under a mutex — Start() runs on an IPC worker thread and the release on libobs' capture thread.

overwrite now suppresses only the on-disk check. It means "clobber the stale file from last session", which cannot coherently extend to a file another output is writing right now. With a single output there is no peer claim, so behaviour is byte-for-byte unchanged.

2. UX — per-canvas naming. (2) is a safety net, not an answer: it says "something was already there", not "this is your vertical recording", and which canvas wins the base name depends on start order. Recordings had no naming knob at all — only the replay buffer did. osn::Recording now has the same prefix/suffix, wrapped around fileFormat before the extension, exposed over IPC and on IRecording:

2026-08-09 14-30-12.mp4
2026-08-09 14-30-12 Vertical.mp4

Reserved path characters are stripped from prefix/suffix as the replay buffer does; fileFormat passes through untouched so no existing pattern changes meaning.

3. Diagnosability. A LOG_WARNING when the claim actually renames a path. Stepping over a file left on disk is ordinary and stays quiet; stepping over a path a running output holds means a client pointed two outputs at one file.

Test plan

• New Catch2 tests (obs-studio-server/tests/test-osn-file-output.cpp) — peer claim, noSpace _2 form, the overwrite decision, separator/case folding, release-and-reuse, and DecoratedFileFormat spacing/sanitisation. Verified failing before the fix by stubbing the peer check: 16 | 12 passed | 4 failed → all passing. Full unit suite 109 assertions in 4 cases.
• Dual canvas recordings can be told apart by suffix (new integration test) — passes locally; asserts both outputs get their requested names with no (2) anywhere, proving the naming path end to end.
• Dual canvas recording avoids name collision — waitForFile() removed from between the two start() calls. That helper existed to force the first file onto disk so the os_file_exists check had something to see; without it this is a genuine regression test for the claim.
• Start Dual Output with advanced recording using the same user audio track — added lastFile() inequality assertion, covering the default-filename case a real user hits.
• obs-studio-server-lib and obs_studio_client.node build clean; clang-format 18.1.3 clean on all 13 changed C++ files; yarn build:javascript regeneration is stable.

All three dual-output tests confirmed green on CI (182 passing overall).

Follow-ups

• Desktop must set suffix = 'Vertical' on the vertical recording in createRecording, and uncomment the per-display addRecordingEntry labelling. Until then dual output still collides — but now logs a warning instead of being silently wrong. Needs an osn bump containing this PR first.
• Split-file chunks 2..N are out of scope. Chunk 1 is covered (it uses settings["path"], which survives the obs_data_apply merge), but ConfigureRecFileSplitting hands directory+format to libobs, which names later chunks from its own os_file_exists check on the muxer thread with no path back into osn. The correct fix is upstream: resolve names by exclusively creating the file rather than by os_file_exists, which closes the same race for every consumer.
• waitForFile in tests/osn-tests/util/general.ts is now unused; left in place as a reasonable general helper.

[Copilot]

Pull request overview

Prevents simultaneous canvas recordings from writing to the same file after the filename behavior introduced in #1675.

Changes:

• Adds synchronized in-memory filename claims.
• Adds recording prefix/suffix naming.
• Adds unit and dual-output integration coverage.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/osn-tests/src/test_osn_dual_output.ts	Tests collision avoidance and suffix naming.
obs-studio-server/tests/test-osn-file-output.cpp	Tests claims and decorated formats.
obs-studio-server/source/osn-simple-recording.cpp	Applies naming and claims to simple recordings.
obs-studio-server/source/osn-recording.hpp	Defines prefix/suffix API.
obs-studio-server/source/osn-recording.cpp	Implements decorated filename formatting.
obs-studio-server/source/osn-output.hpp	Adds a stop lifecycle hook.
obs-studio-server/source/osn-output.cpp	Invokes cleanup after stops and failed starts.
obs-studio-server/source/osn-file-output.hpp	Stores and exposes filename claims.
obs-studio-server/source/osn-file-output.cpp	Implements synchronized claim resolution.
obs-studio-server/source/osn-advanced-recording.cpp	Applies naming and claims to advanced recordings.
obs-studio-server/CMakeLists.txt	Registers the new unit tests.
obs-studio-client/source/simple-recording.cpp	Exposes prefix/suffix accessors.
obs-studio-client/source/recording.hpp	Declares client accessors.
obs-studio-client/source/recording.cpp	Implements prefix/suffix IPC calls.
obs-studio-client/source/advanced-recording.cpp	Exposes advanced recording accessors.
js/module.ts	Documents the TypeScript API.
js/module.d.ts	Updates generated declarations.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[summeroff]

Correct, and fixed in 01e3c2a. claim_key now applies Windows path-equivalence only under WIN32 and compares byte-exact elsewhere, for exactly the reason you give: on POSIX a backslash is an ordinary filename character.

The macOS runners caught the same thing independently — my local verification was Windows-only, so it could not have.

[summeroff]

Fixed in 01e3c2a, alongside the claim_key change. The section is now #ifdef WIN32, with a POSIX counterpart asserting the opposite: that two paths differing only by separator stay distinct.

[summeroff]

Good catch — real, and fixed in 8e392f2.

Confirmed the mechanism: obs_output_can_begin_data_capture() returns false while active(output), so a second start() on a running recording does land here with the first muxer still writing. Now guarded:

if (!obs_output_active(m_output))
    OnOutputStopped();

Worth noting the on-disk check masks this in the common case (the file exists by then), but not with overwrite = true — which suppresses exactly that check — so your parenthetical was the important part.

[summeroff]

Confirmed and fixed in 8e392f2. Both Destroy handlers now call DeleteOutput() before Manager::free(), so the object stays visible for the whole stop wait. DeleteOutput() is idempotent (if (!m_output) return;), so the destructor call becomes a no-op and blocking behaviour is unchanged — only the ordering.

I preferred this to moving claims into longer-lived storage: keeping the claim on the object is what makes a missed release self-limiting rather than a permanent phantom.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

obs-studio-server/source/osn-file-output.cpp:69

• Calling start() again on an active recording re-enters this resolver. The existing file then makes candidate become ... (2), replacing the owner's original claim even though StartOutput() fails and the original muxer remains active. A subsequent output with overwrite=true can therefore claim the original path and recreate the corruption this guard is intended to prevent. Make resolution idempotent whenever the owner already has a live/pending claim (or reject an active second start before resolving).

	std::lock_guard<std::mutex> lock(s_filenameClaimMutex);

[summeroff]

Real, and reachable exactly as you describe — fixed in 5fe4151.

Confirmed the truncation: osn_generate_formatted_filename() appends the extension and then clamps, so the extension is what gets cut:

dstr_cat_ch(&sf, '.');
dstr_cat(&sf, extension);
...
if (sf.len > 255)
    dstr_mid(&sf, &sf, 0, 255);

and the new prefix/suffix API makes 255 bytes easier to reach. The insertion point is now the end of the name when there is no extension, so the live-output guarantee holds either way.

Verifying this also turned up an adjacent bug the same fix covers: strrchr searched the whole path, so a dot in a directory name was treated as the extension — C:/my.dir/rec became C:/my (2).dir/rec. The search is now scoped to the final path component. That one predates this PR.

Both are covered by new unit cases (an extensionless name is still made unique, a dot in a directory is not treated as the extension); each fails against the previous logic.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

obs-studio-server/source/osn-file-output.cpp:78

• This changes collision handling when fileFormat is empty. The formatter then produces .mp4, but dot == nameStart is treated as extensionless, yielding .mp4 (2) instead of the previous (2).mp4. Preserve the leading appended extension while still excluding dots in directory components.

	const size_t insertAt = (dot != std::string::npos && dot > nameStart) ? dot : strPath.size();

[summeroff]

Right again, and it is the other half of the one before it — fixed in 9795070.

I guarded the release path in 8e392f2 but left the resolve path free to move the claim. The sequence you describe:

1. start() resolves A, claims A, muxer writes A
2. start() again — the whole Start() handler reruns, the timestamp has advanced, so FindBestFilename resolves B and reassigns claimedFilePath = B
3. obs_output_start() refuses because the output is active; the muxer is still on A, and nothing claims A

With overwrite = true suppressing the on-disk check, another output can then take A. The claim is now only taken when the output is not already active:

if (owner && !obs_output_active(owner->GetOutput()))
    owner->claimedFilePath = strPath;

obs_output_active() is null-safe (return (output != NULL) ? ... : false), so an output that has not been created yet still claims normally.

On your alternative — rejecting the duplicate start before resolving a filename — I agree it is the better fix, and it would also stop the stale obs_output_update(path = B) that currently lands on a running output. I did not take it here because it changes what the Start IPC call returns for a case Desktop reaches routinely through validateOrCreateOutputInstance, which calls start() on an already-validated instance. That deserves deciding on its own terms rather than as a side effect of this PR.

One caveat on coverage: the unit tests exercise the not-active branch only, since GetOutput() is null there. The active branch has no unit coverage.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (2)

obs-studio-server/source/osn-file-output.cpp:42

• This only lowercases individual UTF-8 bytes, so non-ASCII case variants are left unchanged even though the default Windows filesystem treats them as the same path (for example, directories differing only by Ä/ä). Two recordings can therefore still claim the same Windows file. Build the key with a Windows Unicode case-insensitive comparison/canonicalization routine rather than byte-wise tolower.

	std::string key = path;
	std::replace(key.begin(), key.end(), '\\', '/');
	std::transform(key.begin(), key.end(), key.begin(), [](unsigned char c) { return (char)tolower(c); });
	return key;

obs-studio-server/source/osn-file-output.cpp:99

• When the generated filename has already reached the formatter's 255-byte limit, inserting the counter makes the final path component exceed the filesystem limit. This is especially concrete for the extensionless truncation case described above: a 255-byte basename becomes 259 bytes after " (2)", so the second recording fails with ENAMETOOLONG instead of receiving a usable unique name. Reserve room for the counter by truncating the basename (without splitting a UTF-8 sequence) before inserting it, and cover the actual 255-byte case rather than the short extensionless string in the test.

		candidate = strPath;
		candidate.insert(insertAt, numStr);