Video Capture Device Win: cleanup error logging

While writing other code, I found out that the error logging
in VCDWin is cumbersome: it is too verbose on Init() and
doesn't dump the HRESULT while capturing.

So: This CL adds a macro DLOG_IF_WITH_HRESULT() that logs
an error message _and_ the stringified HRESULT. It also
sends an HRESULT to SetErrorMessage() to dump it on
stdout.

In my experience, all these loggings are only used
by developers to try and guess what goes wrong in Win
bots when those fail, so the less release code they take
up the better.

BUG=405016, 648490

Review-Url: https://codereview.chromium.org/2365533004
Cr-Commit-Position: refs/heads/master@{#420493}

media/capture/video/win/video_capture_device_win.cc

@@ -24,6 +24,17 @@ using base::win::ScopedVariant;
 
 namespace media {
 
+#if DCHECK_IS_ON()
+#define DLOG_IF_FAILED_WITH_HRESULT(message, hr)                        \
+  {                                                                     \
+    DLOG_IF(ERROR, FAILED(hr)) << (message) << ": "                     \
+                               << logging::SystemErrorCodeToString(hr); \
+  }
+#else
+#define DLOG_IF_FAILED_WITH_HRESULT(message, hr) \
+  {}
+#endif
+
 // Check if a Pin matches a category.
 bool PinMatchesCategory(IPin* pin, REFGUID category) {
   DCHECK(pin);
@@ -242,12 +253,9 @@ bool VideoCaptureDeviceWin::Init() {
   HRESULT hr;
 
   hr = GetDeviceFilter(device_descriptor_.device_id, capture_filter_.Receive());
-
-  if (!capture_filter_.get()) {
-    DLOG(ERROR) << "Failed to create capture filter: "
-                << logging::SystemErrorCodeToString(hr);
+  DLOG_IF_FAILED_WITH_HRESULT("Failed to create capture filter", hr);
+  if (!capture_filter_.get())
     return false;
-  }
 
   output_capture_pin_ = GetPin(capture_filter_.get(), PINDIR_OUTPUT,
                                PIN_CATEGORY_CAPTURE, GUID_NULL);
@@ -267,54 +275,43 @@ bool VideoCaptureDeviceWin::Init() {
 
   hr = graph_builder_.CreateInstance(CLSID_FilterGraph, NULL,
                                      CLSCTX_INPROC_SERVER);
-  if (FAILED(hr)) {
-    DLOG(ERROR) << "Failed to create graph builder: "
-                << logging::SystemErrorCodeToString(hr);
+  DLOG_IF_FAILED_WITH_HRESULT("Failed to create capture filter", hr);
+  if (FAILED(hr))
     return false;
-  }
 
   hr = capture_graph_builder_.CreateInstance(CLSID_CaptureGraphBuilder2, NULL,
                                              CLSCTX_INPROC);
-  if (FAILED(hr)) {
-    DLOG(ERROR) << "Failed to create the Capture Graph Builder: "
-                << logging::SystemErrorCodeToString(hr);
+  DLOG_IF_FAILED_WITH_HRESULT("Failed to create the Capture Graph Builder", hr);
+  if (FAILED(hr))
     return false;
-  }
 
   hr = capture_graph_builder_->SetFiltergraph(graph_builder_.get());
-  if (FAILED(hr)) {
-    DLOG(ERROR) << "Failed to give graph to capture graph builder: "
-                << logging::SystemErrorCodeToString(hr);
+  DLOG_IF_FAILED_WITH_HRESULT("Failed to give graph to capture graph builder",
+                              hr);
+  if (FAILED(hr))
     return false;
-  }
 
   hr = graph_builder_.QueryInterface(media_control_.Receive());
-  if (FAILED(hr)) {
-    DLOG(ERROR) << "Failed to create media control builder: "
-                << logging::SystemErrorCodeToString(hr);
+  DLOG_IF_FAILED_WITH_HRESULT("Failed to create media control builder", hr);
+  if (FAILED(hr))
     return false;
-  }
 
   hr = graph_builder_->AddFilter(capture_filter_.get(), NULL);
-  if (FAILED(hr)) {
-    DLOG(ERROR) << "Failed to add the capture device to the graph: "
-                << logging::SystemErrorCodeToString(hr);
+  DLOG_IF_FAILED_WITH_HRESULT("Failed to add the capture device to the graph",
+                              hr);
+  if (FAILED(hr))
     return false;
-  }
 
   hr = graph_builder_->AddFilter(sink_filter_.get(), NULL);
-  if (FAILED(hr)) {
-    DLOG(ERROR) << "Failed to add the sink filter to the graph: "
-                << logging::SystemErrorCodeToString(hr);
+  DLOG_IF_FAILED_WITH_HRESULT("Failed to add the sink filter to the graph", hr);
+  if (FAILED(hr))
     return false;
-  }
 
-  // The following code builds the upstream portions of the graph,
-  // for example if a capture device uses a Windows Driver Model (WDM)
-  // driver, the graph may require certain filters upstream from the
-  // WDM Video Capture filter, such as a TV Tuner filter or an Analog
-  // Video Crossbar filter. We try using the more prevalent
-  // MEDIATYPE_Interleaved first.
+  // The following code builds the upstream portions of the graph, for example
+  // if a capture device uses a Windows Driver Model (WDM) driver, the graph may
+  // require certain filters upstream from the WDM Video Capture filter, such as
+  // a TV Tuner filter or an Analog Video Crossbar filter. We try using the more
+  // prevalent MEDIATYPE_Interleaved first.
   base::win::ScopedComPtr<IAMStreamConfig> stream_config;
 
   hr = capture_graph_builder_->FindInterface(
@@ -324,8 +321,7 @@ bool VideoCaptureDeviceWin::Init() {
     hr = capture_graph_builder_->FindInterface(
         &PIN_CATEGORY_CAPTURE, &MEDIATYPE_Video, capture_filter_.get(),
         IID_IAMStreamConfig, (void**)stream_config.Receive());
-    DLOG_IF(ERROR, FAILED(hr)) << "Failed to find CapFilter:IAMStreamConfig: "
-                               << logging::SystemErrorCodeToString(hr);
+    DLOG_IF_FAILED_WITH_HRESULT("Failed to find CapFilter:IAMStreamConfig", hr);
   }
 
   return CreateCapabilityMap();
@@ -353,14 +349,14 @@ void VideoCaptureDeviceWin::AllocateAndStart(
   ScopedComPtr<IAMStreamConfig> stream_config;
   HRESULT hr = output_capture_pin_.QueryInterface(stream_config.Receive());
   if (FAILED(hr)) {
-    SetErrorState(FROM_HERE, "Can't get the Capture format settings");
+    SetErrorState(FROM_HERE, "Can't get the Capture format settings", hr);
     return;
   }
 
   int count = 0, size = 0;
   hr = stream_config->GetNumberOfCapabilities(&count, &size);
   if (FAILED(hr)) {
-    SetErrorState(FROM_HERE, "Failed to GetNumberOfCapabilities");
+    SetErrorState(FROM_HERE, "Failed to GetNumberOfCapabilities", hr);
     return;
   }
 
@@ -373,7 +369,7 @@ void VideoCaptureDeviceWin::AllocateAndStart(
   hr = stream_config->GetStreamCaps(found_capability.stream_index,
                                     media_type.Receive(), caps.get());
   if (hr != S_OK) {
-    SetErrorState(FROM_HERE, "Failed to get capture device capabilities");
+    SetErrorState(FROM_HERE, "Failed to get capture device capabilities", hr);
     return;
   }
   if (media_type->formattype == FORMAT_VideoInfo) {
@@ -389,8 +385,7 @@ void VideoCaptureDeviceWin::AllocateAndStart(
   // Order the capture device to use this format.
   hr = stream_config->SetFormat(media_type.get());
   if (FAILED(hr)) {
-    // TODO(grunell): Log the error. http://crbug.com/405016.
-    SetErrorState(FROM_HERE, "Failed to set capture device output format");
+    SetErrorState(FROM_HERE, "Failed to set capture device output format", hr);
     return;
   }
 
@@ -407,15 +402,13 @@ void VideoCaptureDeviceWin::AllocateAndStart(
   }
 
   if (FAILED(hr)) {
-    SetErrorState(FROM_HERE, "Failed to connect the Capture graph.");
+    SetErrorState(FROM_HERE, "Failed to connect the Capture graph.", hr);
     return;
   }
 
   hr = media_control_->Pause();
   if (FAILED(hr)) {
-    SetErrorState(
-        FROM_HERE,
-        "Failed to pause the Capture device, is it already occupied?");
+    SetErrorState(FROM_HERE, "Failed to pause the Capture device", hr);
     return;
   }
 
@@ -426,7 +419,7 @@ void VideoCaptureDeviceWin::AllocateAndStart(
   // Start capturing.
   hr = media_control_->Run();
   if (FAILED(hr)) {
-    SetErrorState(FROM_HERE, "Failed to start the Capture device.");
+    SetErrorState(FROM_HERE, "Failed to start the Capture device.", hr);
     return;
   }
 
@@ -440,7 +433,7 @@ void VideoCaptureDeviceWin::StopAndDeAllocate() {
 
   HRESULT hr = media_control_->Stop();
   if (FAILED(hr)) {
-    SetErrorState(FROM_HERE, "Failed to stop the capture graph.");
+    SetErrorState(FROM_HERE, "Failed to stop the capture graph.", hr);
     return;
   }
 
@@ -489,25 +482,20 @@ bool VideoCaptureDeviceWin::CreateCapabilityMap() {
   DCHECK(thread_checker_.CalledOnValidThread());
   ScopedComPtr<IAMStreamConfig> stream_config;
   HRESULT hr = output_capture_pin_.QueryInterface(stream_config.Receive());
-  if (FAILED(hr)) {
-    DPLOG(ERROR) << "Failed to get IAMStreamConfig interface from "
-                    "capture device: " << logging::SystemErrorCodeToString(hr);
+  DLOG_IF_FAILED_WITH_HRESULT(
+      "Failed to get IAMStreamConfig from capture device", hr);
+  if (FAILED(hr))
     return false;
-  }
 
   // Get interface used for getting the frame rate.
   ScopedComPtr<IAMVideoControl> video_control;
   hr = capture_filter_.QueryInterface(video_control.Receive());
-  DLOG_IF(WARNING, FAILED(hr)) << "IAMVideoControl Interface NOT SUPPORTED: "
-                               << logging::SystemErrorCodeToString(hr);
 
   int count = 0, size = 0;
   hr = stream_config->GetNumberOfCapabilities(&count, &size);
-  if (FAILED(hr)) {
-    DLOG(ERROR) << "Failed to GetNumberOfCapabilities: "
-                << logging::SystemErrorCodeToString(hr);
+  DLOG_IF_FAILED_WITH_HRESULT("Failed to GetNumberOfCapabilities", hr);
+  if (FAILED(hr))
     return false;
-  }
 
   std::unique_ptr<BYTE[]> caps(new BYTE[size]);
   for (int stream_index = 0; stream_index < count; ++stream_index) {
@@ -517,8 +505,7 @@ bool VideoCaptureDeviceWin::CreateCapabilityMap() {
     // GetStreamCaps() may return S_FALSE, so don't use FAILED() or SUCCEED()
     // macros here since they'll trigger incorrectly.
     if (hr != S_OK) {
-      DLOG(ERROR) << "Failed to GetStreamCaps: "
-                  << logging::SystemErrorCodeToString(hr);
+      DLOG(ERROR) << "Failed to GetStreamCaps";
       return false;
     }
 
@@ -595,18 +582,16 @@ void VideoCaptureDeviceWin::SetAntiFlickerInCaptureFilter(
     hr = ks_propset->Set(PROPSETID_VIDCAP_VIDEOPROCAMP,
                          KSPROPERTY_VIDEOPROCAMP_POWERLINE_FREQUENCY, &data,
                          sizeof(data), &data, sizeof(data));
-    DLOG_IF(ERROR, FAILED(hr)) << "Anti-flicker setting failed: "
-                               << logging::SystemErrorCodeToString(hr);
-    DVLOG_IF(2, SUCCEEDED(hr)) << "Anti-flicker set correctly.";
-  } else {
-    DVLOG(2) << "Anti-flicker setting not supported.";
+    DLOG_IF_FAILED_WITH_HRESULT("Anti-flicker setting failed", hr);
   }
 }
 
 void VideoCaptureDeviceWin::SetErrorState(
     const tracked_objects::Location& from_here,
-    const std::string& reason) {
+    const std::string& reason,
+    HRESULT hr) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DLOG_IF_FAILED_WITH_HRESULT(reason, hr);
   state_ = kError;
   client_->OnError(from_here, reason);
 }

media/capture/video/win/video_capture_device_win.h

@@ -93,7 +93,8 @@ class VideoCaptureDeviceWin : public VideoCaptureDevice,
   bool CreateCapabilityMap();
   void SetAntiFlickerInCaptureFilter(const VideoCaptureParams& params);
   void SetErrorState(const tracked_objects::Location& from_here,
-                     const std::string& reason);
+                     const std::string& reason,
+                     HRESULT hr);
 
   const VideoCaptureDeviceDescriptor device_descriptor_;
   InternalState state_;