Added AVFoundation Glue and Device Monitoring for Mac.

content/browser/device_monitor_mac.mm

Index: content/browser/device_monitor_mac.mm
diff --git a/content/browser/device_monitor_mac.mm b/content/browser/device_monitor_mac.mm
index 7b12e91cbdf4ed8b57fc020d3b75c0579240d0ef..ba24e5b49d1e585495644f6ae44aeee136869aa8 100644
--- a/content/browser/device_monitor_mac.mm
+++ b/content/browser/device_monitor_mac.mm
@@ -7,39 +7,61 @@
 #import <QTKit/QTKit.h>
 
 #include "base/logging.h"
+#include "base/synchronization/lock.h"
+#include "media/video/capture/mac/avfoundation_glue.h"
 
-namespace content {
+namespace {
 
-class DeviceMonitorMac::QTMonitorImpl {
+// Interface used by DeviceMonitorMac to interact with either a QTKit or an
+// AVFoundation implementation of events and notifications.
+class MacMonitorInterface {
  public:
-  explicit QTMonitorImpl(DeviceMonitorMac* monitor);
-  virtual ~QTMonitorImpl() {}
+  virtual ~MacMonitorInterface() {};
 
-  void Start();
-  void Stop();
+  virtual void OnDeviceChanged() = 0;
+};
 
- private:
-  void OnDeviceChanged();
+// This class is used to keep track of system devices names and their types.
+class DeviceInfo{

[Mark Mentovai, 2013/10/09 16:05:12]

Fix the formatting of this class to match the style guide. Space before { on
this line and line 27, space before public on the next line, name the enum
values properly, name variables (including arguments and member values)
properly, no semicolon after the {} on line 33, etc.

[mcasas, 2013/10/12 09:59:27]

Done.

+public:
+  enum DeviceType{
+    audio,
+    video
+  };
 
-  DeviceMonitorMac* monitor_;
-  int number_audio_devices_;
-  int number_video_devices_;
+  DeviceInfo(std::string the_uniqueID, DeviceType the_type):
+      uniqueID(the_uniqueID), type(the_type) {};
+
+  bool operator==(const DeviceInfo& device) const {
+    return uniqueID == device.uniqueID;
+  }
+
+  std::string uniqueID;
+  DeviceType type;

[Mark Mentovai, 2013/10/09 16:05:12]

Maybe this can be a bit mask, so that you can stuff a device that does both
audio and video into the list only once?

[mcasas, 2013/10/12 09:59:27]

Instead of that I added a new category "kMuxed" and used it, which cleaned the
implementation of OnDeviceChanged() quite a bit. IMHO bit mask would complicate
the situation, having to define const ints, etc.

+  // Allow generated copy constructor and assignment.
+};
+
+class QTKitMonitorImpl : public MacMonitorInterface {
+ public:
+  QTKitMonitorImpl() {};
+  explicit QTKitMonitorImpl(content::DeviceMonitorMac* monitor);
+  virtual ~QTKitMonitorImpl();
+
+  virtual void OnDeviceChanged() OVERRIDE;
+ private:
+  content::DeviceMonitorMac* monitor_;
+  std::vector<DeviceInfo> cached_devices;

[Mark Mentovai, 2013/10/09 16:05:12]

Naming: members have trailing underscores.

[mcasas, 2013/10/12 09:59:27]

Done.

   id device_arrival_;
   id device_removal_;
-
-  DISALLOW_COPY_AND_ASSIGN(QTMonitorImpl);
 };
 
-DeviceMonitorMac::QTMonitorImpl::QTMonitorImpl(DeviceMonitorMac* monitor)
+QTKitMonitorImpl::QTKitMonitorImpl(content::DeviceMonitorMac* monitor)
     : monitor_(monitor),
-      number_audio_devices_(0),
-      number_video_devices_(0),
+      cached_devices(),
       device_arrival_(nil),
       device_removal_(nil) {
   DCHECK(monitor);
-}
 
-void DeviceMonitorMac::QTMonitorImpl::Start() {
   NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
   device_arrival_ =
     [nc addObserverForName:QTCaptureDeviceWasConnectedNotification
@@ -56,7 +78,7 @@ void DeviceMonitorMac::QTMonitorImpl::Start() {
                       OnDeviceChanged();}];
 }
 
-void DeviceMonitorMac::QTMonitorImpl::Stop() {
+QTKitMonitorImpl::~QTKitMonitorImpl() {
   if (!monitor_)
     return;
 
@@ -65,39 +87,184 @@ void DeviceMonitorMac::QTMonitorImpl::Stop() {
   [nc removeObserver:device_removal_];
 }
 
-void DeviceMonitorMac::QTMonitorImpl::OnDeviceChanged() {
+void QTKitMonitorImpl::OnDeviceChanged() {
+  base::AutoLock lock(monitor_->lock_);
+  int video_device_added = 0;
+  int audio_device_added = 0;
+  int video_device_removed = 0;
+  int audio_device_removed = 0;
+
+  std::vector<DeviceInfo> snapshot_devices;

[Mark Mentovai, 2013/10/09 16:05:12]

Maybe snapshot_devices and cached_devices would be better as a set? Their IDs
are supposed to be unique. You could DCHECK that they in fact are unique as you
build snapshot_devices.

[mcasas, 2013/10/12 09:59:27]

I think that would make sense if we would like to order |snapshot_devices| and
|cached_devices| based on their unique_id; AVFoundation always list devices in
its own internal order which is constant but not unique_id-wise increasing.
Practically, |snapshot_devices| and |cached_devices| have what would appear to
be random, but the _same_ random, hence comparisons (find()) between them are OK
without the need to re-order. Does this make sense?

[Robert Sesek, 2013/10/14 17:26:44]

Side node: unless the order is explicitly guaranteed as part of the AVFoundation
docs, this is liable to change in future OS releases and the order is not
guaranteed to be stable. You're obviously free to sort them if you like.

+
   NSArray* devices = [QTCaptureDevice inputDevices];
-  int number_video_devices = 0;
-  int number_audio_devices = 0;
   for (QTCaptureDevice* device in devices) {
     if ([device hasMediaType:QTMediaTypeVideo] ||
-        [device hasMediaType:QTMediaTypeMuxed])
-      ++number_video_devices;
-
+        [device hasMediaType:QTMediaTypeMuxed]) {
+      snapshot_devices.push_back(DeviceInfo(
+          [[device uniqueID] UTF8String], DeviceInfo::video));
+    }
     if ([device hasMediaType:QTMediaTypeSound] ||
-        [device hasMediaType:QTMediaTypeMuxed])
-      ++number_audio_devices;
+        [device hasMediaType:QTMediaTypeMuxed]) {
+      snapshot_devices.push_back(DeviceInfo(
+          [[device uniqueID] UTF8String], DeviceInfo::audio));
+    }
   }
 
-  if (number_video_devices_ != number_video_devices) {
-    number_video_devices_ = number_video_devices;
-    monitor_->NotifyDeviceChanged(base::SystemMonitor::DEVTYPE_VIDEO_CAPTURE);
+  std::vector<DeviceInfo>::iterator it;
+  for (it = snapshot_devices.begin(); it != snapshot_devices.end(); ++it) {
+    if (std::find(cached_devices.begin(), cached_devices.end(), *it)
+        == cached_devices.end()) {
+      video_device_added += (it->type == DeviceInfo::video);
+      audio_device_added += (it->type == DeviceInfo::audio);
+      DVLOG(1) << "Device has been added id: " << it->uniqueID;
+    }

[Mark Mentovai, 2013/10/09 16:05:12]

If cached_devices isn’t going to be used on any other threads while this
operation is occurring, you can remove entries from cached_devices as you walk
through snapshot_devices to handle device adds. Then, when you’re done with
that, anything remaining in cached_devices is guaranteed to not be in
snapshot_devices, and you can just walk cached_devices to pick up the removals
without having to do a find on snapshot_devices.

If cached_devices might still be used on other threads, you’ll probably need to
bring the lock back anyway.

[mcasas, 2013/10/12 09:59:27]

Done.
On second thoughts, the lock was brought due to my -incorrect- understanding
that two or more notifications could be served in parallel, hence having a race
condition in this OnDeviceChanged() method. Otherwise the device monitor is a
singleton so there should be no problem. I preventively removed the lock but
accept criticism ;)

   }
 
-  if (number_audio_devices_ != number_audio_devices) {
-    number_audio_devices_ = number_audio_devices;
+  for (it = cached_devices.begin(); it != cached_devices.end(); ++it) {
+    if (std::find(snapshot_devices.begin(), snapshot_devices.end(), *it)
+        == snapshot_devices.end()) {
+      video_device_removed += (it->type == DeviceInfo::video);
+      audio_device_removed += (it->type == DeviceInfo::audio);
+      DVLOG(1) << "Device has been removed id: " << it->uniqueID;
+    }
+  }
+
+  cached_devices.swap(snapshot_devices);
+
+  if (video_device_added || video_device_removed)
+    monitor_->NotifyDeviceChanged(base::SystemMonitor::DEVTYPE_VIDEO_CAPTURE);
+  if (audio_device_added || video_device_removed)
     monitor_->NotifyDeviceChanged(base::SystemMonitor::DEVTYPE_AUDIO_CAPTURE);
+}
+
+class AVFoundationMonitorImpl : public MacMonitorInterface {
+ public:
+  AVFoundationMonitorImpl() {};
+  explicit AVFoundationMonitorImpl(content::DeviceMonitorMac* monitor);
+  virtual ~AVFoundationMonitorImpl();
+
+  virtual void OnDeviceChanged() OVERRIDE;
+ private:
+  content::DeviceMonitorMac* monitor_;
+  std::vector<DeviceInfo> cached_devices;

[Mark Mentovai, 2013/10/09 16:05:12]

Similar comments apply in here.

[mcasas, 2013/10/12 09:59:27]

Done.

+  id device_arrival_;
+  id device_removal_;
+};
+
+AVFoundationMonitorImpl::AVFoundationMonitorImpl(
+    content::DeviceMonitorMac* monitor)
+    : monitor_(monitor),
+      cached_devices(),
+      device_arrival_(nil),
+      device_removal_(nil) {
+  DCHECK(monitor);
+
+  NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
+  NSOperationQueue *mainQueue = [NSOperationQueue mainQueue];
+
+  device_arrival_ =
+      [nc addObserverForName:[AVFoundationGlue
+      avCaptureDeviceWasConnectedNotification]
+                      object:nil
+                       queue:mainQueue
+                  usingBlock:^(NSNotification* notification) {
+                      OnDeviceChanged();}];
+  DCHECK(device_arrival_);
+   device_removal_ =
+      [nc addObserverForName:[AVFoundationGlue
+      avCaptureDeviceWasDisconnectedNotification]
+                      object:nil
+                       queue:mainQueue
+                  usingBlock:^(NSNotification* notification) {
+                      OnDeviceChanged();}];
+  DCHECK(device_removal_);
+}
+
+AVFoundationMonitorImpl::~AVFoundationMonitorImpl() {
+  if (!monitor_)
+    return;
+
+  NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
+  [nc removeObserver:device_arrival_];
+  [nc removeObserver:device_removal_];
+}
+
+void AVFoundationMonitorImpl::OnDeviceChanged() {
+  base::AutoLock lock(monitor_->lock_);
+  int video_device_added = 0;
+  int audio_device_added = 0;
+  int video_device_removed = 0;
+  int audio_device_removed = 0;
+
+  std::vector<DeviceInfo> snapshot_devices;
+  NSArray* devices = [AVCaptureDeviceGlue devices];
+  for (CrAVCaptureDevice* device in devices) {
+    if ([AVCaptureDeviceGlue hasMediaType:[AVFoundationGlue avMediaTypeVideo]
+                         forCaptureDevice:device] ||
+        [AVCaptureDeviceGlue hasMediaType:[AVFoundationGlue avMediaTypeMuxed]
+                         forCaptureDevice:device]) {
+      snapshot_devices.push_back(DeviceInfo([[AVCaptureDeviceGlue
+          uniqueID:device] UTF8String], DeviceInfo::video));
+    }
+    if ([AVCaptureDeviceGlue hasMediaType:[AVFoundationGlue avMediaTypeAudio]
+                         forCaptureDevice:device] ||
+        [AVCaptureDeviceGlue hasMediaType:[AVFoundationGlue avMediaTypeMuxed]
+                         forCaptureDevice:device]) {
+      snapshot_devices.push_back(DeviceInfo([[AVCaptureDeviceGlue
+          uniqueID:device] UTF8String], DeviceInfo::audio));
+    }
+  }
+
+  // Compare the current system devices snapshot with the ones cached to detect

[Mark Mentovai, 2013/10/09 16:05:12]

Looks like all, or almost all, of the rest of this function is common to both
the QTKit and AVFoundation implementations. You should refactor it so that they
share code.

[mcasas, 2013/10/12 09:59:27]

Done. Beefing the MacMonitorInterface actually made it no more and interface so
I put some more common parts (variables) in it.

+  // additions, present in the former but not in the latter.
+  std::vector<DeviceInfo>::iterator it;
+  for (it = snapshot_devices.begin(); it != snapshot_devices.end(); ++it) {
+    if( std::find(cached_devices.begin(), cached_devices.end(), *it)

[Mark Mentovai, 2013/10/09 16:05:12]

Spacing is interesting here and on line 233.

[mcasas, 2013/10/12 09:59:27]

Done.

+        == cached_devices.end()) {
+      video_device_added += (it->type == DeviceInfo::video);
+      audio_device_added += (it->type == DeviceInfo::audio);
+      DVLOG(1) << "Device has been added id: " << it->uniqueID;
+    }
+  }
+
+  // Compare the cached devices snapshot with the current ones to detect missing
+  // devices. These will be present in the former but not in the latter.
+  for (it = cached_devices.begin(); it != cached_devices.end(); ++it) {
+    if( std::find(snapshot_devices.begin(), snapshot_devices.end(), *it)
+        == snapshot_devices.end()) {
+      video_device_removed += (it->type == DeviceInfo::video);
+      audio_device_removed += (it->type == DeviceInfo::audio);
+      DVLOG(1) << "Device has been removed id: " << it->uniqueID;
+    }
   }
+  // Update the cached devices with the current system snapshot.
+  cached_devices.swap(snapshot_devices);
+
+  if (video_device_added || video_device_removed)
+    monitor_->NotifyDeviceChanged(base::SystemMonitor::DEVTYPE_VIDEO_CAPTURE);
+  if (audio_device_added || video_device_removed)
+    monitor_->NotifyDeviceChanged(base::SystemMonitor::DEVTYPE_AUDIO_CAPTURE);
 }
 
+}  // namespace
+
+namespace content {
+
 DeviceMonitorMac::DeviceMonitorMac() {
-  qt_monitor_.reset(new QTMonitorImpl(this));
-  qt_monitor_->Start();
+  if ([AVFoundationGlue isAVFoundationSupported]){
+    DVLOG(1) << "Monitoring via AVFoundation";
+    device_monitor_impl_.reset(new AVFoundationMonitorImpl(this));
+    // Force the device enumeration so we enumerate correctly devices already in
+    // the system and at the same time use the AVCaptureDeviceGlue so it in
+    // turn forces the AVCaptureDeviceGlue alloc-init.
+    device_monitor_impl_->OnDeviceChanged();
+  } else {
+    DVLOG(1) << "Monitoring via QTKit";
+    device_monitor_impl_.reset(new QTKitMonitorImpl(this));
+  }
 }
 
-DeviceMonitorMac::~DeviceMonitorMac() {
-  qt_monitor_->Stop();
-}
+DeviceMonitorMac::~DeviceMonitorMac() {}
 
 void DeviceMonitorMac::NotifyDeviceChanged(
     base::SystemMonitor::DeviceType type) {