From 8bd0f97fe6873f2c1b80d220b66fdebd30eb4f65 Mon Sep 17 00:00:00 2001 From: Austin Orion Date: Thu, 28 Jan 2021 12:18:26 -0800 Subject: [PATCH] Address CL comments from 200161. This is a follow up to https://webrtc-review.googlesource.com/c/src/+/200161 I forgot to actually upload the code that addressed reviewer's comments, and since they were minor comments I went ahead and completed the CL before I realized. Fortunately no harm done because all of this code is behind a disabled build flag. This is a great example of move fast, make mistakes. Lesson learned. Bug: webrtc:9273 Change-Id: I5e35b31719b264855568de4b5e595fe0f192654e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/204420 Reviewed-by: Jamie Walch Commit-Queue: Austin Orion Cr-Commit-Position: refs/heads/master@{#33095} --- modules/desktop_capture/desktop_capturer.cc | 8 ++------ modules/desktop_capture/win/screen_capture_utils.cc | 6 ++++-- modules/desktop_capture/win/screen_capture_utils.h | 2 +- .../desktop_capture/win/screen_capture_utils_unittest.cc | 7 +++++-- modules/desktop_capture/win/wgc_capture_session.cc | 8 ++------ 5 files changed, 14 insertions(+), 17 deletions(-) diff --git a/modules/desktop_capture/desktop_capturer.cc b/modules/desktop_capture/desktop_capturer.cc index e90fce4dda..8d8bdd5835 100644 --- a/modules/desktop_capture/desktop_capturer.cc +++ b/modules/desktop_capture/desktop_capturer.cc @@ -23,8 +23,6 @@ #if defined(RTC_ENABLE_WIN_WGC) #include "modules/desktop_capture/win/wgc_capturer_win.h" #include "rtc_base/win/windows_version.h" - -const bool kUseWinWgcCapturer = false; #endif // defined(RTC_ENABLE_WIN_WGC) namespace webrtc { @@ -59,8 +57,7 @@ std::unique_ptr DesktopCapturer::CreateWindowCapturer( // TODO(bugs.webrtc.org/11760): Add a WebRTC field trial (or similar // mechanism) check here that leads to use of the WGC capturer once it is // fully implemented. - if (kUseWinWgcCapturer && - rtc::rtc_win::GetVersion() >= rtc::rtc_win::Version::VERSION_WIN10_RS5) { + if (rtc::rtc_win::GetVersion() >= rtc::rtc_win::Version::VERSION_WIN10_RS5) { return WgcCapturerWin::CreateRawWindowCapturer(options); } #endif // defined(RTC_ENABLE_WIN_WGC) @@ -86,8 +83,7 @@ std::unique_ptr DesktopCapturer::CreateScreenCapturer( // TODO(bugs.webrtc.org/11760): Add a WebRTC field trial (or similar // mechanism) check here that leads to use of the WGC capturer once it is // fully implemented. - if (kUseWinWgcCapturer && - rtc::rtc_win::GetVersion() >= rtc::rtc_win::Version::VERSION_WIN10_RS5) { + if (rtc::rtc_win::GetVersion() >= rtc::rtc_win::Version::VERSION_WIN10_RS5) { return WgcCapturerWin::CreateRawScreenCapturer(options); } #endif // defined(RTC_ENABLE_WIN_WGC) diff --git a/modules/desktop_capture/win/screen_capture_utils.cc b/modules/desktop_capture/win/screen_capture_utils.cc index caa1716bc2..c88602342e 100644 --- a/modules/desktop_capture/win/screen_capture_utils.cc +++ b/modules/desktop_capture/win/screen_capture_utils.cc @@ -32,8 +32,10 @@ BOOL CALLBACK GetMonitorListHandler(HMONITOR monitor, // Get the name of the monitor. MONITORINFOEXA monitor_info; monitor_info.cbSize = sizeof(MONITORINFOEXA); - if (!GetMonitorInfoA(monitor, &monitor_info)) - return FALSE; + if (!GetMonitorInfoA(monitor, &monitor_info)) { + // Continue the enumeration, but don't add this monitor to |monitor_list|. + return TRUE; + } DesktopCapturer::Source monitor_source; monitor_source.id = reinterpret_cast(monitor); diff --git a/modules/desktop_capture/win/screen_capture_utils.h b/modules/desktop_capture/win/screen_capture_utils.h index 1edd744c93..f9c457da8d 100644 --- a/modules/desktop_capture/win/screen_capture_utils.h +++ b/modules/desktop_capture/win/screen_capture_utils.h @@ -19,7 +19,7 @@ namespace webrtc { -// Output the HMONITOR values of all display monitors into |monitors|. Returns +// Outputs the HMONITOR values of all display monitors into |monitors|. Returns // true if succeeded, or false if it fails to enumerate the display monitors. bool GetMonitorList(DesktopCapturer::SourceList* monitors); diff --git a/modules/desktop_capture/win/screen_capture_utils_unittest.cc b/modules/desktop_capture/win/screen_capture_utils_unittest.cc index 1ced2a0a04..cd122b7950 100644 --- a/modules/desktop_capture/win/screen_capture_utils_unittest.cc +++ b/modules/desktop_capture/win/screen_capture_utils_unittest.cc @@ -14,6 +14,7 @@ #include #include "modules/desktop_capture/desktop_capturer.h" +#include "rtc_base/logging.h" #include "test/gtest.h" namespace webrtc { @@ -33,14 +34,16 @@ TEST(ScreenCaptureUtilsTest, GetMonitorList) { DesktopCapturer::SourceList monitors; ASSERT_TRUE(GetMonitorList(&monitors)); - ASSERT_GT(monitors.size(), 0ULL); } TEST(ScreenCaptureUtilsTest, IsMonitorValid) { DesktopCapturer::SourceList monitors; ASSERT_TRUE(GetMonitorList(&monitors)); - ASSERT_GT(monitors.size(), 0ULL); + if (monitors.size() == 0) { + RTC_LOG(LS_INFO) << "Skip screen capture test on systems with no monitors."; + GTEST_SKIP(); + } ASSERT_TRUE(IsMonitorValid(monitors[0].id)); } diff --git a/modules/desktop_capture/win/wgc_capture_session.cc b/modules/desktop_capture/win/wgc_capture_session.cc index f8ba6d403c..0fdb6ec98a 100644 --- a/modules/desktop_capture/win/wgc_capture_session.cc +++ b/modules/desktop_capture/win/wgc_capture_session.cc @@ -116,10 +116,8 @@ HRESULT WgcCaptureSession::StartCapture() { return hr; hr = session_->StartCapture(); - if (FAILED(hr)) { - RTC_LOG(LS_ERROR) << "Failed to start CaptureSession: " << hr; + if (FAILED(hr)) return hr; - } is_capture_started_ = true; return hr; @@ -138,10 +136,8 @@ HRESULT WgcCaptureSession::GetFrame( ComPtr capture_frame; HRESULT hr = frame_pool_->TryGetNextFrame(&capture_frame); - if (FAILED(hr)) { - RTC_LOG(LS_ERROR) << "TryGetNextFrame failed: " << hr; + if (FAILED(hr)) return hr; - } if (!capture_frame) return hr;