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 <jamiewalch@chromium.org>
Commit-Queue: Joe Downing <joedow@google.com>
Cr-Commit-Position: refs/heads/master@{#32417}

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",

modules/desktop_capture/win/d3d_device.cc

@@ -12,6 +12,7 @@
 
 #include <utility>
 
+#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<IDXGIAdapter>& 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 "
+    RTC_LOG(LS_WARNING) << "D3D11CreateDevice returned: "
-                        << error.ErrorMessage() << " with code "
+                        << desktop_capture::utils::ComErrorToString(error);
-                        << error.Error();
     return false;
   }
 
   if (feature_level < D3D_FEATURE_LEVEL_11_0) {
     RTC_LOG(LS_WARNING)
-        << "D3D11CreateDevice returns an instance without DirectX "
+        << "D3D11CreateDevice returned an instance without DirectX 11 support, "
-           "11 support, level "
+        << "level " << feature_level << ". Following initialization may fail.";
-        << 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<IDXGIAdapter>& 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 "
+        << "this usually means the system does not support DirectX "
-           "11. Error "
+        << "11. Error received: "
-        << error.ErrorMessage() << " with code " << error.Error();
+        << desktop_capture::utils::ComErrorToString(error);
     return false;
   }
 
@@ -73,7 +72,8 @@ std::vector<D3dDevice> D3dDevice::EnumDevices() {
       CreateDXGIFactory1(__uuidof(IDXGIFactory1),
                          reinterpret_cast<void**>(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<D3dDevice>();
   }
 
@@ -90,9 +90,8 @@ std::vector<D3dDevice> D3dDevice::EnumDevices() {
       break;
     } else {
       RTC_LOG(LS_WARNING)
-          << "IDXGIFactory1::EnumAdapters returns an unexpected "
+          << "IDXGIFactory1::EnumAdapters returned an unexpected error: "
-             "error "
+          << desktop_capture::utils::ComErrorToString(error);
-          << error.ErrorMessage() << " with code " << error.Error();
     }
   }
   return result;

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<const char*>(error_message));
+  return string_builder.str();
+}
+
+}  // namespace utils
+}  // namespace desktop_capture
+}  // namespace webrtc

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 <comdef.h>
+
+#include <string>
+
+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_

modules/desktop_capture/win/dxgi_adapter_duplicator.cc

@@ -15,6 +15,7 @@
 
 #include <algorithm>
 
+#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 "
+      RTC_LOG(LS_WARNING) << "IDXGIAdapter::EnumOutputs returned "
-                             "NOT_CURRENTLY_AVAILABLE. This may happen when "
+                          << "NOT_CURRENTLY_AVAILABLE. This may happen when "
-                             "running in session 0.";
+                          << "running in session 0.";
       break;
     }
 
     if (error.Error() != S_OK || !output) {
-      RTC_LOG(LS_WARNING) << "IDXGIAdapter::EnumOutputs returns an unexpected "
+      RTC_LOG(LS_WARNING) << "IDXGIAdapter::EnumOutputs returned an unexpected "
-                             "result "
+                          << "result: "
-                          << error.ErrorMessage() << " with error code"
+                          << desktop_capture::utils::ComErrorToString(error);
-                          << error.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, "
+              << "Failed to convert IDXGIOutput to IDXGIOutput1, this usually "
-                 "this usually means the system does not support "
+              << "means the system does not support DirectX 11";
-                 "DirectX 11";
           continue;
         }
         DxgiOutputDuplicator duplicator(device_, output1, desc);
         if (!duplicator.Initialize()) {
           RTC_LOG(LS_WARNING) << "Failed to initialize DxgiOutputDuplicator on "
-                                 "output "
+                              << "output " << i;
-                              << i;
           continue;
         }
 

modules/desktop_capture/win/dxgi_output_duplicator.cc

@@ -18,6 +18,7 @@
 
 #include <algorithm>
 
+#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<IUnknown*>(device_.d3d_device()),
                                duplication_.GetAddressOf());
   if (error.Error() != S_OK || !duplication_) {
-    RTC_LOG(LS_WARNING)
+    RTC_LOG(LS_WARNING) << "Failed to duplicate output from IDXGIOutput1: "
-        << "Failed to duplicate output from IDXGIOutput1, error "
+                        << desktop_capture::utils::ComErrorToString(error);
-        << error.ErrorMessage() << ", with code " << error.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, which is required by downstream components, "
-                         "format is "
+                      << "format is " << desc_.ModeDesc.Format;
-                      << desc_.ModeDesc.Format;
     return false;
   }
 
@@ -123,7 +122,7 @@ bool DxgiOutputDuplicator::DuplicateOutput() {
       static_cast<int>(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, "
+    RTC_LOG(LS_ERROR) << "Failed to release frame from IDXGIOutputDuplication: "
-                         "error"
+                      << desktop_capture::utils::ComErrorToString(error);
-                      << error.ErrorMessage() << ", code " << error.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 "
+    RTC_LOG(LS_ERROR) << "Failed to capture frame: "
-                      << error.ErrorMessage() << ", code " << error.Error();
+                      << 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<UINT>(metadata_.capacity()), move_rects, &buff_size);
   if (error.Error() != S_OK) {
-    RTC_LOG(LS_ERROR) << "Failed to get move rectangles, error "
+    RTC_LOG(LS_ERROR) << "Failed to get move rectangles: "
-                      << error.ErrorMessage() << ", code " << error.Error();
+                      << 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<UINT>(metadata_.capacity()) - buff_size, dirty_rects,
       &buff_size);
   if (error.Error() != S_OK) {
-    RTC_LOG(LS_ERROR) << "Failed to get dirty rectangles, error "
+    RTC_LOG(LS_ERROR) << "Failed to get dirty rectangles: "
-                      << error.ErrorMessage() << ", code " << error.Error();
+                      << desktop_capture::utils::ComErrorToString(error);
     return false;
   }
   dirty_rects_count = buff_size / sizeof(RECT);

modules/desktop_capture/win/dxgi_texture.cc

@@ -15,6 +15,7 @@
 #include <wrl/client.h>
 
 #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<void**>(texture.GetAddressOf()));
   if (error.Error() != S_OK || !texture) {
-    RTC_LOG(LS_ERROR) << "Failed to convert IDXGIResource to ID3D11Texture2D, "
+    RTC_LOG(LS_ERROR) << "Failed to convert IDXGIResource to ID3D11Texture2D: "
-                         "error "
+                      << desktop_capture::utils::ComErrorToString(error);
-                      << error.ErrorMessage() << ", code " << error.Error();
     return false;
   }
 

modules/desktop_capture/win/dxgi_texture_mapping.cc

@@ -14,6 +14,7 @@
 #include <dxgi.h>
 #include <dxgi1_2.h>
 
+#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, "
+        << "Failed to map the IDXGIOutputDuplication to a bitmap: "
-           "error "
+        << desktop_capture::utils::ComErrorToString(error);
-        << error.ErrorMessage() << ", code " << error.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 "
+    RTC_LOG(LS_ERROR) << "Failed to unmap the IDXGIOutputDuplication: "
-                      << error.ErrorMessage() << ", code " << error.Error();
+                      << desktop_capture::utils::ComErrorToString(error);
     return false;
   }
   return true;

modules/desktop_capture/win/dxgi_texture_staging.cc

@@ -15,6 +15,7 @@
 #include <dxgi1_2.h>
 #include <unknwn.h>
 
+#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)
+    RTC_LOG(LS_ERROR) << "Failed to create a new ID3D11Texture2D as stage: "
-        << "Failed to create a new ID3D11Texture2D as stage, error "
+                      << desktop_capture::utils::ComErrorToString(error);
-        << error.ErrorMessage() << ", code " << error.Error();
     return false;
   }
 
   error = stage_.As(&surface_);
   if (error.Error() != S_OK || !surface_) {
-    RTC_LOG(LS_ERROR)
+    RTC_LOG(LS_ERROR) << "Failed to convert ID3D11Texture2D to IDXGISurface: "
-        << "Failed to convert ID3D11Texture2D to IDXGISurface, error "
+                      << desktop_capture::utils::ComErrorToString(error);
-        << error.ErrorMessage() << ", code " << error.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 "
+    RTC_LOG(LS_ERROR) << "Failed to map the IDXGISurface to a bitmap: "
-                      << error.ErrorMessage() << ", code " << error.Error();
+                      << desktop_capture::utils::ComErrorToString(error);
     return false;
   }