Skip to content

Commit e40a416

Browse files
hanatyan128claude
andcommitted
fix: Code review fixes — extract unregisterAllHotkeys, add null check, fix thread safety
- Extract unregisterAllHotkeys() helper to eliminate duplicate hotkey unregistration code in registerHotkey() and removeCallback() - Add obs_filter_get_parent() null check in registerHotkey() - Initialize togglePauseRecordingHotkeyPairId and addChapterToRecordingHotkeyId in constructor initializer list to avoid undefined behavior - Use Qt::QueuedConnection for outputUserEnabledChanged signal connection to ensure UI updates run on the UI thread when emitted from OBS core thread - Remove trailing blank lines before closing braces in streaming hotkey functions - Document OBS hotkey persistence mechanism (name-string-based keybinding restoration) in registerHotkey() comments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 287accc commit e40a416

4 files changed

Lines changed: 36 additions & 52 deletions

File tree

src/UI/output-status-dock.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,8 +373,11 @@ void BranchOutputStatusDock::addFilter(BranchOutputFilter *filter)
373373
removeFilter(filter);
374374

375375
// Immediate checkbox sync when output user-enabled state changes (e.g. from hotkeys)
376-
connect(filter, &BranchOutputFilter::outputUserEnabledChanged, this,
377-
&BranchOutputStatusDock::onOutputUserEnabledChanged, Qt::UniqueConnection);
376+
connect(
377+
filter, &BranchOutputFilter::outputUserEnabledChanged, this,
378+
&BranchOutputStatusDock::onOutputUserEnabledChanged,
379+
static_cast<Qt::ConnectionType>(Qt::UniqueConnection | Qt::QueuedConnection)
380+
);
378381

379382
OBSDataAutoRelease settings = obs_source_get_settings(filter->filterSource);
380383

src/plugin-main.cpp

Lines changed: 30 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ BranchOutputFilter::BranchOutputFilter(obs_data_t *settings, obs_source_t *sourc
7272
cropScene(nullptr),
7373
toggleEnableHotkeyPairId(OBS_INVALID_HOTKEY_PAIR_ID),
7474
splitRecordingHotkeyId(OBS_INVALID_HOTKEY_ID),
75+
togglePauseRecordingHotkeyPairId(OBS_INVALID_HOTKEY_PAIR_ID),
76+
addChapterToRecordingHotkeyId(OBS_INVALID_HOTKEY_ID),
7577
splitRecordingEnabled(false),
7678
recordingSettingsOverridden(false),
7779
replayBufferActive(false),
@@ -148,9 +150,8 @@ BranchOutputFilter::~BranchOutputFilter()
148150
pthread_mutex_destroy(&audioMutex);
149151
}
150152

151-
void BranchOutputFilter::registerHotkey()
153+
void BranchOutputFilter::unregisterAllHotkeys()
152154
{
153-
// Unregister all previous hotkeys
154155
if (toggleEnableHotkeyPairId != OBS_INVALID_HOTKEY_PAIR_ID) {
155156
obs_hotkey_pair_unregister(toggleEnableHotkeyPairId);
156157
toggleEnableHotkeyPairId = OBS_INVALID_HOTKEY_PAIR_ID;
@@ -193,9 +194,32 @@ void BranchOutputFilter::registerHotkey()
193194
obs_hotkey_pair_unregister(toggleReplayBufferHotkeyPairId);
194195
toggleReplayBufferHotkeyPairId = OBS_INVALID_HOTKEY_PAIR_ID;
195196
}
197+
}
196198

197-
auto uuid = obs_source_get_uuid(filterSource);
199+
// OBS hotkey persistence note:
200+
// OBS persists hotkey keybindings in source->context.hotkey_data using the hotkey **name string**
201+
// as the dictionary key (NOT the numeric obs_hotkey_id / obs_hotkey_pair_id).
202+
// When a hotkey is unregistered and re-registered with the same name string,
203+
// obs_hotkey_register_internal() automatically restores saved keybindings from context.hotkey_data.
204+
// Therefore, unregister→re-register cycles do NOT lose user keybindings.
205+
// See: libobs/obs-hotkey.c — obs_hotkey_register_internal(), load_bindings(), enum_save_hotkey().
206+
//
207+
// This function conditionally registers hotkeys based on which output types are currently enabled
208+
// in settings, keeping OBS's hotkey settings UI clean. It is called from:
209+
// - addCallback() — initial registration when filter is added
210+
// - filterRenamedSignal handler — re-register with updated description text
211+
// - updateCallback() — re-register when settings change (output types enabled/disabled)
212+
void BranchOutputFilter::registerHotkey()
213+
{
198214
auto parent = obs_filter_get_parent(filterSource);
215+
if (!parent) {
216+
return;
217+
}
218+
219+
// Unregister all previous hotkeys
220+
unregisterAllHotkeys();
221+
222+
auto uuid = obs_source_get_uuid(filterSource);
199223
OBSDataAutoRelease settings = obs_source_get_settings(filterSource);
200224

201225
// Register enable/disable filter hotkey (always registered)
@@ -254,8 +278,7 @@ void BranchOutputFilter::registerHotkey()
254278
auto splitDescription = QString(obs_module_text("SplitRecordingFileHotkey")).arg(name);
255279

256280
splitRecordingHotkeyId = obs_hotkey_register_source(
257-
parent, qUtf8Printable(splitName), qUtf8Printable(splitDescription), onSplitRecordingFileHotkeyPressed,
258-
this
281+
parent, qUtf8Printable(splitName), qUtf8Printable(splitDescription), onSplitRecordingFileHotkeyPressed, this
259282
);
260283

261284
auto pauseRecordingName = QString("PauseRecording.%1").arg(uuid);
@@ -1680,49 +1703,8 @@ void BranchOutputFilter::removeCallback()
16801703
QMetaObject::invokeMethod(statusDock, "removeFilter", Qt::QueuedConnection, Q_ARG(BranchOutputFilter *, this));
16811704
}
16821705

1683-
// Unregsiter hotkeys
1684-
if (toggleEnableHotkeyPairId != OBS_INVALID_HOTKEY_PAIR_ID) {
1685-
obs_hotkey_pair_unregister(toggleEnableHotkeyPairId);
1686-
toggleEnableHotkeyPairId = OBS_INVALID_HOTKEY_PAIR_ID;
1687-
}
1688-
if (splitRecordingHotkeyId != OBS_INVALID_HOTKEY_ID) {
1689-
obs_hotkey_unregister(splitRecordingHotkeyId);
1690-
splitRecordingHotkeyId = OBS_INVALID_HOTKEY_ID;
1691-
}
1692-
if (togglePauseRecordingHotkeyPairId != OBS_INVALID_HOTKEY_PAIR_ID) {
1693-
obs_hotkey_pair_unregister(togglePauseRecordingHotkeyPairId);
1694-
togglePauseRecordingHotkeyPairId = OBS_INVALID_HOTKEY_PAIR_ID;
1695-
}
1696-
if (addChapterToRecordingHotkeyId != OBS_INVALID_HOTKEY_ID) {
1697-
obs_hotkey_unregister(addChapterToRecordingHotkeyId);
1698-
addChapterToRecordingHotkeyId = OBS_INVALID_HOTKEY_ID;
1699-
}
1700-
if (saveReplayBufferHotkeyId != OBS_INVALID_HOTKEY_ID) {
1701-
obs_hotkey_unregister(saveReplayBufferHotkeyId);
1702-
saveReplayBufferHotkeyId = OBS_INVALID_HOTKEY_ID;
1703-
}
1704-
if (enableAllStreamingHotkeyId != OBS_INVALID_HOTKEY_ID) {
1705-
obs_hotkey_unregister(enableAllStreamingHotkeyId);
1706-
enableAllStreamingHotkeyId = OBS_INVALID_HOTKEY_ID;
1707-
}
1708-
if (disableAllStreamingHotkeyId != OBS_INVALID_HOTKEY_ID) {
1709-
obs_hotkey_unregister(disableAllStreamingHotkeyId);
1710-
disableAllStreamingHotkeyId = OBS_INVALID_HOTKEY_ID;
1711-
}
1712-
for (size_t i = 0; i < MAX_SERVICES; i++) {
1713-
if (toggleStreamingServiceHotkeyPairIds[i] != OBS_INVALID_HOTKEY_PAIR_ID) {
1714-
obs_hotkey_pair_unregister(toggleStreamingServiceHotkeyPairIds[i]);
1715-
toggleStreamingServiceHotkeyPairIds[i] = OBS_INVALID_HOTKEY_PAIR_ID;
1716-
}
1717-
}
1718-
if (toggleRecordingHotkeyPairId != OBS_INVALID_HOTKEY_PAIR_ID) {
1719-
obs_hotkey_pair_unregister(toggleRecordingHotkeyPairId);
1720-
toggleRecordingHotkeyPairId = OBS_INVALID_HOTKEY_PAIR_ID;
1721-
}
1722-
if (toggleReplayBufferHotkeyPairId != OBS_INVALID_HOTKEY_PAIR_ID) {
1723-
obs_hotkey_pair_unregister(toggleReplayBufferHotkeyPairId);
1724-
toggleReplayBufferHotkeyPairId = OBS_INVALID_HOTKEY_PAIR_ID;
1725-
}
1706+
// Unregister hotkeys
1707+
unregisterAllHotkeys();
17261708

17271709
obs_log(LOG_INFO, "%s: Filter removed", qUtf8Printable(name));
17281710
}

src/plugin-main.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ class BranchOutputFilter : public QObject {
190190
void restartOutput();
191191
void stopOutputGracefully();
192192
void registerHotkey();
193+
void unregisterAllHotkeys();
193194
size_t findStreamingSlotByHotkeyPairId(obs_hotkey_pair_id id) const;
194195
void setBlankingActive(bool active, bool muteAudio, obs_source_t *parent);
195196
void setAudioCapturesActive(bool active);

src/plugin-streaming.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,6 @@ void BranchOutputFilter::onEnableAllStreamingHotkeyPressed(void *data, obs_hotke
561561
filter->setStreamingUserEnabled(i, true);
562562
}
563563
}
564-
565564
}
566565

567566
void BranchOutputFilter::onDisableAllStreamingHotkeyPressed(void *data, obs_hotkey_id, obs_hotkey *, bool pressed)
@@ -581,7 +580,6 @@ void BranchOutputFilter::onDisableAllStreamingHotkeyPressed(void *data, obs_hotk
581580
filter->setStreamingUserEnabled(i, false);
582581
}
583582
}
584-
585583
}
586584

587585
bool BranchOutputFilter::onEnableStreamingServiceHotkeyPressed(

0 commit comments

Comments
 (0)