Fix problem when we did not play beginning of HTML5 audio stream in low latency mode.

Problem manifested itself on Mac because Mac code is especially sensitive to timing, and because Mac code ignores length of data in buffer -- it assumes buffer is always full, and gives to OS the entire buffer (zeroed out in that case).

Fix consists of 2 parts:
* When starting audio stream, send request for the data -- previously code depended on data length in the buffer being 0, so it assumed no data would be played, and data would be returned starting from 2nd request.
* Implement polling "is data ready?" when starting new stream. Start physical stream only after first chunk data is available (or after ~9ms had passed, to correctly handle "old" clients not writing metadata into buffer). Polling is not continuous, it is done by task delayed by 3ms.

Fix is definitely not necessary on Windows, where we have mechanism that handles "bad" timing when requesting new data, but I decided it is better to implement it on all platforms.

Added test.

BUG=98674

TEST=Go to http://www.jimmysparkle.me/dump/chrome-bug/ and listen to the audio stream.


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

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

3 files changed, 116 insertions, 1 deletions

diff --git a/media/audio/audio_output_controller.cc b/media/audio/audio_output_controller.cc
index b8536f9..3331701 100644
--- a/media/audio/audio_output_controller.cc
+++ b/media/audio/audio_output_controller.cc
@@ -13,6 +13,10 @@ namespace media {
 // Signal a pause in low-latency mode.
 const int AudioOutputController::kPauseMark = -1;
 
+// Polling-related constants.
+const int AudioOutputController::kPollNumAttempts = 3;
+const int AudioOutputController::kPollPauseInMilliseconds = 3;
+
 AudioOutputController::AudioOutputController(EventHandler* handler,
                                              uint32 capacity,
                                              SyncReader* sync_reader)
@@ -23,7 +27,8 @@ AudioOutputController::AudioOutputController(EventHandler* handler,
       buffer_(0, capacity),
       pending_request_(false),
       sync_reader_(sync_reader),
-      message_loop_(NULL) {
+      message_loop_(NULL),
+      number_polling_attempts_left_(0) {
 }
 
 AudioOutputController::~AudioOutputController() {
@@ -173,6 +178,43 @@ void AudioOutputController::DoPlay() {
     return;
   state_ = kPlaying;
 
+  if (LowLatencyMode()) {
+    // Ask for first packet.
+    sync_reader_->UpdatePendingBytes(0);
+
+    // Cannot start stream immediately, should give renderer some time
+    // to deliver data.
+    number_polling_attempts_left_ = kPollNumAttempts;
+    message_loop_->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&AudioOutputController::PollAndStartIfDataReady, this),
+        kPollPauseInMilliseconds);
+  } else {
+    StartStream();
+  }
+}
+
+void AudioOutputController::PollAndStartIfDataReady() {
+  DCHECK_EQ(message_loop_, MessageLoop::current());
+
+  // Being paranoic: do nothing if we were stopped/paused
+  // after DoPlay() but before DoStartStream().
+  if (state_ != kPlaying)
+    return;
+
+  if (--number_polling_attempts_left_ == 0 || sync_reader_->DataReady()) {
+    StartStream();
+  } else {
+    message_loop_->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&AudioOutputController::PollAndStartIfDataReady, this),
+        kPollPauseInMilliseconds);
+  }
+}
+
+void AudioOutputController::StartStream() {
+  DCHECK_EQ(message_loop_, MessageLoop::current());
+
   // We start the AudioOutputStream lazily.
   stream_->Start(this);
 
diff --git a/media/audio/audio_output_controller.h b/media/audio/audio_output_controller.h
index 88c4e06..2c2246b 100644
--- a/media/audio/audio_output_controller.h
+++ b/media/audio/audio_output_controller.h
@@ -103,6 +103,13 @@ class MEDIA_EXPORT AudioOutputController
 
     // Close this synchronous reader.
     virtual void Close() = 0;
+
+    // Poll if data is ready.
+    // Not reliable, as there is no guarantee that renderer is "new-style"
+    // renderer that writes metadata into buffer. After several unsuccessful
+    // attempts caller should assume the data is ready even if that function
+    // returns false.
+    virtual bool DataReady() = 0;
   };
 
   virtual ~AudioOutputController();
@@ -163,12 +170,17 @@ class MEDIA_EXPORT AudioOutputController
   virtual void OnError(AudioOutputStream* stream, int code);
 
  private:
+  // We are polling sync reader if data became available.
+  static const int kPollNumAttempts;
+  static const int kPollPauseInMilliseconds;
+
   AudioOutputController(EventHandler* handler,
                         uint32 capacity, SyncReader* sync_reader);
 
   // The following methods are executed on the audio controller thread.
   void DoCreate(const AudioParameters& params);
   void DoPlay();
+  void PollAndStartIfDataReady();
   void DoPause();
   void DoFlush();
   void DoClose(const base::Closure& closed_task);
@@ -178,6 +190,9 @@ class MEDIA_EXPORT AudioOutputController
   // Helper method to submit a OnMoreData() call to the event handler.
   void SubmitOnMoreData_Locked();
 
+  // Helper method that starts physical stream.
+  void StartStream();
+
   // |handler_| may be called only if |state_| is not kClosed.
   EventHandler* handler_;
   AudioOutputStream* stream_;
@@ -205,6 +220,10 @@ class MEDIA_EXPORT AudioOutputController
   // The message loop of audio thread that this object runs on.
   MessageLoop* message_loop_;
 
+  // When starting stream we wait for data to become available.
+  // Number of times left.
+  int number_polling_attempts_left_;
+
   DISALLOW_COPY_AND_ASSIGN(AudioOutputController);
 };
 
diff --git a/media/audio/audio_output_controller_unittest.cc b/media/audio/audio_output_controller_unittest.cc
index cab9929..f6917a7 100644
--- a/media/audio/audio_output_controller_unittest.cc
+++ b/media/audio/audio_output_controller_unittest.cc
@@ -14,6 +14,7 @@
 
 using ::testing::_;
 using ::testing::AtLeast;
+using ::testing::DoAll;
 using ::testing::Exactly;
 using ::testing::InvokeWithoutArgs;
 using ::testing::NotNull;
@@ -54,6 +55,7 @@ class MockAudioOutputControllerSyncReader
   MOCK_METHOD1(UpdatePendingBytes, void(uint32 bytes));
   MOCK_METHOD2(Read, uint32(void* data, uint32 size));
   MOCK_METHOD0(Close, void());
+  MOCK_METHOD0(DataReady, bool());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockAudioOutputControllerSyncReader);
@@ -150,6 +152,58 @@ TEST(AudioOutputControllerTest, PlayAndClose) {
   CloseAudioController(controller);
 }
 
+TEST(AudioOutputControllerTest, PlayAndCloseLowLatency) {
+  if (!HasAudioOutputDevices() || IsRunningHeadless())
+    return;
+
+  MockAudioOutputControllerEventHandler event_handler;
+  base::WaitableEvent event(false, false);
+
+  // If OnCreated is called then signal the event.
+  EXPECT_CALL(event_handler, OnCreated(NotNull()))
+      .WillOnce(SignalEvent(&event));
+
+  // OnPlaying() will be called only once.
+  EXPECT_CALL(event_handler, OnPlaying(NotNull()))
+      .Times(Exactly(1));
+
+  MockAudioOutputControllerSyncReader sync_reader;
+  EXPECT_CALL(sync_reader, UpdatePendingBytes(_))
+      .Times(AtLeast(10));
+  EXPECT_CALL(sync_reader, DataReady())
+      .WillOnce(Return(false))
+      .WillOnce(Return(false))
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(sync_reader, Read(_, kHardwareBufferSize))
+      .Times(AtLeast(10))
+      .WillRepeatedly(DoAll(SignalEvent(&event),
+                            Return(1)));
+  EXPECT_CALL(sync_reader, Close());
+
+  AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR, kChannelLayout,
+                         kSampleRate, kBitsPerSample, kSamplesPerPacket);
+  scoped_refptr<AudioOutputController> controller =
+      AudioOutputController::CreateLowLatency(&event_handler,
+                                              params,
+                                              &sync_reader);
+  ASSERT_TRUE(controller.get());
+
+  // Wait for OnCreated() to be called.
+  event.Wait();
+
+  controller->Play();
+
+  // Wait until the date is requested at least 10 times.
+  for (int i = 0; i < 10; i++) {
+    event.Wait();
+    uint8 buf[1];
+    controller->EnqueueData(buf, 0);
+  }
+
+  // Now stop the controller.
+  CloseAudioController(controller);
+}
+
 TEST(AudioOutputControllerTest, PlayPauseClose) {
   if (!HasAudioOutputDevices() || IsRunningHeadless())
     return;