Have AudioRendererImpl advance time until it's told to stop.

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>
 
@@ skipping 130 matching lines @@
 
 void AudioRendererImpl::SetMediaTime(base::TimeDelta time) {
   DVLOG(1) << __FUNCTION__ << "(" << time.InMicroseconds() << ")";
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   base::AutoLock auto_lock(lock_);
   DCHECK(!rendering_);
   DCHECK_EQ(state_, kFlushed);
 
   start_timestamp_ = time;
+  ended_timestamp_ = kInfiniteDuration();
   audio_clock_.reset(new AudioClock(time, audio_parameters_.sample_rate()));
 }
 
 base::TimeDelta AudioRendererImpl::CurrentMediaTime() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // TODO(scherkus): Finish implementing when ready to switch Pipeline to using
   // TimeSource http://crbug.com/370634
   NOTIMPLEMENTED();
@@ skipping 379 matching lines @@
 }
 
 int AudioRendererImpl::Render(AudioBus* audio_bus,
                               int audio_delay_milliseconds) {
   const int requested_frames = audio_bus->frames();
   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::AutoLock auto_lock(lock_);
 
     // Ensure Stop() hasn't destroyed our |algorithm_| on the pipeline thread.
     if (!algorithm_) {
       audio_clock_->WroteAudio(
           0, requested_frames, delay_frames, playback_rate_);
       return 0;
     }
 
@@ skipping 19 matching lines @@
     // 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.
     if (algorithm_->frames_buffered() > 0) {
       frames_written =
           algorithm_->FillBuffer(audio_bus, requested_frames, playback_rate_);
     }
-    audio_clock_->WroteAudio(
-        frames_written, requested_frames, delay_frames, playback_rate_);
 
+    int frames_after_end_of_stream = 0;
     if (frames_written == 0) {
-      if (received_end_of_stream_ && !rendered_end_of_stream_ &&
-          !audio_clock_->audio_data_buffered()) {
-        rendered_end_of_stream_ = true;
-        task_runner_->PostTask(FROM_HERE, ended_cb_);
-      } else if (!received_end_of_stream_ && state_ == kPlaying) {
-        if (buffering_state_ != BUFFERING_HAVE_NOTHING) {
-          algorithm_->IncreaseQueueCapacity();
-          SetBufferingState_Locked(BUFFERING_HAVE_NOTHING);
-        }
+      if (received_end_of_stream_) {
+        if (ended_timestamp_ == kInfiniteDuration())
+          ended_timestamp_ = audio_clock_->latest_media_timestamp();
+        frames_after_end_of_stream = requested_frames;
+      } else if (state_ == kPlaying &&
+                 buffering_state_ != BUFFERING_HAVE_NOTHING) {
+        algorithm_->IncreaseQueueCapacity();
+        SetBufferingState_Locked(BUFFERING_HAVE_NOTHING);

[DaleCurtis, 2014/08/29 20:17:23]

Shouldn't you be accounting for the frames written last cycle here too?

[scherkus (not reviewing), 2014/08/29 20:32:02]

Not sure I follow.

What case are you thinking of?

[DaleCurtis, 2014/08/29 20:59:17]

Audio still advances from the last write cycle despite underflow.  Do you want
to adjust time to compensate?  I don't think I follow why you're treating the
two cases differently. 

[scherkus (not reviewing), 2014/08/29 21:29:00]

The moment we enter underflow we don't want to advance time because we know we
have more decoded audio frames that will eventually arrive when we exit
underflow. If we advanced time at this point we would throw things off.

When we know we *won't* have more decoded frames arriving (end of stream), we
keep inserting silence to keep time advancing.

I hope I understood your question! Let me know if you want to chat offline.

[DaleCurtis, 2014/08/29 22:04:42]

Okay, this makes sense, but could do with a comment explaining why :)

       }
     }
 
+    audio_clock_->WroteAudio(frames_written + frames_after_end_of_stream,
+                             requested_frames,
+                             delay_frames,
+                             playback_rate_);
+
     if (CanRead_Locked()) {
       task_runner_->PostTask(FROM_HERE,
                              base::Bind(&AudioRendererImpl::AttemptRead,
                                         weak_factory_.GetWeakPtr()));
     }
 
-    // Firing |ended_cb_| means we no longer need to run |time_cb_|.
-    if (!rendered_end_of_stream_ &&
-        last_timestamp_update_ != audio_clock_->current_media_timestamp()) {
+    if (last_timestamp_update_ != audio_clock_->current_media_timestamp()) {
       // Since |max_time| uses linear interpolation, only provide an upper bound
       // that is for audio data at the same playback rate. Failing to do so can
       // make time jump backwards when the linear interpolated time advances
       // past buffered regions of audio at different rates.
       last_timestamp_update_ = audio_clock_->current_media_timestamp();
       base::TimeDelta max_time =
           last_timestamp_update_ +
           audio_clock_->contiguous_audio_data_buffered_at_same_rate();
-      time_cb = base::Bind(time_cb_, last_timestamp_update_, max_time);
+      task_runner_->PostTask(
+          FROM_HERE, base::Bind(time_cb_, last_timestamp_update_, max_time));
+
+      if (last_timestamp_update_ >= ended_timestamp_ &&
+          !rendered_end_of_stream_) {
+        rendered_end_of_stream_ = true;
+        task_runner_->PostTask(FROM_HERE, ended_cb_);
+      }
     }
   }
 
-  if (!time_cb.is_null())
-    task_runner_->PostTask(FROM_HERE, time_cb);
-
   DCHECK_LE(frames_written, requested_frames);
   return frames_written;
 }
 
 void AudioRendererImpl::OnRenderError() {
   // UMA data tells us this happens ~0.01% of the time. Trigger an error instead
   // of trying to gracefully fall back to a fake sink. It's very likely
   // OnRenderError() should be removed and the audio stack handle errors without
   // notifying clients. See http://crbug.com/234708 for details.
   HistogramRendererEvent(RENDER_ERROR);
@@ skipping 59 matching lines @@
            << buffering_state;
   DCHECK_NE(buffering_state_, buffering_state);
   lock_.AssertAcquired();
   buffering_state_ = buffering_state;
 
   task_runner_->PostTask(FROM_HERE,
                          base::Bind(buffering_state_cb_, buffering_state_));
 }
 
 }  // namespace media