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 <jamiewalch@chromium.org>
Commit-Queue: Austin Orion <auorion@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#33095}
diff --git a/modules/desktop_capture/desktop_capturer.cc b/modules/desktop_capture/desktop_capturer.cc
index e90fce4..8d8bdd5 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 @@
// 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 @@
// 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 caa1716..c886023 100644
--- a/modules/desktop_capture/win/screen_capture_utils.cc
+++ b/modules/desktop_capture/win/screen_capture_utils.cc
@@ -32,8 +32,10 @@
// 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<intptr_t>(monitor);
diff --git a/modules/desktop_capture/win/screen_capture_utils.h b/modules/desktop_capture/win/screen_capture_utils.h
index 1edd744..f9c457d 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 1ced2a0..cd122b7 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 <vector>
#include "modules/desktop_capture/desktop_capturer.h"
+#include "rtc_base/logging.h"
#include "test/gtest.h"
namespace webrtc {
@@ -33,14 +34,16 @@
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 f8ba6d4..0fdb6ec 100644
--- a/modules/desktop_capture/win/wgc_capture_session.cc
+++ b/modules/desktop_capture/win/wgc_capture_session.cc
@@ -116,10 +116,8 @@
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 @@
ComPtr<WGC::IDirect3D11CaptureFrame> 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;
