Introduce AudioClock to improve playback delay calculations.

media/filters/audio_renderer_impl.cc

 // Copyright (c) 2012 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/filters/audio_renderer_impl.h"
 
 #include <math.h>
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/single_thread_task_runner.h"
 #include "media/base/audio_buffer.h"
 #include "media/base/audio_buffer_converter.h"
 #include "media/base/audio_hardware_config.h"
 #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 {
 
 namespace {
 
 enum AudioRendererEvent {
   INITIALIZED,
   RENDER_ERROR,
   RENDER_EVENT_MAX = RENDER_ERROR,
@@ skipping 17 matching lines @@
       audio_buffer_stream_(task_runner,
                            decoders.Pass(),
                            set_decryptor_ready_cb),
       hardware_config_(hardware_config),
       now_cb_(base::Bind(&base::TimeTicks::Now)),
       state_(kUninitialized),
       sink_playing_(false),
       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) {
   audio_buffer_stream_.set_splice_observer(base::Bind(
       &AudioRendererImpl::OnNewSpliceBuffer, weak_factory_.GetWeakPtr()));
   audio_buffer_stream_.set_config_change_observer(base::Bind(
       &AudioRendererImpl::OnConfigChange, weak_factory_.GetWeakPtr()));
 }
 
 AudioRendererImpl::~AudioRendererImpl() {
@@ skipping 90 matching lines @@
 void AudioRendererImpl::ResetDecoderDone() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   {
     base::AutoLock auto_lock(lock_);
     if (state_ == kStopped)
       return;
 
     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;
 
     earliest_end_time_ = now_cb_.Run();
     splicer_->Reset();
     if (buffer_converter_)
       buffer_converter_->Reset();
     algorithm_->FlushBuffers();
   }
@@ skipping 97 matching lines @@
     const AudioParameters& hw_params = hardware_config_->GetOutputConfig();
     audio_parameters_.Reset(hw_params.format(),
                             hw_params.channel_layout(),
                             hw_params.channels(),
                             hw_params.input_channels(),
                             hw_params.sample_rate(),
                             hw_params.bits_per_sample(),
                             hardware_config_->GetHighLatencyBufferSize());
   }
 
+  audio_clock_.reset(new AudioClock(audio_parameters_.sample_rate()));
+
   audio_buffer_stream_.Initialize(
       stream,
       statistics_cb,
       base::Bind(&AudioRendererImpl::OnAudioBufferStreamInitialized,
                  weak_factory_.GetWeakPtr()));
 }
 
 void AudioRendererImpl::OnAudioBufferStreamInitialized(bool success) {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
@@ skipping 256 matching lines @@
 bool AudioRendererImpl::IsBeforePrerollTime(
     const scoped_refptr<AudioBuffer>& buffer) {
   DCHECK_EQ(state_, kPrerolling);
   return buffer && !buffer->end_of_stream() &&
          (buffer->timestamp() + buffer->duration()) < preroll_timestamp_;
 }
 
 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
     //   2) We received an end of stream buffer
     //   3) We haven't already signalled that we've ended
     //   4) Our estimated earliest end time has expired
     //
     // TODO(enal): we should replace (4) with a check that the browser has no
     // more audio data or at least use a delayed callback.
     //
     // We use the following conditions to determine underflow:
     //   1) Algorithm can not fill the audio callback buffer
     //   2) We have NOT received an end of stream buffer
     //   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();
 
       if (received_end_of_stream_ && !rendered_end_of_stream_ &&
           now >= earliest_end_time_) {

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

I know more of this code can get cleaned up (e.g., removing earliest_end_time_
completely...) but I'd rather do that as a separate CL

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

I agree. Is it possible to get rid of earliest_end_time_ though? I'd think we
still need that.

One optimization I thought of in the past, was that if received_end_of_stream_
&& wrote silence, to post delayed task the ended cb based on a the media clock
time. Experiments in the past showed just using earliest_end_time_ was not
accurate enough.

         rendered_end_of_stream_ = true;
         ended_cb_.Run();
       } else if (!received_end_of_stream_ && state_ == kPlaying &&
                  !underflow_disabled_) {
         ChangeState_Locked(kUnderflow);
         underflow_cb = underflow_cb_;
       } else {
         // We can't write any data this cycle. For example, we may have
         // sent all available data to the audio device while not reaching
         // |earliest_end_time_|.
       }
     }
 
     if (CanRead_Locked()) {
       task_runner_->PostTask(FROM_HERE,

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

Really this should be TryPostTask() since PostTask requires acquiring a lock and
we don't want to stall this thread. Another CL.

                              base::Bind(&AudioRendererImpl::AttemptRead,
                                         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();
 
   DCHECK_LE(frames_written, requested_frames);
   return frames_written;
 }
 
 void AudioRendererImpl::UpdateEarliestEndTime_Locked(
     int frames_filled, const base::TimeDelta& playback_delay,
@@ skipping 74 matching lines @@
   DCHECK(expecting_config_changes_);
   buffer_converter_->ResetTimestampState();
   // Drain flushed buffers from the converter so the AudioSplicer receives all
   // data ahead of any OnNewSpliceBuffer() calls.  Since discontinuities should
   // only appear after config changes, AddInput() should never fail here.
   while (buffer_converter_->HasNextBuffer())
     CHECK(splicer_->AddInput(buffer_converter_->GetNextBuffer()));
 }
 
 }  // namespace media