From 92fa17660f0036d416f3eecb0f9849a141963c6f Mon Sep 17 00:00:00 2001 From: Joe Downing Date: Thu, 15 Oct 2020 10:19:26 -0700 Subject: [PATCH] Fixing the error logging in desktop_capture for Windows The desktop_capture code logs failed HRESULTs in several places, the problem is that it tries to use a wchar* (when a char* is required) for the error message and doesn't display the HRESULT in hex. This makes the error logging less useful than it could be. Example failure: error 08406B28, with code -2005270488 In this CL, I add a simple utility function to convert a _com_error object to a std::string whic can be logged. With this change the output looks like this (linebreak added for CL description): Failed to capture frame: HRESULT: 0x887A0026, Message: 'The keyed mutex was abandoned.' I also adjusted the formatting of a few logging statements to be more consistent (adding '<<' operators mostly). Bug: webrtc:12051 Change-Id: I3e88ff6f2ff079fbe210626e1e89b2b053a742a9 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/188522 Reviewed-by: Jamie Walch Commit-Queue: Joe Downing Cr-Commit-Position: refs/heads/master@{#32417} --- modules/desktop_capture/BUILD.gn | 2 ++ modules/desktop_capture/win/d3d_device.cc | 25 +++++++-------- .../win/desktop_capture_utils.cc | 32 +++++++++++++++++++ .../win/desktop_capture_utils.h | 29 +++++++++++++++++ .../win/dxgi_adapter_duplicator.cc | 22 ++++++------- .../win/dxgi_output_duplicator.cc | 32 +++++++++---------- modules/desktop_capture/win/dxgi_texture.cc | 6 ++-- .../win/dxgi_texture_mapping.cc | 10 +++--- .../win/dxgi_texture_staging.cc | 15 ++++----- 9 files changed, 115 insertions(+), 58 deletions(-) create mode 100644 modules/desktop_capture/win/desktop_capture_utils.cc create mode 100644 modules/desktop_capture/win/desktop_capture_utils.h diff --git a/modules/desktop_capture/BUILD.gn b/modules/desktop_capture/BUILD.gn index 2cb6891c9e..956e0a68e5 100644 --- a/modules/desktop_capture/BUILD.gn +++ b/modules/desktop_capture/BUILD.gn @@ -445,6 +445,8 @@ rtc_library("desktop_capture_generic") { "win/d3d_device.h", "win/desktop.cc", "win/desktop.h", + "win/desktop_capture_utils.cc", + "win/desktop_capture_utils.h", "win/display_configuration_monitor.cc", "win/display_configuration_monitor.h", "win/dxgi_adapter_duplicator.cc", diff --git a/modules/desktop_capture/win/d3d_device.cc b/modules/desktop_capture/win/d3d_device.cc index b220b138a5..3d46117501 100644 --- a/modules/desktop_capture/win/d3d_device.cc +++ b/modules/desktop_capture/win/d3d_device.cc @@ -12,6 +12,7 @@ #include +#include "modules/desktop_capture/win/desktop_capture_utils.h" #include "rtc_base/logging.h" namespace webrtc { @@ -38,17 +39,15 @@ bool D3dDevice::Initialize(const ComPtr& adapter) { nullptr, 0, D3D11_SDK_VERSION, d3d_device_.GetAddressOf(), &feature_level, context_.GetAddressOf()); if (error.Error() != S_OK || !d3d_device_ || !context_) { - RTC_LOG(LS_WARNING) << "D3D11CreateDeivce returns error " - << error.ErrorMessage() << " with code " - << error.Error(); + RTC_LOG(LS_WARNING) << "D3D11CreateDevice returned: " + << desktop_capture::utils::ComErrorToString(error); return false; } if (feature_level < D3D_FEATURE_LEVEL_11_0) { RTC_LOG(LS_WARNING) - << "D3D11CreateDevice returns an instance without DirectX " - "11 support, level " - << feature_level << ". Following initialization may fail."; + << "D3D11CreateDevice returned an instance without DirectX 11 support, " + << "level " << feature_level << ". Following initialization may fail."; // D3D_FEATURE_LEVEL_11_0 is not officially documented on MSDN to be a // requirement of Dxgi duplicator APIs. } @@ -57,9 +56,9 @@ bool D3dDevice::Initialize(const ComPtr& adapter) { if (error.Error() != S_OK || !dxgi_device_) { RTC_LOG(LS_WARNING) << "ID3D11Device is not an implementation of IDXGIDevice, " - "this usually means the system does not support DirectX " - "11. Error " - << error.ErrorMessage() << " with code " << error.Error(); + << "this usually means the system does not support DirectX " + << "11. Error received: " + << desktop_capture::utils::ComErrorToString(error); return false; } @@ -73,7 +72,8 @@ std::vector D3dDevice::EnumDevices() { CreateDXGIFactory1(__uuidof(IDXGIFactory1), reinterpret_cast(factory.GetAddressOf())); if (error.Error() != S_OK || !factory) { - RTC_LOG(LS_WARNING) << "Cannot create IDXGIFactory1."; + RTC_LOG(LS_WARNING) << "Cannot create IDXGIFactory1: " + << desktop_capture::utils::ComErrorToString(error); return std::vector(); } @@ -90,9 +90,8 @@ std::vector D3dDevice::EnumDevices() { break; } else { RTC_LOG(LS_WARNING) - << "IDXGIFactory1::EnumAdapters returns an unexpected " - "error " - << error.ErrorMessage() << " with code " << error.Error(); + << "IDXGIFactory1::EnumAdapters returned an unexpected error: " + << desktop_capture::utils::ComErrorToString(error); } } return result; diff --git a/modules/desktop_capture/win/desktop_capture_utils.cc b/modules/desktop_capture/win/desktop_capture_utils.cc new file mode 100644 index 0000000000..476ddc4aba --- /dev/null +++ b/modules/desktop_capture/win/desktop_capture_utils.cc @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2020 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "modules/desktop_capture/win/desktop_capture_utils.h" + +#include "rtc_base/strings/string_builder.h" + +namespace webrtc { +namespace desktop_capture { +namespace utils { + +// Generates a human-readable string from a COM error. +std::string ComErrorToString(const _com_error& error) { + char buffer[1024]; + rtc::SimpleStringBuilder string_builder(buffer); + // Use _bstr_t to simplify the wchar to char conversion for ErrorMessage(). + _bstr_t error_message(error.ErrorMessage()); + string_builder.AppendFormat("HRESULT: 0x%08X, Message: %s", error.Error(), + static_cast(error_message)); + return string_builder.str(); +} + +} // namespace utils +} // namespace desktop_capture +} // namespace webrtc diff --git a/modules/desktop_capture/win/desktop_capture_utils.h b/modules/desktop_capture/win/desktop_capture_utils.h new file mode 100644 index 0000000000..ebf31419ce --- /dev/null +++ b/modules/desktop_capture/win/desktop_capture_utils.h @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2020 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef MODULES_DESKTOP_CAPTURE_WIN_DESKTOP_CAPTURE_UTILS_H_ +#define MODULES_DESKTOP_CAPTURE_WIN_DESKTOP_CAPTURE_UTILS_H_ + +#include + +#include + +namespace webrtc { +namespace desktop_capture { +namespace utils { + +// Generates a human-readable string from a COM error. +std::string ComErrorToString(const _com_error& error); + +} // namespace utils +} // namespace desktop_capture +} // namespace webrtc + +#endif // MODULES_DESKTOP_CAPTURE_WIN_DESKTOP_CAPTURE_UTILS_H_ diff --git a/modules/desktop_capture/win/dxgi_adapter_duplicator.cc b/modules/desktop_capture/win/dxgi_adapter_duplicator.cc index e3f11ac30a..88ec4e25bf 100644 --- a/modules/desktop_capture/win/dxgi_adapter_duplicator.cc +++ b/modules/desktop_capture/win/dxgi_adapter_duplicator.cc @@ -15,6 +15,7 @@ #include +#include "modules/desktop_capture/win/desktop_capture_utils.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -53,17 +54,16 @@ bool DxgiAdapterDuplicator::DoInitialize() { } if (error.Error() == DXGI_ERROR_NOT_CURRENTLY_AVAILABLE) { - RTC_LOG(LS_WARNING) << "IDXGIAdapter::EnumOutputs returns " - "NOT_CURRENTLY_AVAILABLE. This may happen when " - "running in session 0."; + RTC_LOG(LS_WARNING) << "IDXGIAdapter::EnumOutputs returned " + << "NOT_CURRENTLY_AVAILABLE. This may happen when " + << "running in session 0."; break; } if (error.Error() != S_OK || !output) { - RTC_LOG(LS_WARNING) << "IDXGIAdapter::EnumOutputs returns an unexpected " - "result " - << error.ErrorMessage() << " with error code" - << error.Error(); + RTC_LOG(LS_WARNING) << "IDXGIAdapter::EnumOutputs returned an unexpected " + << "result: " + << desktop_capture::utils::ComErrorToString(error); continue; } @@ -75,16 +75,14 @@ bool DxgiAdapterDuplicator::DoInitialize() { error = output.As(&output1); if (error.Error() != S_OK || !output1) { RTC_LOG(LS_WARNING) - << "Failed to convert IDXGIOutput to IDXGIOutput1, " - "this usually means the system does not support " - "DirectX 11"; + << "Failed to convert IDXGIOutput to IDXGIOutput1, this usually " + << "means the system does not support DirectX 11"; continue; } DxgiOutputDuplicator duplicator(device_, output1, desc); if (!duplicator.Initialize()) { RTC_LOG(LS_WARNING) << "Failed to initialize DxgiOutputDuplicator on " - "output " - << i; + << "output " << i; continue; } diff --git a/modules/desktop_capture/win/dxgi_output_duplicator.cc b/modules/desktop_capture/win/dxgi_output_duplicator.cc index 2d56b9af36..c90e2f1ab5 100644 --- a/modules/desktop_capture/win/dxgi_output_duplicator.cc +++ b/modules/desktop_capture/win/dxgi_output_duplicator.cc @@ -18,6 +18,7 @@ #include +#include "modules/desktop_capture/win/desktop_capture_utils.h" #include "modules/desktop_capture/win/dxgi_texture_mapping.h" #include "modules/desktop_capture/win/dxgi_texture_staging.h" #include "rtc_base/checks.h" @@ -103,9 +104,8 @@ bool DxgiOutputDuplicator::DuplicateOutput() { output_->DuplicateOutput(static_cast(device_.d3d_device()), duplication_.GetAddressOf()); if (error.Error() != S_OK || !duplication_) { - RTC_LOG(LS_WARNING) - << "Failed to duplicate output from IDXGIOutput1, error " - << error.ErrorMessage() << ", with code " << error.Error(); + RTC_LOG(LS_WARNING) << "Failed to duplicate output from IDXGIOutput1: " + << desktop_capture::utils::ComErrorToString(error); return false; } @@ -113,9 +113,8 @@ bool DxgiOutputDuplicator::DuplicateOutput() { duplication_->GetDesc(&desc_); if (desc_.ModeDesc.Format != DXGI_FORMAT_B8G8R8A8_UNORM) { RTC_LOG(LS_ERROR) << "IDXGIDuplicateOutput does not use RGBA (8 bit) " - "format, which is required by downstream components, " - "format is " - << desc_.ModeDesc.Format; + << "format, which is required by downstream components, " + << "format is " << desc_.ModeDesc.Format; return false; } @@ -123,7 +122,7 @@ bool DxgiOutputDuplicator::DuplicateOutput() { static_cast(desc_.ModeDesc.Height) != desktop_rect_.height()) { RTC_LOG(LS_ERROR) << "IDXGIDuplicateOutput does not return a same size as its " - "IDXGIOutput1, size returned by IDXGIDuplicateOutput is " + << "IDXGIOutput1, size returned by IDXGIDuplicateOutput is " << desc_.ModeDesc.Width << " x " << desc_.ModeDesc.Height << ", size returned by IDXGIOutput1 is " << desktop_rect_.width() << " x " << desktop_rect_.height(); @@ -140,9 +139,8 @@ bool DxgiOutputDuplicator::ReleaseFrame() { RTC_DCHECK(duplication_); _com_error error = duplication_->ReleaseFrame(); if (error.Error() != S_OK) { - RTC_LOG(LS_ERROR) << "Failed to release frame from IDXGIOutputDuplication, " - "error" - << error.ErrorMessage() << ", code " << error.Error(); + RTC_LOG(LS_ERROR) << "Failed to release frame from IDXGIOutputDuplication: " + << desktop_capture::utils::ComErrorToString(error); return false; } return true; @@ -166,8 +164,8 @@ bool DxgiOutputDuplicator::Duplicate(Context* context, _com_error error = duplication_->AcquireNextFrame( kAcquireTimeoutMs, &frame_info, resource.GetAddressOf()); if (error.Error() != S_OK && error.Error() != DXGI_ERROR_WAIT_TIMEOUT) { - RTC_LOG(LS_ERROR) << "Failed to capture frame, error " - << error.ErrorMessage() << ", code " << error.Error(); + RTC_LOG(LS_ERROR) << "Failed to capture frame: " + << desktop_capture::utils::ComErrorToString(error); return false; } @@ -269,7 +267,7 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion( if (frame_info.TotalMetadataBufferSize == 0) { // This should not happen, since frame_info.AccumulatedFrames > 0. RTC_LOG(LS_ERROR) << "frame_info.AccumulatedFrames > 0, " - "but TotalMetadataBufferSize == 0"; + << "but TotalMetadataBufferSize == 0"; return false; } @@ -285,8 +283,8 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion( _com_error error = duplication_->GetFrameMoveRects( static_cast(metadata_.capacity()), move_rects, &buff_size); if (error.Error() != S_OK) { - RTC_LOG(LS_ERROR) << "Failed to get move rectangles, error " - << error.ErrorMessage() << ", code " << error.Error(); + RTC_LOG(LS_ERROR) << "Failed to get move rectangles: " + << desktop_capture::utils::ComErrorToString(error); return false; } move_rects_count = buff_size / sizeof(DXGI_OUTDUPL_MOVE_RECT); @@ -297,8 +295,8 @@ bool DxgiOutputDuplicator::DoDetectUpdatedRegion( static_cast(metadata_.capacity()) - buff_size, dirty_rects, &buff_size); if (error.Error() != S_OK) { - RTC_LOG(LS_ERROR) << "Failed to get dirty rectangles, error " - << error.ErrorMessage() << ", code " << error.Error(); + RTC_LOG(LS_ERROR) << "Failed to get dirty rectangles: " + << desktop_capture::utils::ComErrorToString(error); return false; } dirty_rects_count = buff_size / sizeof(RECT); diff --git a/modules/desktop_capture/win/dxgi_texture.cc b/modules/desktop_capture/win/dxgi_texture.cc index 2919692c40..b8f5b81f90 100644 --- a/modules/desktop_capture/win/dxgi_texture.cc +++ b/modules/desktop_capture/win/dxgi_texture.cc @@ -15,6 +15,7 @@ #include #include "modules/desktop_capture/desktop_region.h" +#include "modules/desktop_capture/win/desktop_capture_utils.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -49,9 +50,8 @@ bool DxgiTexture::CopyFrom(const DXGI_OUTDUPL_FRAME_INFO& frame_info, __uuidof(ID3D11Texture2D), reinterpret_cast(texture.GetAddressOf())); if (error.Error() != S_OK || !texture) { - RTC_LOG(LS_ERROR) << "Failed to convert IDXGIResource to ID3D11Texture2D, " - "error " - << error.ErrorMessage() << ", code " << error.Error(); + RTC_LOG(LS_ERROR) << "Failed to convert IDXGIResource to ID3D11Texture2D: " + << desktop_capture::utils::ComErrorToString(error); return false; } diff --git a/modules/desktop_capture/win/dxgi_texture_mapping.cc b/modules/desktop_capture/win/dxgi_texture_mapping.cc index 9e138d1d6f..7ecf1adc61 100644 --- a/modules/desktop_capture/win/dxgi_texture_mapping.cc +++ b/modules/desktop_capture/win/dxgi_texture_mapping.cc @@ -14,6 +14,7 @@ #include #include +#include "modules/desktop_capture/win/desktop_capture_utils.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -36,9 +37,8 @@ bool DxgiTextureMapping::CopyFromTexture( if (error.Error() != S_OK) { *rect() = {0}; RTC_LOG(LS_ERROR) - << "Failed to map the IDXGIOutputDuplication to a bitmap, " - "error " - << error.ErrorMessage() << ", code " << error.Error(); + << "Failed to map the IDXGIOutputDuplication to a bitmap: " + << desktop_capture::utils::ComErrorToString(error); return false; } @@ -48,8 +48,8 @@ bool DxgiTextureMapping::CopyFromTexture( bool DxgiTextureMapping::DoRelease() { _com_error error = duplication_->UnMapDesktopSurface(); if (error.Error() != S_OK) { - RTC_LOG(LS_ERROR) << "Failed to unmap the IDXGIOutputDuplication, error " - << error.ErrorMessage() << ", code " << error.Error(); + RTC_LOG(LS_ERROR) << "Failed to unmap the IDXGIOutputDuplication: " + << desktop_capture::utils::ComErrorToString(error); return false; } return true; diff --git a/modules/desktop_capture/win/dxgi_texture_staging.cc b/modules/desktop_capture/win/dxgi_texture_staging.cc index 2bd1eb9a6f..17e8518a7d 100644 --- a/modules/desktop_capture/win/dxgi_texture_staging.cc +++ b/modules/desktop_capture/win/dxgi_texture_staging.cc @@ -15,6 +15,7 @@ #include #include +#include "modules/desktop_capture/win/desktop_capture_utils.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "system_wrappers/include/metrics.h" @@ -64,17 +65,15 @@ bool DxgiTextureStaging::InitializeStage(ID3D11Texture2D* texture) { _com_error error = device_.d3d_device()->CreateTexture2D( &desc, nullptr, stage_.GetAddressOf()); if (error.Error() != S_OK || !stage_) { - RTC_LOG(LS_ERROR) - << "Failed to create a new ID3D11Texture2D as stage, error " - << error.ErrorMessage() << ", code " << error.Error(); + RTC_LOG(LS_ERROR) << "Failed to create a new ID3D11Texture2D as stage: " + << desktop_capture::utils::ComErrorToString(error); return false; } error = stage_.As(&surface_); if (error.Error() != S_OK || !surface_) { - RTC_LOG(LS_ERROR) - << "Failed to convert ID3D11Texture2D to IDXGISurface, error " - << error.ErrorMessage() << ", code " << error.Error(); + RTC_LOG(LS_ERROR) << "Failed to convert ID3D11Texture2D to IDXGISurface: " + << desktop_capture::utils::ComErrorToString(error); return false; } @@ -110,8 +109,8 @@ bool DxgiTextureStaging::CopyFromTexture( _com_error error = surface_->Map(rect(), DXGI_MAP_READ); if (error.Error() != S_OK) { *rect() = {0}; - RTC_LOG(LS_ERROR) << "Failed to map the IDXGISurface to a bitmap, error " - << error.ErrorMessage() << ", code " << error.Error(); + RTC_LOG(LS_ERROR) << "Failed to map the IDXGISurface to a bitmap: " + << desktop_capture::utils::ComErrorToString(error); return false; }