Simplify AudioRendererImpl by using AudioDevice.

content/renderer/media/audio_device.cc

 // Copyright (c) 2011 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/renderer/media/audio_device.h"
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/message_loop.h"
 #include "base/time.h"
 #include "content/common/child_process.h"
 #include "content/common/media/audio_messages.h"
 #include "content/common/view_messages.h"
 #include "content/renderer/render_thread_impl.h"
+#include "media/audio/audio_output_controller.h"
 #include "media/audio/audio_util.h"
 
+AudioDevice::AudioDevice()
+    : buffer_size_(0),
+      channels_(0),
+      bits_per_sample_(16),
+      sample_rate_(0),
+      latency_format_(AudioParameters::AUDIO_PCM_LOW_LATENCY),
+      callback_(0),
+      is_initialized_(false),
+      audio_delay_milliseconds_(0),
+      volume_(1.0),
+      stream_id_(0),
+      play_on_start_(true),
+      is_started_(false),
+      shared_memory_size_(0) {
+  filter_ = RenderThreadImpl::current()->audio_message_filter();
+}
+
 AudioDevice::AudioDevice(size_t buffer_size,
                          int channels,
                          double sample_rate,
                          RenderCallback* callback)
-    : buffer_size_(buffer_size),
-      channels_(channels),
-      bits_per_sample_(16),
-      sample_rate_(sample_rate),
-      callback_(callback),
+    : bits_per_sample_(16),
+      is_initialized_(false),
       audio_delay_milliseconds_(0),
       volume_(1.0),
-      stream_id_(0) {
+      stream_id_(0),
+      play_on_start_(true),
+      is_started_(false),
+      shared_memory_size_(0) {
   filter_ = RenderThreadImpl::current()->audio_message_filter();
+  Initialize(buffer_size,
+             channels,
+             sample_rate,
+             AudioParameters::AUDIO_PCM_LOW_LATENCY,
+             callback);
+}
+
+void AudioDevice::Initialize(size_t buffer_size,
+                             int channels,
+                             double sample_rate,
+                             AudioParameters::Format latency_format,
+                             RenderCallback* callback) {
+  CHECK_EQ(0, stream_id_) <<
+      "AudioDevice::Initialize() must be called before Start()";
+
+  buffer_size_ = buffer_size;
+  channels_ = channels;
+  sample_rate_ = sample_rate;
+  latency_format_ = latency_format;
+  callback_ = callback;
+
+  // Cleanup from any previous initialization.
+  for (size_t i = 0; i < audio_data_.size(); ++i)
+    delete [] audio_data_[i];
+
   audio_data_.reserve(channels);
   for (int i = 0; i < channels; ++i) {
     float* channel_data = new float[buffer_size];
     audio_data_.push_back(channel_data);
   }
+
+  is_initialized_ = true;
+}
+
+bool AudioDevice::IsInitialized() {
+  return is_initialized_;
 }
 
 AudioDevice::~AudioDevice() {
   // The current design requires that the user calls Stop() before deleting
   // this class.
   CHECK_EQ(0, stream_id_);
   for (int i = 0; i < channels_; ++i)
     delete [] audio_data_[i];
 }
 
 void AudioDevice::Start() {
   AudioParameters params;
-  params.format = AudioParameters::AUDIO_PCM_LOW_LATENCY;
+  params.format = latency_format_;
   params.channels = channels_;
   params.sample_rate = static_cast<int>(sample_rate_);
   params.bits_per_sample = bits_per_sample_;
   params.samples_per_packet = buffer_size_;
 
   ChildProcess::current()->io_message_loop()->PostTask(
       FROM_HERE,
       base::Bind(&AudioDevice::InitializeOnIOThread, this, params));
 }
 
 bool AudioDevice::Stop() {
   // Max waiting time for Stop() to complete. If this time limit is passed,
   // we will stop waiting and return false. It ensures that Stop() can't block
   // the calling thread forever.
   const base::TimeDelta kMaxTimeOut = base::TimeDelta::FromMilliseconds(1000);
 
   base::WaitableEvent completion(false, false);
 
   ChildProcess::current()->io_message_loop()->PostTask(
       FROM_HERE,
       base::Bind(&AudioDevice::ShutDownOnIOThread, this, &completion));

[no longer working on chromium, 2011/11/28 15:17:41]

We use a WitableEvent in ShutDownOnIOThread() to sync with IO thread, so after
the event is signaled, stream_id_ is set to 0, we have no chance to access
socket_ and audio_thread_ in OnLowLatencyCreated().
So no lock is needed in for socket_ and audio_thread_ for the shutdown.

[vrk (LEFT CHROMIUM), 2011/12/02 22:54:46]

Hmm, I think you're right, but since this isn't my CL and I'm not that familiar
with the code, I'm not comfortable making that change before landing this. :/

Write a follow-up CL to delete the lock after this CL lands? I don't think it's
critical, so it seems like something to ask crogers about when he comes back
from vacation.

 
   // We wait here for the IO task to be completed to remove race conflicts
   // with OnLowLatencyCreated() and to ensure that Stop() acts as a synchronous
   // function call.
   if (completion.TimedWait(kMaxTimeOut)) {
-    if (audio_thread_.get()) {
-      socket_->Close();
-      audio_thread_->Join();
-      audio_thread_.reset(NULL);
-    }
+    ShutDownAudioThread();
   } else {
     LOG(ERROR) << "Failed to shut down audio output on IO thread";
     return false;
   }
 
   return true;
 }
 
+void AudioDevice::Play() {
+  ChildProcess::current()->io_message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&AudioDevice::PlayOnIOThread, this));
+}
+
+void AudioDevice::Pause(bool flush) {
+  ChildProcess::current()->io_message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&AudioDevice::PauseOnIOThread, this, flush));
+}
+
 bool AudioDevice::SetVolume(double volume) {
   if (volume < 0 || volume > 1.0)
     return false;
 
   ChildProcess::current()->io_message_loop()->PostTask(
       FROM_HERE,
       base::Bind(&AudioDevice::SetVolumeOnIOThread, this, volume));
 
   volume_ = volume;
 
   return true;
 }
 
 void AudioDevice::GetVolume(double* volume) {
   // Return a locally cached version of the current scaling factor.
   *volume = volume_;
 }
 
 void AudioDevice::InitializeOnIOThread(const AudioParameters& params) {
-  // Make sure we don't call Start() more than once.
+  // Make sure we don't create the stream more than once.
   DCHECK_EQ(0, stream_id_);
   if (stream_id_)
     return;
 
   stream_id_ = filter_->AddDelegate(this);
   Send(new AudioHostMsg_CreateStream(stream_id_, params, true));
 }
 
-void AudioDevice::StartOnIOThread() {
-  if (stream_id_)
+void AudioDevice::PlayOnIOThread() {

[no longer working on chromium, 2011/11/28 15:17:41]

What happens if the clients call Play() or Pause() for multiple times? How do
you think adding a flag on IO thread to protect them?

[vrk (LEFT CHROMIUM), 2011/12/02 22:54:46]

This is a good question, but I think it may be something to examine as part of
another CL. This CL preserves the same behavior as the original
AudioRendererImpl in this situation, so I don't think it's necessary to change.

+  if (stream_id_ && is_started_)
     Send(new AudioHostMsg_PlayStream(stream_id_));
+  else
+    play_on_start_ = true;
+}
+
+void AudioDevice::PauseOnIOThread(bool flush) {

[no longer working on chromium, 2011/11/28 15:17:41]

Same question as PlayOnIOThread()

[vrk (LEFT CHROMIUM), 2011/12/02 22:54:46]

Ditto above.

+  if (stream_id_ && is_started_) {
+    Send(new AudioHostMsg_PauseStream(stream_id_));
+    if (flush)
+      Send(new AudioHostMsg_FlushStream(stream_id_));
+  } else {
+    // Note that |flush| isn't relevant here since this is the case where
+    // the stream is first starting.
+    play_on_start_ = false;
+  }
 }
 
 void AudioDevice::ShutDownOnIOThread(base::WaitableEvent* completion) {

[no longer working on chromium, 2011/11/28 15:17:41]

We should clean up all the states like is_started, play_on_start_.

[vrk (LEFT CHROMIUM), 2011/12/02 22:54:46]

Yeah, these states are probably an indication that the API needs some tweaking
yet. But let's leave that to another CL.  It seems like a reasonable solution
for now for the problem that was discussed here:
http://codereview.chromium.org/8477037/diff/1/content/renderer/media/audio_de....

+  is_started_ = false;
+
   // Make sure we don't call shutdown more than once.
   if (!stream_id_) {
-    completion->Signal();
+    if (completion)
+      completion->Signal();
     return;
   }
 
   filter_->RemoveDelegate(stream_id_);
   Send(new AudioHostMsg_CloseStream(stream_id_));

[vrk (LEFT CHROMIUM), 2011/11/22 01:17:39]

Doesn't AudioHostMsg_CloseStream need to be synchronous, i.e.
IPC_SYNC_MESSAGE_ROUTED?

Otherwise I believe this will only wait for the AudioHostMsg_CloseStream message
to be sent, not for the receiver to handle the message.

[vrk (LEFT CHROMIUM), 2011/12/02 22:54:46]

Since this potential problem was not introduced by the CL, I'll make a note on
my own to examine it in a follow-up CL.

   stream_id_ = 0;
 
-  completion->Signal();
+  if (completion)
+    completion->Signal();
 }
 
 void AudioDevice::SetVolumeOnIOThread(double volume) {
   if (stream_id_)
     Send(new AudioHostMsg_SetVolume(stream_id_, volume));
 }
 
 void AudioDevice::OnRequestPacket(AudioBuffersState buffers_state) {
   // This method does not apply to the low-latency system.
-  NOTIMPLEMENTED();
 }
 
 void AudioDevice::OnStateChanged(AudioStreamState state) {
   if (state == kAudioStreamError) {
     DLOG(WARNING) << "AudioDevice::OnStateChanged(kError)";
   }
-  NOTIMPLEMENTED();
 }
 
 void AudioDevice::OnCreated(
     base::SharedMemoryHandle handle, uint32 length) {
   // Not needed in this simple implementation.
-  NOTIMPLEMENTED();
 }
 
 void AudioDevice::OnLowLatencyCreated(
     base::SharedMemoryHandle handle,
     base::SyncSocket::Handle socket_handle,
     uint32 length) {
   DCHECK(MessageLoop::current() == ChildProcess::current()->io_message_loop());
 #if defined(OS_WIN)
   DCHECK(handle);
   DCHECK(socket_handle);
 #else
   DCHECK_GE(handle.fd, 0);
   DCHECK_GE(socket_handle, 0);
 #endif
   DCHECK(length);
 
   // Takes care of the case when Stop() is called before OnLowLatencyCreated().
   if (!stream_id_) {
     base::SharedMemory::CloseHandle(handle);
     // Close the socket handler.
     base::SyncSocket socket(socket_handle);
     return;
   }
 
   shared_memory_.reset(new base::SharedMemory(handle, false));
   shared_memory_->Map(length);
+  shared_memory_size_ = length;
 
   DCHECK_GE(length, buffer_size_ * sizeof(int16) * channels_);
 
-  socket_.reset(new base::SyncSocket(socket_handle));
-  // Allow the client to pre-populate the buffer.
-  FireRenderCallback();
+  {
+    // Synchronize with ShutDownAudioThread().
+    base::AutoLock auto_lock(lock_);
 
-  audio_thread_.reset(
-      new base::DelegateSimpleThread(this, "renderer_audio_thread"));
-  audio_thread_->Start();
+    socket_.reset(new base::SyncSocket(socket_handle));
+    // Allow the client to pre-populate the buffer.
+    FireRenderCallback();
 
-  MessageLoop::current()->PostTask(
-      FROM_HERE,
-      base::Bind(&AudioDevice::StartOnIOThread, this));
+    DCHECK(!audio_thread_.get());
+    audio_thread_.reset(
+        new base::DelegateSimpleThread(this, "renderer_audio_thread"));
+    audio_thread_->Start();
+  }
+
+  // We handle the case where Play() and/or Pause() may have been called
+  // multiple times before OnLowLatencyCreated() gets called.
+  is_started_ = true;
+  if (play_on_start_) {
+    MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&AudioDevice::PlayOnIOThread, this));

[no longer working on chromium, 2011/11/28 15:17:41]

One potential racing condition here, for example, the following calls can happen
in sequence on IO thread:
PlayOnIOThread()
OnLowLatencyCreated()
PauseOnIOThread()

Since OnLowLatencyCreated() will post a PlayOnIOThread(), so 
PauseOnIOThread() will send a pause PauseStream message to the brwoser before
the stream is played.

Maybe we should just call PlayOnIOThread() directly, instead of posting a task.

[vrk (LEFT CHROMIUM), 2011/12/02 22:54:46]

Yes, you're right; good catch! I think calling PlayOnIOThread() directly makes
the most sense, but I'm not sure if there was a reason this task was essentially
delayed in the past... 

Looking at history, this was introduced in
http://codereview.chromium.org/7157001/. Doesn't look like it was intentionally
delayed, so I changed this to call PlayOnIOThread() directly.

+  }
 }
 
 void AudioDevice::OnVolume(double volume) {
   NOTIMPLEMENTED();
 }
 
 void AudioDevice::Send(IPC::Message* message) {
   filter_->Send(message);
 }
 
 // Our audio thread runs here.
 void AudioDevice::Run() {
   audio_thread_->SetThreadPriority(base::kThreadPriority_RealtimeAudio);
 
   int pending_data;
   const int samples_per_ms = static_cast<int>(sample_rate_) / 1000;
   const int bytes_per_ms = channels_ * (bits_per_sample_ / 8) * samples_per_ms;
 
-  while ((sizeof(pending_data) == socket_->Receive(&pending_data,
-                                                   sizeof(pending_data))) &&
-         (pending_data >= 0)) {
+  while (sizeof(pending_data) ==
+      socket_->Receive(&pending_data, sizeof(pending_data))) {
+    if (pending_data == media::AudioOutputController::kPauseMark) {
+      memset(shared_memory_data(), 0, shared_memory_size_);
+      continue;
+    } else if (pending_data < 0) {
+      break;
+    }
     // Convert the number of pending bytes in the render buffer
     // into milliseconds.
     audio_delay_milliseconds_ = pending_data / bytes_per_ms;
     FireRenderCallback();
   }
 }
 
 void AudioDevice::FireRenderCallback() {
   TRACE_EVENT0("audio", "AudioDevice::FireRenderCallback");
 
   if (callback_) {
     // Update the audio-delay measurement then ask client to render audio.
     callback_->Render(audio_data_, buffer_size_, audio_delay_milliseconds_);
 
     // Interleave, scale, and clip to int16.
+    // TODO(crogers): avoid converting to integer here, and pass the data
+    // to the browser process as float, so we don't lose precision for
+    // audio hardware which has better than 16bit precision.
     media::InterleaveFloatToInt16(audio_data_,
                                   static_cast<int16*>(shared_memory_data()),
                                   buffer_size_);
   }
 }
 
+void AudioDevice::ShutDownAudioThread() {

[no longer working on chromium, 2011/11/28 15:17:41]

Please see comment in Stop().

[vrk (LEFT CHROMIUM), 2011/12/02 22:54:46]

Done.

+  // Synchronize with OnLowLatencyCreated().
+  base::AutoLock auto_lock(lock_);
+
+  if (socket_.get()) {
+    socket_->Close();
+  }
+  if (audio_thread_.get()) {
+    audio_thread_->Join();
+    audio_thread_.reset(NULL);
+  }
+  // Note that the socket can't be reset until *after* joining the thread.
+  socket_.reset(NULL);
+}
+
 double AudioDevice::GetAudioHardwareSampleRate() {
   // Uses cached value if possible.
   static double hardware_sample_rate = 0;
   if (!hardware_sample_rate) {
     RenderThreadImpl::current()->Send(
         new ViewHostMsg_GetHardwareSampleRate(&hardware_sample_rate));
   }
   return hardware_sample_rate;
 }
 
 size_t AudioDevice::GetAudioHardwareBufferSize() {
   // Uses cached value if possible.

[acolwell GONE FROM CHROMIUM, 2011/11/17 19:16:12]

FYI. You might need to coordinate w/ tommi on this. He's moved this to a helper
function in http://codereview.chromium.org/8588030/

[vrk (LEFT CHROMIUM), 2011/12/02 22:54:46]

Thanks Aaron! These methods weren't being used in this class, so rebasing to ToT
was smooth sailing.

   static uint32 buffer_size = 0;
 
   if (!buffer_size) {
     RenderThreadImpl::current()->Send(
         new ViewHostMsg_GetHardwareBufferSize(&buffer_size));
   }
 
   return static_cast<size_t>(buffer_size);
 }