Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(762)

Unified Diff: webrtc/modules/desktop_capture/screen_capturer_x11.cc

Issue 1861893002: Fix screen capturers to initialize on the same thread on which Start() is called. (Closed) Base URL: https://chromium.googlesource.com/external/webrtc.git@master
Patch Set: Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « webrtc/modules/desktop_capture/screen_capturer_mac.mm ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/modules/desktop_capture/screen_capturer_x11.cc
diff --git a/webrtc/modules/desktop_capture/screen_capturer_x11.cc b/webrtc/modules/desktop_capture/screen_capturer_x11.cc
index 65e682b6f8bdc1bd28ba99d515077122fc105adb..986fe2155f7abd62a7644c18be73fd0d91842496 100644
--- a/webrtc/modules/desktop_capture/screen_capturer_x11.cc
+++ b/webrtc/modules/desktop_capture/screen_capturer_x11.cc
@@ -21,6 +21,7 @@
#include <X11/Xutil.h>
#include "webrtc/base/checks.h"
+#include "webrtc/base/thread_checker.h"
#include "webrtc/modules/desktop_capture/desktop_capture_options.h"
#include "webrtc/modules/desktop_capture/desktop_frame.h"
#include "webrtc/modules/desktop_capture/differ.h"
@@ -37,12 +38,9 @@ namespace {
class ScreenCapturerLinux : public ScreenCapturer,
public SharedXDisplay::XEventHandler {
public:
- ScreenCapturerLinux();
+ explicit ScreenCapturerLinux(const DesktopCaptureOptions& options);
virtual ~ScreenCapturerLinux();
- // TODO(ajwong): Do we really want this to be synchronous?
- bool Init(const DesktopCaptureOptions& options);
-
// DesktopCapturer interface.
void Start(Callback* delegate) override;
void Capture(const DesktopRegion& region) override;
@@ -54,11 +52,13 @@ class ScreenCapturerLinux : public ScreenCapturer,
private:
Display* display() { return options_.x_display()->display(); }
- // SharedXDisplay::XEventHandler interface.
- bool HandleXEvent(const XEvent& event) override;
+ bool Initialize();
void InitXDamage();
+ // SharedXDisplay::XEventHandler interface.
+ bool HandleXEvent(const XEvent& event) override;
+
// Capture screen pixels to the current buffer in the queue. In the DAMAGE
// case, the ScreenCapturerHelper already holds the list of invalid rectangles
// from HandleXEvent(). In the non-DAMAGE case, this captures the
@@ -78,25 +78,27 @@ class ScreenCapturerLinux : public ScreenCapturer,
void DeinitXlib();
+ rtc::ThreadChecker thread_checker_;
+
DesktopCaptureOptions options_;
- Callback* callback_;
+ Callback* callback_ = nullptr;
// X11 graphics context.
- GC gc_;
- Window root_window_;
+ GC gc_ = nullptr;
+ Window root_window_ = BadValue;
// XFixes.
- bool has_xfixes_;
- int xfixes_event_base_;
- int xfixes_error_base_;
+ bool has_xfixes_ = false;
+ int xfixes_event_base_ = -1;
+ int xfixes_error_base_ = -1;
// XDamage information.
- bool use_damage_;
- Damage damage_handle_;
- int damage_event_base_;
- int damage_error_base_;
- XserverRegion damage_region_;
+ bool use_damage_ = false;
+ Damage damage_handle_ = 0;
+ int damage_event_base_ = -1;
+ int damage_error_base_ = -1;
+ XserverRegion damage_region_ = 0;
// Access to the X Server's pixel buffer.
XServerPixelBuffer x_server_pixel_buffer_;
@@ -118,22 +120,16 @@ class ScreenCapturerLinux : public ScreenCapturer,
RTC_DISALLOW_COPY_AND_ASSIGN(ScreenCapturerLinux);
};
-ScreenCapturerLinux::ScreenCapturerLinux()
- : callback_(NULL),
- gc_(NULL),
- root_window_(BadValue),
- has_xfixes_(false),
- xfixes_event_base_(-1),
- xfixes_error_base_(-1),
- use_damage_(false),
- damage_handle_(0),
- damage_event_base_(-1),
- damage_error_base_(-1),
- damage_region_(0) {
- helper_.SetLogGridSize(4);
+ScreenCapturerLinux::ScreenCapturerLinux(const DesktopCaptureOptions& options)
+ : options_(options) {
+ // ScreenCapturer can be used on a thread different from the thread on which
+ // it's created.
+ thread_checker_.DetachFromThread();
}
ScreenCapturerLinux::~ScreenCapturerLinux() {
+ RTC_DCHECK(thread_checker_.CalledOnValidThread());
+
options_.x_display()->RemoveEventHandler(ConfigureNotify, this);
if (use_damage_) {
options_.x_display()->RemoveEventHandler(
@@ -142,8 +138,8 @@ ScreenCapturerLinux::~ScreenCapturerLinux() {
DeinitXlib();
}
-bool ScreenCapturerLinux::Init(const DesktopCaptureOptions& options) {
- options_ = options;
+bool ScreenCapturerLinux::Initialize() {
+ helper_.SetLogGridSize(4);
root_window_ = RootWindow(display(), DefaultScreen(display()));
if (root_window_ == BadValue) {
@@ -198,11 +194,6 @@ void ScreenCapturerLinux::InitXDamage() {
return;
}
- // TODO(lambroslambrou): Disable DAMAGE in situations where it is known
- // to fail, such as when Desktop Effects are enabled, with graphics
- // drivers (nVidia, ATI) that fail to report DAMAGE notifications
- // properly.
-
// Request notifications every time the screen becomes damaged.
damage_handle_ = XDamageCreate(display(), root_window_,
XDamageReportNonEmpty);
@@ -227,13 +218,20 @@ void ScreenCapturerLinux::InitXDamage() {
}
void ScreenCapturerLinux::Start(Callback* callback) {
+ RTC_DCHECK(thread_checker_.CalledOnValidThread());
RTC_DCHECK(!callback_);
RTC_DCHECK(callback);
callback_ = callback;
+
+ if (!Initialize()) {
+ callback_->OnInitializationFailed();
+ }
}
void ScreenCapturerLinux::Capture(const DesktopRegion& region) {
+ RTC_DCHECK(thread_checker_.CalledOnValidThread());
+
TickTime capture_start_time = TickTime::Now();
queue_.MoveToNextFrame();
@@ -279,7 +277,9 @@ void ScreenCapturerLinux::Capture(const DesktopRegion& region) {
}
bool ScreenCapturerLinux::GetScreenList(ScreenList* screens) {
+ RTC_DCHECK(thread_checker_.CalledOnValidThread());
RTC_DCHECK(screens->size() == 0);
+
// TODO(jiayl): implement screen enumeration.
Screen default_screen;
default_screen.id = 0;
@@ -288,11 +288,14 @@ bool ScreenCapturerLinux::GetScreenList(ScreenList* screens) {
}
bool ScreenCapturerLinux::SelectScreen(ScreenId id) {
+ RTC_DCHECK(thread_checker_.CalledOnValidThread());
// TODO(jiayl): implement screen selection.
return true;
}
bool ScreenCapturerLinux::HandleXEvent(const XEvent& event) {
+ RTC_DCHECK(thread_checker_.CalledOnValidThread());
+
if (use_damage_ && (event.type == damage_event_base_ + XDamageNotify)) {
const XDamageNotifyEvent* damage_event =
reinterpret_cast<const XDamageNotifyEvent*>(&event);
@@ -309,7 +312,7 @@ bool ScreenCapturerLinux::HandleXEvent(const XEvent& event) {
DesktopFrame* ScreenCapturerLinux::CaptureScreen() {
DesktopFrame* frame = queue_.current_frame()->Share();
- assert(x_server_pixel_buffer_.window_size().equals(frame->size()));
+ RTC_DCHECK(x_server_pixel_buffer_.window_size().equals(frame->size()));
// Pass the screen size to the helper, so it can clip the invalid region if it
// expands that region to a grid.
@@ -436,10 +439,7 @@ ScreenCapturer* ScreenCapturer::Create(const DesktopCaptureOptions& options) {
if (!options.x_display())
return NULL;
- std::unique_ptr<ScreenCapturerLinux> capturer(new ScreenCapturerLinux());
- if (!capturer->Init(options))
- capturer.reset();
- return capturer.release();
+ return new ScreenCapturerLinux(options);
}
} // namespace webrtc
« no previous file with comments | « webrtc/modules/desktop_capture/screen_capturer_mac.mm ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698