Skip to content

Commit 035b6a6

Browse files
summeroffclaude
andauthored
fix: stop dual output recording two canvases to one file (#1759)
fix: prevent dual canvas recordings writing to same file Both canvases used "%CCYY-%MM-%DD %hh-%mm-%ss" (1s granularity), so two starts together resolved to same path. FindBestFilename only checked disk and ffmpeg creates async, so second Start() reused the name. Track claimed paths in-memory on the output and skip live claims in FindBestFilename. overwrite now only suppresses on-disk check. Add recording prefix/suffix (like replay buffer) so clients can name per-canvas files, e.g. "Vertical.mp4", avoiding collision by design. Also fix: claim lifetime on failed/duplicate start and destroy, Windows-only path normalization, extensionless path handling, and generated JS types. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 19278f8 commit 035b6a6

17 files changed

Lines changed: 574 additions & 39 deletions

js/module.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -884,6 +884,8 @@ export interface IFileOutput {
884884
lastFile(): string;
885885
}
886886
export interface IRecording extends IFileOutput {
887+
prefix: string;
888+
suffix: string;
887889
videoEncoder: IVideoEncoder;
888890
enableFileSplit: boolean;
889891
splitType: ERecSplitType;

js/module.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1832,6 +1832,14 @@ export interface IFileOutput {
18321832
}
18331833

18341834
export interface IRecording extends IFileOutput {
1835+
/**
1836+
* Wrapped around fileFormat, space-separated, before the extension is added.
1837+
* Set distinct values per canvas in dual output so the two recordings are
1838+
* distinguishable on disk; otherwise both derive the same name and the second
1839+
* one is renamed to "... (2)".
1840+
*/
1841+
prefix: string,
1842+
suffix: string,
18351843
videoEncoder: IVideoEncoder,
18361844
enableFileSplit: boolean,
18371845
splitType: ERecSplitType,

obs-studio-client/source/advanced-recording.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ Napi::Object osn::AdvancedRecording::Init(Napi::Env env, Napi::Object exports)
3636
InstanceAccessor("fileFormat", &osn::AdvancedRecording::GetFileFormat, &osn::AdvancedRecording::SetFileFormat),
3737
InstanceAccessor("overwrite", &osn::AdvancedRecording::GetOverwrite, &osn::AdvancedRecording::SetOverwrite),
3838
InstanceAccessor("noSpace", &osn::AdvancedRecording::GetNoSpace, &osn::AdvancedRecording::SetNoSpace),
39+
InstanceAccessor("prefix", &osn::AdvancedRecording::GetPrefix, &osn::AdvancedRecording::SetPrefix),
40+
InstanceAccessor("suffix", &osn::AdvancedRecording::GetSuffix, &osn::AdvancedRecording::SetSuffix),
3941

4042
InstanceAccessor("videoEncoder", &osn::AdvancedRecording::GetVideoEncoder, &osn::AdvancedRecording::SetVideoEncoder),
4143
InstanceAccessor("signalHandler", &osn::AdvancedRecording::GetSignalHandler, &osn::AdvancedRecording::SetSignalHandler),

obs-studio-client/source/recording.cpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,54 @@
2020
#include "utility.hpp"
2121
#include "video-encoder.hpp"
2222

23+
Napi::Value osn::Recording::GetPrefix(const Napi::CallbackInfo &info)
24+
{
25+
auto conn = GetConnection(info);
26+
if (!conn)
27+
return info.Env().Undefined();
28+
29+
auto response = conn->call_synchronous_helper(className, "GetPrefix", {ipc::value(this->uid)});
30+
31+
if (!ValidateResponse(info, response))
32+
return info.Env().Undefined();
33+
34+
return Napi::String::New(info.Env(), response[1].value_str);
35+
}
36+
37+
void osn::Recording::SetPrefix(const Napi::CallbackInfo &info, const Napi::Value &value)
38+
{
39+
auto conn = GetConnection(info);
40+
if (!conn)
41+
return;
42+
43+
auto response = conn->call_synchronous_helper(className, "SetPrefix", {ipc::value(this->uid), ipc::value(value.ToString().Utf8Value())});
44+
ValidateResponse(info, response);
45+
}
46+
47+
Napi::Value osn::Recording::GetSuffix(const Napi::CallbackInfo &info)
48+
{
49+
auto conn = GetConnection(info);
50+
if (!conn)
51+
return info.Env().Undefined();
52+
53+
auto response = conn->call_synchronous_helper(className, "GetSuffix", {ipc::value(this->uid)});
54+
55+
if (!ValidateResponse(info, response))
56+
return info.Env().Undefined();
57+
58+
return Napi::String::New(info.Env(), response[1].value_str);
59+
}
60+
61+
void osn::Recording::SetSuffix(const Napi::CallbackInfo &info, const Napi::Value &value)
62+
{
63+
auto conn = GetConnection(info);
64+
if (!conn)
65+
return;
66+
67+
auto response = conn->call_synchronous_helper(className, "SetSuffix", {ipc::value(this->uid), ipc::value(value.ToString().Utf8Value())});
68+
ValidateResponse(info, response);
69+
}
70+
2371
Napi::Value osn::Recording::GetVideoEncoder(const Napi::CallbackInfo &info)
2472
{
2573
return videoEncoderRef.IsEmpty() ? info.Env().Undefined() : videoEncoderRef.Value();

obs-studio-client/source/recording.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ class Recording : public WorkerSignals, public FileOutput {
3333
Napi::Reference<Napi::Object> streamingRef;
3434
Napi::Reference<Napi::Object> audioEncoderRef;
3535

36+
Napi::Value GetPrefix(const Napi::CallbackInfo &info);
37+
void SetPrefix(const Napi::CallbackInfo &info, const Napi::Value &value);
38+
Napi::Value GetSuffix(const Napi::CallbackInfo &info);
39+
void SetSuffix(const Napi::CallbackInfo &info, const Napi::Value &value);
3640
Napi::Value GetVideoEncoder(const Napi::CallbackInfo &info);
3741
void SetVideoEncoder(const Napi::CallbackInfo &info, const Napi::Value &value);
3842
Napi::Value GetSignalHandler(const Napi::CallbackInfo &info);

obs-studio-client/source/simple-recording.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ Napi::Object osn::SimpleRecording::Init(Napi::Env env, Napi::Object exports)
4646
InstanceAccessor("fileFormat", &osn::SimpleRecording::GetFileFormat, &osn::SimpleRecording::SetFileFormat),
4747
InstanceAccessor("overwrite", &osn::SimpleRecording::GetOverwrite, &osn::SimpleRecording::SetOverwrite),
4848
InstanceAccessor("noSpace", &osn::SimpleRecording::GetNoSpace, &osn::SimpleRecording::SetNoSpace),
49+
InstanceAccessor("prefix", &osn::SimpleRecording::GetPrefix, &osn::SimpleRecording::SetPrefix),
50+
InstanceAccessor("suffix", &osn::SimpleRecording::GetSuffix, &osn::SimpleRecording::SetSuffix),
4951
InstanceAccessor("lowCPU", &osn::SimpleRecording::GetLowCPU, &osn::SimpleRecording::SetLowCPU),
5052
InstanceAccessor("streaming", &osn::SimpleRecording::GetStreaming, &osn::SimpleRecording::SetStreaming),
5153
InstanceAccessor("enableFileSplit", &osn::SimpleRecording::GetEnableFileSplit, &osn::SimpleRecording::SetEnableFileSplit),

obs-studio-server/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,7 @@ if(BUILD_TESTING)
625625
add_executable(
626626
obs_studio_server_unit_tests
627627
"tests/test-osn-source.cpp"
628+
"tests/test-osn-file-output.cpp"
628629
"tests/obs-setup.cpp"
629630
"tests/obs-setup.hpp"
630631
)

obs-studio-server/source/osn-advanced-recording.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ void osn::IAdvancedRecording::Register(ipc::server &srv)
6767
cls->register_function(std::make_shared<ipc::function>("SetFileResetTimestamps", std::vector<ipc::type>{ipc::type::UInt64, ipc::type::UInt32},
6868
SetFileResetTimestamps));
6969
cls->register_function(std::make_shared<ipc::function>("GetAvailableEncoders", std::vector<ipc::type>{ipc::type::UInt64}, GetAvailableEncoders));
70+
cls->register_function(std::make_shared<ipc::function>("GetPrefix", std::vector<ipc::type>{ipc::type::UInt64}, GetPrefix));
71+
cls->register_function(std::make_shared<ipc::function>("SetPrefix", std::vector<ipc::type>{ipc::type::UInt64, ipc::type::String}, SetPrefix));
72+
cls->register_function(std::make_shared<ipc::function>("GetSuffix", std::vector<ipc::type>{ipc::type::UInt64}, GetSuffix));
73+
cls->register_function(std::make_shared<ipc::function>("SetSuffix", std::vector<ipc::type>{ipc::type::UInt64, ipc::type::String}, SetSuffix));
7074

7175
srv.register_collection(cls);
7276
}
@@ -90,6 +94,11 @@ void osn::IAdvancedRecording::Destroy(void *data, const int64_t id, const std::v
9094
PRETTY_ERROR_RETURN(ErrorCode::InvalidReference, "Recording reference is not valid.");
9195
}
9296

97+
// Stop before deregistering. The destructor would do this anyway, but by then the object is out
98+
// of the manager, and DeleteOutput() can wait up to 20s for the muxer to drain -- a window where
99+
// the file is still being written but its claim is invisible to a concurrent Start.
100+
recording->DeleteOutput();
101+
93102
osn::IAdvancedRecording::Manager::GetInstance().free(recording);
94103
delete recording;
95104

@@ -297,10 +306,9 @@ void osn::IAdvancedRecording::Start(void *data, const int64_t id, const std::vec
297306
if (lastChar != '/' && lastChar != '\\')
298307
path += "/";
299308

300-
path += GenerateSpecifiedFilename(recording->format, recording->noSpace, recording->fileFormat, recording->GetCanvas());
309+
path += GenerateSpecifiedFilename(recording->format, recording->noSpace, recording->DecoratedFileFormat(), recording->GetCanvas());
301310

302-
if (!recording->overwrite)
303-
FindBestFilename(path, recording->noSpace);
311+
FindBestFilename(path, recording->noSpace, recording, recording->overwrite);
304312

305313
obs_data_t *settings = obs_data_create();
306314
obs_data_set_string(settings, "path", path.c_str());

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,105 @@
2020
#include "osn-error.hpp"
2121
#include "shared.hpp"
2222
#include <osn-video.hpp>
23+
#include <util/platform.h>
24+
#include <algorithm>
25+
#include <mutex>
26+
27+
// Serialises resolve-and-claim against the release in OnOutputStopped(). Start() runs on an IPC
28+
// worker thread and the release on libobs' capture thread, and the check-then-set has to be atomic
29+
// as a unit -- the manager's own lock is dropped when for_each returns, so it is not enough.
30+
static std::mutex s_filenameClaimMutex;
31+
32+
// Windows paths are case-insensitive and accept either separator, so compare on a folded key while
33+
// the claim itself keeps the exact string the muxer was given. Elsewhere the comparison is exact:
34+
// on POSIX a backslash is an ordinary filename character, so folding it into a separator would make
35+
// two different files look like one.
36+
static std::string claim_key(const std::string &path)
37+
{
38+
#ifdef WIN32
39+
std::string key = path;
40+
std::replace(key.begin(), key.end(), '\\', '/');
41+
std::transform(key.begin(), key.end(), key.begin(), [](unsigned char c) { return (char)tolower(c); });
42+
return key;
43+
#else
44+
return path;
45+
#endif
46+
}
47+
48+
// Caller must hold s_filenameClaimMutex.
49+
static bool path_claimed_by_other(const std::string &candidate, const osn::FileOutput *owner)
50+
{
51+
const std::string wanted = claim_key(candidate);
52+
bool claimed = false;
53+
54+
osn::IFileOutput::Manager::GetInstance().for_each([&](osn::FileOutput *output) {
55+
// Idle outputs, replay buffers (which never claim) and GetLegacySettings' throwaway
56+
// objects all carry an empty claim.
57+
if (claimed || !output || output == owner || output->claimedFilePath.empty())
58+
return;
59+
60+
if (claim_key(output->claimedFilePath) == wanted)
61+
claimed = true;
62+
});
63+
64+
return claimed;
65+
}
66+
67+
void osn::IFileOutput::FindBestFilename(std::string &strPath, bool noSpace, FileOutput *owner, bool allowOverwrite)
68+
{
69+
std::lock_guard<std::mutex> lock(s_filenameClaimMutex);
70+
71+
// Insert before the extension, or at the very end when there is none. Only the final component
72+
// counts: a dot in a directory name is not an extension. An extensionless name is reachable --
73+
// osn_generate_formatted_filename() appends the extension and then truncates the whole thing to
74+
// 255 bytes -- and giving up there would hand two live outputs the same path.
75+
const size_t sep = strPath.find_last_of("/\\");
76+
const size_t nameStart = (sep == std::string::npos) ? 0 : sep + 1;
77+
const size_t dot = strPath.find_last_of('.');
78+
const size_t insertAt = (dot != std::string::npos && dot > nameStart) ? dot : strPath.size();
79+
80+
std::string candidate = strPath;
81+
int num = 2;
82+
bool blockedByLiveOutput = false;
83+
84+
for (;;) {
85+
const bool onDisk = !allowOverwrite && os_file_exists(candidate.c_str());
86+
const bool heldByPeer = path_claimed_by_other(candidate, owner);
87+
88+
if (!onDisk && !heldByPeer)
89+
break;
90+
if (heldByPeer)
91+
blockedByLiveOutput = true;
92+
93+
std::string numStr = noSpace ? "_" : " (";
94+
numStr += std::to_string(num++);
95+
if (!noSpace)
96+
numStr += ")";
97+
98+
candidate = strPath;
99+
candidate.insert(insertAt, numStr);
100+
}
101+
102+
// Stepping over a file left on disk is ordinary and stays quiet. Stepping over a path another
103+
// running output holds is not: it means a client pointed two outputs at one file -- dual output
104+
// without distinct prefix/suffix, most likely -- and got a name it did not ask for.
105+
if (blockedByLiveOutput)
106+
blog(LOG_WARNING, "Recording path '%s' is in use by another active output, writing to '%s' instead.", strPath.c_str(), candidate.c_str());
107+
108+
strPath = candidate;
109+
110+
// Never move the claim of an output that is already running. A duplicate start() resolves a
111+
// fresh name -- the timestamp has moved on -- but obs_output_start() then refuses and the muxer
112+
// stays on its original file. Reassigning here would unclaim the file actually being written.
113+
if (owner && !obs_output_active(owner->GetOutput()))
114+
owner->claimedFilePath = strPath;
115+
}
116+
117+
void osn::FileOutput::OnOutputStopped()
118+
{
119+
std::lock_guard<std::mutex> lock(s_filenameClaimMutex);
120+
claimedFilePath.clear();
121+
}
23122

24123
void osn::IFileOutput::Register(ipc::server &srv)
25124
{

obs-studio-server/source/osn-file-output.hpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,20 @@ class FileOutput : public Output {
3535
}
3636
virtual ~FileOutput() {}
3737

38+
protected:
39+
void OnOutputStopped() override;
40+
3841
public:
3942
std::string path;
4043
std::string format;
4144
std::string fileFormat;
4245
bool overwrite;
4346
bool noSpace;
4447
std::string muxerSettings;
48+
49+
// Full resolved path this output is recording to, empty while idle. Guarded by the
50+
// claim mutex in osn-file-output.cpp; read it through that, not directly.
51+
std::string claimedFilePath;
4552
};
4653

4754
class IFileOutput {
@@ -82,7 +89,12 @@ class IFileOutput {
8289
static void GetLastFile(void *data, const int64_t id, const std::vector<ipc::value> &args, std::vector<ipc::value> &rval);
8390

8491
static std::string GenerateSpecifiedFilename(const std::string &extension, bool noSpace, const std::string &format, obs_video_info *ovi);
85-
static void FindBestFilename(std::string &strPath, bool noSpace);
92+
93+
// Resolves strPath to a name no other live output has claimed, appending " (2)", " (3)"
94+
// (or "_2" when noSpace) as needed, and records the result against owner. allowOverwrite
95+
// suppresses the on-disk check only -- a path held by another running output is never
96+
// reused, because two muxers on one file corrupt it whatever the overwrite setting says.
97+
static void FindBestFilename(std::string &strPath, bool noSpace, FileOutput *owner, bool allowOverwrite);
8698

8799
static obs_encoder_t *duplicate_encoder(obs_encoder_t *src, uint64_t trackIndex = 0);
88100
};

0 commit comments

Comments
 (0)