Since the introduction of PushSource, there are two buffering layers in the

browser process, the hardware buffer used in AudioOutputStream and
transportation buffer in PushSource. Together with the latency in the IPC
audio layer we have a serious AV sync problem.

To compensate the delay and latency introduced by these three factors
two parameters are added in RequestAudioPacket message that include
the buffer fill level and timestamp of the request. These two parameters
are used to determine the playback delay to be used by the audio
renderer to update the pipeline with the time delta.

So we have three parameters we need to care about:
1. Hardware buffer in AudioOutputStream
2. Buffered data in PushSource
3. IPC latency

We have accurate values for 2 and 3 but not 1. We currently don't have the
API in AudioOutputStream to query the remaining buffer in the hardware
buffer. But usually there is a large amount of data in it, e.g. on Windows
400ms worth of data. Since we now detached the hardware buffer request of
OnMoreData() from the actual packet request of IPC (by the introduction of
PushSource), it is really critical to know the buffer level in the hardware.
I made a guess of this buffer level by using the amount of last buffer copy.

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

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

4 files changed, 33 insertions, 6 deletions

diff --git a/media/filters/audio_renderer_base.cc b/media/filters/audio_renderer_base.cc
index 22d275b..681e84b 100644
--- a/media/filters/audio_renderer_base.cc
+++ b/media/filters/audio_renderer_base.cc
@@ -99,10 +99,15 @@ void AudioRendererBase::OnReadComplete(Buffer* buffer_in) {
 }
 
 // TODO(scherkus): clean up FillBuffer().. it's overly complex!!
-size_t AudioRendererBase::FillBuffer(uint8* dest, size_t dest_len,
-                                     float rate) {
+size_t AudioRendererBase::FillBuffer(uint8* dest,
+                                     size_t dest_len,
+                                     float rate,
+                                     const base::TimeDelta& playback_delay) {
   size_t buffers_released = 0;
   size_t dest_written = 0;
+
+  // The timestamp of the last buffer written during the last call to
+  // FillBuffer().
   base::TimeDelta last_fill_buffer_time;
   {
     AutoLock auto_lock(lock_);
@@ -198,6 +203,14 @@ size_t AudioRendererBase::FillBuffer(uint8* dest, size_t dest_len,
 
   // Update the pipeline's time if it was set last time.
   if (last_fill_buffer_time.InMicroseconds() > 0) {
+    // Adjust the |last_fill_buffer_time| with the playback delay.
+    // TODO(hclam): If there is a playback delay, the pipeline would not be
+    // updated with a correct timestamp when the stream is played at the very
+    // end since we use decoded packets to trigger time updates. A better
+    // solution is to start a timer when an audio packet is decoded to allow
+    // finer time update events.
+    if (playback_delay < last_fill_buffer_time)
+      last_fill_buffer_time -= playback_delay;
     host_->SetTime(last_fill_buffer_time);
   }
 
diff --git a/media/filters/audio_renderer_base.h b/media/filters/audio_renderer_base.h
index 83111aa..2273c7d 100644
--- a/media/filters/audio_renderer_base.h
+++ b/media/filters/audio_renderer_base.h
@@ -58,7 +58,7 @@ class AudioRendererBase : public AudioRenderer {
   virtual void OnReadComplete(Buffer* buffer_in);
 
   // Fills the given buffer with audio data by dequeuing buffers and copying the
-  // data into the |dest|.  FillBuffer also takes care of updating the clock.
+  // data into the |dest|.  FillBuffer() also takes care of updating the clock.
   // Returns the number of bytes copied into |dest|, which may be less than
   // equal to |len|.
   //
@@ -67,8 +67,18 @@ class AudioRendererBase : public AudioRenderer {
   // enough.  In such scenarios, the callee should zero out unused portions
   // of their buffer to playback silence.
   //
+  // FillBuffer() updates the pipeline's playback timestamp. If FillBuffer() is
+  // not called at the same rate as audio samples are played, then the reported
+  // timestamp in the pipeline will be ahead of the actual audio playback. In
+  // this case |playback_delay| should be used to indicate when in the future
+  // should the filled buffer be played. If FillBuffer() is called as the audio
+  // hardware plays the buffer, then |playback_delay| should be zero.
+  //
   // Safe to call on any thread.
-  size_t FillBuffer(uint8* dest, size_t len, float rate);
+  size_t FillBuffer(uint8* dest,
+                    size_t len,
+                    float rate,
+                    const base::TimeDelta& playback_delay);
 
   // Helper to parse a media format and return whether we were successful
   // retrieving all the information we care about.
diff --git a/media/filters/audio_renderer_impl.cc b/media/filters/audio_renderer_impl.cc
index 95e1401..180aa76 100644
--- a/media/filters/audio_renderer_impl.cc
+++ b/media/filters/audio_renderer_impl.cc
@@ -57,7 +57,8 @@ size_t AudioRendererImpl::OnMoreData(AudioOutputStream* stream, void* dest_void,
   // TODO(scherkus): Maybe change OnMoreData to pass in char/uint8 or similar.
   // TODO(fbarchard): Waveout_output_win.h should handle zero length buffers
   //                  without clicking.
-  return FillBuffer(static_cast<uint8*>(dest_void), len, playback_rate_);
+  return FillBuffer(static_cast<uint8*>(dest_void), len,
+                    playback_rate_, base::TimeDelta());
 }
 
 void AudioRendererImpl::OnClose(AudioOutputStream* stream) {
diff --git a/media/filters/null_audio_renderer.cc b/media/filters/null_audio_renderer.cc
index b01e939..db72837 100644
--- a/media/filters/null_audio_renderer.cc
+++ b/media/filters/null_audio_renderer.cc
@@ -51,7 +51,10 @@ void NullAudioRenderer::ThreadMain() {
 
     // Only consume buffers when actually playing.
     if (playback_rate_ > 0.0f)  {
-      size_t bytes = FillBuffer(buffer_.get(), buffer_size_, playback_rate_);
+      size_t bytes = FillBuffer(buffer_.get(),
+                                buffer_size_,
+                                playback_rate_,
+                                base::TimeDelta());
 
       // Calculate our sleep duration, taking playback rate into consideration.
       sleep_in_milliseconds =