Skip to content

Commit

Permalink
fix: use StartUpdating method for PipeWire capturer (#39116)
Browse files Browse the repository at this point in the history
* fix: use StartUpdating method for PipeWire capturer

Fixed a crash related to PipeWire capturer by adapting to Chromium's
interface changes. Chromium expects a call to
`NativeDesktopMediaList::StartUpdating` with an implementation of
`DesktopMediaListObserver` for delegated capturers like PipeWire. This
interface allows listening to user permission events and listing
sources only after the user has made a choice on the permission dialog.

The interface has been implemented by an inner class to allow listening
to screen and window capture permissions concurrently using two
instances of the class. A patch that was resetting the capturer on the
first refresh has been changed to exclude PipeWire. PipeWire capturer
object will follow the lifecycle of `NativeDesktopMediaList`, as is the
case in Chromium.

Fixes #37463

* fix: wait for thumbnails from PipeWire when necessary

The PipeWire stream starts after the dialog is dismissed. If the sources
are listed immediately afterwards, the thumbnail may not have been
generated by that time. Explicitly wait for both thumbnail generation
and a selection on the source dialog before listing sources.
  • Loading branch information
aiddya committed Jul 24, 2023
1 parent 7fe5925 commit 595e25a
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 20 deletions.
11 changes: 7 additions & 4 deletions patches/chromium/desktop_media_list.patch
Expand Up @@ -82,7 +82,7 @@ index 33ca7a53dfb6d2c9e3a33f0065a3acd806e82e01..9fdf2e8ff0056ff407015b914c6b03eb
const Source& GetSource(int index) const override;
DesktopMediaList::Type GetMediaListType() const override;
diff --git a/chrome/browser/media/webrtc/native_desktop_media_list.cc b/chrome/browser/media/webrtc/native_desktop_media_list.cc
index 836f7683b77b3b28f33984f27649d675ddddd5b7..84135d3cf983c88fc165a0fc012200e5132a52ff 100644
index 836f7683b77b3b28f33984f27649d675ddddd5b7..18f795dd77754e115aa66a7404a2e4075372bc5f 100644
--- a/chrome/browser/media/webrtc/native_desktop_media_list.cc
+++ b/chrome/browser/media/webrtc/native_desktop_media_list.cc
@@ -141,7 +141,7 @@ BOOL CALLBACK AllHwndCollector(HWND hwnd, LPARAM param) {
Expand All @@ -94,17 +94,20 @@ index 836f7683b77b3b28f33984f27649d675ddddd5b7..84135d3cf983c88fc165a0fc012200e5
#endif

} // namespace
@@ -451,6 +451,9 @@ void NativeDesktopMediaList::Worker::RefreshNextThumbnail() {
@@ -451,6 +451,12 @@ void NativeDesktopMediaList::Worker::RefreshNextThumbnail() {
FROM_HERE,
base::BindOnce(&NativeDesktopMediaList::UpdateNativeThumbnailsFinished,
media_list_));
+
+ // This call is necessary to release underlying OS screen capture mechanisms.
+ capturer_.reset();
+ // Skip if the source list is delegated, as the source list window will be active.
+ if (!capturer_->GetDelegatedSourceListController()) {
+ capturer_.reset();
+ }
}

void NativeDesktopMediaList::Worker::OnCaptureResult(
@@ -823,6 +826,11 @@ void NativeDesktopMediaList::RefreshForVizFrameSinkWindows(
@@ -823,6 +829,11 @@ void NativeDesktopMediaList::RefreshForVizFrameSinkWindows(
FROM_HERE, base::BindOnce(&Worker::RefreshThumbnails,
base::Unretained(worker_.get()),
std::move(native_ids), thumbnail_size_));
Expand Down
92 changes: 76 additions & 16 deletions shell/browser/api/electron_api_desktop_capturer.cc
Expand Up @@ -181,6 +181,41 @@ DesktopCapturer::DesktopCapturer(v8::Isolate* isolate) {}

DesktopCapturer::~DesktopCapturer() = default;

DesktopCapturer::DesktopListListener::DesktopListListener(
OnceCallback update_callback,
OnceCallback failure_callback,
bool skip_thumbnails)
: update_callback_(std::move(update_callback)),
failure_callback_(std::move(failure_callback)),
have_thumbnail_(skip_thumbnails) {}

DesktopCapturer::DesktopListListener::~DesktopListListener() = default;

void DesktopCapturer::DesktopListListener::OnDelegatedSourceListSelection() {
if (have_thumbnail_) {
std::move(update_callback_).Run();
} else {
have_selection_ = true;
}
}

void DesktopCapturer::DesktopListListener::OnSourceThumbnailChanged(int index) {
if (have_selection_) {
// This is called every time a thumbnail is refreshed. Reset variable to
// ensure that the callback is not run again.
have_selection_ = false;

// PipeWire returns a single source, so index is not relevant.
std::move(update_callback_).Run();
} else {
have_thumbnail_ = true;
}
}

void DesktopCapturer::DesktopListListener::OnDelegatedSourceListDismissed() {
std::move(failure_callback_).Run();
}

void DesktopCapturer::StartHandling(bool capture_window,
bool capture_screen,
const gfx::Size& thumbnail_size,
Expand Down Expand Up @@ -212,11 +247,22 @@ void DesktopCapturer::StartHandling(bool capture_window,
window_capturer_ = std::make_unique<NativeDesktopMediaList>(
DesktopMediaList::Type::kWindow, std::move(capturer));
window_capturer_->SetThumbnailSize(thumbnail_size);
window_capturer_->Update(
base::BindOnce(&DesktopCapturer::UpdateSourcesList,
weak_ptr_factory_.GetWeakPtr(),
window_capturer_.get()),
/* refresh_thumbnails = */ true);

OnceCallback update_callback = base::BindOnce(
&DesktopCapturer::UpdateSourcesList, weak_ptr_factory_.GetWeakPtr(),
window_capturer_.get());

if (window_capturer_->IsSourceListDelegated()) {
OnceCallback failure_callback = base::BindOnce(
&DesktopCapturer::HandleFailure, weak_ptr_factory_.GetWeakPtr());
window_listener_ = std::make_unique<DesktopListListener>(
std::move(update_callback), std::move(failure_callback),
thumbnail_size.IsEmpty());
window_capturer_->StartUpdating(window_listener_.get());
} else {
window_capturer_->Update(std::move(update_callback),
/* refresh_thumbnails = */ true);
}
}
}

Expand All @@ -226,11 +272,22 @@ void DesktopCapturer::StartHandling(bool capture_window,
screen_capturer_ = std::make_unique<NativeDesktopMediaList>(
DesktopMediaList::Type::kScreen, std::move(capturer));
screen_capturer_->SetThumbnailSize(thumbnail_size);
screen_capturer_->Update(
base::BindOnce(&DesktopCapturer::UpdateSourcesList,
weak_ptr_factory_.GetWeakPtr(),
screen_capturer_.get()),
/* refresh_thumbnails = */ true);

OnceCallback update_callback = base::BindOnce(
&DesktopCapturer::UpdateSourcesList, weak_ptr_factory_.GetWeakPtr(),
screen_capturer_.get());

if (screen_capturer_->IsSourceListDelegated()) {
OnceCallback failure_callback = base::BindOnce(
&DesktopCapturer::HandleFailure, weak_ptr_factory_.GetWeakPtr());
screen_listener_ = std::make_unique<DesktopListListener>(
std::move(update_callback), std::move(failure_callback),
thumbnail_size.IsEmpty());
screen_capturer_->StartUpdating(screen_listener_.get());
} else {
screen_capturer_->Update(std::move(update_callback),
/* refresh_thumbnails = */ true);
}
}
}
}
Expand Down Expand Up @@ -270,12 +327,7 @@ void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) {
// |media_list_sources|.
if (!webrtc::DxgiDuplicatorController::Instance()->GetDeviceNames(
&device_names)) {
v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
v8::HandleScope scope(isolate);
gin_helper::CallMethod(this, "_onerror", "Failed to get sources.");

Unpin();

HandleFailure();
return;
}

Expand Down Expand Up @@ -325,6 +377,14 @@ void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) {
}
}

void DesktopCapturer::HandleFailure() {
v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
v8::HandleScope scope(isolate);
gin_helper::CallMethod(this, "_onerror", "Failed to get sources.");

Unpin();
}

// static
gin::Handle<DesktopCapturer> DesktopCapturer::Create(v8::Isolate* isolate) {
auto handle = gin::CreateHandle(isolate, new DesktopCapturer(isolate));
Expand Down
29 changes: 29 additions & 0 deletions shell/browser/api/electron_api_desktop_capturer.h
Expand Up @@ -62,8 +62,37 @@ class DesktopCapturer : public gin::Wrappable<DesktopCapturer>,
void OnDelegatedSourceListDismissed() override {}

private:
using OnceCallback = base::OnceClosure;

class DesktopListListener : public DesktopMediaListObserver {
public:
DesktopListListener(OnceCallback update_callback,
OnceCallback failure_callback,
bool skip_thumbnails);
~DesktopListListener() override;

protected:
void OnSourceAdded(int index) override {}
void OnSourceRemoved(int index) override {}
void OnSourceMoved(int old_index, int new_index) override {}
void OnSourceNameChanged(int index) override {}
void OnSourceThumbnailChanged(int index) override;
void OnSourcePreviewChanged(size_t index) override {}
void OnDelegatedSourceListSelection() override;
void OnDelegatedSourceListDismissed() override;

private:
OnceCallback update_callback_;
OnceCallback failure_callback_;
bool have_selection_ = false;
bool have_thumbnail_ = false;
};

void UpdateSourcesList(DesktopMediaList* list);
void HandleFailure();

std::unique_ptr<DesktopListListener> window_listener_;
std::unique_ptr<DesktopListListener> screen_listener_;
std::unique_ptr<DesktopMediaList> window_capturer_;
std::unique_ptr<DesktopMediaList> screen_capturer_;
std::vector<DesktopCapturer::Source> captured_sources_;
Expand Down

0 comments on commit 595e25a

Please sign in to comment.