Windows video capture: Remove duplicated code from GetDeviceSupportedFormats*

media/video/capture/win/video_capture_device_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/video/capture/win/video_capture_device_win.h"
 
 #include <ks.h>
 #include <ksmedia.h>
 
 #include <algorithm>
@@ skipping 205 matching lines @@
     if (capture_filter_)
       graph_builder_->RemoveFilter(capture_filter_);
 
     if (mjpg_filter_)
       graph_builder_->RemoveFilter(mjpg_filter_);
   }
 }
 
 bool VideoCaptureDeviceWin::Init() {
   DCHECK(CalledOnValidThread());
-  HRESULT hr = GetDeviceFilter(device_name_.id(), capture_filter_.Receive());
-  if (!capture_filter_) {
-    DLOG(ERROR) << "Failed to create capture filter: "
-                << logging::SystemErrorCodeToString(hr);
+
+  if (!VideoCaptureDeviceWin::GetDeviceSupportedFormats(device_name_,
+                                                        &capture_filter_,
+                                                        &output_capture_pin_,
+                                                        &capabilities_)) {
     return false;
   }
 
-  output_capture_pin_ =
-      GetPin(capture_filter_, PINDIR_OUTPUT, PIN_CATEGORY_CAPTURE);
-  if (!output_capture_pin_) {
-    DLOG(ERROR) << "Failed to get capture output pin";
-    return false;
-  }
-
   // Create the sink filter used for receiving Captured frames.
   sink_filter_ = new SinkFilter(this);
   if (sink_filter_ == NULL) {
     DLOG(ERROR) << "Failed to create send filter";
     return false;
   }
 
   input_sink_pin_ = sink_filter_->GetPin(0);
 
-  hr = graph_builder_.CreateInstance(CLSID_FilterGraph, NULL,
-                                     CLSCTX_INPROC_SERVER);
+  HRESULT hr = graph_builder_.CreateInstance(
+      CLSID_FilterGraph, NULL, CLSCTX_INPROC_SERVER);
   if (FAILED(hr)) {
     DLOG(ERROR) << "Failed to create graph builder: "
                 << logging::SystemErrorCodeToString(hr);
     return false;
   }
 
   hr = graph_builder_.QueryInterface(media_control_.Receive());
   if (FAILED(hr)) {
     DLOG(ERROR) << "Failed to create media control builder: "
                 << logging::SystemErrorCodeToString(hr);
     return false;
   }
 
   hr = graph_builder_->AddFilter(capture_filter_, NULL);
   if (FAILED(hr)) {
     DLOG(ERROR) << "Failed to add the capture device to the graph: "
                 << logging::SystemErrorCodeToString(hr);
     return false;
   }
 
   hr = graph_builder_->AddFilter(sink_filter_, NULL);
   if (FAILED(hr)) {
     DLOG(ERROR) << "Failed to add the send filter to the graph: "
                 << logging::SystemErrorCodeToString(hr);
     return false;
   }
 
-  return CreateCapabilityMap();
+  return true;
 }
 
 void VideoCaptureDeviceWin::AllocateAndStart(
     const VideoCaptureParams& params,
     scoped_ptr<VideoCaptureDevice::Client> client) {
   DCHECK(CalledOnValidThread());
   if (state_ != kIdle)
     return;
 
   client_ = client.Pass();
@@ skipping 143 matching lines @@
   state_ = kIdle;
 }
 
 // Implements SinkFilterObserver::SinkFilterObserver.
 void VideoCaptureDeviceWin::FrameReceived(const uint8* buffer,
                                           int length) {
   client_->OnIncomingCapturedData(
       buffer, length, capture_format_, 0, base::TimeTicks::Now());
 }
 
-bool VideoCaptureDeviceWin::CreateCapabilityMap() {
-  DCHECK(CalledOnValidThread());
+// static
+bool VideoCaptureDeviceWin::GetDeviceSupportedFormats(

[perkj_chrome, 2014/09/19 10:04:55]

It looks like this is doing much more that gettign the supported formats. Can it
be split up in multiple functions that only do what the function name suggests?

+    const Name& device,
+    ScopedComPtr<IBaseFilter>* capture_filter,
+    ScopedComPtr<IPin>* output_capture_pin,
+    CapabilityList* capabilities) {
+  HRESULT hr = GetDeviceFilter(device.id(), capture_filter->Receive());
+  if (!capture_filter) {
+    DLOG(ERROR) << "Failed to create capture filter: "
+                << logging::SystemErrorCodeToString(hr);
+    return false;
+  }
+
+  *output_capture_pin =
+      GetPin(*capture_filter, PINDIR_OUTPUT, PIN_CATEGORY_CAPTURE);
+  if (!*output_capture_pin) {
+    DLOG(ERROR) << "Failed to get capture output pin";
+    return false;
+  }
+
   ScopedComPtr<IAMStreamConfig> stream_config;
-  HRESULT hr = output_capture_pin_.QueryInterface(stream_config.Receive());
+  hr = output_capture_pin->QueryInterface(stream_config.Receive());
   if (FAILED(hr)) {
     DPLOG(ERROR) << "Failed to get IAMStreamConfig interface from "
                     "capture device: " << logging::SystemErrorCodeToString(hr);
     return false;
   }
 
   // Get interface used for getting the frame rate.
   ScopedComPtr<IAMVideoControl> video_control;
-  hr = capture_filter_.QueryInterface(video_control.Receive());
+  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);
     return false;
   }
 
   scoped_ptr<BYTE[]> caps(new BYTE[size]);
   for (int i = 0; i < count; ++i) {
     ScopedMediaType media_type;
     hr = stream_config->GetStreamCaps(i, media_type.Receive(), caps.get());
     // 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);
-      return false;
+      continue;
     }
 
     if (media_type->majortype == MEDIATYPE_Video &&
         media_type->formattype == FORMAT_VideoInfo) {
       VideoCaptureCapabilityWin capability(i);
       capability.supported_format.pixel_format =
           TranslateMediaSubtypeToPixelFormat(media_type->subtype);
       if (capability.supported_format.pixel_format == PIXEL_FORMAT_UNKNOWN)
         continue;
 
       VIDEOINFOHEADER* h =
           reinterpret_cast<VIDEOINFOHEADER*>(media_type->pbFormat);
       capability.supported_format.frame_size.SetSize(h->bmiHeader.biWidth,
                                                      h->bmiHeader.biHeight);
 
       // Try to get a better |time_per_frame| from IAMVideoControl.  If not, use
       // the value from VIDEOINFOHEADER.
       REFERENCE_TIME time_per_frame = h->AvgTimePerFrame;
       if (video_control) {
         ScopedCoMem<LONGLONG> max_fps;
         LONG list_size = 0;
         SIZE size = {capability.supported_format.frame_size.width(),
                      capability.supported_format.frame_size.height()};
 
         // GetFrameRateList doesn't return max frame rate always
         // eg: Logitech Notebook. This may be due to a bug in that API
         // because GetFrameRateList array is reversed in the above camera. So
         // a util method written. Can't assume the first value will return
         // the max fps.
-        hr = video_control->GetFrameRateList(output_capture_pin_, i, size,
+        hr = video_control->GetFrameRateList(*output_capture_pin, i, size,
                                              &list_size, &max_fps);
         // Sometimes |list_size| will be > 0, but max_fps will be NULL.  Some
         // drivers may return an HRESULT of S_FALSE which SUCCEEDED() translates
         // into success, so explicitly check S_OK.  See http://crbug.com/306237.
         if (hr == S_OK && list_size > 0 && max_fps) {
           time_per_frame = *std::min_element(max_fps.get(),
                                              max_fps.get() + list_size);
         }
       }
 
       capability.supported_format.frame_rate =
           (time_per_frame > 0)
               ? (kSecondsToReferenceTime / static_cast<float>(time_per_frame))
               : 0.0;
 
-      // DirectShow works at the moment only on integer frame_rate but the
-      // best capability matching class works on rational frame rates.
-      capability.frame_rate_numerator = capability.supported_format.frame_rate;
-      capability.frame_rate_denominator = 1;
-
-      capabilities_.Add(capability);
+      capabilities->Add(capability);
     }
   }
 
-  return !capabilities_.empty();
+  return !capabilities->empty();
 }
 
 // Set the power line frequency removal in |capture_filter_| if available.
 void VideoCaptureDeviceWin::SetAntiFlickerInCaptureFilter() {
   const int power_line_frequency = GetPowerLineFrequencyForLocation();
   if (power_line_frequency != kPowerLine50Hz &&
       power_line_frequency != kPowerLine60Hz) {
     return;
   }
   ScopedComPtr<IKsPropertySet> ks_propset;
@@ skipping 19 matching lines @@
     DVLOG(2) << "Anti-flicker setting not supported.";
   }
 }
 
 void VideoCaptureDeviceWin::SetErrorState(const std::string& reason) {
   DCHECK(CalledOnValidThread());
   state_ = kError;
   client_->OnError(reason);
 }
 }  // namespace media