Windows video capture: Remove duplicated code from GetDeviceSupportedFormats*

media/video/capture/win/video_capture_device_factory_win.cc

Index: media/video/capture/win/video_capture_device_factory_win.cc
diff --git a/media/video/capture/win/video_capture_device_factory_win.cc b/media/video/capture/win/video_capture_device_factory_win.cc
index 1630baf35730602bffdeaf5287d1c65127df0703..de13111feb01d032b100e38eeb1eae97d4665a66 100644
--- a/media/video/capture/win/video_capture_device_factory_win.cc
+++ b/media/video/capture/win/video_capture_device_factory_win.cc
@@ -190,89 +190,16 @@ static void GetDeviceNamesMediaFoundation(Names* device_names) {
 static void GetDeviceSupportedFormatsDirectShow(const Name& device,
                                                 VideoCaptureFormats* formats) {
   DVLOG(1) << "GetDeviceSupportedFormatsDirectShow for " << device.name();
-  ScopedComPtr<ICreateDevEnum> dev_enum;
-  HRESULT hr = dev_enum.CreateInstance(CLSID_SystemDeviceEnum, NULL,
-                                       CLSCTX_INPROC);
-  if (FAILED(hr))
-    return;
-
-  ScopedComPtr<IEnumMoniker> enum_moniker;
-  hr = dev_enum->CreateClassEnumerator(CLSID_VideoInputDeviceCategory,
-                                       enum_moniker.Receive(), 0);
-  // CreateClassEnumerator returns S_FALSE on some Windows OS when no camera
-  // exists. Therefore the FAILED macro can't be used.
-  if (hr != S_OK)
-    return;
 
-  // Walk the capture devices. No need to check for device presence again since
-  // that is anyway needed in GetDeviceFilter(). "google camera adapter" and old
-  // VFW devices are already skipped previously in GetDeviceNames() enumeration.
   base::win::ScopedComPtr<IBaseFilter> capture_filter;
-  hr = VideoCaptureDeviceWin::GetDeviceFilter(device.capabilities_id(),
-                                              capture_filter.Receive());
-  if (!capture_filter) {
-    DLOG(ERROR) << "Failed to create capture filter: "
-                << logging::SystemErrorCodeToString(hr);
+  base::win::ScopedComPtr<IPin> output_capture_pin;
+  CapabilityList capabilities;
+  if (!VideoCaptureDeviceWin::GetDeviceSupportedFormats(
+          device, &capture_filter, &output_capture_pin, &capabilities)) {
     return;
   }
 
-  base::win::ScopedComPtr<IPin> output_capture_pin(
-      VideoCaptureDeviceWin::GetPin(capture_filter,
-                                    PINDIR_OUTPUT,
-                                    PIN_CATEGORY_CAPTURE));
-  if (!output_capture_pin) {
-    DLOG(ERROR) << "Failed to get capture output pin";
-    return;
-  }
-
-  ScopedComPtr<IAMStreamConfig> stream_config;
-  hr = output_capture_pin.QueryInterface(stream_config.Receive());
-  if (FAILED(hr)) {
-    DLOG(ERROR) << "Failed to get IAMStreamConfig interface from "
-                   "capture device: " << logging::SystemErrorCodeToString(hr);
-    return;
-  }
-
-  int count = 0, size = 0;
-  hr = stream_config->GetNumberOfCapabilities(&count, &size);
-  if (FAILED(hr)) {
-    DLOG(ERROR) << "GetNumberOfCapabilities failed: "
-                << logging::SystemErrorCodeToString(hr);
-    return;
-  }
-
-  scoped_ptr<BYTE[]> caps(new BYTE[size]);
-  for (int i = 0; i < count; ++i) {
-    VideoCaptureDeviceWin::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) << "GetStreamCaps failed: "
-                  << logging::SystemErrorCodeToString(hr);
-      return;
-    }
-
-    if (media_type->majortype == MEDIATYPE_Video &&
-        media_type->formattype == FORMAT_VideoInfo) {
-      VideoCaptureFormat format;
-      format.pixel_format =
-          VideoCaptureDeviceWin::TranslateMediaSubtypeToPixelFormat(
-              media_type->subtype);
-      if (format.pixel_format == PIXEL_FORMAT_UNKNOWN)
-        continue;
-      VIDEOINFOHEADER* h =
-          reinterpret_cast<VIDEOINFOHEADER*>(media_type->pbFormat);
-      format.frame_size.SetSize(h->bmiHeader.biWidth,
-                                h->bmiHeader.biHeight);
-      // Trust the frame rate from the VIDEOINFOHEADER.
-      format.frame_rate = (h->AvgTimePerFrame > 0) ?
-          kSecondsToReferenceTime / static_cast<float>(h->AvgTimePerFrame) :
-          0.0f;
-      formats->push_back(format);
-      DVLOG(1) << device.name() << " " << format.ToString();
-    }
-  }
+  capabilities.CapabilitiesToVideoCaptureFormats(formats);
 }
 
 static void GetDeviceSupportedFormatsMediaFoundation(
@@ -294,45 +221,15 @@ static void GetDeviceSupportedFormatsMediaFoundation(
     return;
   }
 
-  DWORD stream_index = 0;
-  ScopedComPtr<IMFMediaType> type;
-  while (SUCCEEDED(reader->GetNativeMediaType(
-             kFirstVideoStream, stream_index, type.Receive()))) {
-    UINT32 width, height;
-    hr = MFGetAttributeSize(type, MF_MT_FRAME_SIZE, &width, &height);
-    if (FAILED(hr)) {
-      DLOG(ERROR) << "MFGetAttributeSize failed: "
-                  << logging::SystemErrorCodeToString(hr);
-      return;
-    }
-    VideoCaptureFormat capture_format;
-    capture_format.frame_size.SetSize(width, height);
-
-    UINT32 numerator, denominator;
-    hr = MFGetAttributeRatio(type, MF_MT_FRAME_RATE, &numerator, &denominator);
-    if (FAILED(hr)) {
-      DLOG(ERROR) << "MFGetAttributeSize failed: "
-                  << logging::SystemErrorCodeToString(hr);
-      return;
-    }
-    capture_format.frame_rate = denominator
-        ? static_cast<float>(numerator) / denominator : 0.0f;
-
-    GUID type_guid;
-    hr = type->GetGUID(MF_MT_SUBTYPE, &type_guid);
-    if (FAILED(hr)) {
-      DLOG(ERROR) << "GetGUID failed: "
-                  << logging::SystemErrorCodeToString(hr);
-      return;
-    }
-    VideoCaptureDeviceMFWin::FormatFromGuid(type_guid,
-                                            &capture_format.pixel_format);
-    type.Release();
-    formats->push_back(capture_format);
-    ++stream_index;
-
-    DVLOG(1) << device.name() << " " << capture_format.ToString();
+  CapabilityList capabilities;
+  hr = VideoCaptureDeviceMFWin::FillCapabilities(reader, &capabilities);
+  if (FAILED(hr)) {
+    DLOG(ERROR) << "FillCapabilities failed: "
+                << logging::SystemErrorCodeToString(hr);
+    return;
   }
+
+  capabilities.CapabilitiesToVideoCaptureFormats(formats);
 }
 
 // Returns true iff the current platform supports the Media Foundation API
@@ -425,10 +322,15 @@ void VideoCaptureDeviceFactoryWin::GetDeviceSupportedFormats(
     const Name& device,
     VideoCaptureFormats* formats) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  formats->clear();

[tommi (sloooow) - chröme, 2014/10/22 16:23:13]

if formats needs to be empty on entry, I prefer to add a
DCHECK(formats->empty()) - but only if that's really a requirement of the
function.  Otherwise, I'd just like to remove this line and leave it up to the
caller to make sure it's empty (which is the most common case - or perhaps the
only case?)

   if (use_media_foundation_)
     GetDeviceSupportedFormatsMediaFoundation(device, formats);
   else
     GetDeviceSupportedFormatsDirectShow(device, formats);
+  for (VideoCaptureFormats::iterator it = formats->begin();

[tommi (sloooow) - chröme, 2014/10/22 16:23:13]

nit: empty line before this one

+       it != formats->end(); ++it) {
+    DVLOG(1) << device.name() << " " << it->ToString();
+  }
 }
 
 }  // namespace media