Added AVFoundation Glue and Device Monitoring for Mac.

content/browser/device_monitor_mac.mm

 // Copyright (c) 2012 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 "content/browser/device_monitor_mac.h"
 
 #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;
+};
 
+class QTKitMonitorImpl : public MacMonitorInterface {
+ public:
+  QTKitMonitorImpl() {};
+  explicit QTKitMonitorImpl(content::DeviceMonitorMac* monitor);
+  virtual ~QTKitMonitorImpl();
+
+  virtual void OnDeviceChanged() OVERRIDE;
  private:
-  void OnDeviceChanged();
-
-  DeviceMonitorMac* monitor_;
+  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
                     object:nil
                      queue:nil
                 usingBlock:^(NSNotification* notification) {
                     OnDeviceChanged();}];
 
   device_removal_ =
       [nc addObserverForName:QTCaptureDeviceWasDisconnectedNotification
                       object:nil
                        queue:nil
                   usingBlock:^(NSNotification* notification) {
                       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;
   for (QTCaptureDevice* device in devices) {
     if ([device hasMediaType:QTMediaTypeVideo] ||
         [device hasMediaType:QTMediaTypeMuxed])
       ++number_video_devices;
 
     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) {
     number_video_devices_ = number_video_devices;
     monitor_->NotifyDeviceChanged(base::SystemMonitor::DEVTYPE_VIDEO_CAPTURE);
   }
 
   if (number_audio_devices_ != number_audio_devices) {
     number_audio_devices_ = number_audio_devices;
     monitor_->NotifyDeviceChanged(base::SystemMonitor::DEVTYPE_AUDIO_CAPTURE);
   }
 }
 
-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.
   base::SystemMonitor::Get()->ProcessDevicesChanged(type);
 }
 
 }  // namespace content