Adding audio unit tests for Android

media/audio/android/opensles_input.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 "media/audio/android/opensles_input.h"
 
 #include "base/logging.h"
 #include "media/audio/android/audio_manager_android.h"
 
 #define LOG_ON_FAILURE_AND_RETURN(op, ...) \
   do { \
     SLresult err = (op);            \
     if (err != SL_RESULT_SUCCESS) { \
       DLOG(ERROR) << #op << " failed: " << err; \
       return __VA_ARGS__; \
     } \
   } while (0)
 
 namespace media {
 
 OpenSLESInputStream::OpenSLESInputStream(AudioManagerAndroid* audio_manager,
                                          const AudioParameters& params)
     : audio_manager_(audio_manager),
       callback_(NULL),
       recorder_(NULL),
       simple_buffer_queue_(NULL),
-      active_queue_(0),
+      active_buffer_(0),
       buffer_size_bytes_(0),
       started_(false) {
+  DVLOG(2) << "OpenSLESInputStream::OpenSLESInputStream()";
   format_.formatType = SL_DATAFORMAT_PCM;
   format_.numChannels = static_cast<SLuint32>(params.channels());
   // Provides sampling rate in milliHertz to OpenSLES.
   format_.samplesPerSec = static_cast<SLuint32>(params.sample_rate() * 1000);
   format_.bitsPerSample = params.bits_per_sample();
   format_.containerSize = params.bits_per_sample();
   format_.endianness = SL_BYTEORDER_LITTLEENDIAN;
   if (format_.numChannels == 1)
     format_.channelMask = SL_SPEAKER_FRONT_CENTER;
   else if (format_.numChannels == 2)
     format_.channelMask = SL_SPEAKER_FRONT_LEFT | SL_SPEAKER_FRONT_RIGHT;
   else
     NOTREACHED() << "Unsupported number of channels: " << format_.numChannels;
 
   buffer_size_bytes_ = params.GetBytesPerBuffer();
 
   memset(&audio_data_, 0, sizeof(audio_data_));
 }
 
 OpenSLESInputStream::~OpenSLESInputStream() {
+  DVLOG(2) << "OpenSLESInputStream::~OpenSLESInputStream()";
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!recorder_object_.Get());
   DCHECK(!engine_object_.Get());
   DCHECK(!recorder_);
   DCHECK(!simple_buffer_queue_);
   DCHECK(!audio_data_[0]);
 }
 
 bool OpenSLESInputStream::Open() {
+  DVLOG(2) << "OpenSLESInputStream::Open()";
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (engine_object_.Get())
     return false;
 
   if (!CreateRecorder())
     return false;
 
   SetupAudioBuffer();
+  active_buffer_ = 0;
 
   return true;
 }
 
 void OpenSLESInputStream::Start(AudioInputCallback* callback) {
+  DVLOG(2) << "OpenSLESInputStream::Start()";
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(callback);
+  DCHECK(!callback_);

[tommi (sloooow) - chröme, 2013/08/30 14:37:13]

nit: keep the dcheck behind the lock

[henrika (OOO until Aug 14), 2013/08/30 14:44:27]

Done.

[henrika (OOO until Aug 14), 2013/09/02 09:36:15]

Actually, this will not work since we don't reset callback in Stop. The same is
true for all other platforms as well and the reason is that callback is used in
Close to signal OnClose and cant be cleared in Stop. Hope it is OK if I don't
add this check here now.

[tommi (sloooow) - chröme, 2013/09/02 09:49:41]

That's either a design flaw or we simply have bugs on all platforms.  If the
callback is set in Start, it should be cleared in Stop (assuming an
implementation bug).  Alternatively, the interface could be changed so that the
callback is set in Open() and then cleared in Close().  Given that the callback
has an OnClose method, I'm leaning towards design flaw.

As a temporary workaround, I'd like the DCHECK to then be to check: DCHECK(NULL
|| callback_ == callback);
As is, a call sequence of Open()/Start()/Stop()/Start()/Stop()/Close() would
potentially only call OnClose() on the second callback, not the first.

   DCHECK(recorder_);
   DCHECK(simple_buffer_queue_);
   if (started_)
     return;
 
-  // Enable the flags before streaming.
+  base::AutoLock lock(lock_);
   callback_ = callback;
-  active_queue_ = 0;
-  started_ = true;
 
-  SLresult err = SL_RESULT_UNKNOWN_ERROR;
-  // Enqueues |kNumOfQueuesInBuffer| zero buffers to get the ball rolling.
-  for (int i = 0; i < kNumOfQueuesInBuffer; ++i) {
+  SLAndroidSimpleBufferQueueState buffer_queue_state;
+  SLresult err = (*simple_buffer_queue_)->GetState(simple_buffer_queue_,
+                                                   &buffer_queue_state);
+  if (SL_RESULT_SUCCESS != err) {
+    HandleError(err);
+    return;
+  }
+
+  // Number of free buffers in the queue.
+  int num_free_buffers = kMaxNumOfBuffersInQueue - buffer_queue_state.count;
+  DCHECK(num_free_buffers == kMaxNumOfBuffersInQueue || num_free_buffers == 0);
+
+  // Enqueues |num_free_buffers| zero buffers to get the ball rolling.
+  // |num_free_buffers| can be zero if Stop has been called followed by a
+  // new call to Start. We do clear the queue in Stop, and the buffer state
+  // should then be cleared, but for some reason it is not. Using this
+  // approach enables call sequences like: Start, Stop, Start, even if there
+  // might be some old data remaining at the second call to Start.
+  for (int i = 0; i < num_free_buffers; ++i) {
     err = (*simple_buffer_queue_)->Enqueue(
         simple_buffer_queue_,
         audio_data_[i],
         buffer_size_bytes_);
     if (SL_RESULT_SUCCESS != err) {
       HandleError(err);
       return;
     }
   }
 
   // Start the recording by setting the state to |SL_RECORDSTATE_RECORDING|.
   err = (*recorder_)->SetRecordState(recorder_, SL_RECORDSTATE_RECORDING);
-  if (SL_RESULT_SUCCESS != err)
+  if (SL_RESULT_SUCCESS != err) {
     HandleError(err);
+  }
+
+  started_ = true;

[tommi (sloooow) - chröme, 2013/08/30 14:37:13]

if we failed to set the state to 'recording' I don't think we should flip this
switch.
Stop() basically just tries to flip the switch back, which won't work if we
didn't succeed in the first place.

[henrika (OOO until Aug 14), 2013/08/30 14:44:27]

Done.

 }
 
 void OpenSLESInputStream::Stop() {
+  DVLOG(2) << "OpenSLESInputStream::Stop()";
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (!started_)
     return;
 
+  base::AutoLock lock(lock_);
+
   // Stop recording by setting the record state to |SL_RECORDSTATE_STOPPED|.
   LOG_ON_FAILURE_AND_RETURN(
       (*recorder_)->SetRecordState(recorder_,
                                    SL_RECORDSTATE_STOPPED));
 
   // Clear the buffer queue to get rid of old data when resuming recording.
   LOG_ON_FAILURE_AND_RETURN(
       (*simple_buffer_queue_)->Clear(simple_buffer_queue_));
 
   started_ = false;
 }
 
 void OpenSLESInputStream::Close() {
+  DVLOG(2) << "OpenSLESInputStream::Close()";
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   // Stop the stream if it is still recording.
   Stop();
+  {
+    base::AutoLock lock(lock_);
 
-  // Explicitly free the player objects and invalidate their associated
-  // interfaces. They have to be done in the correct order.
-  recorder_object_.Reset();
-  engine_object_.Reset();
-  simple_buffer_queue_ = NULL;
-  recorder_ = NULL;
+    if (callback_) {
+      callback_->OnClose(this);
+      callback_ = NULL;
+    }
 
-  ReleaseAudioBuffer();
+    // Destroy the buffer queue recorder object and invalidate all associated
+    // interfaces.
+    recorder_object_.Reset();
+    simple_buffer_queue_ = NULL;
+    recorder_ = NULL;
+
+    // Destroy the engine object. We don't store any associated interface for
+    // this object.
+    engine_object_.Reset();
+    ReleaseAudioBuffer();
+  }
 
   audio_manager_->ReleaseInputStream(this);
 }
 
 double OpenSLESInputStream::GetMaxVolume() {
   NOTIMPLEMENTED();
   return 0.0;
 }
 
 void OpenSLESInputStream::SetVolume(double volume) {
   NOTIMPLEMENTED();
 }
 
 double OpenSLESInputStream::GetVolume() {
   NOTIMPLEMENTED();
   return 0.0;
 }
 
 void OpenSLESInputStream::SetAutomaticGainControl(bool enabled) {
   NOTIMPLEMENTED();
 }
 
 bool OpenSLESInputStream::GetAutomaticGainControl() {
   NOTIMPLEMENTED();
   return false;
 }
 
 bool OpenSLESInputStream::CreateRecorder() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!engine_object_.Get());
+  DCHECK(!recorder_object_.Get());
+  DCHECK(!recorder_);
+  DCHECK(!simple_buffer_queue_);
+
   // Initializes the engine object with specific option. After working with the
   // object, we need to free the object and its resources.
   SLEngineOption option[] = {
     { SL_ENGINEOPTION_THREADSAFE, static_cast<SLuint32>(SL_BOOLEAN_TRUE) }
   };
   LOG_ON_FAILURE_AND_RETURN(slCreateEngine(engine_object_.Receive(),
                                            1,
                                            option,
                                            0,
                                            NULL,
@@ skipping 15 matching lines @@
   // Audio source configuration.
   SLDataLocator_IODevice mic_locator = {
     SL_DATALOCATOR_IODEVICE, SL_IODEVICE_AUDIOINPUT,
     SL_DEFAULTDEVICEID_AUDIOINPUT, NULL
   };
   SLDataSource audio_source = { &mic_locator, NULL };
 
   // Audio sink configuration.
   SLDataLocator_AndroidSimpleBufferQueue buffer_queue = {
     SL_DATALOCATOR_ANDROIDSIMPLEBUFFERQUEUE,  // Locator type.
-    static_cast<SLuint32>(kNumOfQueuesInBuffer)  // Number of buffers.
+    static_cast<SLuint32>(kMaxNumOfBuffersInQueue)  // Number of buffers.
   };
   SLDataSink audio_sink = { &buffer_queue, &format_ };
 
   // Create an audio recorder.
   const SLInterfaceID interface_id[] = {
     SL_IID_ANDROIDSIMPLEBUFFERQUEUE,
     SL_IID_ANDROIDCONFIGURATION
   };
   const SLboolean interface_required[] = {
     SL_BOOLEAN_TRUE,
     SL_BOOLEAN_TRUE
   };
+
   // Create AudioRecorder and specify SL_IID_ANDROIDCONFIGURATION.
   LOG_ON_FAILURE_AND_RETURN(
       (*engine)->CreateAudioRecorder(engine,
                                      recorder_object_.Receive(),
                                      &audio_source,
                                      &audio_sink,
                                      arraysize(interface_id),
                                      interface_id,
                                      interface_required),
       false);
 
   SLAndroidConfigurationItf recorder_config;
   LOG_ON_FAILURE_AND_RETURN(
       recorder_object_->GetInterface(recorder_object_.Get(),
                                      SL_IID_ANDROIDCONFIGURATION,
                                      &recorder_config),
       false);
 
+  // Uses the main microphone tuned for audio communications.
   SLint32 stream_type = SL_ANDROID_RECORDING_PRESET_VOICE_COMMUNICATION;
   LOG_ON_FAILURE_AND_RETURN(
       (*recorder_config)->SetConfiguration(recorder_config,
                                            SL_ANDROID_KEY_RECORDING_PRESET,
                                            &stream_type, sizeof(SLint32)),
       false);
 
   // Realize the recorder object in synchronous mode.
   LOG_ON_FAILURE_AND_RETURN(
       recorder_object_->Realize(recorder_object_.Get(),
@@ skipping 26 matching lines @@
 }
 
 void OpenSLESInputStream::SimpleBufferQueueCallback(
     SLAndroidSimpleBufferQueueItf buffer_queue, void* instance) {
   OpenSLESInputStream* stream =
       reinterpret_cast<OpenSLESInputStream*>(instance);
   stream->ReadBufferQueue();
 }
 
 void OpenSLESInputStream::ReadBufferQueue() {
-  if (!started_)
+  base::AutoLock lock(lock_);
+
+  // Verify that we are in a recording state.
+  SLuint32 state;
+  SLresult err = (*recorder_)->GetRecordState(recorder_, &state);
+  if (SL_RESULT_SUCCESS != err) {
+    HandleError(err);
     return;
+  }
+  if (state != SL_RECORDSTATE_RECORDING) {
+    DLOG(WARNING) << "Received callback in non-recording state";
+    return;
+  }
 
   // TODO(xians): Get an accurate delay estimation.
   callback_->OnData(this,
-                    audio_data_[active_queue_],
+                    audio_data_[active_buffer_],
                     buffer_size_bytes_,
                     buffer_size_bytes_,
                     0.0);
 
   // Done with this buffer. Send it to device for recording.
-  SLresult err = (*simple_buffer_queue_)->Enqueue(
+  err = (*simple_buffer_queue_)->Enqueue(

[tommi (sloooow) - chröme, 2013/08/30 14:37:13]

I just noticed that a buffer_queue pointer is supplied to the callback.  Would
it make sense to dcheck that it's the same pointer as we use here or should we
use the pointer that's given to us?

[henrika (OOO until Aug 14), 2013/08/30 14:44:27]

Offline discussions => we decided that I could ignore this comment.

       simple_buffer_queue_,
-      audio_data_[active_queue_],
+      audio_data_[active_buffer_],
       buffer_size_bytes_);
   if (SL_RESULT_SUCCESS != err)
     HandleError(err);
 
-  active_queue_ = (active_queue_ + 1) % kNumOfQueuesInBuffer;
+  active_buffer_ = (active_buffer_ + 1) % kMaxNumOfBuffersInQueue;
 }
 
 void OpenSLESInputStream::SetupAudioBuffer() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!audio_data_[0]);
-  for (int i = 0; i < kNumOfQueuesInBuffer; ++i) {
+  for (int i = 0; i < kMaxNumOfBuffersInQueue; ++i) {
     audio_data_[i] = new uint8[buffer_size_bytes_];
   }
 }
 
 void OpenSLESInputStream::ReleaseAudioBuffer() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   if (audio_data_[0]) {
-    for (int i = 0; i < kNumOfQueuesInBuffer; ++i) {
+    for (int i = 0; i < kMaxNumOfBuffersInQueue; ++i) {
       delete [] audio_data_[i];
       audio_data_[i] = NULL;
     }
   }
 }
 
 void OpenSLESInputStream::HandleError(SLresult error) {
-  DLOG(FATAL) << "OpenSLES Input error " << error;
+  DLOG(ERROR) << "OpenSLES Input error " << error;
   if (callback_)
     callback_->OnError(this);
 }
 
 }  // namespace media