PepperPlatformAudioInputImpl: support audio input device selection.

content/renderer/pepper/pepper_platform_audio_input_impl.cc

 // 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/renderer/pepper/pepper_platform_audio_input_impl.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
 #include "build/build_config.h"
 #include "content/common/child_process.h"
 #include "content/common/media/audio_messages.h"
+#include "content/renderer/pepper/pepper_plugin_delegate_impl.h"
 #include "content/renderer/render_thread_impl.h"
 #include "media/audio/audio_manager_base.h"
 
 PepperPlatformAudioInputImpl::PepperPlatformAudioInputImpl()
     : client_(NULL),
       stream_id_(0),
-      main_message_loop_proxy_(base::MessageLoopProxy::current()) {
+      main_message_loop_proxy_(base::MessageLoopProxy::current()),
+      shutdown_called_(false) {

[no longer working on chromium, 2012/03/16 14:33:23]

why do we need a new shutdown_called flag? can we use stream_id_ instead?

[yzshen1, 2012/03/17 09:32:56]

ShutDown() may be called before stream_id_ is set to non-zero. This is possible
because there is an async step before initializing, i.e., OpenDevice ->
OnDeviceOpened.

So we need this extra variable.

[no longer working on chromium, 2012/03/18 00:38:59]

Do I miss something here? It looks to me they all are on IO thread. If it is
true, they are synchronous. Would you please double check?

[yzshen1, 2012/03/19 21:47:54]

OpenDevice() and OnDeviceOpened() happens on the main thread (as you can see,
there are DCHECKs to guarantee that.)
I don't understand what do you mean by "they are synchronous". Would you please
explain?

The following example illustrates what I was trying to say:
- [on main thread] Initialize() is called, which calls
plugin_delegate_->OpenDevice() (assuming device_id is not empty).
- [on main thread] ShutDown() is called, which posts a task to run
ShutDownOnIOThread().
- [on main thread] OnDeviceOpened() is called, which posts a task to run
InitializeOnIOThread().
- [on IO thread] ShutDownOnIOThread() is called, which sets shutdown_called_ to
true.
- [on IO thread] InitializeOnIOThread() is called. Please note that we cannot
use stream_id_ here to determine whether shutdown has occurred, because
stream_id_ is generated in _this_ method. That is why we need shutdown_called_.

   filter_ = RenderThreadImpl::current()->audio_input_message_filter();
 }
 
 PepperPlatformAudioInputImpl::~PepperPlatformAudioInputImpl() {
   // Make sure we have been shut down. Warning: this will usually happen on
   // the I/O thread!
   DCHECK_EQ(0, stream_id_);
   DCHECK(!client_);
+  DCHECK(label_.empty());
+  DCHECK(shutdown_called_);
 }
 
 // static
 PepperPlatformAudioInputImpl* PepperPlatformAudioInputImpl::Create(
+    const base::WeakPtr<PepperPluginDelegateImpl>& plugin_delegate,
+    const std::string& device_id,
     uint32_t sample_rate,
     uint32_t sample_count,
-    webkit::ppapi::PluginDelegate::PlatformAudioCommonClient* client) {
+    webkit::ppapi::PluginDelegate::PlatformAudioInputClient* client) {
   scoped_refptr<PepperPlatformAudioInputImpl> audio_input(
       new PepperPlatformAudioInputImpl);
-  if (audio_input->Initialize(sample_rate, sample_count, client)) {
+  if (audio_input->Initialize(plugin_delegate, device_id, sample_rate,
+                              sample_count, client)) {
     // Balanced by Release invoked in
     // PepperPlatformAudioInputImpl::ShutDownOnIOThread().
     return audio_input.release();
   }
   return NULL;
 }
 
 bool PepperPlatformAudioInputImpl::StartCapture() {

[no longer working on chromium, 2012/03/16 14:33:23]

It looks like this function has no way to return false, change it to void?

[yzshen1, 2012/03/17 09:32:56]

Done.

+  DCHECK(main_message_loop_proxy_->BelongsToCurrentThread());
+
   ChildProcess::current()->io_message_loop()->PostTask(
-    FROM_HERE,
-    base::Bind(&PepperPlatformAudioInputImpl::StartCaptureOnIOThread, this));
+      FROM_HERE,
+      base::Bind(&PepperPlatformAudioInputImpl::StartCaptureOnIOThread, this));
   return true;
 }
 
 bool PepperPlatformAudioInputImpl::StopCapture() {

[no longer working on chromium, 2012/03/16 14:33:23]

void?

[yzshen1, 2012/03/17 09:32:56]

Done.

+  DCHECK(main_message_loop_proxy_->BelongsToCurrentThread());
+
   ChildProcess::current()->io_message_loop()->PostTask(
-    FROM_HERE,
-    base::Bind(&PepperPlatformAudioInputImpl::StopCaptureOnIOThread, this));
+      FROM_HERE,
+      base::Bind(&PepperPlatformAudioInputImpl::StopCaptureOnIOThread, this));
   return true;
 }
 
 void PepperPlatformAudioInputImpl::ShutDown() {
+  DCHECK(main_message_loop_proxy_->BelongsToCurrentThread());
+
   // Called on the main thread to stop all audio callbacks. We must only change
   // the client on the main thread, and the delegates from the I/O thread.
   client_ = NULL;
   ChildProcess::current()->io_message_loop()->PostTask(
       FROM_HERE,
       base::Bind(&PepperPlatformAudioInputImpl::ShutDownOnIOThread, this));
 }
 
 bool PepperPlatformAudioInputImpl::Initialize(

[no longer working on chromium, 2012/03/16 14:33:23]

void?

[yzshen1, 2012/03/17 09:32:56]

I changed two DCHECKs to input parameter validation, so I kept the bool return
value.

On 2012/03/16 14:33:23, xians1 wrote:

+    const base::WeakPtr<PepperPluginDelegateImpl>& plugin_delegate,
+    const std::string& device_id,
     uint32_t sample_rate,
     uint32_t sample_count,
-    webkit::ppapi::PluginDelegate::PlatformAudioCommonClient* client) {
+    webkit::ppapi::PluginDelegate::PlatformAudioInputClient* client) {
+  DCHECK(main_message_loop_proxy_->BelongsToCurrentThread());
+
+  DCHECK(plugin_delegate);
   DCHECK(client);
-  // Make sure we don't call init more than once.
-  DCHECK_EQ(0, stream_id_);
 
+  plugin_delegate_ = plugin_delegate;
   client_ = client;
 
-  AudioParameters params;
-  params.format = AudioParameters::AUDIO_PCM_LINEAR;
-  params.channels = 1;
-  params.sample_rate = sample_rate;
-  params.bits_per_sample = 16;
-  params.samples_per_packet = sample_count;
+  params_.format = AudioParameters::AUDIO_PCM_LINEAR;
+  params_.channels = 1;
+  params_.sample_rate = sample_rate;
+  params_.bits_per_sample = 16;
+  params_.samples_per_packet = sample_count;
 
-  ChildProcess::current()->io_message_loop()->PostTask(
-      FROM_HERE,
-      base::Bind(&PepperPlatformAudioInputImpl::InitializeOnIOThread,
-                 this, params));
+  if (device_id.empty()) {
+    // Use the default device.
+    ChildProcess::current()->io_message_loop()->PostTask(
+        FROM_HERE,
+        base::Bind(&PepperPlatformAudioInputImpl::InitializeOnIOThread,
+                   this, 0));
+  } else {
+    // We need to open the device and obtain the label and session ID before
+    // initializing.
+    plugin_delegate_->OpenDevice(
+        PP_DEVICETYPE_DEV_AUDIOCAPTURE, device_id,
+        base::Bind(&PepperPlatformAudioInputImpl::OnDeviceOpened, this));
+  }
   return true;
 }
 
-void PepperPlatformAudioInputImpl::InitializeOnIOThread(
-    const AudioParameters& params) {
+void PepperPlatformAudioInputImpl::InitializeOnIOThread(int session_id) {
+  DCHECK(ChildProcess::current()->io_message_loop_proxy()->
+      BelongsToCurrentThread());
+
+  if (shutdown_called_)
+    return;
+
+  // Make sure we don't call init more than once.
+  DCHECK_EQ(0, stream_id_);
   stream_id_ = filter_->AddDelegate(this);
-  filter_->Send(new AudioInputHostMsg_CreateStream(
-      stream_id_, params, AudioManagerBase::kDefaultDeviceId));
+  DCHECK_NE(0, stream_id_);
+
+  if (!session_id) {
+    // We will be notified by OnStreamCreated().
+    filter_->Send(new AudioInputHostMsg_CreateStream(
+        stream_id_, params_, AudioManagerBase::kDefaultDeviceId));
+  } else {
+    // We will be notified by OnDeviceReady().
+    filter_->Send(new AudioInputHostMsg_StartDevice(stream_id_, session_id));
+  }
 }
 
 void PepperPlatformAudioInputImpl::StartCaptureOnIOThread() {
+  DCHECK(ChildProcess::current()->io_message_loop_proxy()->
+      BelongsToCurrentThread());
+
   if (stream_id_)
     filter_->Send(new AudioInputHostMsg_RecordStream(stream_id_));

[no longer working on chromium, 2012/03/16 14:33:23]

What happens if we are in the middle of starting the device, while the stream
has not been created, maybe we should not send AudioInputHostMsg_RecordStream in
this case.

[yzshen1, 2012/03/17 09:32:56]

Thanks for pointing this out!

This class doesn't maintain state to enforce the order of those public
operations: StartCapture/StopCapture/Shutdown.

ppb_audio_input_shared.cc maintains a simple state machine which makes sure
PepperPlatformAudioInputImpl is used in a reasonable way. So I added the check
there.

(Besides, it won't do any harm if a RecordStream is sent before the device is
started.)

On 2012/03/16 14:33:23, xians1 wrote:

[no longer working on chromium, 2012/03/18 00:38:59]

Thanks for clarification.

 }
 
 void PepperPlatformAudioInputImpl::StopCaptureOnIOThread() {
+  DCHECK(ChildProcess::current()->io_message_loop_proxy()->
+      BelongsToCurrentThread());
+
+  // TODO(yzshen): We cannot re-start capturing if the stream is closed.
   if (stream_id_)
     filter_->Send(new AudioInputHostMsg_CloseStream(stream_id_));

[no longer working on chromium, 2012/03/16 14:33:23]

same situation here, we need to do similar things as in audio_input_device.cc to
handle two possibilities.

[yzshen1, 2012/03/17 09:32:56]

Please see my comment above.
On 2012/03/16 14:33:23, xians1 wrote:

 }
 
 void PepperPlatformAudioInputImpl::ShutDownOnIOThread() {
+  DCHECK(ChildProcess::current()->io_message_loop_proxy()->
+      BelongsToCurrentThread());
+
   // Make sure we don't call shutdown more than once.
-  if (!stream_id_)
+  if (shutdown_called_)
     return;
+  shutdown_called_ = true;
 
-  filter_->Send(new AudioInputHostMsg_CloseStream(stream_id_));
-  filter_->RemoveDelegate(stream_id_);
-  stream_id_ = 0;
+  if (stream_id_) {
+    filter_->Send(new AudioInputHostMsg_CloseStream(stream_id_));
+    filter_->RemoveDelegate(stream_id_);
+    stream_id_ = 0;
+  }
+
+  main_message_loop_proxy_->PostTask(
+      FROM_HERE,
+      base::Bind(&PepperPlatformAudioInputImpl::CloseDevice, this));
 
   Release();  // Release for the delegate, balances out the reference taken in

[no longer working on chromium, 2012/03/16 14:33:23]

What does this Release() do? We execute CloseDevice after Release(), is it
intentional?

[yzshen1, 2012/03/17 09:32:56]

This is a ref-counted object, Release() decreases the ref count by 1.

CloseDevice() has to be executed on the main thread, so a task is posted. It
will be safe to execute it after Release(), because Bind will maintain ref
count, too.

On 2012/03/16 14:33:23, xians1 wrote:

[no longer working on chromium, 2012/03/18 00:38:59]

True, I know it is safe due to PepperPlatformAudioInputImpl is ref-counted. I
asked the question because we come from Shutdown() in main thread, then here we
post a task back to main thread. Can we just call CloseDevice in Shutdown()? Or
they have to be done in this way?

[yzshen1, 2012/03/19 21:47:54]

I try to maintain the following order:
open device -> start device -> create stream -> record stream -> close stream ->
close device
Because "open device" comes the first, I would like to make sure "close device"
comes the last.

I haven't investigated whether there are any issues if "close device" is called
prior to "close stream". I will take a closer look at the browser side code.

               // PepperPluginDelegateImpl::CreateAudioInput.
 }
 
 void PepperPlatformAudioInputImpl::OnStreamCreated(
     base::SharedMemoryHandle handle,
     base::SyncSocket::Handle socket_handle,
     uint32 length) {
 #if defined(OS_WIN)
   DCHECK(handle);
   DCHECK(socket_handle);
 #else
   DCHECK_NE(-1, handle.fd);
   DCHECK_NE(-1, socket_handle);
 #endif
   DCHECK(length);
 

[no longer working on chromium, 2012/03/16 14:33:23]

Which thread does this OnStreamCreated() happen? IO thread?
If it does, we can check the stream_id_, and free the handles if it is 0, then
we don't need to post a task to the main thread.

What about making it more readable by:
if (base::MessageLoopProxy::current() != main_message_loop_proxy_) {
  main_message_loop_proxy_->PostTask(
      FROM_HERE,
      base::Bind(&PepperPlatformAudioInputImpl::OnStreamCreated, this, handle,
socket_handle, length));
  return;
}

// it looks like client_ is not thread safe anyway, consider a lock here?
if (client_) {
  ....

[yzshen1, 2012/03/17 09:32:56]

Yes. And then we will post a task (using the same method) to the main thread.
Yes. That will work. But we still need to check client_ and do clean up on the
main thread if it is NULL. Because ShutDown may be called after the task was
posted but before it gets processed on the main thread.
Okay. I changed the condition and switched the two branches.
 
client_ is never accessed on threads other than the main thread, so we don't
need a lock.

   if (base::MessageLoopProxy::current() == main_message_loop_proxy_) {
     // Must dereference the client only on the main thread. Shutdown may have
     // occurred while the request was in-flight, so we need to NULL check.
-    if (client_)
+    if (client_) {
       client_->StreamCreated(handle, length, socket_handle);
+    } else {
+      // Clean up the handles.
+      base::SyncSocket temp_socket(socket_handle);
+      base::SharedMemory temp_shared_memory(handle, false);
+    }
   } else {
+    // No need to check |shutdown_called_| here. If shutdown has occurred,
+    // |client_| will be NULL and the handles will be cleaned up on the main
+    // thread.
     main_message_loop_proxy_->PostTask(
         FROM_HERE,
         base::Bind(&PepperPlatformAudioInputImpl::OnStreamCreated, this,
                    handle, socket_handle, length));
   }
 }
 
 void PepperPlatformAudioInputImpl::OnVolume(double volume) {
 }
 
 void PepperPlatformAudioInputImpl::OnStateChanged(AudioStreamState state) {
 }
 
-void PepperPlatformAudioInputImpl::OnDeviceReady(const std::string&) {
+void PepperPlatformAudioInputImpl::OnDeviceReady(const std::string& device_id) {
+  DCHECK(ChildProcess::current()->io_message_loop_proxy()->
+      BelongsToCurrentThread());
+
+  if (shutdown_called_)
+    return;
+
+  if (device_id.empty()) {
+    main_message_loop_proxy_->PostTask(
+        FROM_HERE,
+        base::Bind(&PepperPlatformAudioInputImpl::NotifyStreamCreationFailed,
+                   this));
+  } else {
+    // We will be notified by OnStreamCreated().
+    filter_->Send(new AudioInputHostMsg_CreateStream(stream_id_, params_,
+                                                     device_id));
+  }
 }
+
+void PepperPlatformAudioInputImpl::OnDeviceOpened(int request_id,
+                                                  bool succeeded,
+                                                  const std::string& label) {
+  DCHECK(main_message_loop_proxy_->BelongsToCurrentThread());
+
+  succeeded = succeeded && plugin_delegate_;
+  if (succeeded) {
+    label_ = label;
+
+    if (client_) {
+      int session_id = plugin_delegate_->GetSessionID(
+          PP_DEVICETYPE_DEV_AUDIOCAPTURE, label);
+      ChildProcess::current()->io_message_loop()->PostTask(
+          FROM_HERE,
+          base::Bind(&PepperPlatformAudioInputImpl::InitializeOnIOThread,
+                     this, session_id));
+    } else {
+      // Shutdown has occurred.
+      CloseDevice();
+    }
+  } else {
+    NotifyStreamCreationFailed();

[no longer working on chromium, 2012/03/16 14:33:23]

shall we still send the notification when plugin_delegate_ == NULL?

[yzshen1, 2012/03/17 09:32:56]

As long as client_ is not NULL, I think it makes sense to send the notification.
(And that is what the current code does.)

+  }
+}
+
+void PepperPlatformAudioInputImpl::CloseDevice() {
+  DCHECK(main_message_loop_proxy_->BelongsToCurrentThread());
+
+  if (plugin_delegate_ && !label_.empty()) {
+    plugin_delegate_->CloseDevice(label_);
+    label_.clear();
+  }
+}
+
+void PepperPlatformAudioInputImpl::NotifyStreamCreationFailed() {
+  DCHECK(main_message_loop_proxy_->BelongsToCurrentThread());
+
+  if (client_)
+    client_->StreamCreationFailed();
+}