Skip to content

frontend: Improve UX around recording paths #12395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Warchamp7 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

frontend: Improve UX around recording paths #12395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions frontend/OBSApp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1390,15 +1390,9 @@ string GetFormatExt(const char *container)

string GetOutputFilename(const char *path, const char *container, bool noSpace, bool overwrite, const char *format)
{
OBSBasic *main = OBSBasic::Get();

os_dir_t *dir = path && path[0] ? os_opendir(path) : nullptr;

if (!dir) {
if (main->isVisible())
OBSMessageBox::warning(main, QTStr("Output.BadPath.Title"), QTStr("Output.BadPath.Text"));
else
main->SysTrayNotify(QTStr("Output.BadPath.Text"), QSystemTrayIcon::Warning);
return "";
}

Expand Down
7 changes: 5 additions & 2 deletions frontend/data/locale/en-US.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,10 +439,12 @@ Output.RecordError.EncodeErrorMsg="An encoder error occurred while recording."
Output.RecordError.EncodeErrorMsg.LastError="An encoder error occurred while recording:<br><br>%1"

# output recording messages
Output.BadPath.Title="Bad File Path"
Output.Error.Title="Output Error"
Output.BadPath.Title="Invalid Path"

# "Recording Path" should match Basic.Settings.Output.Simple.SavePath
Output.BadPath.Text="The configured Recording Path could not be opened. Please check your Recording Path under Settings → Output → Recording."
Output.PathError.Text="The configured Recording Path could not be opened. The folder specified in Settings → Output → Recording does not exist."
Output.BadPath.Text="Recording Path '%1' does not exist.\n\nWould you like to create it?"

# broadcast setup messages
Output.NoBroadcast.Title="No Broadcast Configured"
Expand Down Expand Up @@ -674,6 +676,7 @@ Basic.StatusBar.DelayStartingStoppingIn="Delay (stopping in %1 sec, starting in
Basic.StatusBar.RecordingSavedTo="Recording saved to '%1'"
Basic.StatusBar.ReplayBufferSavedTo="Replay buffer saved to '%1'"
Basic.StatusBar.ScreenshotSavedTo="Screenshot saved to '%1'"
Basic.StatusBar.ScreenshotError="Error while saving screenshot"
Basic.StatusBar.AutoRemuxedTo="Recording auto remuxed to '%1'"

# filters window
Expand Down
6 changes: 6 additions & 0 deletions frontend/forms/OBSBasicSettings.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2615,6 +2615,9 @@
<property name="enabled">
<bool>true</bool>
</property>
<property name="readOnly">
<bool>true</bool>
</property>
</widget>
</item>
<item>
Expand Down Expand Up @@ -3622,6 +3625,9 @@
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="readOnly">
<bool>true</bool>
</property>
</widget>
</item>
<item>
Expand Down
24 changes: 17 additions & 7 deletions frontend/utility/ScreenshotObj.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,16 @@ ScreenshotObj::~ScreenshotObj()

if (cx && cy) {
OBSBasic *main = OBSBasic::Get();
main->ShowStatusBarMessage(
QTStr("Basic.StatusBar.ScreenshotSavedTo").arg(QT_UTF8(path.c_str())));
if (success) {
main->ShowStatusBarMessage(
QTStr("Basic.StatusBar.ScreenshotSavedTo").arg(QT_UTF8(path.c_str())));

main->lastScreenshot = path;
main->lastScreenshot = path;

main->OnEvent(OBS_FRONTEND_EVENT_SCREENSHOT_TAKEN);
main->OnEvent(OBS_FRONTEND_EVENT_SCREENSHOT_TAKEN);
} else {
main->ShowStatusBarMessage(QTStr("Basic.StatusBar.ScreenshotError"));
}
}
}
}
Expand Down Expand Up @@ -262,9 +266,15 @@ static HRESULT SaveJxr(LPCWSTR path, uint8_t *pixels, uint32_t cx, uint32_t cy)

void ScreenshotObj::MuxAndFinish()
{
if (half_bytes.empty()) {
image.save(QT_UTF8(path.c_str()));
blog(LOG_INFO, "Saved screenshot to '%s'", path.c_str());
success = false;

if (half_bytes.empty() && !path.empty()) {
success = image.save(QT_UTF8(path.c_str()));
if (success) {
blog(LOG_INFO, "Saved screenshot to '%s'", path.c_str());
} else {
blog(LOG_INFO, "Error while saving screenshot");
}
} else {
#ifdef _WIN32
wchar_t *path_w = nullptr;
Expand Down
1 change: 1 addition & 0 deletions frontend/utility/ScreenshotObj.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class ScreenshotObj : public QObject {
uint32_t cx;
uint32_t cy;
std::thread th;
bool success = false;
Copy link
Copy Markdown
Member

@PatTheMav PatTheMav Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool success = false;
bool isScreenshotSaved = false;

Success by itself is ambiguous - did it successfully render the texture? Generate the image data? Store it to disk?

The only way to understand this is to go backwards from the Basic.StatusBar.ScreenshotSavedTo string being generated upon success but parsing the code that far shouldn't be necessary.


int stage = 0;

Expand Down
1 change: 1 addition & 0 deletions frontend/widgets/OBSBasic.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,7 @@ private slots:

bool IsFFmpegOutputToURL() const;
bool OutputPathValid();
bool promptCreateOutputPath();
void OutputPathInvalidMessage();

// TODO: Unimplemented, remove.
Expand Down
26 changes: 25 additions & 1 deletion frontend/widgets/OBSBasic_OutputHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ void OBSBasic::OutputPathInvalidMessage()
{
blog(LOG_ERROR, "Recording stopped because of bad output path");

OBSMessageBox::critical(this, QTStr("Output.BadPath.Title"), QTStr("Output.BadPath.Text"));
OBSMessageBox::critical(this, QTStr("Output.Error.Title"), QTStr("Output.PathError.Text"));
}

bool OBSBasic::IsFFmpegOutputToURL() const
Expand All @@ -140,3 +140,27 @@ bool OBSBasic::OutputPathValid()
const char *path = GetCurrentOutputPath();
return path && *path && QDir(path).exists();
}

bool OBSBasic::promptCreateOutputPath()
Copy link
Copy Markdown
Member

@PatTheMav PatTheMav Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool OBSBasic::promptCreateOutputPath()
bool OBSBasic::ensureOutputPathExists()

It's probably irrelevant to the calling code how this is ensured, but it only cares that it is ensured.

Copy link
Copy Markdown
Member Author

@Warchamp7 Warchamp7 Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not ensured in this case since the user could click no. Would that still be appropriate here?

Copy link
Copy Markdown
Member

@PatTheMav PatTheMav Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it's fine if the function that is supposed to ensure this path exists tells you it "couldn't do it", because then you know it wasn't able to fulfil its job. It also makes sense that the code will not continue, because a precondition isn't true.

{
const char *path = GetCurrentOutputPath();

if (!path || !*path) {
return false;
}

auto result = OBSMessageBox::question(this, QTStr("Output.BadPath.Title"),
QTStr("Output.BadPath.Text").arg(path),
QMessageBox::Yes | QMessageBox::No);

if (result == QMessageBox::No) {
return false;
}

auto makeDirectory = os_mkdir(path);
if (makeDirectory == MKDIR_ERROR) {
OBSMessageBox::critical(this, QTStr("Error"), QTStr("Failed to create directory '%1'").arg(path));
}

return makeDirectory != MKDIR_ERROR;
}
7 changes: 5 additions & 2 deletions frontend/widgets/OBSBasic_Recording.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ void OBSBasic::on_actionShow_Recordings_triggered()
: config_get_string(activeConfiguration, "AdvOut", "RecFilePath");
const char *path = strcmp(mode, "Advanced") ? config_get_string(activeConfiguration, "SimpleOutput", "FilePath")
: adv_path;
QDesktopServices::openUrl(QUrl::fromLocalFile(path));
bool success = QDesktopServices::openUrl(QUrl::fromLocalFile(path));
if (!success) {
OutputPathInvalidMessage();
}
}

#define RECORDING_START "==== Recording Start ==============================================="
Expand Down Expand Up @@ -113,7 +116,7 @@ void OBSBasic::StartRecording()
if (disableOutputsRef)
return;

if (!OutputPathValid()) {
if (!OutputPathValid() && !promptCreateOutputPath()) {
OutputPathInvalidMessage();
return;
}
Expand Down
2 changes: 1 addition & 1 deletion frontend/widgets/OBSBasic_ReplayBuffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void OBSBasic::StartReplayBuffer()
if (!UIValidation::NoSourcesConfirmation(this))
return;

if (!OutputPathValid()) {
if (!OutputPathValid() && !promptCreateOutputPath()) {
OutputPathInvalidMessage();
return;
}
Expand Down
Loading