Some fixups for AudioController and unit tests

Some fixes to AudioController and more unit tests. AudioController is
now ready to be used by AudioRendererHost.

BUG=39885
TEST=media_unittests

Review URL: http://codereview.chromium.org/2861010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50480 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 81 insertions, 21 deletions

diff --git a/media/audio/audio_controller.cc b/media/audio/audio_controller.cc
index b30f722..21ecdfc 100644
--- a/media/audio/audio_controller.cc
+++ b/media/audio/audio_controller.cc
@@ -6,8 +6,8 @@
 
 // The following parameters limit the request buffer and packet size from the
 // renderer to avoid renderer from requesting too much memory.
-static const int kMegabytes = 1024 * 1024;
-static const int kMaxHardwareBufferSize = 2 * kMegabytes;
+static const uint32 kMegabytes = 1024 * 1024;
+static const uint32 kMaxHardwareBufferSize = 2 * kMegabytes;
 static const int kMaxChannels = 32;
 static const int kMaxBitsPerSample = 64;
 static const int kMaxSampleRate = 192000;
@@ -15,7 +15,7 @@ static const int kMaxSampleRate = 192000;
 // Return true if the parameters for creating an audio stream is valid.
 // Return false otherwise.
 static bool CheckParameters(int channels, int sample_rate,
-                            int bits_per_sample, int hardware_buffer_size) {
+                            int bits_per_sample, uint32 hardware_buffer_size) {
   if (channels <= 0 || channels > kMaxChannels)
     return false;
   if (sample_rate <= 0 || sample_rate > kMaxSampleRate)
@@ -52,7 +52,7 @@ scoped_refptr<AudioController> AudioController::Create(
     int channels,
     int sample_rate,
     int bits_per_sample,
-    int hardware_buffer_size,
+    uint32 hardware_buffer_size,
     uint32 buffer_capacity) {
 
   if (!CheckParameters(channels, sample_rate, bits_per_sample,
@@ -60,18 +60,18 @@ scoped_refptr<AudioController> AudioController::Create(
     return NULL;
 
   // Starts the audio controller thread.
-  scoped_refptr<AudioController> source = new AudioController(
+  scoped_refptr<AudioController> controller = new AudioController(
       event_handler, buffer_capacity, NULL);
 
   // Start the audio controller thread and post a task to create the
   // audio stream.
-  source->thread_.Start();
-  source->thread_.message_loop()->PostTask(
+  controller->thread_.Start();
+  controller->thread_.message_loop()->PostTask(
       FROM_HERE,
-      NewRunnableMethod(source.get(), &AudioController::DoCreate,
+      NewRunnableMethod(controller.get(), &AudioController::DoCreate,
                         format, channels, sample_rate, bits_per_sample,
                         hardware_buffer_size));
-  return source;
+  return controller;
 }
 
 // static
@@ -81,7 +81,7 @@ scoped_refptr<AudioController> AudioController::CreateLowLatency(
     int channels,
     int sample_rate,
     int bits_per_sample,
-    int hardware_buffer_size,
+    uint32 hardware_buffer_size,
     SyncReader* sync_reader) {
 
   DCHECK(sync_reader);
@@ -91,18 +91,18 @@ scoped_refptr<AudioController> AudioController::CreateLowLatency(
     return NULL;
 
   // Starts the audio controller thread.
-  scoped_refptr<AudioController> source = new AudioController(
+  scoped_refptr<AudioController> controller = new AudioController(
       event_handler, 0, sync_reader);
 
   // Start the audio controller thread and post a task to create the
   // audio stream.
-  source->thread_.Start();
-  source->thread_.message_loop()->PostTask(
+  controller->thread_.Start();
+  controller->thread_.message_loop()->PostTask(
       FROM_HERE,
-      NewRunnableMethod(source.get(), &AudioController::DoCreate,
+      NewRunnableMethod(controller.get(), &AudioController::DoCreate,
                         format, channels, sample_rate, bits_per_sample,
                         hardware_buffer_size));
-  return source;
+  return controller;
 }
 
 void AudioController::Play() {
@@ -127,7 +127,13 @@ void AudioController::Flush() {
 }
 
 void AudioController::Close() {
-  DCHECK(thread_.IsRunning());
+  if (!thread_.IsRunning()) {
+    // If the thread is not running make sure we are stopped.
+    DCHECK_EQ(kClosed, state_);
+    return;
+  }
+
+  // Wait for all tasks to complete on the audio thread.
   thread_.message_loop()->PostTask(
       FROM_HERE,
       NewRunnableMethod(this, &AudioController::DoClose));
@@ -149,8 +155,8 @@ void AudioController::EnqueueData(const uint8* data, uint32 size) {
 }
 
 void AudioController::DoCreate(AudioManager::Format format, int channels,
-                               int sample_rate, int bits_per_sample,
-                               int hardware_buffer_size) {
+                              int sample_rate, int bits_per_sample,
+                              uint32 hardware_buffer_size) {
   // Create the stream in the first place.
   stream_ = AudioManager::GetAudioManager()->MakeAudioStream(
       format, channels, sample_rate, bits_per_sample);
@@ -170,6 +176,12 @@ void AudioController::DoCreate(AudioManager::Format format, int channels,
     return;
   }
   handler_->OnCreated(this);
+
+  // If in normal latency mode then start buffering.
+  if (!LowLatencyMode()) {
+    AutoLock auto_lock(lock_);
+    SubmitOnMoreData_Locked();
+  }
 }
 
 void AudioController::DoPlay() {
diff --git a/media/audio/audio_controller.h b/media/audio/audio_controller.h
index 21d6d22..7b02af4 100644
--- a/media/audio/audio_controller.h
+++ b/media/audio/audio_controller.h
@@ -107,7 +107,7 @@ class AudioController : public base::RefCountedThreadSafe<AudioController>,
       int channels,                   // Number of channels.
       int sample_rate,                // Sampling frequency/rate.
       int bits_per_sample,            // Number of bits per sample.
-      int hardware_buffer_size,       // Size of the hardware buffer.
+      uint32 hardware_buffer_size,    // Size of the hardware buffer.
 
       // Soft limit for buffer capacity in this controller. This parameter
       // is used only in regular latency mode.
@@ -120,7 +120,7 @@ class AudioController : public base::RefCountedThreadSafe<AudioController>,
       int channels,                   // Number of channels.
       int sample_rate,                // Sampling frequency/rate.
       int bits_per_sample,            // Number of bits per sample.
-      int hardware_buffer_size,       // Size of the hardware buffer.
+      uint32 hardware_buffer_size,    // Size of the hardware buffer.
 
       // External synchronous reader for audio controller.
       SyncReader* sync_reader);
@@ -140,6 +140,9 @@ class AudioController : public base::RefCountedThreadSafe<AudioController>,
   // Closes the audio output stream and shutdown the audio controller thread.
   // This method returns only after all operations are completed. This
   // controller cannot be used after this method is called.
+  //
+  // It is safe to call this method more than once. Calls after the first one
+  // will have no effect.
   void Close();
 
   // Sets the volume of the audio output stream.
@@ -150,6 +153,8 @@ class AudioController : public base::RefCountedThreadSafe<AudioController>,
   // SyncReader is present.
   void EnqueueData(const uint8* data, uint32 size);
 
+  bool LowLatencyMode() const { return sync_reader_ != NULL; }
+
   ///////////////////////////////////////////////////////////////////////////
   // AudioSourceCallback methods.
   virtual uint32 OnMoreData(AudioOutputStream* stream, void* dest,
@@ -164,7 +169,7 @@ class AudioController : public base::RefCountedThreadSafe<AudioController>,
   // The following methods are executed on the audio controller thread.
   void DoCreate(AudioManager::Format format, int channels,
                 int sample_rate, int bits_per_sample,
-                int hardware_buffer_size);
+                uint32 hardware_buffer_size);
   void DoPlay();
   void DoPause();
   void DoFlush();
diff --git a/media/audio/audio_controller_unittest.cc b/media/audio/audio_controller_unittest.cc
index c2a30ae..0246788 100644
--- a/media/audio/audio_controller_unittest.cc
+++ b/media/audio/audio_controller_unittest.cc
@@ -71,6 +71,24 @@ ACTION_P3(SignalEvent, event, count, limit) {
   }
 }
 
+TEST(AudioControllerTest, CreateAndClose) {
+  if (!HasAudioDevices() || IsRunningHeadless())
+    return;
+
+  MockAudioControllerEventHandler event_handler;
+  scoped_refptr<AudioController> controller = AudioController::Create(
+      &event_handler, AudioManager::AUDIO_PCM_LINEAR, kChannels,
+      kSampleRate, kBitsPerSample, kHardwareBufferSize, kBufferCapacity);
+  ASSERT_TRUE(controller.get());
+
+  // Close the controller immediately.
+  controller->Close();
+
+  // TODO(hclam): Make sure releasing the reference to this
+  // object actually destruct it.
+  controller = NULL;
+}
+
 TEST(AudioControllerTest, PlayAndClose) {
   if (!HasAudioDevices() || IsRunningHeadless())
     return;
@@ -184,4 +202,23 @@ TEST(AudioControllerTest, HardwareBufferTooLarge) {
   ASSERT_FALSE(controller);
 }
 
+TEST(AudioControllerTest, CloseTwice) {
+  if (!HasAudioDevices() || IsRunningHeadless())
+    return;
+
+  MockAudioControllerEventHandler event_handler;
+  scoped_refptr<AudioController> controller = AudioController::Create(
+      &event_handler, AudioManager::AUDIO_PCM_LINEAR, kChannels,
+      kSampleRate, kBitsPerSample, kHardwareBufferSize, kBufferCapacity);
+  ASSERT_TRUE(controller.get());
+
+  // Close the controller immediately.
+  controller->Close();
+  controller->Close();
+
+  // TODO(hclam): Make sure releasing the reference to this
+  // object actually destruct it.
+  controller = NULL;
+}
+
 }  // namespace media
diff --git a/media/audio/linux/audio_manager_linux.cc b/media/audio/linux/audio_manager_linux.cc
index b3d21f5..9fa4799 100644
--- a/media/audio/linux/audio_manager_linux.cc
+++ b/media/audio/linux/audio_manager_linux.cc
@@ -59,6 +59,12 @@ AudioManagerLinux::AudioManagerLinux()
 }
 
 AudioManagerLinux::~AudioManagerLinux() {
+  // Make sure we stop the thread first. If we let the default destructor to
+  // destruct the members, we may destroy audio streams before stopping the
+  // thread, resulting an unexpected behavior.
+  // This way we make sure activities of the audio streams are all stopped
+  // before we destroy them.
+  audio_thread_.Stop();
   active_streams_.clear();
 }