Skip to content

Commit

Permalink
fix: delete desktop capturers when they're not needed (electron#39194)
Browse files Browse the repository at this point in the history
* fix: delete desktop capturers when they're not needed

Delete desktop capturer objects by resetting the DesktopMediaList
objects that own them after the sources have been collected. Capturers
that are not delegated are already being reset via a patch on
NativeDesktopMediaList. That is not safe for delegated capturers as
thumbnail generation depends on user events. Deleting the
DesktopMediaList operation is safe for all capturers and releases OS
capture resources as soon as possible.

* fix: add a patch to clean up PipeWire resources

Adding a patch to workaround a Chromium issue:
https://bugs.chromium.org/p/chromium/issues/detail?id=1467060

The patch can be removed when the issue is resolved.
  • Loading branch information
aiddya authored Jul 26, 2023
1 parent 38c3d8d commit fa5b1be
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/webrtc/.patches
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
fix_fallback_to_x11_capturer_on_wayland.patch
fix_mark_pipewire_capturer_as_failed_after_session_is_closed.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Athul Iddya <[email protected]>
Date: Sat, 22 Jul 2023 11:11:01 -0700
Subject: fix: mark PipeWire capturer as failed after session is closed

After a PipeWire screencast session is successfully started, the
consumer is expected to keep calling CaptureFrame() from the
DesktopCapturer interface in a loop to pull frames. A PipeWire
screencast session can be closed by the server on user action. Once the
session is closed, there's no point in calling CaptureFrame() again as
the capture has to be restarted. Inform the caller of the failure by
returning Result::ERROR_PERMANENT on the next invocation of
CaptureFrame().

diff --git a/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc b/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc
index 4ef00e68ab58333648b8acaf4256f4b57a42734d..5a67c18c1d1f62aa5e3162d9778ca665bac4a1bb 100644
--- a/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc
+++ b/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc
@@ -109,6 +109,7 @@ void BaseCapturerPipeWire::OnScreenCastRequestResult(RequestResponse result,
void BaseCapturerPipeWire::OnScreenCastSessionClosed() {
if (!capturer_failed_) {
options_.screencast_stream()->StopScreenCastStream();
+ capturer_failed_ = true;
}
}

6 changes: 6 additions & 0 deletions shell/browser/api/electron_api_desktop_capturer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,9 @@ void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) {
v8::HandleScope scope(isolate);
gin_helper::CallMethod(this, "_onfinished", captured_sources_);

screen_capturer_.reset();
window_capturer_.reset();

Unpin();
}
}
Expand All @@ -408,6 +411,9 @@ void DesktopCapturer::HandleFailure() {
v8::HandleScope scope(isolate);
gin_helper::CallMethod(this, "_onerror", "Failed to get sources.");

screen_capturer_.reset();
window_capturer_.reset();

Unpin();
}

Expand Down

0 comments on commit fa5b1be

Please sign in to comment.