VideoCaptureDeviceFactory: Change device enumeration to callback + QTKit enumerates in UI thread.

media/video/capture/mac/video_capture_device_factory_mac.mm

Index: media/video/capture/mac/video_capture_device_factory_mac.mm
diff --git a/media/video/capture/mac/video_capture_device_factory_mac.mm b/media/video/capture/mac/video_capture_device_factory_mac.mm
index 0100ba1a038d28643b2dd5967170ae05d3d4681b..d58a25c658687e18ee8261874b4a8076b50f4c1e 100644
--- a/media/video/capture/mac/video_capture_device_factory_mac.mm
+++ b/media/video/capture/mac/video_capture_device_factory_mac.mm
@@ -4,6 +4,9 @@
 
 #include "media/video/capture/mac/video_capture_device_factory_mac.h"
 
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/task_runner_util.h"
 #import "media/video/capture/mac/avfoundation_glue.h"
 #include "media/video/capture/mac/video_capture_device_mac.h"
 #import "media/video/capture/mac/video_capture_device_avfoundation_mac.h"
@@ -22,26 +25,64 @@ const struct NameAndVid {
 // In device identifiers, the USB VID and PID are stored in 4 bytes each.
 const size_t kVidPidSize = 4;
 
-VideoCaptureDeviceFactoryMac::VideoCaptureDeviceFactoryMac() {
+static scoped_ptr<media::VideoCaptureDevice::Names>
+EnumerateDevicesUsingQTKit() {
+  scoped_ptr<VideoCaptureDevice::Names> device_names(
+        new VideoCaptureDevice::Names());
+  NSMutableDictionary* capture_devices =
+      [[[NSMutableDictionary alloc] init] autorelease];
+  [VideoCaptureDeviceQTKit getDeviceNames:capture_devices];
+  for (NSString* key in capture_devices) {
+    VideoCaptureDevice::Name name(
+        [[capture_devices valueForKey:key] UTF8String],
+        [key UTF8String], VideoCaptureDevice::Name::QTKIT);
+    device_names->push_back(name);
+  }
+  return device_names.Pass();
+}
+
+static void RunDevicesEnumeratedCallback(
+    const base::Callback<void(scoped_ptr<media::VideoCaptureDevice::Names>)>&
+        callback,
+    scoped_ptr<media::VideoCaptureDevice::Names> device_names) {
+  callback.Run(device_names.Pass());
+}
+
+// static
+bool VideoCaptureDeviceFactoryMac::PlatformSupportsAVFoundation() {
+  return AVFoundationGlue::IsAVFoundationSupported();
+}
+
+VideoCaptureDeviceFactoryMac::VideoCaptureDeviceFactoryMac(
+    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner)
+    : ui_task_runner_(ui_task_runner) {
   thread_checker_.DetachFromThread();
 }
 
+VideoCaptureDeviceFactoryMac::~VideoCaptureDeviceFactoryMac() {}
+
 scoped_ptr<VideoCaptureDevice> VideoCaptureDeviceFactoryMac::Create(
-    scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
     const VideoCaptureDevice::Name& device_name) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_NE(device_name.capture_api_type(),
             VideoCaptureDevice::Name::API_TYPE_UNKNOWN);
 
-  VideoCaptureDevice::Names device_names;
-  GetDeviceNames(&device_names);
-  VideoCaptureDevice::Names::iterator it = device_names.begin();
-  for (; it != device_names.end(); ++it) {
-    if (it->id() == device_name.id())
-      break;
+  // Check device presence only for AVFoundation API, since it is too expensive
+  // and brittle for QTKit. The actual initialization at device level will fail
+  // subsequently if the device is not present.
+  if (AVFoundationGlue::IsAVFoundationSupported()) {
+    scoped_ptr<VideoCaptureDevice::Names> device_names(
+        new VideoCaptureDevice::Names());
+    GetDeviceNames(device_names.get());

[tommi (sloooow) - chröme, 2014/06/01 11:25:22]

Doesn't this also enumerate QTKit devices? (Blackmagic) 

[mcasas, 2014/06/02 13:19:04]

Yes, if present in the system.

+
+    VideoCaptureDevice::Names::iterator it = device_names->begin();
+    for (; it != device_names->end(); ++it) {
+      if (it->id() == device_name.id())
+        break;
+    }
+    if (it == device_names->end())
+      return scoped_ptr<VideoCaptureDevice>();

[tommi (sloooow) - chröme, 2014/06/01 11:25:22]

If this check indeed only looks at devices from avfoundation, then won't this
cause the bm devices to be blocked? 

[mcasas, 2014/06/02 13:19:04]

Just to be clear of the process, there are currently 2 enumerations
from the VCD family side: One is the common EnumerateDeviceNames,
that's cached in VCManager, and this secondary one on device Create().
This secondary check is in place to avoid trying to Create() a device
that has been disconnected from the system after enumeration;
this is no longer a possibility since the DeviceMonitorMac is listening
to connect/disconnect (triggering an enumeration), hence the time
span between enumeration and Create() is quite reduced; even if a
non-connected device is Create()d, the subsequent call to 
AllocateAndStart() would fail (they are done almost 
back-to-back [1] and [2]).

Since QTKit is very sensitive to enumerations, the fewer the better,
so this CL removes this second one.  AVFoundation double checking
is, however, left in place so that the functionality (and the 
unittest) stay the same, but I think we could remove this secondary
enumeration, which indeed might even trigger QTKit enumeration
of blacklisted devices.

Blacklisted devices have their own path inside AVFoundation 
GetDeviceNames, i.e. they don't use the normal QTKit path, so
they should still work fine.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

[tommi (sloooow) - chröme, 2014/06/03 08:52:11]

OK, thanks.

So just to recap and verify that my understanding is correct:

1.  In this scope we enumerate both AVFoundation devices *and* Blackmagic
devices (via qtkit).

2. This check (|if (it == device_names->end())|) will never be true for
Blackmagic devices.

[mcasas, 2014/06/03 13:56:46]

That's correct.
Correct, it should not verify as long as the Blackmagic device is still
connected in the system (from the moment it was first enumerated).

   }
-  if (it == device_names.end())
-    return scoped_ptr<VideoCaptureDevice>();
 
   scoped_ptr<VideoCaptureDeviceMac> capture_device(
       new VideoCaptureDeviceMac(device_name));
@@ -53,7 +94,7 @@ scoped_ptr<VideoCaptureDevice> VideoCaptureDeviceFactoryMac::Create(
 }
 
 void VideoCaptureDeviceFactoryMac::GetDeviceNames(
-    VideoCaptureDevice::Names* const device_names) {
+    VideoCaptureDevice::Names* device_names) {
   DCHECK(thread_checker_.CalledOnValidThread());
   // Loop through all available devices and add to |device_names|.
   NSDictionary* capture_devices;
@@ -99,14 +140,24 @@ void VideoCaptureDeviceFactoryMac::GetDeviceNames(
       }
     }
   } else {
+    // We should not enumerate QTKit devices in Device Thread;
+    NOTREACHED();
+  }
+}
+
+void VideoCaptureDeviceFactoryMac::EnumerateDeviceNames(const base::Callback<
+    void(scoped_ptr<media::VideoCaptureDevice::Names>)>& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (AVFoundationGlue::IsAVFoundationSupported()) {
+    scoped_ptr<VideoCaptureDevice::Names> device_names(
+        new VideoCaptureDevice::Names());
+    GetDeviceNames(device_names.get());
+    callback.Run(device_names.Pass());
+  } else {
     DVLOG(1) << "Enumerating video capture devices using QTKit";
-    capture_devices = [VideoCaptureDeviceQTKit deviceNames];
-    for (NSString* key in capture_devices) {
-      VideoCaptureDevice::Name name(
-          [[capture_devices valueForKey:key] UTF8String],
-          [key UTF8String], VideoCaptureDevice::Name::QTKIT);
-      device_names->push_back(name);
-    }
+    base::PostTaskAndReplyWithResult(ui_task_runner_, FROM_HERE,
+        base::Bind(&EnumerateDevicesUsingQTKit),
+        base::Bind(&RunDevicesEnumeratedCallback, callback));
   }
 }