Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(393)

Unified Diff: media/audio/mac/audio_manager_mac.cc

Issue 1901583005: Revert of Pass task runners to AudioManager constructor. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « media/audio/mac/audio_manager_mac.h ('k') | media/audio/mock_audio_manager.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/audio/mac/audio_manager_mac.cc
diff --git a/media/audio/mac/audio_manager_mac.cc b/media/audio/mac/audio_manager_mac.cc
index 0b29dcc13967e6c15caf57f412181f1cb3a8e581..20cd4c88a9c69bd5c6687acebe8d9350a6902168 100644
--- a/media/audio/mac/audio_manager_mac.cc
+++ b/media/audio/mac/audio_manager_mac.cc
@@ -293,6 +293,18 @@
return GetDefaultDevice(device, false);
}
+template <class T>
+void StopStreams(std::list<T*>* streams) {
+ for (typename std::list<T*>::iterator it = streams->begin();
+ it != streams->end();
+ ++it) {
+ // Stop() is safe to call multiple times, so it doesn't matter if a stream
+ // has already been stopped.
+ (*it)->Stop();
+ }
+ streams->clear();
+}
+
class AudioManagerMac::AudioPowerObserver : public base::PowerObserver {
public:
AudioPowerObserver()
@@ -358,27 +370,53 @@
DISALLOW_COPY_AND_ASSIGN(AudioPowerObserver);
};
-AudioManagerMac::AudioManagerMac(
- scoped_refptr<base::SingleThreadTaskRunner> task_runner,
- scoped_refptr<base::SingleThreadTaskRunner> worker_task_runner,
- AudioLogFactory* audio_log_factory)
- : AudioManagerBase(std::move(task_runner),
- std::move(worker_task_runner),
- audio_log_factory),
+AudioManagerMac::AudioManagerMac(AudioLogFactory* audio_log_factory)
+ : AudioManagerBase(audio_log_factory),
current_sample_rate_(0),
current_output_device_(kAudioDeviceUnknown) {
SetMaxOutputStreamsAllowed(kMaxOutputStreams);
+ // CoreAudio calls must occur on the main thread of the process, which in our
+ // case is sadly the browser UI thread. Failure to execute calls on the right
+ // thread leads to crashes and odd behavior. See http://crbug.com/158170.
+ // TODO(dalecurtis): We should require the message loop to be passed in.
+ task_runner_ = base::MessageLoopForUI::IsCurrent()
+ ? base::ThreadTaskRunnerHandle::Get()
+ : AudioManagerBase::GetTaskRunner();
+
// Task must be posted last to avoid races from handing out "this" to the
// audio thread. Always PostTask even if we're on the right thread since
// AudioManager creation is on the startup path and this may be slow.
- GetTaskRunner()->PostTask(
- FROM_HERE, base::Bind(&AudioManagerMac::InitializeOnAudioThread,
- base::Unretained(this)));
+ task_runner_->PostTask(FROM_HERE,
+ base::Bind(&AudioManagerMac::InitializeOnAudioThread,
+ base::Unretained(this)));
}
AudioManagerMac::~AudioManagerMac() {
+ if (task_runner_->BelongsToCurrentThread()) {
+ ShutdownOnAudioThread();
+ } else {
+ // It's safe to post a task here since Shutdown() will wait for all tasks to
+ // complete before returning.
+ task_runner_->PostTask(FROM_HERE,
+ base::Bind(&AudioManagerMac::ShutdownOnAudioThread,
+ base::Unretained(this)));
+ }
+
Shutdown();
+}
+
+scoped_refptr<base::SingleThreadTaskRunner> AudioManagerMac::GetTaskRunner() {
+ return task_runner_;
+}
+
+scoped_refptr<base::SingleThreadTaskRunner>
+AudioManagerMac::GetWorkerTaskRunner() {
+ if (!worker_thread_) {
+ worker_thread_.reset(new base::Thread("AudioWorkerThread"));
+ CHECK(worker_thread_->Start());
+ }
+ return worker_thread_->task_runner();
}
bool AudioManagerMac::HasAudioOutputDevices() {
@@ -518,7 +556,7 @@
std::string AudioManagerMac::GetAssociatedOutputDeviceID(
const std::string& input_device_id) {
- DCHECK(GetTaskRunner()->BelongsToCurrentThread());
+ DCHECK(task_runner_->BelongsToCurrentThread());
AudioDeviceID device = GetAudioDeviceIdByUId(true, input_device_id);
if (device == kAudioObjectUnknown)
return std::string();
@@ -643,7 +681,7 @@
}
std::string AudioManagerMac::GetDefaultOutputDeviceID() {
- DCHECK(GetTaskRunner()->BelongsToCurrentThread());
+ DCHECK(task_runner_->BelongsToCurrentThread());
AudioDeviceID device_id = kAudioObjectUnknown;
if (!GetDefaultOutputDevice(&device_id))
return std::string();
@@ -742,12 +780,31 @@
}
void AudioManagerMac::InitializeOnAudioThread() {
- DCHECK(GetTaskRunner()->BelongsToCurrentThread());
+ DCHECK(task_runner_->BelongsToCurrentThread());
power_observer_.reset(new AudioPowerObserver());
}
+void AudioManagerMac::ShutdownOnAudioThread() {
+ DCHECK(task_runner_->BelongsToCurrentThread());
+ output_device_listener_.reset();
+ power_observer_.reset();
+
+ // Since CoreAudio calls have to run on the UI thread and browser shutdown
+ // doesn't wait for outstanding tasks to complete, we may have input/output
+ // streams still running at shutdown.
+ //
+ // To avoid calls into destructed classes, we need to stop the OS callbacks
+ // by stopping the streams. Note: The streams are leaked since process
+ // destruction is imminent.
+ //
+ // See http://crbug.com/354139 for crash details.
+ StopStreams(&basic_input_streams_);
+ StopStreams(&low_latency_input_streams_);
+ StopStreams(&output_streams_);
+}
+
void AudioManagerMac::HandleDeviceChanges() {
- DCHECK(GetTaskRunner()->BelongsToCurrentThread());
+ DCHECK(task_runner_->BelongsToCurrentThread());
const int new_sample_rate = HardwareSampleRate();
AudioDeviceID new_output_device;
GetDefaultOutputDevice(&new_output_device);
@@ -789,22 +846,22 @@
}
bool AudioManagerMac::IsSuspending() const {
- DCHECK(GetTaskRunner()->BelongsToCurrentThread());
+ DCHECK(task_runner_->BelongsToCurrentThread());
return power_observer_->IsSuspending();
}
bool AudioManagerMac::ShouldDeferStreamStart() const {
- DCHECK(GetTaskRunner()->BelongsToCurrentThread());
+ DCHECK(task_runner_->BelongsToCurrentThread());
return power_observer_->ShouldDeferStreamStart();
}
bool AudioManagerMac::IsOnBatteryPower() const {
- DCHECK(GetTaskRunner()->BelongsToCurrentThread());
+ DCHECK(task_runner_->BelongsToCurrentThread());
return power_observer_->IsOnBatteryPower();
}
size_t AudioManagerMac::GetNumberOfResumeNotifications() const {
- DCHECK(GetTaskRunner()->BelongsToCurrentThread());
+ DCHECK(task_runner_->BelongsToCurrentThread());
return power_observer_->num_resume_notifications();
}
@@ -814,7 +871,7 @@
size_t desired_buffer_size,
bool* size_was_changed,
size_t* io_buffer_frame_size) {
- DCHECK(GetTaskRunner()->BelongsToCurrentThread());
+ DCHECK(task_runner_->BelongsToCurrentThread());
const bool is_input = (element == 1);
DVLOG(1) << "MaybeChangeBufferSize(id=0x" << std::hex << device_id
<< ", is_input=" << is_input << ", desired_buffer_size=" << std::dec
@@ -934,13 +991,8 @@
AudioManagerBase::ReleaseInputStream(stream);
}
-ScopedAudioManagerPtr CreateAudioManager(
- scoped_refptr<base::SingleThreadTaskRunner> task_runner,
- scoped_refptr<base::SingleThreadTaskRunner> worker_task_runner,
- AudioLogFactory* audio_log_factory) {
- return ScopedAudioManagerPtr(
- new AudioManagerMac(std::move(task_runner), std::move(worker_task_runner),
- audio_log_factory));
+AudioManager* CreateAudioManager(AudioLogFactory* audio_log_factory) {
+ return new AudioManagerMac(audio_log_factory);
}
} // namespace media
« no previous file with comments | « media/audio/mac/audio_manager_mac.h ('k') | media/audio/mock_audio_manager.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698