Skip to content

Commit

Permalink
UI: Wait for full vcam deactivation to destroy its view
Browse files Browse the repository at this point in the history
A bug which was technically introduced in df446c3: the author
assigned video mixes to the virtual camera and did not realize you have
to wait for full capture stop with the "deactivate" signal rather than
the "stop" signal. This situation is understandable because these
signals are likely confusing and need more documentation.

The way an output works when it stops is it has three stages:
- "stopping", which is the moment the user themselves stop the output
- "stop", when the output's implementation has stopped and returns
  success or an error code
- "deactivate", when the output inself (not the implementation) has
  fully stopped capturing encoded or raw frame data after the
  implementation has stopped. This is done in a separate thread,
  end_data_capture_thread(), for performance and data race reasons, and
  is when it's "actually fully stopped for real this time"

(Lain note: I sincerely apologize for this confusing signal design)

The author of df446c3 was likely confused as to why they could not
destroy the video mix in the "stop" signal without triggering a race
condition in end_data_capture_thread(), and instead decided to make a
workaround to clear the video mix set to the output before destroying
the video mix. Unsetting the video mix I'm guessing *seemed* to fix the
problem for them at the time, and destroying the video mix separately
after that automatically stops capture, so it *technically* worked
because the deactivate thread cannot be called until the
implementation's stop signal has executed. However, it was still the
incorrect way to handle the problem, because it circumvents the output's
mechanism for deactivating the frame data capture by destroying the view
instead.

The reason this was the incorrect way to handle the problem became
exposed in 7cd7ca8, when tytan made it so it uses the main video mix
under certain circumstances. The main view is special and cannot be
destroyed, so the mechanism to stop frame data capture that the
virtualcam was using before failed, and raw frame data capture continued
perpetually until the end of the program erroneously.

This fix solves the bug by using the "deactivate" signal rather than the
"stop" signal, thus allowing the output to fully end its data capture
and *then* destroy the video mix.

Fixes obsproject#9153
  • Loading branch information
Lain-B committed Jul 6, 2023
1 parent 9712b33 commit cececf4
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
7 changes: 6 additions & 1 deletion UI/window-basic-main-outputs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,11 @@ static void OBSStopVirtualCam(void *data, calldata_t *params)
os_atomic_set_bool(&virtualcam_active, false);
QMetaObject::invokeMethod(output->main, "OnVirtualCamStop",
Q_ARG(int, code));
}

obs_output_set_media(output->virtualCam, nullptr, nullptr);
static void OBSDeactivateVirtualCam(void *data, calldata_t * /* params */)
{
BasicOutputHandler *output = static_cast<BasicOutputHandler *>(data);
output->DestroyVirtualCamView();
}

Expand Down Expand Up @@ -293,6 +296,8 @@ inline BasicOutputHandler::BasicOutputHandler(OBSBasic *main_) : main(main_)
startVirtualCam.Connect(signal, "start", OBSStartVirtualCam,
this);
stopVirtualCam.Connect(signal, "stop", OBSStopVirtualCam, this);
deactivateVirtualCam.Connect(signal, "deactivate",
OBSDeactivateVirtualCam, this);
}
}

Expand Down
1 change: 1 addition & 0 deletions UI/window-basic-main-outputs.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ struct BasicOutputHandler {
OBSSignal stopStreaming;
OBSSignal startVirtualCam;
OBSSignal stopVirtualCam;
OBSSignal deactivateVirtualCam;
OBSSignal streamDelayStarting;
OBSSignal streamStopping;
OBSSignal recordStopping;
Expand Down

0 comments on commit cececf4

Please sign in to comment.