Introduce AudioClock to improve playback delay calculations.

media/filters/audio_renderer_impl.cc

Index: media/filters/audio_renderer_impl.cc
diff --git a/media/filters/audio_renderer_impl.cc b/media/filters/audio_renderer_impl.cc
index 4f71d3d81cc9d636e9d28b469ef75ba05d0f746e..b73cb2ac473d05ddec28716d498678fbfad95d07 100644
--- a/media/filters/audio_renderer_impl.cc
+++ b/media/filters/audio_renderer_impl.cc
@@ -20,6 +20,7 @@
 #include "media/base/audio_splicer.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/demuxer_stream.h"
+#include "media/filters/audio_clock.h"
 #include "media/filters/decrypting_demuxer_stream.h"
 
 namespace media {
@@ -57,8 +58,6 @@ AudioRendererImpl::AudioRendererImpl(
       pending_read_(false),
       received_end_of_stream_(false),
       rendered_end_of_stream_(false),
-      audio_time_buffered_(kNoTimestamp()),
-      current_time_(kNoTimestamp()),
       underflow_disabled_(false),
       preroll_aborted_(false),
       weak_factory_(this) {
@@ -169,8 +168,7 @@ void AudioRendererImpl::ResetDecoderDone() {
     DCHECK_EQ(state_, kPaused);
     DCHECK(!flush_cb_.is_null());
 
-    audio_time_buffered_ = kNoTimestamp();
-    current_time_ = kNoTimestamp();
+    audio_clock_.reset(new AudioClock(audio_parameters_.sample_rate()));

[DaleCurtis, 2014/04/30 20:36:33]

Kind of stinks to allocate a whole new object. You could have a Reset() method
which just clears buffered_audio_.

[scherkus (not reviewing), 2014/05/02 19:26:05]

Considering the low frequency of this operation I feel it's fine. This way
there's less uncertainty around maintaining a Reset() method.

     received_end_of_stream_ = false;
     rendered_end_of_stream_ = false;
     preroll_aborted_ = false;
@@ -288,6 +286,8 @@ void AudioRendererImpl::Initialize(DemuxerStream* stream,
                             hardware_config_->GetHighLatencyBufferSize());
   }
 
+  audio_clock_.reset(new AudioClock(audio_parameters_.sample_rate()));
+
   audio_buffer_stream_.Initialize(
       stream,
       statistics_cb,
@@ -564,27 +564,33 @@ bool AudioRendererImpl::IsBeforePrerollTime(
 int AudioRendererImpl::Render(AudioBus* audio_bus,
                               int audio_delay_milliseconds) {
   const int requested_frames = audio_bus->frames();
-  base::TimeDelta current_time = kNoTimestamp();
-  base::TimeDelta max_time = kNoTimestamp();
   base::TimeDelta playback_delay = base::TimeDelta::FromMilliseconds(
       audio_delay_milliseconds);
-
+  const int delay_frames = static_cast<int>(playback_delay.InSecondsF() *
+                                            audio_parameters_.sample_rate());
   int frames_written = 0;
+  base::Closure time_cb;
   base::Closure underflow_cb;
   {
     base::AutoLock auto_lock(lock_);
 
     // Ensure Stop() hasn't destroyed our |algorithm_| on the pipeline thread.
-    if (!algorithm_)
+    if (!algorithm_) {
+      audio_clock_->WroteSilence(requested_frames, delay_frames);

[DaleCurtis, 2014/04/30 20:36:33]

This should only be true after Stop() has been called, so you don't really need
this call. Up to you though.

[scherkus (not reviewing), 2014/05/02 19:26:05]

Will leave it in for now ... I suspect this function will get cleaned up some
more

       return 0;
+    }
 
     float playback_rate = algorithm_->playback_rate();
-    if (playback_rate == 0)
+    if (playback_rate == 0) {
+      audio_clock_->WroteSilence(requested_frames, delay_frames);
       return 0;
+    }
 
     // Mute audio by returning 0 when not playing.
-    if (state_ != kPlaying)
+    if (state_ != kPlaying) {
+      audio_clock_->WroteSilence(requested_frames, delay_frames);
       return 0;
+    }
 
     // We use the following conditions to determine end of playback:
     //   1) Algorithm can not fill the audio callback buffer
@@ -601,8 +607,13 @@ int AudioRendererImpl::Render(AudioBus* audio_bus,
     //   3) We are in the kPlaying state
     //
     // Otherwise the buffer has data we can send to the device.
-    const base::TimeDelta time_before_filling = algorithm_->GetTime();
+    const base::TimeDelta media_timestamp_before_filling =
+        audio_clock_->CurrentMediaTimestamp();
     frames_written = algorithm_->FillBuffer(audio_bus, requested_frames);
+    audio_clock_->WroteAudio(
+        frames_written, delay_frames, playback_rate, algorithm_->GetTime());
+    audio_clock_->WroteSilence(requested_frames - frames_written, delay_frames);
+
     if (frames_written == 0) {
       const base::TimeTicks now = now_cb_.Run();
 
@@ -627,55 +638,36 @@ int AudioRendererImpl::Render(AudioBus* audio_bus,
                                         weak_factory_.GetWeakPtr()));
     }
 
-    // Adjust the delay according to playback rate.
-    base::TimeDelta adjusted_playback_delay = base::TimeDelta::FromMicroseconds(
-        ceil(playback_delay.InMicroseconds() * playback_rate));
-
-    // The |audio_time_buffered_| is the ending timestamp of the last frame
-    // buffered at the audio device. |playback_delay| is the amount of time
-    // buffered at the audio device. The current time can be computed by their
-    // difference.
-    if (audio_time_buffered_ != kNoTimestamp()) {
-      base::TimeDelta previous_time = current_time_;
-      current_time_ = audio_time_buffered_ - adjusted_playback_delay;
-
-      // Time can change in one of two ways:
-      //   1) The time of the audio data at the audio device changed, or
-      //   2) The playback delay value has changed
-      //
-      // We only want to set |current_time| (and thus execute |time_cb_|) if
-      // time has progressed and we haven't signaled end of stream yet.
-      //
-      // Why? The current latency of the system results in getting the last call
-      // to FillBuffer() later than we'd like, which delays firing the 'ended'
-      // event, which delays the looping/trigging performance of short sound
-      // effects.
-      //
-      // TODO(scherkus): revisit this and switch back to relying on playback
-      // delay after we've revamped our audio IPC subsystem.
-      if (current_time_ > previous_time && !rendered_end_of_stream_) {
-        current_time = current_time_;
-      }
-    } else if (frames_written > 0) {
-      // Nothing has been buffered yet, so use the first buffer's timestamp.
-      DCHECK(time_before_filling != kNoTimestamp());
-      current_time_ = current_time =
-          time_before_filling - adjusted_playback_delay;
+    // Time can change in one of two ways:
+    //   1) The time of the audio data at the audio device changed, or
+    //   2) The playback delay value has changed
+    //
+    // We only want to execute |time_cb_| if time has progressed and we haven't
+    // signaled end of stream yet.
+    //
+    // Why? The current latency of the system results in getting the last call
+    // to FillBuffer() later than we'd like, which delays firing the 'ended'
+    // event, which delays the looping/trigging performance of short sound
+    // effects.
+    //
+    // TODO(scherkus): revisit this and switch back to relying on playback
+    // delay after we've revamped our audio IPC subsystem.

[scherkus (not reviewing), 2014/04/30 19:13:58]

I'm not sure how much of this whole comment block is useful anymore

WDYT?

[DaleCurtis, 2014/04/30 20:36:33]

Agreed.

[scherkus (not reviewing), 2014/05/02 19:26:05]

Trimmed it down.

+    if (media_timestamp_before_filling !=
+            audio_clock_->CurrentMediaTimestamp() &&
+        !rendered_end_of_stream_) {
+      time_cb = base::Bind(time_cb_,
+                           audio_clock_->CurrentMediaTimestamp(),
+                           audio_clock_->LastEndpointTimestamp());
     }
 
-    // The call to FillBuffer() on |algorithm_| has increased the amount of
-    // buffered audio data. Update the new amount of time buffered.
-    max_time = algorithm_->GetTime();
-    audio_time_buffered_ = max_time;
-
     if (frames_written > 0) {
       UpdateEarliestEndTime_Locked(
           frames_written, playback_delay, now_cb_.Run());
     }
   }
 
-  if (current_time != kNoTimestamp() && max_time != kNoTimestamp())
-    time_cb_.Run(current_time, max_time);
+  if (!time_cb.is_null())
+    time_cb.Run();
 
   if (!underflow_cb.is_null())
     underflow_cb.Run();