refactor AudioInputDevice to remove race condition.

content/renderer/media/webrtc_audio_device_impl.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/webrtc_audio_device_impl.h"
 
 #include "base/string_util.h"
 #include "media/audio/audio_util.h"
 
 // TODO(henrika): come up with suitable value(s) for all platforms.
 // Max supported size for input and output buffers.
 // Unit is in #(audio frames), hence 1440 <=> 30ms @ 48kHz.
 static const size_t kMaxBufferSize = 1440;
 static const int kMaxChannels = 2;
 static const int64 kMillisecondsBetweenProcessCalls = 5000;
 static const char kVersion[] = "WebRTC AudioDevice 1.0.0.Chrome";
 
 WebRtcAudioDeviceImpl::WebRtcAudioDeviceImpl(
-  size_t input_buffer_size, size_t output_buffer_size,
-  int input_channels, int output_channels,
-  double input_sample_rate, double output_sample_rate)
+    size_t input_buffer_size, size_t output_buffer_size,
+    int input_channels, int output_channels,
+    double input_sample_rate, double output_sample_rate)
     : audio_transport_callback_(NULL),
       last_error_(AudioDeviceModule::kAdmErrNone),
       input_buffer_size_(input_buffer_size),
       output_buffer_size_(output_buffer_size),
       input_channels_(input_channels),
       output_channels_(output_channels),
       input_sample_rate_(input_sample_rate),
       output_sample_rate_(output_sample_rate),
+      adm_thread_("WebRtcAudioDeviceImpl"),
+      recording_stop_event_(false, false),
       initialized_(false),
       playing_(false),
-      recording_(false),
+      recording_state_(kStopped),
       input_delay_ms_(0),
       output_delay_ms_(0),
       last_process_time_(base::TimeTicks::Now()) {
     VLOG(1) << "WebRtcAudioDeviceImpl::WebRtcAudioDeviceImpl()";
 
+    adm_thread_.Start();
+    adm_message_loop_ = adm_thread_.message_loop_proxy();
+
     // Create an AudioInputDevice client if the requested buffer size
     // is an even multiple of 10 milliseconds.
     if (BufferSizeIsValid(input_buffer_size, input_sample_rate)) {
       audio_input_device_ = new AudioInputDevice(
           input_buffer_size,
           input_channels,
           input_sample_rate,
+          adm_message_loop_.get(),

[henrika_dont_use, 2011/08/07 16:52:27]

Perhaps add a comment on what we want to achieve by feeding this thread as input
to the AudioInputDevice. 

[wjia(left Chromium), 2011/08/09 01:40:36]

Added some comments in AudioInputDevice constructor.

+          this,
           this);
     }
 
     // Create an AudioDevice client if the requested buffer size
     // is an even multiple of 10 milliseconds.
     if (BufferSizeIsValid(output_buffer_size, output_sample_rate)) {
       audio_output_device_ = new AudioDevice(
           output_buffer_size,
           output_channels,
           output_sample_rate,
           this);
     }
     DCHECK(audio_input_device_);
     DCHECK(audio_output_device_);
 
     input_buffer_.reset(new int16[kMaxBufferSize * kMaxChannels]);
     output_buffer_.reset(new int16[kMaxBufferSize * kMaxChannels]);
 
     bytes_per_sample_ = sizeof(*input_buffer_.get());
 }
 
 WebRtcAudioDeviceImpl::~WebRtcAudioDeviceImpl() {
   VLOG(1) << "WebRtcAudioDeviceImpl::~WebRtcAudioDeviceImpl()";
   if (playing_)
     StopPlayout();
-  if (recording_)
-    StopRecording();
+  StopRecording();
   if (initialized_)
     Terminate();
+  adm_thread_.Stop();
 }
 
 void WebRtcAudioDeviceImpl::Render(
     const std::vector<float*>& audio_data,
     size_t number_of_frames,
     size_t audio_delay_milliseconds) {
   DCHECK_LE(number_of_frames, kMaxBufferSize);
 
   // Store the reported audio delay locally.
+  lock_.Acquire();
   output_delay_ms_ = audio_delay_milliseconds;
+  lock_.Release();
 
   const int channels = audio_data.size();
   DCHECK_LE(channels, kMaxChannels);
 
   const int samples_per_sec = static_cast<int>(input_sample_rate_);
   uint32_t samples_per_10_msec = (samples_per_sec / 100);
   const int bytes_per_10_msec =
       channels * samples_per_10_msec * bytes_per_sample_;
 
   uint32_t num_audio_samples = 0;
@@ skipping 53 matching lines @@
   const int bytes_per_10_msec =
       channels * samples_per_10_msec * bytes_per_sample_;
   size_t accumulated_audio_samples = 0;
 
   char* audio_byte_buffer = reinterpret_cast<char*>(input_buffer_.get());
 
   // Write audio samples in blocks of 10 milliseconds to the registered
   // webrtc::AudioTransport sink. Keep writing until our internal byte
   // buffer is empty.
   while (accumulated_audio_samples < number_of_frames) {
+    lock_.Acquire();
+    int output_delay_ms = output_delay_ms_;
+    lock_.Release();
     // Deliver 10ms of recorded PCM audio.
     // TODO(henrika): add support for analog AGC?
     audio_transport_callback_->RecordedDataIsAvailable(
         audio_byte_buffer,
         samples_per_10_msec,
         bytes_per_sample_,
         channels,
         samples_per_sec,
-        input_delay_ms_ + output_delay_ms_,
+        input_delay_ms_ + output_delay_ms,
         0,  // clock_drift
         0,  // current_mic_level
         new_mic_level);  // not used
     accumulated_audio_samples += samples_per_10_msec;
     audio_byte_buffer += bytes_per_10_msec;
   }
 }
 
 int32_t WebRtcAudioDeviceImpl::Version(char* version,
                                        uint32_t& remaining_buffer_in_bytes,
@@ skipping 49 matching lines @@
 int32_t WebRtcAudioDeviceImpl::RegisterEventObserver(
     webrtc::AudioDeviceObserver* event_callback) {
   VLOG(1) << "RegisterEventObserver()";
   NOTIMPLEMENTED();
   return -1;
 }
 
 int32_t WebRtcAudioDeviceImpl::RegisterAudioCallback(
     webrtc::AudioTransport* audio_callback) {
   VLOG(1) << "RegisterAudioCallback()";
-  if (playing_ || recording_)  {
+  int32_t error = 0;
+  base::WaitableEvent event(false, false);
+  adm_message_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(
+          this,
+          &WebRtcAudioDeviceImpl::RegisterAudioCallbackOnAdmThread,
+          audio_callback, &error, &event));
+  event.Wait();
+  return error;
+}
+
+void WebRtcAudioDeviceImpl::RegisterAudioCallbackOnAdmThread(
+    webrtc::AudioTransport* audio_callback,
+    int32_t* error,
+    base::WaitableEvent* event) {
+  VLOG(1) << "RegisterAudioCallbackOnAdmThread()";
+  if (playing_ || recording_state_ != kStopped)  {
     LOG(ERROR) << "Unable to (de)register transport during active media";
-    return -1;
+    *error = -1;
+    return;
   }
   audio_transport_callback_ = audio_callback;
-  return 0;
+  *error = 0;
+  event->Signal();
 }
 
 int32_t WebRtcAudioDeviceImpl::Init() {
   VLOG(1) << "Init()";
   if (initialized_)
     return 0;
   initialized_ = true;
   return 0;
 }
 
@@ skipping 120 matching lines @@
   playing_ = !audio_output_device_->Stop();
   return (!playing_ ? 0 : -1);
 }
 
 bool WebRtcAudioDeviceImpl::Playing() const {
   return playing_;
 }
 
 int32_t WebRtcAudioDeviceImpl::StartRecording() {
   VLOG(1) << "StartRecording()";
-  LOG_IF(ERROR, !audio_transport_callback_) << "Audio transport is missing";
-  if (!audio_transport_callback_) {
-    LOG(ERROR) << "Audio transport is missing";
-    return -1;
-  }
-  if (recording_) {
+  adm_message_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(this,
+                        &WebRtcAudioDeviceImpl::StartRecordingOnAdmThread));
+  return 0;
+}
+
+void WebRtcAudioDeviceImpl::StartRecordingOnAdmThread() {
+  VLOG(1) << "StartRecordingOnAdmThread()";
+  // Required to set audio_transport_callback_ before starting recording.
+  DCHECK(audio_transport_callback_);
+  if (recording_state_ != kStopped) {
     // webrtc::VoiceEngine assumes that it is OK to call Start() twice and
     // that the call is ignored the second time.
     LOG(WARNING) << "Recording is already active";
-    return 0;
+    return;
   }
-  recording_ = audio_input_device_->Start();
-  return (recording_ ? 0 : -1);
+  audio_input_device_->Start();
+  recording_state_ = kStarting;
 }
 
 int32_t WebRtcAudioDeviceImpl::StopRecording() {
   VLOG(1) << "StopRecording()";
-  DCHECK(audio_input_device_);
-  if (!recording_) {
+  adm_message_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(this,
+                        &WebRtcAudioDeviceImpl::StopRecordingOnAdmThread));
+  recording_stop_event_.Wait();
+  return 0;
+}
+
+void WebRtcAudioDeviceImpl::StopRecordingOnAdmThread() {
+  VLOG(1) << "StopRecordingOnAdmThread()";
+  // Client never sees kStopping state since it's always blocked on
+  // StopRecording() call.
+  DCHECK_NE(recording_state_, kStopping);
+  if (recording_state_ == kStopped) {
     // webrtc::VoiceEngine assumes that it is OK to call Stop() just in case.
     LOG(WARNING) << "Recording was already stopped";
-    return 0;
+    recording_stop_event_.Signal();
+    return;
   }
-  recording_ = !audio_input_device_->Stop();
-  return (!recording_ ? 0 : -1);
+  audio_input_device_->Stop();
+  recording_state_ = kStopping;
+}
+
+void WebRtcAudioDeviceImpl::OnRecordingStarted() {
+  VLOG(1) << "OnRecordingStarted()";
+  adm_message_loop_->PostTask(FROM_HERE,

[henrika_dont_use, 2011/08/07 16:52:27]

Can't we set state in this method directly? It is called from the ADM thread,
right?

[wjia(left Chromium), 2011/08/09 01:40:36]

Setting state here put some assumption on AudioInputDevice implementation.

+      NewRunnableMethod(this,
+                        &WebRtcAudioDeviceImpl::OnRecordingStartedOnAdmThread));
+}
+
+void WebRtcAudioDeviceImpl::OnRecordingStartedOnAdmThread() {
+  VLOG(1) << "OnRecordingStartedOnAdmThread()";
+  recording_state_ = kStarted;
+}
+
+void WebRtcAudioDeviceImpl::OnRecordingStopped() {
+  VLOG(1) << "OnRecordingStopped()";
+  adm_message_loop_->PostTask(FROM_HERE,

[henrika_dont_use, 2011/08/07 16:52:27]

Same comment as for Start.

[wjia(left Chromium), 2011/08/09 01:40:36]

See comments above.

+      NewRunnableMethod(this,
+                        &WebRtcAudioDeviceImpl::OnRecordingStoppedOnAdmThread));
+}
+
+void WebRtcAudioDeviceImpl::OnRecordingStoppedOnAdmThread() {
+  VLOG(1) << "OnRecordingStoppedOnAdmThread()";
+  recording_state_ = kStopped;
+  // Always signal "stopped" since client must be waiting for it.
+  recording_stop_event_.Signal();
 }
 
 bool WebRtcAudioDeviceImpl::Recording() const {
-  return recording_;
+  bool recording = false;
+  base::WaitableEvent event(false, false);
+  adm_message_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(this,
+                        &WebRtcAudioDeviceImpl::GetRecordingOnAdmThread,
+                        &recording, &event));
+  event.Wait();
+  return recording;
+}
+
+void WebRtcAudioDeviceImpl::GetRecordingOnAdmThread(
+    bool* recording,
+    base::WaitableEvent* event) const {
+  *recording = recording_state_ == kStarting ||
+               recording_state_ == kStarted ||
+               recording_state_ == kStopping;
+  event->Signal();
 }
 
 int32_t WebRtcAudioDeviceImpl::SetAGC(bool enable) {
   NOTIMPLEMENTED();
   return -1;
 }
 
 bool WebRtcAudioDeviceImpl::AGC() const {
   NOTIMPLEMENTED();
   return false;
@@ skipping 217 matching lines @@
   return -1;
 }
 
 int32_t WebRtcAudioDeviceImpl::PlayoutDelay(uint16_t* delay_ms) const {
   // Report the cached output delay value.
   *delay_ms = static_cast<uint16_t>(output_delay_ms_);
   return 0;
 }
 
 int32_t WebRtcAudioDeviceImpl::RecordingDelay(uint16_t* delay_ms) const {
-  // Report the cached output delay value.
-  *delay_ms = static_cast<uint16_t>(input_delay_ms_);
+  base::WaitableEvent event(false, false);
+  adm_message_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(this,
+                        &WebRtcAudioDeviceImpl::GetRecordingDelayOnAdmThread,
+                        delay_ms, &event));
+  event.Wait();
   return 0;
 }
 
+void WebRtcAudioDeviceImpl::GetRecordingDelayOnAdmThread(
+    uint16_t* delay_ms,
+    base::WaitableEvent* event) const {
+  // Report the cached output delay value.
+  *delay_ms = static_cast<uint16_t>(input_delay_ms_);
+  event->Signal();
+}
+
 int32_t WebRtcAudioDeviceImpl::CPULoad(uint16_t* load) const {
   NOTIMPLEMENTED();
   return -1;
 }
 
 int32_t WebRtcAudioDeviceImpl::StartRawOutputFileRecording(
     const char pcm_file_name_utf8[webrtc::kAdmMaxFileNameSize]) {
   NOTIMPLEMENTED();
   return -1;
 }
@@ skipping 61 matching lines @@
     size_t buffer_size, float sample_rate) const {
   const int samples_per_sec = static_cast<int>(sample_rate);
   const int samples_per_10_msec = (samples_per_sec / 100);
   bool size_is_valid = (((buffer_size % samples_per_10_msec) == 0) &&
                          (buffer_size <= kMaxBufferSize));
   DLOG_IF(WARNING, !size_is_valid) << "Size of buffer must be and even "
                                    << "multiple of 10 ms and less than "
                                    << kMaxBufferSize;
   return size_is_valid;
 }