From 05bba2be48f241e865675fdffb17fff8f9efb62d Mon Sep 17 00:00:00 2001 From: zijiehe Date: Wed, 21 Sep 2016 17:25:41 -0700 Subject: [PATCH] Several minor changes to ScreenCapturerWinMagnifier 1. Remove legacy screen-saver-blocking logic 2. tls_index_ is not a good choice, we can use thread-static 3. ScreenCapturerHelper is not designed for this scenario 4. Disable this capturer on 2+ monitors system BUG=638802 Review-Url: https://codereview.webrtc.org/2319383002 Cr-Commit-Position: refs/heads/master@{#14342} --- .../screen_capturer_unittest.cc | 32 ++++--- .../win/screen_capturer_win_magnifier.cc | 94 +++++++++---------- .../win/screen_capturer_win_magnifier.h | 7 -- 3 files changed, 62 insertions(+), 71 deletions(-) diff --git a/webrtc/modules/desktop_capture/screen_capturer_unittest.cc b/webrtc/modules/desktop_capture/screen_capturer_unittest.cc index 5c3671672a..6a70dc23cc 100644 --- a/webrtc/modules/desktop_capture/screen_capturer_unittest.cc +++ b/webrtc/modules/desktop_capture/screen_capturer_unittest.cc @@ -277,15 +277,14 @@ TEST_F(ScreenCapturerTest, Capture) { EXPECT_TRUE(it.IsAtEnd()); } -// Disabled due to being flaky due to the fact that it useds rendering / UI, -// see webrtc/6366. +// Disabled due to being flaky due to the fact that it uses rendering / UI, see +// webrtc/6366. TEST_F(ScreenCapturerTest, DISABLED_CaptureUpdatedRegion) { TestCaptureUpdatedRegion(); } -// Disabled due to being flaky due to the fact that it useds rendering / UI, -// see webrtc/6366. -// TODO(zijiehe): Find out the reason of failure of this test on trybot. +// Disabled due to being flaky due to the fact that it uses rendering / UI, see +// webrtc/6366. TEST_F(ScreenCapturerTest, DISABLED_TwoCapturers) { std::unique_ptr capturer2 = std::move(capturer_); SetUp(); @@ -357,8 +356,8 @@ TEST_F(ScreenCapturerTest, UseDirectxCapturerWithSharedBuffers) { EXPECT_EQ(frame->shared_memory()->id(), kTestSharedMemoryId); } -// Disabled due to being flaky due to the fact that it useds rendering / UI, -// see webrtc/6366. +// Disabled due to being flaky due to the fact that it uses rendering / UI, see +// webrtc/6366. TEST_F(ScreenCapturerTest, DISABLED_CaptureUpdatedRegionWithDirectxCapturer) { if (!CreateDirectxCapturer()) { return; @@ -367,8 +366,8 @@ TEST_F(ScreenCapturerTest, DISABLED_CaptureUpdatedRegionWithDirectxCapturer) { TestCaptureUpdatedRegion(); } -// Disabled due to being flaky due to the fact that it useds rendering / UI, -// see webrtc/6366. +// Disabled due to being flaky due to the fact that it uses rendering / UI, see +// webrtc/6366. TEST_F(ScreenCapturerTest, DISABLED_TwoDirectxCapturers) { if (!CreateDirectxCapturer()) { return; @@ -379,8 +378,15 @@ TEST_F(ScreenCapturerTest, DISABLED_TwoDirectxCapturers) { TestCaptureUpdatedRegion({capturer_.get(), capturer2.get()}); } -// Disabled due to being flaky due to the fact that it useds rendering / UI, -// see webrtc/6366. +// Disabled due to being flaky due to the fact that it uses rendering / UI, see +// webrtc/6366. +TEST_F(ScreenCapturerTest, DISABLED_CaptureUpdatedRegionWithMagnifierCapturer) { + CreateMagnifierCapturer(); + TestCaptureUpdatedRegion(); +} + +// Disabled due to being flaky due to the fact that it uses rendering / UI, see +// webrtc/6366. TEST_F(ScreenCapturerTest, DISABLED_TwoMagnifierCapturers) { CreateMagnifierCapturer(); std::unique_ptr capturer2 = std::move(capturer_); @@ -388,8 +394,8 @@ TEST_F(ScreenCapturerTest, DISABLED_TwoMagnifierCapturers) { TestCaptureUpdatedRegion({capturer_.get(), capturer2.get()}); } -// Disabled due to being flaky due to the fact that it useds rendering / UI, -// see webrtc/6366. +// Disabled due to being flaky due to the fact that it uses rendering / UI, see +// webrtc/6366. TEST_F(ScreenCapturerTest, DISABLED_MaybeCaptureUpdatedRegionWithDirectxCapturer) { // Even DirectX capturer is not supported in current system, we should be able diff --git a/webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.cc b/webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.cc index 7fdc8eb43f..e22b272d14 100644 --- a/webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.cc +++ b/webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.cc @@ -10,10 +10,9 @@ #include "webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.h" -#include - #include +#include "webrtc/base/checks.h" #include "webrtc/base/timeutils.h" #include "webrtc/modules/desktop_capture/desktop_capture_options.h" #include "webrtc/modules/desktop_capture/desktop_frame.h" @@ -58,11 +57,15 @@ ScreenCapturerWinMagnifier::~ScreenCapturerWinMagnifier() { } void ScreenCapturerWinMagnifier::Start(Callback* callback) { - assert(!callback_); - assert(callback); + RTC_DCHECK(!callback_); + RTC_DCHECK(callback); callback_ = callback; - InitializeMagnifier(); + if (!InitializeMagnifier()) { + LOG_F(LS_WARNING) << "Switching to fallback screen capturer becuase " + "magnifier initialization failed."; + StartFallbackCapturer(); + } } void ScreenCapturerWinMagnifier::SetSharedMemoryFactory( @@ -71,18 +74,21 @@ void ScreenCapturerWinMagnifier::SetSharedMemoryFactory( } void ScreenCapturerWinMagnifier::Capture(const DesktopRegion& region) { + if (!magnifier_initialized_ || + !magnifier_capture_succeeded_ || + GetSystemMetrics(SM_CMONITORS) != 1) { + // Do not try to use the magnifier if it failed before and in multi-screen + // setup (where the API crashes sometimes). + LOG_F(LS_WARNING) << "Switching to the fallback screen capturer because " + "initialization or last capture attempt failed, or " + "execute on multi-screen system."; + StartFallbackCapturer(); + fallback_capturer_->Capture(region); + return; + } + int64_t capture_start_time_nanos = rtc::TimeNanos(); - queue_.MoveToNextFrame(); - - // Request that the system not power-down the system, or the display hardware. - if (!SetThreadExecutionState(ES_DISPLAY_REQUIRED | ES_SYSTEM_REQUIRED)) { - if (!set_thread_execution_state_failed_) { - set_thread_execution_state_failed_ = true; - LOG_F(LS_WARNING) << "Failed to make system & display power assertion: " - << GetLastError(); - } - } // Switch to the desktop receiving user input if different from the current // one. std::unique_ptr input_desktop(Desktop::GetInputDesktop()); @@ -97,22 +103,14 @@ void ScreenCapturerWinMagnifier::Capture(const DesktopRegion& region) { desktop_.SetThreadDesktop(input_desktop.release()); } - bool succeeded = false; - - // Do not try to use the magnifier if it failed before and in multi-screen - // setup (where the API crashes sometimes). - if (magnifier_initialized_ && (GetSystemMetrics(SM_CMONITORS) == 1) && - magnifier_capture_succeeded_) { - DesktopRect rect = GetScreenRect(current_screen_id_, current_device_key_); - CreateCurrentFrameIfNecessary(rect.size()); - - // CaptureImage may fail in some situations, e.g. windows8 metro mode. - succeeded = CaptureImage(rect); - } - - // Defer to the fallback capturer if magnifier capturer did not work. - if (!succeeded) { - LOG_F(LS_WARNING) << "Switching to the fallback screen capturer."; + DesktopRect rect = GetScreenRect(current_screen_id_, current_device_key_); + queue_.MoveToNextFrame(); + CreateCurrentFrameIfNecessary(rect.size()); + // CaptureImage may fail in some situations, e.g. windows8 metro mode. So + // defer to the fallback capturer if magnifier capturer did not work. + if (!CaptureImage(rect)) { + LOG_F(LS_WARNING) << "Switching to the fallback screen capturer because " + "last capture attempt failed."; StartFallbackCapturer(); fallback_capturer_->Capture(region); return; @@ -120,6 +118,7 @@ void ScreenCapturerWinMagnifier::Capture(const DesktopRegion& region) { const DesktopFrame* current_frame = queue_.current_frame(); const DesktopFrame* last_frame = queue_.previous_frame(); + DesktopRegion updated_region; if (last_frame && last_frame->size().equals(current_frame->size())) { // Make sure the differencer is set up correctly for these previous and // current screens. @@ -133,24 +132,17 @@ void ScreenCapturerWinMagnifier::Capture(const DesktopRegion& region) { } // Calculate difference between the two last captured frames. - DesktopRegion region; differ_->CalcDirtyRegion( - last_frame->data(), current_frame->data(), ®ion); - helper_.InvalidateRegion(region); + last_frame->data(), current_frame->data(), &updated_region); } else { - // No previous frame is available, or the screen is resized. Invalidate the - // whole screen. - helper_.InvalidateScreen(current_frame->size()); + updated_region.SetRect(DesktopRect::MakeSize(current_frame->size())); } - helper_.set_size_most_recent(current_frame->size()); - // Emit the current frame. std::unique_ptr frame = queue_.current_frame()->Share(); frame->set_dpi(DesktopVector(GetDeviceCaps(desktop_dc_, LOGPIXELSX), GetDeviceCaps(desktop_dc_, LOGPIXELSY))); - frame->mutable_updated_region()->Clear(); - helper_.TakeInvalidRegion(frame->mutable_updated_region()); + frame->mutable_updated_region()->Swap(&updated_region); frame->set_capture_time_ms((rtc::TimeNanos() - capture_start_time_nanos) / rtc::kNumNanosecsPerMillisec); callback_->OnCaptureResult(Result::SUCCESS, std::move(frame)); @@ -183,7 +175,7 @@ void ScreenCapturerWinMagnifier::SetExcludedWindow(WindowId excluded_window) { } bool ScreenCapturerWinMagnifier::CaptureImage(const DesktopRect& rect) { - assert(magnifier_initialized_); + RTC_DCHECK(magnifier_initialized_); // Set the magnifier control to cover the captured rect. The content of the // magnifier control will be the captured image. @@ -200,6 +192,8 @@ bool ScreenCapturerWinMagnifier::CaptureImage(const DesktopRect& rect) { RECT native_rect = {rect.left(), rect.top(), rect.right(), rect.bottom()}; + RTC_DCHECK(tls_index_.Value() != static_cast(TLS_OUT_OF_INDEXES)); + TlsSetValue(tls_index_.Value(), this); // OnCaptured will be called via OnMagImageScalingCallback and fill in the // frame before set_window_source_func_ returns. result = set_window_source_func_(magnifier_window_, native_rect); @@ -223,20 +217,21 @@ BOOL ScreenCapturerWinMagnifier::OnMagImageScalingCallback( RECT unclipped, RECT clipped, HRGN dirty) { - assert(tls_index_.Value() != static_cast(TLS_OUT_OF_INDEXES)); - + RTC_DCHECK(tls_index_.Value() != static_cast(TLS_OUT_OF_INDEXES)); ScreenCapturerWinMagnifier* owner = reinterpret_cast( TlsGetValue(tls_index_.Value())); - + TlsSetValue(tls_index_.Value(), nullptr); owner->OnCaptured(srcdata, srcheader); return TRUE; } +// TODO(zijiehe): These functions are available on Windows Vista or upper, so we +// do not need to use LoadLibrary and GetProcAddress anymore. Use regular +// include and function calls instead of a dynamical loaded library. bool ScreenCapturerWinMagnifier::InitializeMagnifier() { - assert(!magnifier_initialized_); - + RTC_DCHECK(!magnifier_initialized_); desktop_dc_ = GetDC(nullptr); mag_lib_handle_ = LoadLibrary(L"Magnification.dll"); @@ -353,9 +348,6 @@ bool ScreenCapturerWinMagnifier::InitializeMagnifier() { TlsFree(new_tls_index); } - assert(tls_index_.Value() != static_cast(TLS_OUT_OF_INDEXES)); - TlsSetValue(tls_index_.Value(), this); - magnifier_initialized_ = true; return true; } @@ -407,7 +399,7 @@ void ScreenCapturerWinMagnifier::CreateCurrentFrameIfNecessary( } void ScreenCapturerWinMagnifier::StartFallbackCapturer() { - assert(fallback_capturer_); + RTC_DCHECK(fallback_capturer_); if (!fallback_capturer_started_) { fallback_capturer_started_ = true; diff --git a/webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.h b/webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.h index 1a90791c03..969cb83bcf 100644 --- a/webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.h +++ b/webrtc/modules/desktop_capture/win/screen_capturer_win_magnifier.h @@ -113,19 +113,12 @@ class ScreenCapturerWinMagnifier : public ScreenCapturer { std::wstring current_device_key_; HWND excluded_window_ = NULL; - // A thread-safe list of invalid rectangles, and the size of the most - // recently captured screen. - ScreenCapturerHelper helper_; - // Queue of the frames buffers. ScreenCaptureFrameQueue queue_; // Class to calculate the difference between two screen bitmaps. std::unique_ptr differ_; - // Used to suppress duplicate logging of SetThreadExecutionState errors. - bool set_thread_execution_state_failed_ = false; - ScopedThreadDesktop desktop_; // Used for getting the screen dpi.