Video capture: Refactor GetBestMatchedFormat

Review URL: https://codereview.chromium.org/558623002

Cr-Commit-Position: refs/heads/master@{#299499}

5 files changed, 93 insertions, 223 deletions

diff --git a/media/video/capture/mac/video_capture_device_mac.mm b/media/video/capture/mac/video_capture_device_mac.mm
index 190f53d..65103e5 100644
--- a/media/video/capture/mac/video_capture_device_mac.mm
+++ b/media/video/capture/mac/video_capture_device_mac.mm
@@ -20,6 +20,7 @@
 #import "media/video/capture/mac/platform_video_capturing_mac.h"
 #import "media/video/capture/mac/video_capture_device_avfoundation_mac.h"
 #import "media/video/capture/mac/video_capture_device_qtkit_mac.h"
+#include "ui/gfx/size.h"
 
 @implementation DeviceNameAndTransportType
 
@@ -94,24 +95,19 @@ typedef struct IOUSBInterfaceDescriptor {
   UInt8 bUnitID;
 } IOUSBInterfaceDescriptor;
 
-// TODO(ronghuawu): Replace this with CapabilityList::GetBestMatchedCapability.
-void GetBestMatchSupportedResolution(int* width, int* height) {
+static void GetBestMatchSupportedResolution(gfx::Size* resolution) {
   int min_diff = kint32max;
-  int matched_width = *width;
-  int matched_height = *height;
-  int desired_res_area = *width * *height;
+  const int desired_area = resolution->GetArea();
   for (size_t i = 0; i < arraysize(kWellSupportedResolutions); ++i) {
-    int area = kWellSupportedResolutions[i]->width *
-               kWellSupportedResolutions[i]->height;
-    int diff = std::abs(desired_res_area - area);
+    const int area = kWellSupportedResolutions[i]->width *
+                     kWellSupportedResolutions[i]->height;
+    const int diff = std::abs(desired_area - area);
     if (diff < min_diff) {
       min_diff = diff;
-      matched_width = kWellSupportedResolutions[i]->width;
-      matched_height = kWellSupportedResolutions[i]->height;
+      resolution->SetSize(kWellSupportedResolutions[i]->width,
+                          kWellSupportedResolutions[i]->height);
     }
   }
-  *width = matched_width;
-  *height = matched_height;
 }
 
 // Tries to create a user-side device interface for a given USB device. Returns
@@ -370,15 +366,13 @@ void VideoCaptureDeviceMac::AllocateAndStart(
   if (state_ != kIdle) {
     return;
   }
-  int width = params.requested_format.frame_size.width();
-  int height = params.requested_format.frame_size.height();
-  float frame_rate = params.requested_format.frame_rate;
 
   // QTKit API can scale captured frame to any size requested, which would lead
   // to undesired aspect ratio changes. Try to open the camera with a known
   // supported format and let the client crop/pad the captured frames.
+  gfx::Size resolution = params.requested_format.frame_size;
   if (!AVFoundationGlue::IsAVFoundationSupported())
-    GetBestMatchSupportedResolution(&width, &height);
+    GetBestMatchSupportedResolution(&resolution);
 
   client_ = client.Pass();
   if (device_name_.capture_api_type() == Name::AVFOUNDATION)
@@ -394,13 +388,11 @@ void VideoCaptureDeviceMac::AllocateAndStart(
     SetErrorState("Could not open capture device.");
     return;
   }
-  if (frame_rate < kMinFrameRate)
-    frame_rate = kMinFrameRate;
-  else if (frame_rate > kMaxFrameRate)
-    frame_rate = kMaxFrameRate;
 
-  capture_format_.frame_size.SetSize(width, height);
-  capture_format_.frame_rate = frame_rate;
+  capture_format_.frame_size = resolution;
+  capture_format_.frame_rate =
+      std::max(kMinFrameRate,
+               std::min(params.requested_format.frame_rate, kMaxFrameRate));
   capture_format_.pixel_format = PIXEL_FORMAT_UYVY;
 
   // QTKit: Set the capture resolution only if this is VGA or smaller, otherwise
@@ -410,8 +402,8 @@ void VideoCaptureDeviceMac::AllocateAndStart(
   // latency, because the webcam will need to be reopened if its default
   // resolution is not HD or VGA.
   // AVfoundation is configured for all resolutions.
-  if (AVFoundationGlue::IsAVFoundationSupported() || width <= kVGA.width ||
-      height <= kVGA.height) {
+  if (AVFoundationGlue::IsAVFoundationSupported() ||
+      resolution.width() <= kVGA.width || resolution.height() <= kVGA.height) {
     if (!UpdateCaptureResolution())
       return;
   }
diff --git a/media/video/capture/win/capability_list_win.cc b/media/video/capture/win/capability_list_win.cc
index 9b4531b..cefda43 100644
--- a/media/video/capture/win/capability_list_win.cc
+++ b/media/video/capture/win/capability_list_win.cc
@@ -5,116 +5,49 @@
 #include "media/video/capture/win/capability_list_win.h"
 
 #include <algorithm>
+#include <functional>
 
 #include "base/logging.h"
 
 namespace media {
-namespace {
 
-// Help structure used for comparing video capture capabilities.
-struct ResolutionDiff {
-  const VideoCaptureCapabilityWin* capability;
-  int diff_height;
-  int diff_width;
-  int diff_frame_rate;
-};
-
-bool CompareHeight(const ResolutionDiff& item1, const ResolutionDiff& item2) {
-  return abs(item1.diff_height) < abs(item2.diff_height);
-}
-
-bool CompareWidth(const ResolutionDiff& item1, const ResolutionDiff& item2) {
-  return abs(item1.diff_width) < abs(item2.diff_width);
-}
-
-bool CompareFrameRate(const ResolutionDiff& item1,
-                      const ResolutionDiff& item2) {
-  return abs(item1.diff_frame_rate) < abs(item2.diff_frame_rate);
-}
-
-bool CompareColor(const ResolutionDiff& item1, const ResolutionDiff& item2) {
-  return item1.capability->supported_format.pixel_format <
-         item2.capability->supported_format.pixel_format;
-}
-
-}  // namespace.
-
-CapabilityList::CapabilityList() {
-  DetachFromThread();
-}
-
-CapabilityList::~CapabilityList() {}
-
-// Appends an entry to the list.
-void CapabilityList::Add(const VideoCaptureCapabilityWin& capability) {
-  DCHECK(CalledOnValidThread());
-  capabilities_.push_back(capability);
+static bool CompareCapability(const VideoCaptureFormat& requested,
+                              const CapabilityWin& capability_lhs,
+                              const CapabilityWin& capability_rhs) {
+  const VideoCaptureFormat& lhs = capability_lhs.supported_format;
+  const VideoCaptureFormat& rhs = capability_rhs.supported_format;
+
+  const int diff_height_lhs =
+      std::abs(lhs.frame_size.height() - requested.frame_size.height());
+  const int diff_height_rhs =
+      std::abs(rhs.frame_size.height() - requested.frame_size.height());
+  if (diff_height_lhs != diff_height_rhs)
+    return diff_height_lhs < diff_height_rhs;
+
+  const int diff_width_lhs =
+      std::abs(lhs.frame_size.width() - requested.frame_size.width());
+  const int diff_width_rhs =
+      std::abs(rhs.frame_size.width() - requested.frame_size.width());
+  if (diff_width_lhs != diff_width_rhs)
+    return diff_width_lhs < diff_width_rhs;
+
+  const float diff_fps_lhs = std::fabs(lhs.frame_rate - requested.frame_rate);
+  const float diff_fps_rhs = std::fabs(rhs.frame_rate - requested.frame_rate);
+  if (diff_fps_lhs != diff_fps_rhs)
+    return diff_fps_lhs < diff_fps_rhs;
+
+  return lhs.pixel_format < rhs.pixel_format;
 }
 
-const VideoCaptureCapabilityWin& CapabilityList::GetBestMatchedFormat(
-    int requested_width,
-    int requested_height,
-    float requested_frame_rate) const {
-  DCHECK(CalledOnValidThread());
-  DCHECK(!capabilities_.empty());
-
-  std::list<ResolutionDiff> diff_list;
-
-  // Loop through the candidates to create a list of differentials between the
-  // requested resolution and the camera capability.
-  for (Capabilities::const_iterator it = capabilities_.begin();
-       it != capabilities_.end(); ++it) {
-    ResolutionDiff diff;
-    diff.capability = &(*it);
-    diff.diff_width = it->supported_format.frame_size.width() - requested_width;
-    diff.diff_height =
-        it->supported_format.frame_size.height() - requested_height;
-    // The 1000 allows using integer arithmetic for f.i. 29.971 fps.
-    diff.diff_frame_rate =
-        1000 * ((static_cast<float>(it->frame_rate_numerator) /
-                 it->frame_rate_denominator) -
-                requested_frame_rate);
-    diff_list.push_back(diff);
-  }
-
-  // Sort the best height candidates.
-  diff_list.sort(&CompareHeight);
-  int best_diff = diff_list.front().diff_height;
-  for (std::list<ResolutionDiff>::iterator it = diff_list.begin();
-       it != diff_list.end(); ++it) {
-    if (it->diff_height != best_diff) {
-      // Remove all candidates but the best.
-      diff_list.erase(it, diff_list.end());
-      break;
-    }
+CapabilityWin GetBestMatchedCapability(const VideoCaptureFormat& requested,
+                                       const CapabilityList& capabilities) {
+  DCHECK(!capabilities.empty());
+  CapabilityWin best_match = capabilities.front();
+  for (const CapabilityWin& capability : capabilities) {
+    if (CompareCapability(requested, capability, best_match))
+      best_match = capability;
   }
-
-  // Sort the best width candidates.
-  diff_list.sort(&CompareWidth);
-  best_diff = diff_list.front().diff_width;
-  for (std::list<ResolutionDiff>::iterator it = diff_list.begin();
-       it != diff_list.end(); ++it) {
-    if (it->diff_width != best_diff) {
-      // Remove all candidates but the best.
-      diff_list.erase(it, diff_list.end());
-      break;
-    }
-  }
-
-  // Sort the best frame rate candidates.
-  diff_list.sort(&CompareFrameRate);
-  best_diff = diff_list.front().diff_frame_rate;
-  for (std::list<ResolutionDiff>::iterator it = diff_list.begin();
-       it != diff_list.end(); ++it) {
-    if (it->diff_frame_rate != best_diff) {
-      diff_list.erase(it, diff_list.end());
-      break;
-    }
-  }
-
-  // Decide the best color format.
-  diff_list.sort(&CompareColor);
-  return *diff_list.front().capability;
+  return best_match;
 }
 
 }  // namespace media
diff --git a/media/video/capture/win/capability_list_win.h b/media/video/capture/win/capability_list_win.h
index c381b4b..e173439 100644
--- a/media/video/capture/win/capability_list_win.h
+++ b/media/video/capture/win/capability_list_win.h
@@ -11,49 +11,21 @@
 
 #include <list>
 
-#include "base/basictypes.h"
-#include "base/threading/non_thread_safe.h"
 #include "media/video/capture/video_capture_types.h"
 
 namespace media {
 
-struct VideoCaptureCapabilityWin {
-  explicit VideoCaptureCapabilityWin(int index)
-      : stream_index(index),
-        frame_rate_numerator(0),
-        frame_rate_denominator(1) {}
+struct CapabilityWin {
+  CapabilityWin(int index, const VideoCaptureFormat& format)
+      : stream_index(index), supported_format(format) {}
   int stream_index;
-  // Internally to Media Foundation Api type devices we use rational framerates
-  // so framerates can be properly represented, f.i. 29.971fps= 30000/1001.
-  int frame_rate_numerator;
-  int frame_rate_denominator;
   VideoCaptureFormat supported_format;
 };
 
-class CapabilityList : public base::NonThreadSafe {
- public:
-  CapabilityList();
-  ~CapabilityList();
+typedef std::list<CapabilityWin> CapabilityList;
 
-  bool empty() const { return capabilities_.empty(); }
-
-  // Appends an entry to the list.
-  void Add(const VideoCaptureCapabilityWin& capability);
-
-  // Loops through the list of capabilities and returns an index of the best
-  // matching capability.  The algorithm prioritizes height, width, frame rate
-  // and color format in that order.
-  const VideoCaptureCapabilityWin& GetBestMatchedFormat(
-      int requested_width,
-      int requested_height,
-      float requested_frame_rate) const;
-
- private:
-  typedef std::list<VideoCaptureCapabilityWin> Capabilities;
-  Capabilities capabilities_;
-
-  DISALLOW_COPY_AND_ASSIGN(CapabilityList);
-};
+CapabilityWin GetBestMatchedCapability(const VideoCaptureFormat& requested,
+                                       const CapabilityList& capabilities);
 
 }  // namespace media
 
diff --git a/media/video/capture/win/video_capture_device_mf_win.cc b/media/video/capture/win/video_capture_device_mf_win.cc
index 2fa25db..a222d1c 100644
--- a/media/video/capture/win/video_capture_device_mf_win.cc
+++ b/media/video/capture/win/video_capture_device_mf_win.cc
@@ -34,34 +34,26 @@ static bool GetFrameSize(IMFMediaType* type, gfx::Size* frame_size) {
   return true;
 }
 
-static bool GetFrameRate(IMFMediaType* type,
-                         int* frame_rate_numerator,
-                         int* frame_rate_denominator) {
+static bool GetFrameRate(IMFMediaType* type, float* frame_rate) {
   UINT32 numerator, denominator;
   if (FAILED(MFGetAttributeRatio(type, MF_MT_FRAME_RATE, &numerator,
                                  &denominator))||
       !denominator) {
     return false;
   }
-  *frame_rate_numerator = numerator;
-  *frame_rate_denominator = denominator;
+  *frame_rate = static_cast<float>(numerator) / denominator;
   return true;
 }
 
-static bool FillCapabilitiesFromType(IMFMediaType* type,
-                                     VideoCaptureCapabilityWin* capability) {
+static bool FillFormat(IMFMediaType* type, VideoCaptureFormat* format) {
   GUID type_guid;
   if (FAILED(type->GetGUID(MF_MT_SUBTYPE, &type_guid)) ||
-      !GetFrameSize(type, &capability->supported_format.frame_size) ||
-      !GetFrameRate(type,
-                    &capability->frame_rate_numerator,
-                    &capability->frame_rate_denominator) ||
+      !GetFrameSize(type, &format->frame_size) ||
+      !GetFrameRate(type, &format->frame_rate) ||
       !VideoCaptureDeviceMFWin::FormatFromGuid(type_guid,
-          &capability->supported_format.pixel_format)) {
+                                               &format->pixel_format)) {
     return false;
   }
-  capability->supported_format.frame_rate =
-      capability->frame_rate_numerator / capability->frame_rate_denominator;
 
   return true;
 }
@@ -71,15 +63,13 @@ HRESULT FillCapabilities(IMFSourceReader* source,
   DWORD stream_index = 0;
   ScopedComPtr<IMFMediaType> type;
   HRESULT hr;
-  for (hr = source->GetNativeMediaType(kFirstVideoStream, stream_index,
-                                       type.Receive());
-       SUCCEEDED(hr);
-       hr = source->GetNativeMediaType(kFirstVideoStream, stream_index,
-                                       type.Receive())) {
-    VideoCaptureCapabilityWin capability(stream_index++);
-    if (FillCapabilitiesFromType(type, &capability))
-      capabilities->Add(capability);
+  while (SUCCEEDED(hr = source->GetNativeMediaType(
+                       kFirstVideoStream, stream_index, type.Receive()))) {
+    VideoCaptureFormat format;
+    if (FillFormat(type, &format))
+      capabilities->emplace_back(stream_index, format);
     type.Release();
+    ++stream_index;
   }
 
   if (capabilities->empty() && (SUCCEEDED(hr) || hr == MF_E_NO_MORE_TYPES))
@@ -251,12 +241,8 @@ void VideoCaptureDeviceMFWin::AllocateAndStart(
   if (reader_) {
     hr = FillCapabilities(reader_, &capabilities);
     if (SUCCEEDED(hr)) {
-      VideoCaptureCapabilityWin found_capability =
-          capabilities.GetBestMatchedFormat(
-              params.requested_format.frame_size.width(),
-              params.requested_format.frame_size.height(),
-              params.requested_format.frame_rate);
-
+      const CapabilityWin found_capability =
+          GetBestMatchedCapability(params.requested_format, capabilities);
       ScopedComPtr<IMFMediaType> type;
       hr = reader_->GetNativeMediaType(
           kFirstVideoStream, found_capability.stream_index, type.Receive());
diff --git a/media/video/capture/win/video_capture_device_win.cc b/media/video/capture/win/video_capture_device_win.cc
index 7dfd834..d1ee6fc 100644
--- a/media/video/capture/win/video_capture_device_win.cc
+++ b/media/video/capture/win/video_capture_device_win.cc
@@ -318,18 +318,15 @@ void VideoCaptureDeviceWin::AllocateAndStart(
 
   client_ = client.Pass();
 
-  // Get the camera capability that best match the requested resolution.
-  const VideoCaptureCapabilityWin& found_capability =
-      capabilities_.GetBestMatchedFormat(
-          params.requested_format.frame_size.width(),
-          params.requested_format.frame_size.height(),
-          params.requested_format.frame_rate);
+  // Get the camera capability that best match the requested format.
+  const CapabilityWin found_capability =
+      GetBestMatchedCapability(params.requested_format, capabilities_);
   VideoCaptureFormat format = found_capability.supported_format;
 
   // Reduce the frame rate if the requested frame rate is lower
   // than the capability.
-  if (format.frame_rate > params.requested_format.frame_rate)
-    format.frame_rate = params.requested_format.frame_rate;
+  format.frame_rate =
+      std::min(format.frame_rate, params.requested_format.frame_rate);
 
   ScopedComPtr<IAMStreamConfig> stream_config;
   HRESULT hr = output_capture_pin_.QueryInterface(stream_config.Receive());
@@ -501,9 +498,10 @@ bool VideoCaptureDeviceWin::CreateCapabilityMap() {
   }
 
   scoped_ptr<BYTE[]> caps(new BYTE[size]);
-  for (int i = 0; i < count; ++i) {
+  for (int stream_index = 0; stream_index < count; ++stream_index) {
     ScopedMediaType media_type;
-    hr = stream_config->GetStreamCaps(i, media_type.Receive(), caps.get());
+    hr = stream_config->GetStreamCaps(
+        stream_index, 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) {
@@ -514,53 +512,42 @@ bool VideoCaptureDeviceWin::CreateCapabilityMap() {
 
     if (media_type->majortype == MEDIATYPE_Video &&
         media_type->formattype == FORMAT_VideoInfo) {
-      VideoCaptureCapabilityWin capability(i);
-      capability.supported_format.pixel_format =
+      VideoCaptureFormat format;
+      format.pixel_format =
           TranslateMediaSubtypeToPixelFormat(media_type->subtype);
-      if (capability.supported_format.pixel_format == PIXEL_FORMAT_UNKNOWN)
+      if (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);
+      format.frame_size.SetSize(h->bmiHeader.biWidth, h->bmiHeader.biHeight);
 
-      // Try to get a better |time_per_frame| from IAMVideoControl.  If not, use
+      // 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,
-                                             &list_size, &max_fps);
-        // Sometimes |list_size| will be > 0, but max_fps will be NULL.  Some
+        const SIZE size = {format.frame_size.width(),
+                           format.frame_size.height()};
+        hr = video_control->GetFrameRateList(
+            output_capture_pin_, stream_index, size, &list_size, &max_fps);
+        // Can't assume the first value will return the 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.
+        // 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 =
+      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_.emplace_back(stream_index, format);
     }
   }