diff --git a/modules/desktop_capture/desktop_capturer.cc b/modules/desktop_capture/desktop_capturer.cc index dc24b387d5..f4676b7fe2 100644 --- a/modules/desktop_capture/desktop_capturer.cc +++ b/modules/desktop_capture/desktop_capturer.cc @@ -29,11 +29,6 @@ namespace webrtc { DesktopCapturer::~DesktopCapturer() = default; -DelegatedSourceListController* -DesktopCapturer::GetDelegatedSourceListController() { - return nullptr; -} - void DesktopCapturer::SetSharedMemoryFactory( std::unique_ptr shared_memory_factory) {} diff --git a/modules/desktop_capture/desktop_capturer.h b/modules/desktop_capture/desktop_capturer.h index cfe7bcd40b..d2ac7be8ad 100644 --- a/modules/desktop_capture/desktop_capturer.h +++ b/modules/desktop_capture/desktop_capturer.h @@ -32,31 +32,6 @@ namespace webrtc { class DesktopCaptureOptions; class DesktopFrame; -// A controller to be implemented and returned by -// GetDelegatedSourceListController in capturers that require showing their own -// source list and managing user selection there. Apart from ensuring the -// visibility of the source list, these capturers should largely be interacted -// with the same as a normal capturer, though there may be some caveats for -// some DesktopCapturer methods. See GetDelegatedSourceListController for more -// information. -class RTC_EXPORT DelegatedSourceListController { - public: - // Used to prompt the capturer to show the delegated source list. If the - // source list is already visible, this will be a no-op. Must be called after - // starting the DesktopCapturer. - // - // Note that any selection from a previous invocation of the source list may - // be cleared when this method is called. - virtual void EnsureVisible() = 0; - - // Used to prompt the capturer to hide the delegated source list. If the - // source list is already hidden, this will be a no-op. - virtual void EnsureHidden() = 0; - - protected: - virtual ~DelegatedSourceListController() {} -}; - // Abstract interface for screen and window capturers. class RTC_EXPORT DesktopCapturer { public: @@ -113,18 +88,6 @@ class RTC_EXPORT DesktopCapturer { // valid until capturer is destroyed. virtual void Start(Callback* callback) = 0; - // Returns a valid pointer if the capturer requires the user to make a - // selection from a source list provided by the capturer. - // Returns nullptr if the capturer does not provide a UI for the user to make - // a selection. - // - // Callers should not take ownership of the returned pointer, but it is - // guaranteed to be valid as long as the desktop_capturer is valid. - // Note that consumers should still use GetSourceList and SelectSource, but - // their behavior may be modified if this returns a value. See those methods - // for a more in-depth discussion of those potential modifications. - virtual DelegatedSourceListController* GetDelegatedSourceListController(); - // Sets SharedMemoryFactory that will be used to create buffers for the // captured frames. The factory can be invoked on a thread other than the one // where CaptureFrame() is called. It will be destroyed on the same thread. @@ -153,19 +116,10 @@ class RTC_EXPORT DesktopCapturer { // should return monitors. // For DesktopCapturer implementations to capture windows, this function // should only return root windows owned by applications. - // - // Note that capturers who use a delegated source list will return a - // SourceList with exactly one value, but it may not be viable for capture - // (e.g. CaptureFrame will return ERROR_TEMPORARY) until a selection has been - // made. virtual bool GetSourceList(SourceList* sources); // Selects a source to be captured. Returns false in case of a failure (e.g. // if there is no source with the specified type and id.) - // - // Note that some capturers with delegated source lists may also support - // selecting a SourceID that is not in the returned source list as a form of - // restore token. virtual bool SelectSource(SourceId id); // Brings the selected source to the front and sets the input focus on it. diff --git a/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc b/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc index bf7a43b0fa..baa97daa76 100644 --- a/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc +++ b/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc @@ -16,6 +16,8 @@ #include "modules/desktop_capture/linux/wayland/xdg_desktop_portal_utils.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" +#include "rtc_base/random.h" +#include "rtc_base/time_utils.h" namespace webrtc { @@ -42,7 +44,8 @@ BaseCapturerPipeWire::BaseCapturerPipeWire( : options_(options), is_screencast_portal_(false), portal_(std::move(portal)) { - source_id_ = RestoreTokenManager::GetInstance().GetUnusedId(); + Random random(rtc::TimeMicros()); + source_id_ = static_cast(random.Rand(1, INT_MAX)); } BaseCapturerPipeWire::~BaseCapturerPipeWire() {} @@ -50,11 +53,6 @@ BaseCapturerPipeWire::~BaseCapturerPipeWire() {} void BaseCapturerPipeWire::OnScreenCastRequestResult(RequestResponse result, uint32_t stream_node_id, int fd) { - is_portal_open_ = false; - - // Reset the value of capturer_failed_ in case we succeed below. If we fail, - // then it'll set it to the right value again soon enough. - capturer_failed_ = false; if (result != RequestResponse::kSuccess || !options_.screencast_stream()->StartScreenCastStream( stream_node_id, fd, options_.get_width(), options_.get_height())) { @@ -97,15 +95,11 @@ void BaseCapturerPipeWire::Start(Callback* callback) { } } - is_portal_open_ = true; portal_->Start(); } void BaseCapturerPipeWire::CaptureFrame() { if (capturer_failed_) { - // This could be recoverable if the source list is re-summoned; but for our - // purposes this is fine, since it requires intervention to resolve and - // essentially starts a new capture. callback_->OnCaptureResult(Result::ERROR_PERMANENT, nullptr); return; } @@ -143,35 +137,6 @@ bool BaseCapturerPipeWire::SelectSource(SourceId id) { return true; } -DelegatedSourceListController* -BaseCapturerPipeWire::GetDelegatedSourceListController() { - return this; -} - -void BaseCapturerPipeWire::EnsureVisible() { - RTC_DCHECK(callback_); - if (is_portal_open_) - return; - - // Clear any previously selected state/capture - portal_->Cleanup(); - options_.screencast_stream()->StopScreenCastStream(); - - // Get a new source id to reflect that the source has changed. - source_id_ = RestoreTokenManager::GetInstance().GetUnusedId(); - - is_portal_open_ = true; - portal_->Start(); -} - -void BaseCapturerPipeWire::EnsureHidden() { - if (!is_portal_open_) - return; - - is_portal_open_ = false; - portal_->Cleanup(); -} - SessionDetails BaseCapturerPipeWire::GetSessionDetails() { return portal_->GetSessionDetails(); } diff --git a/modules/desktop_capture/linux/wayland/base_capturer_pipewire.h b/modules/desktop_capture/linux/wayland/base_capturer_pipewire.h index 62878536e2..6e18f4d11b 100644 --- a/modules/desktop_capture/linux/wayland/base_capturer_pipewire.h +++ b/modules/desktop_capture/linux/wayland/base_capturer_pipewire.h @@ -23,7 +23,6 @@ namespace webrtc { class BaseCapturerPipeWire : public DesktopCapturer, - public DelegatedSourceListController, public ScreenCastPortal::PortalNotifier { public: explicit BaseCapturerPipeWire(const DesktopCaptureOptions& options); @@ -40,11 +39,6 @@ class BaseCapturerPipeWire : public DesktopCapturer, void CaptureFrame() override; bool GetSourceList(SourceList* sources) override; bool SelectSource(SourceId id) override; - DelegatedSourceListController* GetDelegatedSourceListController() override; - - // DelegatedSourceListController - void EnsureVisible() override; - void EnsureHidden() override; // ScreenCastPortal::PortalNotifier interface. void OnScreenCastRequestResult(xdg_portal::RequestResponse result, @@ -62,7 +56,6 @@ class BaseCapturerPipeWire : public DesktopCapturer, Callback* callback_ = nullptr; bool capturer_failed_ = false; bool is_screencast_portal_ = false; - bool is_portal_open_ = false; // SourceId that is selected using SelectSource() and that we previously // returned in GetSourceList(). This should be a SourceId that has a restore diff --git a/modules/desktop_capture/linux/wayland/restore_token_manager.cc b/modules/desktop_capture/linux/wayland/restore_token_manager.cc index 5ca9b957a9..cc626d3065 100644 --- a/modules/desktop_capture/linux/wayland/restore_token_manager.cc +++ b/modules/desktop_capture/linux/wayland/restore_token_manager.cc @@ -30,8 +30,4 @@ std::string RestoreTokenManager::TakeToken(DesktopCapturer::SourceId id) { return token; } -DesktopCapturer::SourceId RestoreTokenManager::GetUnusedId() { - return ++last_source_id_; -} - } // namespace webrtc diff --git a/modules/desktop_capture/linux/wayland/restore_token_manager.h b/modules/desktop_capture/linux/wayland/restore_token_manager.h index 174bef121f..37c9a39cac 100644 --- a/modules/desktop_capture/linux/wayland/restore_token_manager.h +++ b/modules/desktop_capture/linux/wayland/restore_token_manager.h @@ -29,15 +29,10 @@ class RestoreTokenManager { void AddToken(DesktopCapturer::SourceId id, const std::string& token); std::string TakeToken(DesktopCapturer::SourceId id); - // Returns a source ID which does not have any token associated with it yet. - DesktopCapturer::SourceId GetUnusedId(); - private: RestoreTokenManager() = default; ~RestoreTokenManager() = default; - DesktopCapturer::SourceId last_source_id_ = 0; - std::unordered_map restore_tokens_; }; diff --git a/modules/desktop_capture/linux/wayland/screen_capture_portal_interface.h b/modules/desktop_capture/linux/wayland/screen_capture_portal_interface.h index 00c4b30390..775ed1facc 100644 --- a/modules/desktop_capture/linux/wayland/screen_capture_portal_interface.h +++ b/modules/desktop_capture/linux/wayland/screen_capture_portal_interface.h @@ -42,9 +42,6 @@ class ScreenCapturePortalInterface { virtual xdg_portal::SessionDetails GetSessionDetails() { return {}; } // Starts the portal setup. virtual void Start() {} - - virtual void Cleanup() {} - // Notifies observers about the success/fail state of the portal // request/response. virtual void OnPortalDone(xdg_portal::RequestResponse result) {} diff --git a/modules/desktop_capture/linux/wayland/screencast_portal.cc b/modules/desktop_capture/linux/wayland/screencast_portal.cc index 1bfdc0fe2a..8e45af7e24 100644 --- a/modules/desktop_capture/linux/wayland/screencast_portal.cc +++ b/modules/desktop_capture/linux/wayland/screencast_portal.cc @@ -66,11 +66,9 @@ void ScreenCastPortal::Cleanup() { session_handle_ = ""; cancellable_ = nullptr; proxy_ = nullptr; - restore_token_ = ""; if (pw_fd_ != -1) { close(pw_fd_); - pw_fd_ = -1; } } diff --git a/modules/desktop_capture/linux/wayland/screencast_portal.h b/modules/desktop_capture/linux/wayland/screencast_portal.h index 4bfba9f1f8..7970710c41 100644 --- a/modules/desktop_capture/linux/wayland/screencast_portal.h +++ b/modules/desktop_capture/linux/wayland/screencast_portal.h @@ -112,7 +112,7 @@ class ScreenCastPortal : public xdg_portal::ScreenCapturePortalInterface { // Sends a create session request to the portal. void RequestSession(GDBusProxy* proxy) override; - void Cleanup() override; + void Cleanup(); // Set of methods leveraged by remote desktop portal to setup a common session // with screen cast portal. diff --git a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc index 99cfa6f456..bfb3ff47d7 100644 --- a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc +++ b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc @@ -98,9 +98,6 @@ class SharedScreenCastStreamPrivate { DesktopVector CaptureCursorPosition(); private: - // Stops the streams and cleans up any in-use elements. - void StopAndCleanupStream(); - uint32_t pw_stream_node_id_ = 0; DesktopSize stream_size_ = {}; @@ -366,7 +363,25 @@ void SharedScreenCastStreamPrivate::OnRenegotiateFormat(void* data, uint64_t) { SharedScreenCastStreamPrivate::SharedScreenCastStreamPrivate() {} SharedScreenCastStreamPrivate::~SharedScreenCastStreamPrivate() { - StopAndCleanupStream(); + if (pw_main_loop_) { + pw_thread_loop_stop(pw_main_loop_); + } + + if (pw_stream_) { + pw_stream_destroy(pw_stream_); + } + + if (pw_core_) { + pw_core_disconnect(pw_core_); + } + + if (pw_context_) { + pw_context_destroy(pw_context_); + } + + if (pw_main_loop_) { + pw_thread_loop_destroy(pw_main_loop_); + } } RTC_NO_SANITIZE("cfi-icall") @@ -535,54 +550,15 @@ void SharedScreenCastStreamPrivate::UpdateScreenCastStreamResolution( } void SharedScreenCastStreamPrivate::StopScreenCastStream() { - StopAndCleanupStream(); -} - -void SharedScreenCastStreamPrivate::StopAndCleanupStream() { - // We get buffers on the PipeWire thread, but this is called from the capturer - // thread, so we need to wait on and stop the pipewire thread before we - // disconnect the stream so that we can guarantee we aren't in the middle of - // processing a new frame. - - // Even if we *do* somehow have the other objects without a pipewire thread, - // destroying them without a thread causes a crash. - if (!pw_main_loop_) - return; - - // While we can stop the thread now, we cannot destroy it until we've cleaned - // up the other members. - pw_thread_loop_wait(pw_main_loop_); - pw_thread_loop_stop(pw_main_loop_); - if (pw_stream_) { pw_stream_disconnect(pw_stream_); - pw_stream_destroy(pw_stream_); - pw_stream_ = nullptr; - - { - webrtc::MutexLock lock(&queue_lock_); - queue_.Reset(); - } } - - if (pw_core_) { - pw_core_disconnect(pw_core_); - pw_core_ = nullptr; - } - - if (pw_context_) { - pw_context_destroy(pw_context_); - pw_context_ = nullptr; - } - - pw_thread_loop_destroy(pw_main_loop_); - pw_main_loop_ = nullptr; } std::unique_ptr SharedScreenCastStreamPrivate::CaptureFrame() { webrtc::MutexLock lock(&queue_lock_); - if (!pw_stream_ || !queue_.current_frame()) { + if (!queue_.current_frame()) { return std::unique_ptr{}; }