Win Video Capture: Add device blacklisting, used for IPCamera and YouCam, collect UMA for those

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 4f68c6edb3841a622246848f9dd42562e1e96d3c..43243423ba86762230a6eaf95cd6176a5ef59083 100644
--- a/media/video/capture/win/video_capture_device_factory_win.cc
+++ b/media/video/capture/win/video_capture_device_factory_win.cc
@@ -9,6 +9,7 @@
 
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
+#include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/win/metro.h"
@@ -27,6 +28,16 @@ using Names = media::VideoCaptureDevice::Names;
 
 namespace media {
 
+// We would like to avoid enumerating and/or using certain devices due to they
+// provoking crashes or any other reason. This enum is defined for the purposes
+// of UMA collection. Existing entries cannot be removed.
+enum BlacklistedCameraNames{

[Alexei Svitkine (slow), 2014/10/14 15:18:45]

Nit: Space before {.

[mcasas, 2014/10/15 07:25:09]

Done.

+  BLACKLISTED_CAMERA_GOOGLE_CAMERA_ADAPTER,
+  BLACKLISTED_CAMERA_IP_CAMERA,
+  BLACKLISTED_CAMERA_CYBERLINK_WEBCAM_SPLITTER,
+  BLACKLISTED_CAMERA_MAX  // This one must be last.
+};
+
 // Lazy Instance to initialize the MediaFoundation Library.
 class MFInitializerSingleton {
  public:
@@ -37,6 +48,16 @@ class MFInitializerSingleton {
 static base::LazyInstance<MFInitializerSingleton> g_mf_initialize =
     LAZY_INSTANCE_INITIALIZER;
 
+// Blacklisted devices are identified by a characteristic prefix of the name.
+// This prefix is used case-insensitively.

[Alexei Svitkine (slow), 2014/10/14 15:18:45]

Mention that this should be updated in sync with |BlacklistedCameraNames| and
vice versa in the comment on that one. Maybe put them next to each other.

[mcasas, 2014/10/15 07:25:09]

Done.
Don't think is needed, in any case there's the DCHECK in l.115.

+static const char* kBlacklistedCameraNames[] = {
+  // Name of a fake DirectShow filter on computers with GTalk installed.
+  {"Google Camera Adapter"},
+  // The following two software WebCams cause crashes.
+  {"IP Camera [JPEG/MJPEG]"},
+  {"CyberLink Webcam Splitter"}
+};
+
 static void EnsureMediaFoundationInit() {
   g_mf_initialize.Get();
 }
@@ -90,6 +111,20 @@ static bool EnumerateVideoDevicesMediaFoundation(IMFActivate*** devices,
   return SUCCEEDED(MFEnumDeviceSources(attributes, devices, count));
 }
 
+static bool IsDeviceBlackListed(const std::string& name) {
+  DCHECK_EQ(static_cast<int>(arraysize(kBlacklistedCameraNames)),
+            BLACKLISTED_CAMERA_MAX);

[Alexei Svitkine (slow), 2014/10/14 15:18:45]

Nit: Expected value should be the first arg, so swap them.

[mcasas, 2014/10/15 07:25:09]

Done.

+  for (size_t i = 0; i < arraysize(kBlacklistedCameraNames); ++i) {
+    if (StartsWithASCII(name, kBlacklistedCameraNames[i], false)) {
+      DVLOG(1) << "Enumerated blacklisted device: " << name;
+      UMA_HISTOGRAM_ENUMERATION("Media.VideoCapture.BlacklistedDevice",
+          i, BLACKLISTED_CAMERA_MAX + 1);

[Alexei Svitkine (slow), 2014/10/14 15:18:45]

Remove "+ 1" from the last param, since it's already 1 greater than the maximum
value of |i|.

[mcasas, 2014/10/15 07:25:09]

Presubmit.py complained:

** Presubmit ERRORS **
UMA_HISTOGRAM_ENUMERATION reports in src/media/ are expected to adhere
to the following guidelines:
 - The max value (3rd argument) should be an enum value equal to the
   last valid value, e.g. FOO_MAX = LAST_VALID_FOO.
 - 1 must be added to that max value.
Contact rileya@chromium.org if you have questions.

So I followed its instructions.

[Alexei Svitkine (slow), 2014/10/15 14:00:50]

Sigh, it's because media code expects the MAX value to be the last value, not +1
from the last value.

So if you follow the presubmit, you'd want BLACKLISTED_CAMERA_MAX =
BLACKLISTED_CAMERA_CYBERLINK_WEBCAM_SPLITTER

I guess it doesn't matter either way - just results in an extra unused bucket
being allocated, but no effect on data sent.

(FWIW, I'm not a fan of the media/ code having a different convention than the
rest of Chromium, but I guess it's hard to change at this point. :\)
  38   BLACKLISTED_CAMERA_MAX

+      return true;
+    }
+  }
+  return false;
+}
+
 static void GetDeviceNamesDirectShow(
     const CLSID& class_id,
     const Name::CaptureApiType capture_api_type,
@@ -128,21 +163,10 @@ static void GetDeviceNamesDirectShow(
     if (FAILED(hr) || name.type() != VT_BSTR)
       continue;
 
-    // Ignore all VFW drivers and the special Google Camera Adapter.
-    // Google Camera Adapter is not a real DirectShow camera device.
-    // VFW are very old Video for Windows drivers that can not be used.
-    const wchar_t* str_ptr = V_BSTR(&name);
-    // Name of a fake DirectShow filter that exist on computers with
-    // GTalk installed.
-    static const char kGoogleCameraAdapter[] = "google camera adapter";
-    if (wcsstr(str_ptr, L"(VFW)") != NULL ||
-        LowerCaseEqualsASCII(str_ptr,
-                             str_ptr + arraysize(kGoogleCameraAdapter) - 1,
-                             kGoogleCameraAdapter)) {
+    const std::string device_name(base::SysWideToUTF8(V_BSTR(&name)));
+    if (IsDeviceBlackListed(device_name))
       continue;
-    }
 
-    const std::string device_name(base::SysWideToUTF8(str_ptr));
     name.Reset();
     hr = prop_bag->Read(L"DevicePath", name.Receive(), 0);
     std::string id;