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

Unified Diff: media/audio/audio_output_device.cc

Issue 1703473002: Make AudioOutputDevice restartable and reinitializable (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@new_mixing
Patch Set: Created 4 years, 10 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
Index: media/audio/audio_output_device.cc
diff --git a/media/audio/audio_output_device.cc b/media/audio/audio_output_device.cc
index 3af32b7835f8f1f8a23c98b0a0a16f7fa4deb31c..b227bc2220e5fd38a4073623312f780408bbbf9c 100644
--- a/media/audio/audio_output_device.cc
+++ b/media/audio/audio_output_device.cc
@@ -17,6 +17,8 @@
#include "media/audio/audio_output_controller.h"
#include "media/base/limits.h"
+#include "base/debug/stack_trace.h"
+
namespace media {
// Takes care of invoking the render callback on the audio thread.
@@ -77,16 +79,19 @@ AudioOutputDevice::AudioOutputDevice(
void AudioOutputDevice::Initialize(const AudioParameters& params,
RenderCallback* callback) {
- DCHECK(!callback_) << "Calling Initialize() twice?";
DCHECK(params.IsValid());
- audio_parameters_ = params;
- callback_ = callback;
+ task_runner()->PostTask(
+ FROM_HERE,
+ base::Bind(&AudioOutputDevice::InitializeOnIOThread,
+ this, params, callback));
}
AudioOutputDevice::~AudioOutputDevice() {
// The current design requires that the user calls Stop() before deleting
- // this class.
- DCHECK(audio_thread_.IsStopped());
+ // this class. Since this object is reference counted, we're the only one
+ // accessing data when we destruct.
+ DCHECK(state_ == IDLE || state_ == IPC_CLOSED);
+ audio_thread_.Stop(base::MessageLoop::current());
}
void AudioOutputDevice::RequestDeviceAuthorization() {
@@ -97,21 +102,19 @@ void AudioOutputDevice::RequestDeviceAuthorization() {
}
void AudioOutputDevice::Start() {
- DCHECK(callback_) << "Initialize hasn't been called";
task_runner()->PostTask(FROM_HERE,
- base::Bind(&AudioOutputDevice::CreateStreamOnIOThread, this,
- audio_parameters_));
+ base::Bind(&AudioOutputDevice::CreateStreamOnIOThread, this));
}
void AudioOutputDevice::Stop() {
{
base::AutoLock auto_lock(audio_thread_lock_);
- audio_thread_.Stop(base::MessageLoop::current());
+ audio_thread_.StopRun();
stopping_hack_ = true;
}
task_runner()->PostTask(FROM_HERE,
- base::Bind(&AudioOutputDevice::ShutDownOnIOThread, this));
+ base::Bind(&AudioOutputDevice::StopOnIOThread, this));
}
void AudioOutputDevice::Play() {
@@ -160,15 +163,36 @@ OutputDeviceStatus AudioOutputDevice::GetDeviceStatus() {
}
void AudioOutputDevice::RequestDeviceAuthorizationOnIOThread() {
+ printf("****** AudioOutputDevice::RequestDeviceAuthorizationOnIOThread this = %p, state_ = %d\n",
+ this, state_);
DCHECK(task_runner()->BelongsToCurrentThread());
DCHECK_EQ(state_, IDLE);
state_ = AUTHORIZING;
+
+ // TODO(grunell): Store authorization so that we don't have to authorize
+ // again when re-starting. This involves mostly changes on the browser side:
+ // associate authorization by a new id (currently the stream id), don't forget
+ // authorization away after start or stop. Here we would have to explicitly
+ // make the browser side forget authorization in the dtor with a new
+ // AudioOutputIPC function.
ipc_->RequestDeviceAuthorization(this, session_id_, device_id_,
security_origin_);
}
-void AudioOutputDevice::CreateStreamOnIOThread(const AudioParameters& params) {
+void AudioOutputDevice::InitializeOnIOThread(const AudioParameters& params,
+ RenderCallback* callback) {
DCHECK(task_runner()->BelongsToCurrentThread());
+ DCHECK(!callback_);
+ audio_parameters_ = params;
+ callback_ = callback;
+}
+
+void AudioOutputDevice::CreateStreamOnIOThread() {
+ printf("****** AudioOutputDevice::CreateStreamOnIOThread this = %p, state_ = %d\n",
+ this, state_);
+ DCHECK(task_runner()->BelongsToCurrentThread());
+ DCHECK(callback_ || state_ == IPC_CLOSED);
+
switch (state_) {
case IPC_CLOSED:
if (callback_)
@@ -179,7 +203,7 @@ void AudioOutputDevice::CreateStreamOnIOThread(const AudioParameters& params) {
if (did_receive_auth_.IsSignaled() && device_id_.empty() &&
security_origin_.unique()) {
state_ = CREATING_STREAM;
- ipc_->CreateStream(this, params);
+ ipc_->CreateStream(this, audio_parameters_);
} else {
RequestDeviceAuthorizationOnIOThread();
start_on_authorized_ = true;
@@ -192,7 +216,7 @@ void AudioOutputDevice::CreateStreamOnIOThread(const AudioParameters& params) {
case AUTHORIZED:
state_ = CREATING_STREAM;
- ipc_->CreateStream(this, params);
+ ipc_->CreateStream(this, audio_parameters_);
start_on_authorized_ = false;
break;
@@ -205,6 +229,8 @@ void AudioOutputDevice::CreateStreamOnIOThread(const AudioParameters& params) {
}
void AudioOutputDevice::PlayOnIOThread() {
+ printf("****** AudioOutputDevice::PlayOnIOThread this = %p, state_ = %d\n",
+ this, state_);
DCHECK(task_runner()->BelongsToCurrentThread());
if (state_ == PAUSED) {
TRACE_EVENT_ASYNC_BEGIN0(
@@ -218,6 +244,8 @@ void AudioOutputDevice::PlayOnIOThread() {
}
void AudioOutputDevice::PauseOnIOThread() {
+ printf("****** AudioOutputDevice::PauseOnIOThread this = %p, state_ = %d\n",
+ this, state_);
DCHECK(task_runner()->BelongsToCurrentThread());
if (state_ == PLAYING) {
TRACE_EVENT_ASYNC_END0(
@@ -228,7 +256,9 @@ void AudioOutputDevice::PauseOnIOThread() {
play_on_start_ = false;
}
-void AudioOutputDevice::ShutDownOnIOThread() {
+void AudioOutputDevice::StopOnIOThread() {
+ printf("****** AudioOutputDevice::StopOnIOThread this = %p, state_ = %d\n",
+ this, state_);
DCHECK(task_runner()->BelongsToCurrentThread());
// Close the stream, if we haven't already.
@@ -238,19 +268,15 @@ void AudioOutputDevice::ShutDownOnIOThread() {
}
start_on_authorized_ = false;
- // We can run into an issue where ShutDownOnIOThread is called right after
+ // We can run into an issue where StopOnIOThread is called right after
// OnStreamCreated is called in cases where Start/Stop are called before we
// get the OnStreamCreated callback. To handle that corner case, we call
// Stop(). In most cases, the thread will already be stopped.
- //
- // Another situation is when the IO thread goes away before Stop() is called
- // in which case, we cannot use the message loop to close the thread handle
- // and can't rely on the main thread existing either.
base::AutoLock auto_lock_(audio_thread_lock_);
- base::ThreadRestrictions::ScopedAllowIO allow_io;
- audio_thread_.Stop(NULL);
+ audio_thread_.StopRun();
audio_callback_.reset();
stopping_hack_ = false;
+ callback_ = nullptr;
}
void AudioOutputDevice::SetVolumeOnIOThread(double volume) {
@@ -260,6 +286,8 @@ void AudioOutputDevice::SetVolumeOnIOThread(double volume) {
}
void AudioOutputDevice::OnStateChanged(AudioOutputIPCDelegateState state) {
+ printf("****** AudioOutputDevice::OnStateChanged this = %p, state_ = %d, state = %d\n",
+ this, state_, state);
DCHECK(task_runner()->BelongsToCurrentThread());
// Do nothing if the stream has been closed.
@@ -274,13 +302,8 @@ void AudioOutputDevice::OnStateChanged(AudioOutputIPCDelegateState state) {
break;
case AUDIO_OUTPUT_IPC_DELEGATE_STATE_ERROR:
DLOG(WARNING) << "AudioOutputDevice::OnStateChanged(ERROR)";
- // Don't dereference the callback object if the audio thread
- // is stopped or stopping. That could mean that the callback
- // object has been deleted.
- // TODO(tommi): Add an explicit contract for clearing the callback
- // object. Possibly require calling Initialize again or provide
- // a callback object via Start() and clear it in Stop().
- if (!audio_thread_.IsStopped())
+ // The callback object is cleared in Stop().
+ if (callback_)
callback_->OnRenderError();
break;
default:
@@ -292,6 +315,8 @@ void AudioOutputDevice::OnStateChanged(AudioOutputIPCDelegateState state) {
void AudioOutputDevice::OnDeviceAuthorized(
OutputDeviceStatus device_status,
const media::AudioParameters& output_params) {
+ printf("****** AudioOutputDevice::OnDeviceAuthorized this = %p, state_ = %d, device_status = %d\n",
+ this, state_, device_status);
DCHECK(task_runner()->BelongsToCurrentThread());
DCHECK_EQ(state_, AUTHORIZING);
@@ -313,7 +338,7 @@ void AudioOutputDevice::OnDeviceAuthorized(
did_receive_auth_.Signal();
}
if (start_on_authorized_)
- CreateStreamOnIOThread(audio_parameters_);
+ CreateStreamOnIOThread();
} else {
// Closing IPC forces a Signal(), so no clients are locked waiting
// indefinitely after this method returns.
@@ -328,6 +353,8 @@ void AudioOutputDevice::OnStreamCreated(
base::SharedMemoryHandle handle,
base::SyncSocket::Handle socket_handle,
int length) {
+ printf("****** AudioOutputDevice::OnStreamCreated this = %p, state_ = %d\n",
+ this, state_);
DCHECK(task_runner()->BelongsToCurrentThread());
DCHECK(base::SharedMemory::IsHandleValid(handle));
#if defined(OS_WIN)
@@ -341,9 +368,9 @@ void AudioOutputDevice::OnStreamCreated(
return;
// We can receive OnStreamCreated() on the IO thread after the client has
- // called Stop() but before ShutDownOnIOThread() is processed. In such a
+ // called Stop() but before StopOnIOThread() is processed. In such a
// situation |callback_| might point to freed memory. Instead of starting
- // |audio_thread_| do nothing and wait for ShutDownOnIOThread() to get called.
+ // |audio_thread_| do nothing and wait for StopOnIOThread() to get called.
//
// TODO(scherkus): The real fix is to have sane ownership semantics. The fact
// that |callback_| (which should own and outlive this object!) can point to
@@ -357,11 +384,16 @@ void AudioOutputDevice::OnStreamCreated(
if (stopping_hack_)
return;
- DCHECK(audio_thread_.IsStopped());
audio_callback_.reset(new AudioOutputDevice::AudioThreadCallback(
audio_parameters_, handle, length, callback_));
- audio_thread_.Start(audio_callback_.get(), socket_handle,
- "AudioOutputDevice", true);
+
+ // If the audio thread hasn't been started before, start it.
+ if (audio_thread_.IsStopped()) {
+ printf("****** Starting audio thread.\n");
+ audio_thread_.Start("AudioOutputDevice", true);
+ }
+ audio_thread_.StartRun(audio_callback_.get(), socket_handle);
+
state_ = PAUSED;
// We handle the case where Play() and/or Pause() may have been called
@@ -372,6 +404,8 @@ void AudioOutputDevice::OnStreamCreated(
}
void AudioOutputDevice::OnIPCClosed() {
+ printf("****** AudioOutputDevice::OnIPCClosed this = %p, state_ = %d\n",
+ this, state_);
DCHECK(task_runner()->BelongsToCurrentThread());
state_ = IPC_CLOSED;
ipc_.reset();
@@ -382,7 +416,7 @@ void AudioOutputDevice::OnIPCClosed() {
void AudioOutputDevice::WillDestroyCurrentMessageLoop() {
LOG(ERROR) << "IO loop going away before the audio device has been stopped";
- ShutDownOnIOThread();
+ StopOnIOThread();
}
// AudioOutputDevice::AudioThreadCallback

Powered by Google App Engine
This is Rietveld 408576698