summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormcasas@chromium.org <mcasas@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-04-22 15:02:56 +0000
committermcasas@chromium.org <mcasas@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-04-22 15:02:56 +0000
commit74c3445f9f933495683b27a986ede3e8e4858e03 (patch)
tree5fa42046324b54428d15e61539bb620e631819a1
parent80b0c8c1adf1991387a038ec157f09b6c81caa45 (diff)
downloadchromium_src-74c3445f9f933495683b27a986ede3e8e4858e03.zip
chromium_src-74c3445f9f933495683b27a986ede3e8e4858e03.tar.gz
chromium_src-74c3445f9f933495683b27a986ede3e8e4858e03.tar.bz2
Mac AVFoundation: Enumerate devices in device/audio thread.
Bug http://crbug.com/359589 describes a Chrome hang where two threads are calling [AVCaptureDeviceGlue devices] at the same time to enumerate the devices. The first call comes from DeviceMonitorMac::StartMonitoring() on UI thread while the second call comes from the Audio Thread, a.k.a. Device Thread on +VideoCaptureDeviceAVFoundation::getDeviceNames:. DMM == DeviceMonitorMac The solution proposed here is to do as much as possible on Device thread : - when there is a device change via DMM::OnDeviceChanged(), enumerate on Device Thread, then consolidate the device list on UI thread. - First |suspend_observer_| registration happens on Device Thread. This suspend observer lives in device thread. - Observer removal happens dutifully on Device Thread destructor in -CrAVFoundationDeviceObserver::dealloc BUG=288562, 359589 Review URL: https://codereview.chromium.org/234563004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@265254 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat
-rw-r--r--content/browser/device_monitor_mac.h8
-rw-r--r--content/browser/device_monitor_mac.mm85
-rw-r--r--content/browser/renderer_host/media/media_stream_manager.cc9
3 files changed, 80 insertions, 22 deletions
diff --git a/content/browser/device_monitor_mac.h b/content/browser/device_monitor_mac.h
index 696b961..b3d3f9f 100644
--- a/content/browser/device_monitor_mac.h
+++ b/content/browser/device_monitor_mac.h
@@ -25,8 +25,10 @@ class DeviceMonitorMac {
// Registers the observers for the audio/video device removal, connection and
// suspension. The AVFoundation library is also loaded and initialised if the
- // OS supports it.
- void StartMonitoring();
+ // OS supports it. The |device_task_runner| argument represents the thread on
+ // which device enumeration will occur.
+ void StartMonitoring(
+ const scoped_refptr<base::SingleThreadTaskRunner>& device_task_runner);
// Method called by the internal DeviceMonitorMacImpl object
// |device_monitor_impl_| when a device of type |type| has been added to or
@@ -38,7 +40,7 @@ class DeviceMonitorMac {
scoped_ptr<DeviceMonitorMacImpl> device_monitor_impl_;
// |thread_checker_| is used to check that constructor and StartMonitoring()
- // are called in the correct thread, that also owns the object.
+ // are called in the correct thread, the UI thread, that also owns the object.
base::ThreadChecker thread_checker_;
DISALLOW_COPY_AND_ASSIGN(DeviceMonitorMac);
diff --git a/content/browser/device_monitor_mac.mm b/content/browser/device_monitor_mac.mm
index 10dd620..9433c59 100644
--- a/content/browser/device_monitor_mac.mm
+++ b/content/browser/device_monitor_mac.mm
@@ -8,6 +8,7 @@
#include <set>
+#include "base/bind_helpers.h"
#include "base/logging.h"
#include "base/mac/scoped_nsobject.h"
#import "media/video/capture/mac/avfoundation_glue.h"
@@ -211,8 +212,10 @@ class AVFoundationMonitorImpl;
} // namespace
-// This class is a Key-Value Observer (KVO) shim. It is needed because C++
-// classes cannot observe Key-Values directly.
+// This class is a Key-Value Observer (KVO) shim. It is needed because C++
+// classes cannot observe Key-Values directly. This class is used by
+// AVfoundationMonitorImpl and executed in its |device_task_runner_|, a.k.a.
+// "Device Thread". -stopObserving is called dutifully on -dealloc on UI thread.
@interface CrAVFoundationDeviceObserver : NSObject {
@private
AVFoundationMonitorImpl* receiver_;
@@ -228,21 +231,41 @@ class AVFoundationMonitorImpl;
namespace {
+// AVFoundation implementation of the Mac Device Monitor, registers as a global
+// device connect/disconnect observer and plugs suspend/wake up device observers
+// per device. This class is created and lives in UI thread; device enumeration
+// and operations involving |suspend_observer_| happen on |device_task_runner_|.
class AVFoundationMonitorImpl : public DeviceMonitorMacImpl {
public:
- explicit AVFoundationMonitorImpl(content::DeviceMonitorMac* monitor);
+ AVFoundationMonitorImpl(
+ content::DeviceMonitorMac* monitor,
+ const scoped_refptr<base::SingleThreadTaskRunner>& device_task_runner);
virtual ~AVFoundationMonitorImpl();
virtual void OnDeviceChanged() OVERRIDE;
private:
+ void OnDeviceChangedOnDeviceThread(
+ const scoped_refptr<base::MessageLoopProxy>& ui_thread);
+ void StartObserverOnDeviceThread();
+
+ base::ThreadChecker thread_checker_;
+
+ // {Video,AudioInput}DeviceManager's "Device" thread task runner used for
+ // device enumeration, valid after MediaStreamManager calls StartMonitoring().
+ const scoped_refptr<base::SingleThreadTaskRunner> device_task_runner_;
+
+ // Created and executed in |device_task_runnner_|.
base::scoped_nsobject<CrAVFoundationDeviceObserver> suspend_observer_;
+
DISALLOW_COPY_AND_ASSIGN(AVFoundationMonitorImpl);
};
AVFoundationMonitorImpl::AVFoundationMonitorImpl(
- content::DeviceMonitorMac* monitor)
- : DeviceMonitorMacImpl(monitor) {
+ content::DeviceMonitorMac* monitor,
+ const scoped_refptr<base::SingleThreadTaskRunner>& device_task_runner)
+ : DeviceMonitorMacImpl(monitor),
+ device_task_runner_(device_task_runner) {
NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
device_arrival_ =
[nc addObserverForName:AVFoundationGlue::
@@ -258,23 +281,31 @@ AVFoundationMonitorImpl::AVFoundationMonitorImpl(
queue:nil
usingBlock:^(NSNotification* notification) {
OnDeviceChanged();}];
- suspend_observer_.reset(
- [[CrAVFoundationDeviceObserver alloc] initWithChangeReceiver:this]);
- for (CrAVCaptureDevice* device in [AVCaptureDeviceGlue devices])
- [suspend_observer_ startObserving:device];
+ device_task_runner_->PostTask(FROM_HERE,
+ base::Bind(&AVFoundationMonitorImpl::StartObserverOnDeviceThread,
+ base::Unretained(this)));
}
AVFoundationMonitorImpl::~AVFoundationMonitorImpl() {
NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
[nc removeObserver:device_arrival_];
[nc removeObserver:device_removal_];
- for (CrAVCaptureDevice* device in [AVCaptureDeviceGlue devices])
- [suspend_observer_ stopObserving:device];
}
void AVFoundationMonitorImpl::OnDeviceChanged() {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ device_task_runner_->PostTask(FROM_HERE,
+ base::Bind(&AVFoundationMonitorImpl::OnDeviceChangedOnDeviceThread,
+ base::Unretained(this),
+ base::MessageLoop::current()->message_loop_proxy()));
+}
+
+void AVFoundationMonitorImpl::OnDeviceChangedOnDeviceThread(
+ const scoped_refptr<base::MessageLoopProxy>& ui_thread) {
+ DCHECK(device_task_runner_->BelongsToCurrentThread());
+ NSArray* devices = [AVCaptureDeviceGlue devices];
std::vector<DeviceInfo> snapshot_devices;
- for (CrAVCaptureDevice* device in [AVCaptureDeviceGlue devices]) {
+ for (CrAVCaptureDevice* device in devices) {
[suspend_observer_ startObserving:device];
BOOL suspended = [device respondsToSelector:@selector(isSuspended)] &&
[device isSuspended];
@@ -291,7 +322,18 @@ void AVFoundationMonitorImpl::OnDeviceChanged() {
snapshot_devices.push_back(DeviceInfo([[device uniqueID] UTF8String],
device_type));
}
- ConsolidateDevicesListAndNotify(snapshot_devices);
+ // Post the consolidation of enumerated devices to be done on UI thread.
+ ui_thread->PostTask(FROM_HERE,
+ base::Bind(&DeviceMonitorMacImpl::ConsolidateDevicesListAndNotify,
+ base::Unretained(this), snapshot_devices));
+}
+
+void AVFoundationMonitorImpl::StartObserverOnDeviceThread() {
+ DCHECK(device_task_runner_->BelongsToCurrentThread());
+ suspend_observer_.reset([[CrAVFoundationDeviceObserver alloc]
+ initWithChangeReceiver:this]);
+ for (CrAVCaptureDevice* device in [AVCaptureDeviceGlue devices])
+ [suspend_observer_ startObserving:device];
}
} // namespace
@@ -299,13 +341,21 @@ void AVFoundationMonitorImpl::OnDeviceChanged() {
@implementation CrAVFoundationDeviceObserver
- (id)initWithChangeReceiver:(AVFoundationMonitorImpl*)receiver {
- if ((self = [super init])) {
+ if (self = [super init]) {
DCHECK(receiver != NULL);
receiver_ = receiver;
}
return self;
}
+- (void)dealloc {
+ for (std::set<CrAVCaptureDevice*>::iterator it = monitoredDevices_.begin();
+ it != monitoredDevices_.end(); ++it) {
+ [self stopObserving:*it];
+ }
+ [super dealloc];
+}
+
- (void)startObserving:(CrAVCaptureDevice*)device {
DCHECK(device != nil);
// Skip this device if there are already observers connected to it.
@@ -359,11 +409,13 @@ DeviceMonitorMac::DeviceMonitorMac() {
DeviceMonitorMac::~DeviceMonitorMac() {}
-void DeviceMonitorMac::StartMonitoring() {
+void DeviceMonitorMac::StartMonitoring(
+ const scoped_refptr<base::SingleThreadTaskRunner>& device_task_runner) {
DCHECK(thread_checker_.CalledOnValidThread());
if (AVFoundationGlue::IsAVFoundationSupported()) {
DVLOG(1) << "Monitoring via AVFoundation";
- device_monitor_impl_.reset(new AVFoundationMonitorImpl(this));
+ device_monitor_impl_.reset(new AVFoundationMonitorImpl(this,
+ device_task_runner));
} else {
DVLOG(1) << "Monitoring via QTKit";
device_monitor_impl_.reset(new QTKitMonitorImpl(this));
@@ -372,6 +424,7 @@ void DeviceMonitorMac::StartMonitoring() {
void DeviceMonitorMac::NotifyDeviceChanged(
base::SystemMonitor::DeviceType type) {
+ DCHECK(thread_checker_.CalledOnValidThread());
// TODO(xians): Remove the global variable for SystemMonitor.
base::SystemMonitor::Get()->ProcessDevicesChanged(type);
}
diff --git a/content/browser/renderer_host/media/media_stream_manager.cc b/content/browser/renderer_host/media/media_stream_manager.cc
index 56bc43a..944ece5 100644
--- a/content/browser/renderer_host/media/media_stream_manager.cc
+++ b/content/browser/renderer_host/media/media_stream_manager.cc
@@ -47,7 +47,8 @@ namespace content {
// Forward declaration of DeviceMonitorMac and its only useable method.
class DeviceMonitorMac {
public:
- void StartMonitoring();
+ void StartMonitoring(
+ const scoped_refptr<base::SingleThreadTaskRunner>& device_task_runner);
};
namespace {
@@ -834,8 +835,10 @@ void MediaStreamManager::StartMonitoring() {
void MediaStreamManager::StartMonitoringOnUIThread() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
BrowserMainLoop* browser_main_loop = content::BrowserMainLoop::GetInstance();
- if (browser_main_loop)
- browser_main_loop->device_monitor_mac()->StartMonitoring();
+ if (browser_main_loop) {
+ browser_main_loop->device_monitor_mac()
+ ->StartMonitoring(audio_manager_->GetWorkerTaskRunner());
+ }
}
#endif
generated by cgit v1.1 at 2025-03-24 23:33:00 +0000