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..cdb2a9b41d273f6f42377aa0af1358c9708ff881 100644
--- a/content/browser/device_monitor_mac.mm
+++ b/content/browser/device_monitor_mac.mm
@@ -7,39 +7,42 @@
 #import <QTKit/QTKit.h>
 
 #include "base/logging.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();
+class QTKitMonitorImpl : public MacMonitorInterface {
+ public:
+  QTKitMonitorImpl() {};
+  explicit QTKitMonitorImpl(content::DeviceMonitorMac* monitor);
+  virtual ~QTKitMonitorImpl();
 
-  DeviceMonitorMac* monitor_;
+  virtual void OnDeviceChanged() OVERRIDE;
+ private:
+  content::DeviceMonitorMac* monitor_;
   int number_audio_devices_;
   int number_video_devices_;
   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),
-      device_arrival_(nil),
-      device_removal_(nil) {
+      device_arrival_(0),

[Mark Mentovai, 2013/10/01 18:53:23]

If these are id again now, the initializers should be nil, not 0.

id = generic Objective-C pointer, the “zero” value literal is “nil.”
Other pointers (Type*) = C/C++ pointer, the “zero” value literal is “NULL.”
Non-pointer integral types, the “zero” value literal is 0 (or possibly 0.0,
0.0f, 0U, etc.)

[mcasas, 2013/10/02 14:10:39]

Done.

+      device_removal_(0) {
   DCHECK(monitor);
-}
 
-void DeviceMonitorMac::QTMonitorImpl::Start() {
   NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
   device_arrival_ =
     [nc addObserverForName:QTCaptureDeviceWasConnectedNotification
@@ -56,16 +59,16 @@ void DeviceMonitorMac::QTMonitorImpl::Start() {
                       OnDeviceChanged();}];
 }
 
-void DeviceMonitorMac::QTMonitorImpl::Stop() {
+QTKitMonitorImpl::~QTKitMonitorImpl() {
   if (!monitor_)
     return;
 
   NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
-  [nc removeObserver:device_arrival_];
-  [nc removeObserver:device_removal_];
+  [nc removeObserver:(id)device_arrival_];

[Mark Mentovai, 2013/10/01 18:53:23]

Shouldn’t need these casts anymore since the type is actually id.

[mcasas, 2013/10/02 14:10:39]

Done.

+  [nc removeObserver:(id)device_removal_];
 }
 
-void DeviceMonitorMac::QTMonitorImpl::OnDeviceChanged() {
+void QTKitMonitorImpl::OnDeviceChanged() {
   NSArray* devices = [QTCaptureDevice inputDevices];
   int number_video_devices = 0;
   int number_audio_devices = 0;
@@ -77,6 +80,7 @@ void DeviceMonitorMac::QTMonitorImpl::OnDeviceChanged() {
     if ([device hasMediaType:QTMediaTypeSound] ||
         [device hasMediaType:QTMediaTypeMuxed])
       ++number_audio_devices;
+    DVLOG(1) << "I found "<< number_video_devices << "video devices";
   }
 
   if (number_video_devices_ != number_video_devices) {
@@ -90,15 +94,117 @@ void DeviceMonitorMac::QTMonitorImpl::OnDeviceChanged() {
   }
 }
 
-DeviceMonitorMac::DeviceMonitorMac() {
-  qt_monitor_.reset(new QTMonitorImpl(this));
-  qt_monitor_->Start();
+class AVFoundationMonitorImpl : public MacMonitorInterface {
+ public:
+  AVFoundationMonitorImpl() {};
+  explicit AVFoundationMonitorImpl(content::DeviceMonitorMac* monitor);
+  virtual ~AVFoundationMonitorImpl();
+
+  virtual void OnDeviceChanged() OVERRIDE;
+ private:
+  content::DeviceMonitorMac* monitor_;
+  int number_audio_devices_;
+  int number_video_devices_;
+  id device_arrival_;
+  id device_removal_;
+};
+
+AVFoundationMonitorImpl::AVFoundationMonitorImpl(
+    content::DeviceMonitorMac* monitor)
+    : monitor_(monitor),
+      number_audio_devices_(0),
+      number_video_devices_(0),
+      device_arrival_(0),
+      device_removal_(0) {
+  DCHECK(monitor);
+
+  NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
+
+  // TEMPORARILY we subscribe to ALL events in the system. It should be done
+  // like further below, cherry picking the ones we want.
+  device_arrival_ =
+      [nc addObserverForName:nil
+                      object:nil
+                       queue:nil
+                  usingBlock:^(NSNotification* notification) {
+                      OnDeviceChanged();}];
+
+  //device_arrival_ =
+  //    [nc addObserverForName:[AVFoundationGlue
+  //    avCaptureDeviceWasConnectedNotification]
+  //                    object:nil
+  //                     queue:nil
+  //                usingBlock:^(NSNotification* notification) {
+  //                    OnDeviceChanged();}];
+
+  // device_removal_ =
+  //    [nc addObserverForName:[AVFoundationGlue
+  //    avCaptureDeviceWasDisconnectedNotification]
+  //                    object:nil
+  //                     queue:nil
+  //                usingBlock:^(NSNotification* notification) {
+  //                    OnDeviceChanged();}];
+
 }
 
-DeviceMonitorMac::~DeviceMonitorMac() {
-  qt_monitor_->Stop();
+AVFoundationMonitorImpl::~AVFoundationMonitorImpl() {
+  if (!monitor_)
+    return;
+
+  NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
+  [nc removeObserver:(id)device_arrival_];
+  [nc removeObserver:(id)device_removal_];
+}
+
+void AVFoundationMonitorImpl::OnDeviceChanged() {
+  NSArray* devices = [AVCaptureDeviceGlue devices];
+  int number_video_devices = 0;
+  int number_audio_devices = 0;
+  for (id device in devices) {
+    if ([AVCaptureDeviceGlue hasMediaType:[AVFoundationGlue avMediaTypeVideo]
+      forId:device] ||

[Mark Mentovai, 2013/10/01 18:53:23]

Objective-C style: line up the colons. Take a look at the Objective-C style
guide.

[mcasas, 2013/10/02 14:10:39]

Done.

+      [AVCaptureDeviceGlue hasMediaType:[AVFoundationGlue avMediaTypeMuxed]
+      forId:device])
+      ++number_video_devices;

[Mark Mentovai, 2013/10/01 18:53:23]

Since the condition took up so many lines, {braces} around the controlled code
are helpful or mandatory depending on your read of the guide. I read it as
mandatory.

[mcasas, 2013/10/02 14:10:39]

Done.

+
+    if ([AVCaptureDeviceGlue hasMediaType:[AVFoundationGlue avMediaTypeAudio]
+      forId:device] ||
+      [AVCaptureDeviceGlue hasMediaType:[AVFoundationGlue avMediaTypeMuxed]
+      forId:device])
+      ++number_audio_devices;
+  }
+
+  if (number_video_devices_ != number_video_devices) {
+    DVLOG(1) << "We have a video device plugged " <<
+      ((number_video_devices_ > number_video_devices)? "out" : "in");
+    number_video_devices_ = number_video_devices;

[Mark Mentovai, 2013/10/01 18:53:23]

The synchronization problem I raised earlier still exists here.

[mcasas, 2013/10/02 14:10:39]

Two things: I added a base::AutoLock to monitor_ so the execution of
OnDeviceChanged() will be as critical section, avoiding the race conditions
between "connected' and "disconnected events". Same for QtKit. 

Other thing is that, AFAIK, as long as every connection and disconnection ends
up in a call to ProcessDevicesChanged() (bottom of the file), it does not matter
if number_video_devices_ increases and decreases very fast.

Does this coincide with your understanding?

+    monitor_->NotifyDeviceChanged(base::SystemMonitor::DEVTYPE_VIDEO_CAPTURE);
+  }
+
+  if (number_audio_devices_ != number_audio_devices) {
+    DVLOG(1) << "We have an audio device plugged " <<
+      ((number_audio_devices_ > number_audio_devices)? "out" : "in");
+    number_audio_devices_ = number_audio_devices;
+    monitor_->NotifyDeviceChanged(base::SystemMonitor::DEVTYPE_AUDIO_CAPTURE);
+  }
 }
 
+} // anonymous namespace

[Mark Mentovai, 2013/10/01 18:53:23]

Style: two spaces between } and //.
In Chrome, it’s usual for namespace-ending comments to just be

}  // namespace

(with no more words if it’s an unnamed namespace)

[mcasas, 2013/10/02 14:10:39]

Done.

+
+namespace content {
+
+DeviceMonitorMac::DeviceMonitorMac() {
+  if ([AVFoundationGlue isAVFoundationSupported]){
+    DVLOG(1) << "Monitoring via AVFoundation";
+    device_monitor_impl_.reset(new AVFoundationMonitorImpl(this));
+  } else {
+    DVLOG(1) << "Monitoring via QTKit";
+    device_monitor_impl_.reset(new QTKitMonitorImpl(this));
+  }
+}
+
+DeviceMonitorMac::~DeviceMonitorMac() {}
+
 void DeviceMonitorMac::NotifyDeviceChanged(
     base::SystemMonitor::DeviceType type) {
   // TODO(xians): Remove the global variable for SystemMonitor.