Fix failing WGC tests on Win10

Several tests starting failing when run on trybots on Win10. This CL
fixes several issues that were uncovered.

Issue 1:
Capture failed to start because `get_Size` returned {0, 0}. This is a
known issue in the WGC API that occurs when there are multiple user
sessions on the same machine.
Solution:
Add a `GetSize` method to the `WgcCaptureSource` interface so we can
fallback to other methods if `get_Size` fails.

Issue 2:
The screen capture tests assume there will be displays attached and
fail if there aren't.
Solution:
Always run `IsWgcSupported` for the appropriate capture type.

Issue 3:
ASAN container-overflow in `GetTestWindowIdFromSourceList`
Solution:
Check the validity of the iterator before dereferencing.

Issue 4:
Occasionally, the call to `GetMessage` in the `CloseWindowMidCapture`
test would hang because there were no messages in the queue.
Solution:
Use `PeekMessage` instead which will return if there are no messages.

Bug: webrtc:14002
Change-Id: I69b2f765db87d34a41d6a1796cd5a81f4029be33
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/260202
Reviewed-by: Alexander Cooper <alcooper@chromium.org>
Commit-Queue: Austin Orion <auorion@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#36802}
This commit is contained in:
Austin Orion 2022-05-06 15:53:30 -07:00 committed by WebRTC LUCI CQ
parent c48ad732d6
commit c5b8c8f36b
8 changed files with 158 additions and 97 deletions

View File

@ -648,7 +648,7 @@ rtc_library("desktop_capture_generic") {
"win/wgc_desktop_frame.cc",
"win/wgc_desktop_frame.h",
]
libs += [ "dwmapi.lib" ]
deps += [ "../../rtc_base/win:hstring" ]
}
}

View File

@ -12,7 +12,9 @@
#include <windows.graphics.capture.interop.h>
#include <windows.graphics.directX.direct3d11.interop.h>
#include <wrl.h>
#include <windows.graphics.h>
#include <wrl/client.h>
#include <wrl/event.h>
#include <memory>
#include <utility>
@ -93,8 +95,11 @@ void RecordGetFrameResult(GetFrameResult error) {
} // namespace
WgcCaptureSession::WgcCaptureSession(ComPtr<ID3D11Device> d3d11_device,
ComPtr<WGC::IGraphicsCaptureItem> item)
: d3d11_device_(std::move(d3d11_device)), item_(std::move(item)) {}
ComPtr<WGC::IGraphicsCaptureItem> item,
ABI::Windows::Graphics::SizeInt32 size)
: d3d11_device_(std::move(d3d11_device)),
item_(std::move(item)),
size_(size) {}
WgcCaptureSession::~WgcCaptureSession() = default;
HRESULT WgcCaptureSession::StartCapture() {
@ -162,18 +167,8 @@ HRESULT WgcCaptureSession::StartCapture() {
return hr;
}
ABI::Windows::Graphics::SizeInt32 item_size;
hr = item_.Get()->get_Size(&item_size);
if (FAILED(hr)) {
RecordStartCaptureResult(StartCaptureResult::kGetItemSizeFailed);
return hr;
}
previous_size_ = item_size;
hr = frame_pool_statics2->CreateFreeThreaded(direct3d_device_.Get(),
kPixelFormat, kNumBuffers,
item_size, &frame_pool_);
hr = frame_pool_statics2->CreateFreeThreaded(
direct3d_device_.Get(), kPixelFormat, kNumBuffers, size_, &frame_pool_);
if (FAILED(hr)) {
RecordStartCaptureResult(StartCaptureResult::kCreateFreeThreadedFailed);
return hr;
@ -272,8 +267,7 @@ HRESULT WgcCaptureSession::GetFrame(
// If the size changed, we must resize `mapped_texture_` and `frame_pool_` to
// fit the new size. This must be done before `CopySubresourceRegion` so that
// the textures are the same size.
if (previous_size_.Height != new_size.Height ||
previous_size_.Width != new_size.Width) {
if (size_.Height != new_size.Height || size_.Width != new_size.Width) {
hr = CreateMappedTexture(texture_2D, new_size.Width, new_size.Height);
if (FAILED(hr)) {
RecordGetFrameResult(GetFrameResult::kResizeMappedTextureFailed);
@ -291,8 +285,8 @@ HRESULT WgcCaptureSession::GetFrame(
// If the size has changed since the last capture, we must be sure to use
// the smaller dimensions. Otherwise we might overrun our buffer, or
// read stale data from the last frame.
int image_height = std::min(previous_size_.Height, new_size.Height);
int image_width = std::min(previous_size_.Width, new_size.Width);
int image_height = std::min(size_.Height, new_size.Height);
int image_width = std::min(size_.Width, new_size.Width);
D3D11_BOX copy_region;
copy_region.left = 0;
@ -337,7 +331,7 @@ HRESULT WgcCaptureSession::GetFrame(
*output_frame = std::make_unique<WgcDesktopFrame>(size, row_data_length,
std::move(image_data));
previous_size_ = new_size;
size_ = new_size;
RecordGetFrameResult(GetFrameResult::kSuccess);
return hr;
}

View File

@ -13,6 +13,7 @@
#include <d3d11.h>
#include <windows.graphics.capture.h>
#include <windows.graphics.h>
#include <wrl/client.h>
#include <memory>
@ -28,7 +29,8 @@ class WgcCaptureSession final {
WgcCaptureSession(
Microsoft::WRL::ComPtr<ID3D11Device> d3d11_device,
Microsoft::WRL::ComPtr<
ABI::Windows::Graphics::Capture::IGraphicsCaptureItem> item);
ABI::Windows::Graphics::Capture::IGraphicsCaptureItem> item,
ABI::Windows::Graphics::SizeInt32 size);
// Disallow copy and assign.
WgcCaptureSession(const WgcCaptureSession&) = delete;
@ -87,10 +89,10 @@ class WgcCaptureSession final {
// frame.
Microsoft::WRL::ComPtr<ID3D11Texture2D> mapped_texture_;
// This lets us know when the source has been resized, which is important
// because we must resize the framepool and our texture to be able to hold
// enough data for the frame.
ABI::Windows::Graphics::SizeInt32 previous_size_;
// This is the size of `mapped_texture_` and the buffers in `frame_pool_`. We
// store this as a member so we can compare it to the size of incoming frames
// and resize if necessary.
ABI::Windows::Graphics::SizeInt32 size_;
// The capture session lets us set properties about the capture before it
// starts such as whether to capture the mouse cursor, and it lets us tell WGC

View File

@ -10,6 +10,7 @@
#include "modules/desktop_capture/win/wgc_capture_source.h"
#include <dwmapi.h>
#include <windows.graphics.capture.interop.h>
#include <windows.h>
@ -40,6 +41,18 @@ bool WgcCaptureSource::FocusOnSource() {
return false;
}
ABI::Windows::Graphics::SizeInt32 WgcCaptureSource::GetSize() {
if (!item_)
return {0, 0};
ABI::Windows::Graphics::SizeInt32 item_size;
HRESULT hr = item_->get_Size(&item_size);
if (FAILED(hr))
return {0, 0};
return item_size;
}
HRESULT WgcCaptureSource::GetCaptureItem(
ComPtr<WGC::IGraphicsCaptureItem>* result) {
HRESULT hr = S_OK;
@ -80,6 +93,18 @@ DesktopVector WgcWindowSource::GetTopLeft() {
return window_rect.top_left();
}
ABI::Windows::Graphics::SizeInt32 WgcWindowSource::GetSize() {
RECT window_rect;
HRESULT hr = ::DwmGetWindowAttribute(
reinterpret_cast<HWND>(GetSourceId()), DWMWA_EXTENDED_FRAME_BOUNDS,
reinterpret_cast<void*>(&window_rect), sizeof(window_rect));
if (FAILED(hr))
return WgcCaptureSource::GetSize();
return {window_rect.right - window_rect.left,
window_rect.bottom - window_rect.top};
}
bool WgcWindowSource::IsCapturable() {
if (!IsWindowValidAndVisible(reinterpret_cast<HWND>(GetSourceId())))
return false;
@ -138,6 +163,15 @@ DesktopVector WgcScreenSource::GetTopLeft() {
return GetMonitorRect(*hmonitor_).top_left();
}
ABI::Windows::Graphics::SizeInt32 WgcScreenSource::GetSize() {
ABI::Windows::Graphics::SizeInt32 size = WgcCaptureSource::GetSize();
if (!hmonitor_ || (size.Width != 0 && size.Height != 0))
return size;
DesktopRect rect = GetMonitorRect(*hmonitor_);
return {rect.width(), rect.height()};
}
bool WgcScreenSource::IsCapturable() {
if (!hmonitor_)
return false;

View File

@ -12,6 +12,7 @@
#define MODULES_DESKTOP_CAPTURE_WIN_WGC_CAPTURE_SOURCE_H_
#include <windows.graphics.capture.h>
#include <windows.graphics.h>
#include <wrl/client.h>
#include <memory>
@ -34,6 +35,7 @@ class WgcCaptureSource {
virtual DesktopVector GetTopLeft() = 0;
virtual bool IsCapturable();
virtual bool FocusOnSource();
virtual ABI::Windows::Graphics::SizeInt32 GetSize();
HRESULT GetCaptureItem(
Microsoft::WRL::ComPtr<
ABI::Windows::Graphics::Capture::IGraphicsCaptureItem>* result);
@ -96,6 +98,7 @@ class WgcWindowSource final : public WgcCaptureSource {
~WgcWindowSource() override;
DesktopVector GetTopLeft() override;
ABI::Windows::Graphics::SizeInt32 GetSize() override;
bool IsCapturable() override;
bool FocusOnSource() override;
@ -117,6 +120,7 @@ class WgcScreenSource final : public WgcCaptureSource {
~WgcScreenSource() override;
DesktopVector GetTopLeft() override;
ABI::Windows::Graphics::SizeInt32 GetSize() override;
bool IsCapturable() override;
private:

View File

@ -19,10 +19,10 @@
#include "modules/desktop_capture/desktop_geometry.h"
#include "modules/desktop_capture/win/screen_capture_utils.h"
#include "modules/desktop_capture/win/test_support/test_window.h"
#include "modules/desktop_capture/win/wgc_capturer_win.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/win/scoped_com_initializer.h"
#include "rtc_base/win/windows_version.h"
#include "test/gtest.h"
namespace webrtc {
@ -35,19 +35,11 @@ const int kFirstYCoord = 50;
const int kSecondXCoord = 50;
const int kSecondYCoord = 75;
enum SourceType { kWindowSource = 0, kScreenSource = 1 };
} // namespace
class WgcCaptureSourceTest : public ::testing::TestWithParam<SourceType> {
class WgcCaptureSourceTest : public ::testing::TestWithParam<CaptureType> {
public:
void SetUp() override {
if (rtc::rtc_win::GetVersion() < rtc::rtc_win::Version::VERSION_WIN10_RS5) {
RTC_LOG(LS_INFO)
<< "Skipping WgcCaptureSourceTests on Windows versions < RS5.";
GTEST_SKIP();
}
com_initializer_ =
std::make_unique<ScopedCOMInitializer>(ScopedCOMInitializer::kMTA);
ASSERT_TRUE(com_initializer_->Succeeded());
@ -82,6 +74,12 @@ class WgcCaptureSourceTest : public ::testing::TestWithParam<SourceType> {
// Window specific test
TEST_F(WgcCaptureSourceTest, WindowPosition) {
if (!IsWgcSupported(CaptureType::kWindow)) {
RTC_LOG(LS_INFO)
<< "Skipping WgcCapturerWinTests on unsupported platforms.";
GTEST_SKIP();
}
SetUpForWindowSource();
source_ = source_factory_->CreateCaptureSource(source_id_);
ASSERT_TRUE(source_);
@ -100,6 +98,12 @@ TEST_F(WgcCaptureSourceTest, WindowPosition) {
// Screen specific test
TEST_F(WgcCaptureSourceTest, ScreenPosition) {
if (!IsWgcSupported(CaptureType::kScreen)) {
RTC_LOG(LS_INFO)
<< "Skipping WgcCapturerWinTests on unsupported platforms.";
GTEST_SKIP();
}
SetUpForScreenSource();
source_ = source_factory_->CreateCaptureSource(source_id_);
ASSERT_TRUE(source_);
@ -113,7 +117,13 @@ TEST_F(WgcCaptureSourceTest, ScreenPosition) {
// Source agnostic test
TEST_P(WgcCaptureSourceTest, CreateSource) {
if (GetParam() == SourceType::kWindowSource) {
if (!IsWgcSupported(GetParam())) {
RTC_LOG(LS_INFO)
<< "Skipping WgcCapturerWinTests on unsupported platforms.";
GTEST_SKIP();
}
if (GetParam() == CaptureType::kWindow) {
SetUpForWindowSource();
} else {
SetUpForScreenSource();
@ -132,7 +142,7 @@ TEST_P(WgcCaptureSourceTest, CreateSource) {
INSTANTIATE_TEST_SUITE_P(SourceAgnostic,
WgcCaptureSourceTest,
::testing::Values(SourceType::kWindowSource,
SourceType::kScreenSource));
::testing::Values(CaptureType::kWindow,
CaptureType::kScreen));
} // namespace webrtc

View File

@ -245,7 +245,8 @@ void WgcCapturerWin::CaptureFrame() {
iter_success_pair = ongoing_captures_.emplace(
std::piecewise_construct,
std::forward_as_tuple(capture_source_->GetSourceId()),
std::forward_as_tuple(d3d11_device_, item));
std::forward_as_tuple(d3d11_device_, item,
capture_source_->GetSize()));
RTC_DCHECK(iter_success_pair.second);
capture_session = &iter_success_pair.first->second;
} else {

View File

@ -24,8 +24,8 @@
#include "rtc_base/thread.h"
#include "rtc_base/time_utils.h"
#include "rtc_base/win/scoped_com_initializer.h"
#include "rtc_base/win/windows_version.h"
#include "system_wrappers/include/metrics.h"
#include "system_wrappers/include/sleep.h"
#include "test/gtest.h"
namespace webrtc {
@ -63,9 +63,8 @@ const int kWindowHeightSubtrahend = 7;
// Custom message constants so we can direct our thread to close windows
// and quit running.
const UINT kNoOp = WM_APP;
const UINT kDestroyWindow = WM_APP + 1;
const UINT kQuitRunning = WM_APP + 2;
const UINT kDestroyWindow = WM_APP;
const UINT kQuitRunning = WM_APP + 1;
} // namespace
@ -77,10 +76,7 @@ class WgcCapturerWinTest : public ::testing::TestWithParam<CaptureType>,
std::make_unique<ScopedCOMInitializer>(ScopedCOMInitializer::kMTA);
EXPECT_TRUE(com_initializer_->Succeeded());
// Most tests (except `CaptureAllMonitors`) avoid the bug in screen capture,
// so we check support for window capture so these tests can run on more
// systems.
if (!IsWgcSupported(CaptureType::kWindow)) {
if (!IsWgcSupported(GetParam())) {
RTC_LOG(LS_INFO)
<< "Skipping WgcCapturerWinTests on unsupported platforms.";
GTEST_SKIP();
@ -165,18 +161,18 @@ class WgcCapturerWinTest : public ::testing::TestWithParam<CaptureType>,
// Frequently, the test window will not show up in GetSourceList because it
// was created too recently. Since we are confident the window will be found
// eventually we loop here until we find it.
intptr_t src_id;
intptr_t src_id = 0;
do {
DesktopCapturer::SourceList sources;
EXPECT_TRUE(capturer_->GetSourceList(&sources));
auto it = std::find_if(
sources.begin(), sources.end(),
[&](const DesktopCapturer::Source& src) {
return src.id == reinterpret_cast<intptr_t>(window_info_.hwnd);
});
src_id = it->id;
if (it != sources.end())
src_id = it->id;
} while (src_id != reinterpret_cast<intptr_t>(window_info_.hwnd));
return src_id;
@ -335,33 +331,7 @@ INSTANTIATE_TEST_SUITE_P(SourceAgnostic,
::testing::Values(CaptureType::kWindow,
CaptureType::kScreen));
// Monitor specific tests.
TEST_F(WgcCapturerWinTest, FocusOnMonitor) {
SetUpForScreenCapture();
EXPECT_TRUE(capturer_->SelectSource(0));
// You can't set focus on a monitor.
EXPECT_FALSE(capturer_->FocusOnSelectedSource());
}
TEST_F(WgcCapturerWinTest, CaptureAllMonitors) {
// Trying to capture all monitors causes a crash on Windows versions <20H1.
if (!IsWgcSupported(CaptureType::kScreen)) {
RTC_LOG(LS_INFO)
<< "Skipping CaptureAllMonitors test on unsupported platforms.";
GTEST_SKIP();
}
SetUpForScreenCapture();
EXPECT_TRUE(capturer_->SelectSource(kFullDesktopScreenId));
capturer_->Start(this);
DoCapture();
EXPECT_GT(frame_->size().width(), 0);
EXPECT_GT(frame_->size().height(), 0);
}
TEST_F(WgcCapturerWinTest, NoMonitors) {
TEST(WgcCapturerNoMonitorTest, NoMonitors) {
if (HasActiveDisplay()) {
RTC_LOG(LS_INFO) << "Skip WgcCapturerWinTest designed specifically for "
"systems with no monitors";
@ -373,8 +343,55 @@ TEST_F(WgcCapturerWinTest, NoMonitors) {
EXPECT_FALSE(IsWgcSupported(CaptureType::kScreen));
}
// Window specific tests.
TEST_F(WgcCapturerWinTest, FocusOnWindow) {
class WgcCapturerMonitorTest : public WgcCapturerWinTest {
public:
void SetUp() {
com_initializer_ =
std::make_unique<ScopedCOMInitializer>(ScopedCOMInitializer::kMTA);
EXPECT_TRUE(com_initializer_->Succeeded());
if (!IsWgcSupported(CaptureType::kScreen)) {
RTC_LOG(LS_INFO)
<< "Skipping WgcCapturerWinTests on unsupported platforms.";
GTEST_SKIP();
}
}
};
TEST_F(WgcCapturerMonitorTest, FocusOnMonitor) {
SetUpForScreenCapture();
EXPECT_TRUE(capturer_->SelectSource(0));
// You can't set focus on a monitor.
EXPECT_FALSE(capturer_->FocusOnSelectedSource());
}
TEST_F(WgcCapturerMonitorTest, CaptureAllMonitors) {
SetUpForScreenCapture();
EXPECT_TRUE(capturer_->SelectSource(kFullDesktopScreenId));
capturer_->Start(this);
DoCapture();
EXPECT_GT(frame_->size().width(), 0);
EXPECT_GT(frame_->size().height(), 0);
}
class WgcCapturerWindowTest : public WgcCapturerWinTest {
public:
void SetUp() {
com_initializer_ =
std::make_unique<ScopedCOMInitializer>(ScopedCOMInitializer::kMTA);
EXPECT_TRUE(com_initializer_->Succeeded());
if (!IsWgcSupported(CaptureType::kWindow)) {
RTC_LOG(LS_INFO)
<< "Skipping WgcCapturerWinTests on unsupported platforms.";
GTEST_SKIP();
}
}
};
TEST_F(WgcCapturerWindowTest, FocusOnWindow) {
capturer_ = WgcCapturerWin::CreateRawWindowCapturer(
DesktopCaptureOptions::CreateDefault());
window_info_ = CreateTestWindow(kWindowTitle);
@ -390,7 +407,7 @@ TEST_F(WgcCapturerWinTest, FocusOnWindow) {
DestroyTestWindow(window_info_);
}
TEST_F(WgcCapturerWinTest, SelectMinimizedWindow) {
TEST_F(WgcCapturerWindowTest, SelectMinimizedWindow) {
SetUpForWindowCapture();
MinimizeTestWindow(reinterpret_cast<HWND>(source_id_));
EXPECT_FALSE(capturer_->SelectSource(source_id_));
@ -399,7 +416,7 @@ TEST_F(WgcCapturerWinTest, SelectMinimizedWindow) {
EXPECT_TRUE(capturer_->SelectSource(source_id_));
}
TEST_F(WgcCapturerWinTest, SelectClosedWindow) {
TEST_F(WgcCapturerWindowTest, SelectClosedWindow) {
SetUpForWindowCapture();
EXPECT_TRUE(capturer_->SelectSource(source_id_));
@ -407,7 +424,7 @@ TEST_F(WgcCapturerWinTest, SelectClosedWindow) {
EXPECT_FALSE(capturer_->SelectSource(source_id_));
}
TEST_F(WgcCapturerWinTest, UnsupportedWindowStyle) {
TEST_F(WgcCapturerWindowTest, UnsupportedWindowStyle) {
// Create a window with the WS_EX_TOOLWINDOW style, which WGC does not
// support.
window_info_ = CreateTestWindow(kWindowTitle, kMediumWindowWidth,
@ -426,7 +443,7 @@ TEST_F(WgcCapturerWinTest, UnsupportedWindowStyle) {
DestroyTestWindow(window_info_);
}
TEST_F(WgcCapturerWinTest, IncreaseWindowSizeMidCapture) {
TEST_F(WgcCapturerWindowTest, IncreaseWindowSizeMidCapture) {
SetUpForWindowCapture(kSmallWindowWidth, kSmallWindowHeight);
EXPECT_TRUE(capturer_->SelectSource(source_id_));
@ -447,7 +464,7 @@ TEST_F(WgcCapturerWinTest, IncreaseWindowSizeMidCapture) {
ValidateFrame(kLargeWindowWidth, kMediumWindowHeight);
}
TEST_F(WgcCapturerWinTest, ReduceWindowSizeMidCapture) {
TEST_F(WgcCapturerWindowTest, ReduceWindowSizeMidCapture) {
SetUpForWindowCapture(kLargeWindowWidth, kLargeWindowHeight);
EXPECT_TRUE(capturer_->SelectSource(source_id_));
@ -466,7 +483,7 @@ TEST_F(WgcCapturerWinTest, ReduceWindowSizeMidCapture) {
ValidateFrame(kSmallWindowWidth, kMediumWindowHeight);
}
TEST_F(WgcCapturerWinTest, MinimizeWindowMidCapture) {
TEST_F(WgcCapturerWindowTest, MinimizeWindowMidCapture) {
SetUpForWindowCapture();
EXPECT_TRUE(capturer_->SelectSource(source_id_));
@ -487,7 +504,7 @@ TEST_F(WgcCapturerWinTest, MinimizeWindowMidCapture) {
// a good test.
}
TEST_F(WgcCapturerWinTest, CloseWindowMidCapture) {
TEST_F(WgcCapturerWindowTest, CloseWindowMidCapture) {
SetUpForWindowCapture();
EXPECT_TRUE(capturer_->SelectSource(source_id_));
@ -497,18 +514,17 @@ TEST_F(WgcCapturerWinTest, CloseWindowMidCapture) {
CloseTestWindow();
// We need to call GetMessage to trigger the Closed event and the capturer's
// event handler for it. If we are too early and the Closed event hasn't
// arrived yet we should keep trying until the capturer receives it and stops.
// We need to pump our message queue so the Closed event will be delivered to
// the capturer's event handler. If we are too early and the Closed event
// hasn't arrived yet we should keep trying until the capturer receives it and
// stops.
auto* wgc_capturer = static_cast<WgcCapturerWin*>(capturer_.get());
MSG msg;
while (wgc_capturer->IsSourceBeingCaptured(source_id_)) {
// Since the capturer handles the Closed message, there will be no message
// for us and GetMessage will hang, unless we send ourselves a message
// first.
::PostThreadMessage(GetCurrentThreadId(), kNoOp, 0, 0);
MSG msg;
::GetMessage(&msg, NULL, 0, 0);
::DispatchMessage(&msg);
// Unlike GetMessage, PeekMessage will not hang if there are no messages in
// the queue.
PeekMessage(&msg, 0, 0, 0, PM_REMOVE);
SleepMs(1);
}
// Occasionally, one last frame will have made it into the frame pool before