Fix several WebAudioSourceProviderImpl issues.

Fixes the following issues:
- WebAudioSourceProviderImpl::Stop() does not block for outstanding
provideInput() calls to complete.  provideInput() now acquires the
sink_lock_ to ensure this is true.
- Volume adjustment does not work.  AudioBus::Scale() handles this
now.
- The sink_ was not restored to its original state after client
disconnection.  State is now tracked via a simple state machine.

BUG=233026,235048
TEST=http://chome-audio-example.s3.amazonaws.com/audio_context.html
TEST=http://images.jayisgames.com/magnetized/index.html
TEST=content_unittests --gtest_filter=WebAudioSource*

Review URL: https://chromiumcodereview.appspot.com/14256009

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

3 files changed, 289 insertions, 38 deletions

diff --git a/webkit/media/webaudiosourceprovider_impl.cc b/webkit/media/webaudiosourceprovider_impl.cc
index d0abaef..4829967 100644
--- a/webkit/media/webaudiosourceprovider_impl.cc
+++ b/webkit/media/webaudiosourceprovider_impl.cc
@@ -13,12 +13,42 @@ using WebKit::WebVector;
 
 namespace webkit_media {
 
+namespace {
+
+// Simple helper class for Try() locks.  Lock is Try()'d on construction and
+// must be checked via the locked() attribute.  If acquisition was successful
+// the lock will be released upon destruction.
+// TODO(dalecurtis): This should probably move to base/ if others start using
+// this pattern.
+class AutoTryLock {
+ public:
+  explicit AutoTryLock(base::Lock& lock)
+      : lock_(lock),
+        acquired_(lock_.Try()) {}
+
+  bool locked() const { return acquired_; }
+
+  ~AutoTryLock() {
+    if (acquired_) {
+      lock_.AssertAcquired();
+      lock_.Release();
+    }
+  }
+
+ private:
+  base::Lock& lock_;
+  const bool acquired_;
+  DISALLOW_COPY_AND_ASSIGN(AutoTryLock);
+};
+
+}  // namespace
+
 WebAudioSourceProviderImpl::WebAudioSourceProviderImpl(
     const scoped_refptr<media::AudioRendererSink>& sink)
-    : is_initialized_(false),
-      channels_(0),
+    : channels_(0),
       sample_rate_(0),
-      is_running_(false),
+      volume_(1.0),
+      state_(kStopped),
       renderer_(NULL),
       client_(NULL),
       sink_(sink) {
@@ -29,7 +59,6 @@ WebAudioSourceProviderImpl::~WebAudioSourceProviderImpl() {}
 void WebAudioSourceProviderImpl::setClient(
     WebKit::WebAudioSourceProviderClient* client) {
   base::AutoLock auto_lock(sink_lock_);
-
   if (client && client != client_) {
     // Detach the audio renderer from normal playback.
     sink_->Stop();
@@ -37,7 +66,7 @@ void WebAudioSourceProviderImpl::setClient(
     // The client will now take control by calling provideInput() periodically.
     client_ = client;
 
-    if (is_initialized_) {
+    if (renderer_) {
       // The client needs to be notified of the audio format, if available.
       // If the format is not yet available, we'll be notified later
       // when Initialize() is called.
@@ -48,65 +77,70 @@ void WebAudioSourceProviderImpl::setClient(
   } else if (!client && client_) {
     // Restore normal playback.
     client_ = NULL;
-    // TODO(crogers): We should call sink_->Play() if we're
-    // in the playing state.
+    sink_->SetVolume(volume_);
+    if (state_ >= kStarted)
+      sink_->Start();
+    if (state_ >= kPlaying)
+      sink_->Play();
   }
 }
 
 void WebAudioSourceProviderImpl::provideInput(
     const WebVector<float*>& audio_data, size_t number_of_frames) {
-  DCHECK(client_);
-
-  if (renderer_ && is_initialized_ && is_running_) {
-    // Wrap WebVector as std::vector.
-    std::vector<float*> v(audio_data.size());
-    for (size_t i = 0; i < audio_data.size(); ++i)
-      v[i] = audio_data[i];
-
-    scoped_ptr<media::AudioBus> audio_bus = media::AudioBus::WrapVector(
-        number_of_frames, v);
-
-    // TODO(crogers): figure out if we should volume scale here or in common
-    // WebAudio code.  In any case we need to take care of volume.
-    renderer_->Render(audio_bus.get(), 0);
+  DCHECK(renderer_);
+  DCHECK_EQ(static_cast<size_t>(bus_wrapper_->channels()), audio_data.size());
+  bus_wrapper_->set_frames(number_of_frames);
+  for (size_t i = 0; i < audio_data.size(); ++i)
+    bus_wrapper_->SetChannelData(i, audio_data[i]);
+
+  // Use a try lock to avoid contention in the real-time audio thread.
+  AutoTryLock auto_try_lock(sink_lock_);
+  if (!auto_try_lock.locked() || state_ != kPlaying) {
+    // Provide silence if we failed to acquire the lock or the source is not
+    // running.
+    bus_wrapper_->Zero();
     return;
   }
 
-  // Provide silence if the source is not running.
-  for (size_t i = 0; i < audio_data.size(); ++i)
-    memset(audio_data[i], 0, sizeof(*audio_data[0]) * number_of_frames);
+  DCHECK(client_);
+  renderer_->Render(bus_wrapper_.get(), 0);
+  bus_wrapper_->Scale(volume_);
 }
 
 void WebAudioSourceProviderImpl::Start() {
   base::AutoLock auto_lock(sink_lock_);
+  DCHECK_EQ(state_, kStopped);
+  state_ = kStarted;
   if (!client_)
     sink_->Start();
-  is_running_ = true;
 }
 
 void WebAudioSourceProviderImpl::Stop() {
   base::AutoLock auto_lock(sink_lock_);
+  state_ = kStopped;
   if (!client_)
     sink_->Stop();
-  is_running_ = false;
 }
 
 void WebAudioSourceProviderImpl::Play() {
   base::AutoLock auto_lock(sink_lock_);
+  DCHECK_EQ(state_, kStarted);
+  state_ = kPlaying;
   if (!client_)
     sink_->Play();
-  is_running_ = true;
 }
 
 void WebAudioSourceProviderImpl::Pause() {
   base::AutoLock auto_lock(sink_lock_);
+  DCHECK(state_ == kPlaying || state_ == kStarted);
+  state_ = kStarted;
   if (!client_)
     sink_->Pause();
-  is_running_ = false;
 }
 
 bool WebAudioSourceProviderImpl::SetVolume(double volume) {
   base::AutoLock auto_lock(sink_lock_);
+  volume_ = volume;
   if (!client_)
     sink_->SetVolume(volume);
   return true;
@@ -116,21 +150,21 @@ void WebAudioSourceProviderImpl::Initialize(
     const media::AudioParameters& params,
     RenderCallback* renderer) {
   base::AutoLock auto_lock(sink_lock_);
-  CHECK(!is_initialized_);
+  CHECK(!renderer_);
   renderer_ = renderer;
 
+  DCHECK_EQ(state_, kStopped);
   sink_->Initialize(params, renderer);
 
   // Keep track of the format in case the client hasn't yet been set.
   channels_ = params.channels();
   sample_rate_ = params.sample_rate();
+  bus_wrapper_ = media::AudioBus::CreateWrapper(channels_);
 
   if (client_) {
     // Inform WebKit about the audio stream format.
     client_->setFormat(channels_, sample_rate_);
   }
-
-  is_initialized_ = true;
 }
 
 }  // namespace webkit_media
diff --git a/webkit/media/webaudiosourceprovider_impl.h b/webkit/media/webaudiosourceprovider_impl.h
index 426d2c5..0030df5 100644
--- a/webkit/media/webaudiosourceprovider_impl.h
+++ b/webkit/media/webaudiosourceprovider_impl.h
@@ -24,10 +24,7 @@ namespace webkit_media {
 // periodically call provideInput() to render a certain number of audio
 // sample-frames using the sink's RenderCallback to get the data.
 //
-// THREAD SAFETY:
-// It is assumed that the callers to setClient() and provideInput()
-// implement appropriate locking for thread safety when making
-// these calls.  This happens in WebKit.
+// All calls are protected by a lock.
 class WebAudioSourceProviderImpl
     : public WebKit::WebAudioSourceProvider,
       public media::AudioRendererSink {
@@ -54,12 +51,13 @@ class WebAudioSourceProviderImpl
 
  private:
   // Set to true when Initialize() is called.
-  bool is_initialized_;
   int channels_;
   int sample_rate_;
+  double volume_;
 
-  // Tracks if |sink_| has been instructed to consume audio.
-  bool is_running_;
+  // Tracks the current playback state.
+  enum PlaybackState { kStopped, kStarted, kPlaying };
+  PlaybackState state_;
 
   // Where audio comes from.
   media::AudioRendererSink::RenderCallback* renderer_;
@@ -70,6 +68,7 @@ class WebAudioSourceProviderImpl
   // Where audio ends up unless overridden by |client_|.
   base::Lock sink_lock_;
   scoped_refptr<media::AudioRendererSink> sink_;
+  scoped_ptr<media::AudioBus> bus_wrapper_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(WebAudioSourceProviderImpl);
 };
diff --git a/webkit/media/webaudiosourceprovider_impl_unittest.cc b/webkit/media/webaudiosourceprovider_impl_unittest.cc
new file mode 100644
index 0000000..4202383
--- /dev/null
+++ b/webkit/media/webaudiosourceprovider_impl_unittest.cc
@@ -0,0 +1,218 @@
+// Copyright (c) 2013 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/audio_parameters.h"
+#include "media/base/fake_audio_render_callback.h"
+#include "media/base/mock_audio_renderer_sink.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebAudioSourceProviderClient.h"
+#include "webkit/media/webaudiosourceprovider_impl.h"
+
+namespace webkit_media {
+
+namespace {
+const float kTestVolume = 0.25;
+}  // namespace
+
+class WebAudioSourceProviderImplTest
+    : public testing::Test,
+      public WebKit::WebAudioSourceProviderClient {
+ public:
+  WebAudioSourceProviderImplTest()
+      : params_(media::AudioParameters::AUDIO_PCM_LINEAR,
+                media::CHANNEL_LAYOUT_STEREO, 48000, 16, 64),
+        fake_callback_(0.1),
+        mock_sink_(new media::MockAudioRendererSink()),
+        wasp_impl_(new WebAudioSourceProviderImpl(mock_sink_)) {
+  }
+
+  virtual ~WebAudioSourceProviderImplTest() {}
+
+  void CallAllSinkMethodsAndVerify(bool verify) {
+    testing::InSequence s;
+
+    EXPECT_CALL(*mock_sink_, Start()).Times(verify);
+    wasp_impl_->Start();
+
+    EXPECT_CALL(*mock_sink_, Play()).Times(verify);
+    wasp_impl_->Play();
+
+    EXPECT_CALL(*mock_sink_, Pause()).Times(verify);
+    wasp_impl_->Pause();
+
+    EXPECT_CALL(*mock_sink_, SetVolume(kTestVolume)).Times(verify);
+    wasp_impl_->SetVolume(kTestVolume);
+
+    EXPECT_CALL(*mock_sink_, Stop()).Times(verify);
+    wasp_impl_->Stop();
+
+    testing::Mock::VerifyAndClear(mock_sink_);
+  }
+
+  void SetClient(WebKit::WebAudioSourceProviderClient* client) {
+    testing::InSequence s;
+
+    if (client) {
+      EXPECT_CALL(*mock_sink_, Stop());
+      EXPECT_CALL(*this, setFormat(params_.channels(), params_.sample_rate()));
+    }
+    wasp_impl_->setClient(client);
+
+    testing::Mock::VerifyAndClear(mock_sink_);
+    testing::Mock::VerifyAndClear(this);
+  }
+
+  bool CompareBusses(const media::AudioBus* bus1, const media::AudioBus* bus2) {
+    EXPECT_EQ(bus1->channels(), bus2->channels());
+    EXPECT_EQ(bus1->frames(), bus2->frames());
+    for (int ch = 0; ch < bus1->channels(); ++ch) {
+      if (memcmp(bus1->channel(ch), bus2->channel(ch),
+                 sizeof(*bus1->channel(ch)) * bus1->frames()) != 0) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  // WebKit::WebAudioSourceProviderClient implementation.
+  MOCK_METHOD2(setFormat, void(size_t numberOfChannels, float sampleRate));
+
+ protected:
+  media::AudioParameters params_;
+  media::FakeAudioRenderCallback fake_callback_;
+  scoped_refptr<media::MockAudioRendererSink> mock_sink_;
+  scoped_refptr<WebAudioSourceProviderImpl> wasp_impl_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebAudioSourceProviderImplTest);
+};
+
+// test start/stop/play/pause all return either silence or real audio data.
+// test setVolume results in volume adjustment
+
+TEST_F(WebAudioSourceProviderImplTest, SetClientBeforeInitialize) {
+  // setClient() with a NULL client should do nothing if no client is set.
+  wasp_impl_->setClient(NULL);
+
+  EXPECT_CALL(*mock_sink_, Stop());
+  wasp_impl_->setClient(this);
+
+  // When Initialize() is called after setClient(), the params should propagate
+  // to the client via setFormat() during the call.
+  EXPECT_CALL(*this, setFormat(params_.channels(), params_.sample_rate()));
+  wasp_impl_->Initialize(params_, &fake_callback_);
+
+  // setClient() with the same client should do nothing.
+  wasp_impl_->setClient(this);
+}
+
+// Verify AudioRendererSink functionality w/ and w/o a client.
+TEST_F(WebAudioSourceProviderImplTest, SinkMethods) {
+  wasp_impl_->Initialize(params_, &fake_callback_);
+  ASSERT_EQ(mock_sink_->callback(), &fake_callback_);
+
+  // Without a client all WASP calls should fall through to the underlying sink.
+  CallAllSinkMethodsAndVerify(true);
+
+  // With a client no calls should reach the Stop()'d sink.  Also, setClient()
+  // should propagate the params provided during Initialize() at call time.
+  SetClient(this);
+  CallAllSinkMethodsAndVerify(false);
+
+  // Removing the client should cause WASP to revert to the underlying sink.
+  EXPECT_CALL(*mock_sink_, SetVolume(kTestVolume));
+  SetClient(NULL);
+  CallAllSinkMethodsAndVerify(true);
+}
+
+// Verify underlying sink state is restored after client removal.
+TEST_F(WebAudioSourceProviderImplTest, SinkStateRestored) {
+  wasp_impl_->Initialize(params_, &fake_callback_);
+
+  // Verify state set before the client is set propagates back afterward.
+  EXPECT_CALL(*mock_sink_, Start());
+  wasp_impl_->Start();
+  SetClient(this);
+
+  EXPECT_CALL(*mock_sink_, SetVolume(1.0));
+  EXPECT_CALL(*mock_sink_, Start());
+  SetClient(NULL);
+
+  // Verify state set while the client was attached propagates back afterward.
+  SetClient(this);
+  wasp_impl_->Play();
+  wasp_impl_->SetVolume(kTestVolume);
+
+  EXPECT_CALL(*mock_sink_, SetVolume(kTestVolume));
+  EXPECT_CALL(*mock_sink_, Start());
+  EXPECT_CALL(*mock_sink_, Play());
+  SetClient(NULL);
+}
+
+// Test the AudioRendererSink state machine and its effects on provideInput().
+TEST_F(WebAudioSourceProviderImplTest, ProvideInput) {
+  scoped_ptr<media::AudioBus> bus1 = media::AudioBus::Create(params_);
+  scoped_ptr<media::AudioBus> bus2 = media::AudioBus::Create(params_);
+
+  // Point the WebVector into memory owned by |bus1|.
+  WebKit::WebVector<float*> audio_data(static_cast<size_t>(bus1->channels()));
+  for (size_t i = 0; i < audio_data.size(); ++i)
+    audio_data[i] = bus1->channel(i);
+
+  wasp_impl_->Initialize(params_, &fake_callback_);
+  SetClient(this);
+
+  // Verify provideInput() is muted prior to Start() and no calls to the render
+  // callback have occurred.
+  bus1->channel(0)[0] = 1;
+  bus2->Zero();
+  wasp_impl_->provideInput(audio_data, params_.frames_per_buffer());
+  ASSERT_TRUE(CompareBusses(bus1.get(), bus2.get()));
+  ASSERT_EQ(fake_callback_.last_audio_delay_milliseconds(), -1);
+
+  wasp_impl_->Start();
+
+  // Ditto for Play().
+  bus1->channel(0)[0] = 1;
+  wasp_impl_->provideInput(audio_data, params_.frames_per_buffer());
+  ASSERT_TRUE(CompareBusses(bus1.get(), bus2.get()));
+  ASSERT_EQ(fake_callback_.last_audio_delay_milliseconds(), -1);
+
+  wasp_impl_->Play();
+
+  // Now we should get real audio data.
+  wasp_impl_->provideInput(audio_data, params_.frames_per_buffer());
+  ASSERT_FALSE(CompareBusses(bus1.get(), bus2.get()));
+
+  // Ensure volume adjustment is working.
+  fake_callback_.reset();
+  fake_callback_.Render(bus2.get(), 0);
+  bus2->Scale(kTestVolume);
+
+  fake_callback_.reset();
+  wasp_impl_->SetVolume(kTestVolume);
+  wasp_impl_->provideInput(audio_data, params_.frames_per_buffer());
+  ASSERT_TRUE(CompareBusses(bus1.get(), bus2.get()));
+
+  // Pause should return to silence.
+  wasp_impl_->Pause();
+  bus1->channel(0)[0] = 1;
+  bus2->Zero();
+  wasp_impl_->provideInput(audio_data, params_.frames_per_buffer());
+  ASSERT_TRUE(CompareBusses(bus1.get(), bus2.get()));
+
+  wasp_impl_->Play();
+
+  // Play should return real audio data again...
+  wasp_impl_->provideInput(audio_data, params_.frames_per_buffer());
+  ASSERT_FALSE(CompareBusses(bus1.get(), bus2.get()));
+
+  // Stop() should return silence.
+  wasp_impl_->Stop();
+  bus1->channel(0)[0] = 1;
+  wasp_impl_->provideInput(audio_data, params_.frames_per_buffer());
+  ASSERT_TRUE(CompareBusses(bus1.get(), bus2.get()));
+}
+
+}  // namespace webkit_media