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

media/video/capture/mac/video_capture_device_factory_mac.mm

 // Copyright 2014 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/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"
 #import "media/video/capture/mac/video_capture_device_qtkit_mac.h"
 
 namespace media {
 
 // Some devices are not correctly supported in AVFoundation, f.i. Blackmagic,
 // see http://crbug.com/347371. The devices are identified by USB Vendor ID and
 // by a characteristic substring of the name, usually the vendor's name.
 const struct NameAndVid {
   const char* vid;
   const char* name;
 } kBlacklistedCameras[] = { { "a82c", "Blackmagic" } };
 
 // 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));
   if (!capture_device->Init(device_name.capture_api_type())) {
     LOG(ERROR) << "Could not initialize VideoCaptureDevice.";
     capture_device.reset();
   }
   return scoped_ptr<VideoCaptureDevice>(capture_device.Pass());
 }
 
 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;
   if (AVFoundationGlue::IsAVFoundationSupported()) {
     bool is_any_device_blacklisted = false;
     DVLOG(1) << "Enumerating video capture devices using AVFoundation";
     capture_devices = [VideoCaptureDeviceAVFoundation deviceNames];
     std::string device_vid;
     // Enumerate all devices found by AVFoundation, translate the info for each
     // to class Name and add it to |device_names|.
@@ skipping 25 matching lines @@
             DVLOG(1) << "Enumerated blacklisted " << [device_name UTF8String];
             VideoCaptureDevice::Name name(
                 "QTKit " + std::string([device_name UTF8String]),
                 [key UTF8String], VideoCaptureDevice::Name::QTKIT);
             device_names->push_back(name);
           }
         }
       }
     }
   } 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);
-    }
+    // 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";
+    base::PostTaskAndReplyWithResult(ui_task_runner_, FROM_HERE,
+        base::Bind(&EnumerateDevicesUsingQTKit),
+        base::Bind(&RunDevicesEnumeratedCallback, callback));
+  }
+}
+
 void VideoCaptureDeviceFactoryMac::GetDeviceSupportedFormats(
     const VideoCaptureDevice::Name& device,
     VideoCaptureFormats* supported_formats) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (device.capture_api_type() == VideoCaptureDevice::Name::AVFOUNDATION) {
     DVLOG(1) << "Enumerating video capture capabilities, AVFoundation";
     [VideoCaptureDeviceAVFoundation getDevice:device
                              supportedFormats:supported_formats];
   } else {
     NOTIMPLEMENTED();
   }
 }
 
 }  // namespace media