Decouple 'give me more data' and 'rendered end of stream' audio callbacks.

content/renderer/media/audio_renderer_impl.cc

Index: content/renderer/media/audio_renderer_impl.cc
===================================================================
--- content/renderer/media/audio_renderer_impl.cc	(revision 122285)
+++ content/renderer/media/audio_renderer_impl.cc	(working copy)
@@ -39,12 +39,15 @@
 AudioRendererImpl::AudioRendererImpl(media::AudioRendererSink* sink)
     : AudioRendererBase(),
       bytes_per_second_(0),
-      stopped_(false),
+      state_(NOT_PLAYING),
       sink_(sink),
       is_initialized_(false) {
 }
 
 AudioRendererImpl::~AudioRendererImpl() {
+  DCHECK(signal_end_of_stream_callback_.IsCancelled());
+  if (!signal_end_of_stream_callback_.IsCancelled())
+    signal_end_of_stream_callback_.Cancel();
 }
 
 base::TimeDelta AudioRendererImpl::ConvertToDuration(int bytes) {
@@ -66,9 +69,10 @@
           static_cast<int64>(ceil(predicted_play_time.InMicroseconds() *
                                   playback_rate)));
     }
-    earliest_end_time_ =
+    base::AutoLock auto_lock(audio_renderer_impl_lock_);
+    set_earliest_end_time(
         std::max(earliest_end_time_,
-                 time_now + request_delay + predicted_play_time);
+                 time_now + request_delay + predicted_play_time));
   }
 }
 
@@ -106,20 +110,20 @@
 }
 
 void AudioRendererImpl::OnStop() {
-  if (stopped_)
+  base::AutoLock auto_lock(audio_renderer_impl_lock_);
+  if (state_ == STOPPED)
     return;
-
+  signal_end_of_stream_callback_.Cancel();
   DCHECK(sink_.get());
   sink_->Stop();
-
-  stopped_ = true;
+  state_ = STOPPED;
 }
 
 void AudioRendererImpl::SetPlaybackRate(float rate) {
   DCHECK_LE(0.0f, rate);
 
   // Handle the case where we stopped due to IO message loop dying.
-  if (stopped_) {
+  if (stopped()) {
     AudioRendererBase::SetPlaybackRate(rate);
     return;
   }
@@ -138,7 +142,7 @@
 
 void AudioRendererImpl::Pause(const base::Closure& callback) {
   AudioRendererBase::Pause(callback);
-  if (stopped_)
+  if (stopped())
     return;
 
   DoPause();
@@ -147,7 +151,7 @@
 void AudioRendererImpl::Seek(base::TimeDelta time,
                              const media::FilterStatusCB& cb) {
   AudioRendererBase::Seek(time, cb);
-  if (stopped_)
+  if (stopped())
     return;
 
   DoSeek();
@@ -155,7 +159,7 @@
 
 void AudioRendererImpl::Play(const base::Closure& callback) {
   AudioRendererBase::Play(callback);
-  if (stopped_)
+  if (stopped())
     return;
 
   if (GetPlaybackRate() != 0.0f) {
@@ -166,25 +170,47 @@
 }
 
 void AudioRendererImpl::SetVolume(float volume) {
-  if (stopped_)
+  if (stopped())
     return;
+
   DCHECK(sink_.get());
   sink_->SetVolume(volume);
 }
 
+void AudioRendererImpl::DoSetOrCheckMessageLoopProxy() {
+  if (pipeline_message_loop_proxy_.get() == NULL) {
+    pipeline_message_loop_proxy_ = current_message_loop_proxy();
+  } else {
+    DCHECK_EQ(pipeline_message_loop_proxy_.get(), current_message_loop_proxy());
+  }
+}
+
 void AudioRendererImpl::DoPlay() {
-  earliest_end_time_ = base::Time::Now();
+  DoSetOrCheckMessageLoopProxy();
+  base::AutoLock auto_lock(audio_renderer_impl_lock_);
+  set_earliest_end_time(base::Time::Now());
+  signal_end_of_stream_callback_.Reset(
+    base::Bind(&AudioRendererImpl::DoSignalEndOfStream, AsWeakPtr()));

[scherkus (not reviewing), 2012/03/03 02:24:32]

are weak pointers enforced by cancellable callbacks?

seems strange considering AudioRendererImpl is refcounted

+  state_ = PLAYING;
   DCHECK(sink_.get());
   sink_->Play();
 }
 
 void AudioRendererImpl::DoPause() {
+  DoSetOrCheckMessageLoopProxy();

[scherkus (not reviewing), 2012/03/03 02:24:32]

I would strongly prefer having pipeline set the message loop as opposed to
hijacking it via these methods.

+  base::AutoLock auto_lock(audio_renderer_impl_lock_);
+  signal_end_of_stream_callback_.Cancel();
+  state_ = NOT_PLAYING;
   DCHECK(sink_.get());
   sink_->Pause(false);
 }
 
 void AudioRendererImpl::DoSeek() {
-  earliest_end_time_ = base::Time::Now();
+  DoSetOrCheckMessageLoopProxy();
+  base::AutoLock auto_lock(audio_renderer_impl_lock_);
+  signal_end_of_stream_callback_.Cancel();
+  state_ = NOT_PLAYING;
+  set_earliest_end_time(base::Time::Now());
 
   // Pause and flush the stream when we seek to a new location.
   DCHECK(sink_.get());
@@ -194,10 +220,10 @@
 size_t AudioRendererImpl::Render(const std::vector<float*>& audio_data,
                                  size_t number_of_frames,
                                  size_t audio_delay_milliseconds) {
-  if (stopped_ || GetPlaybackRate() == 0.0f) {
+  if (state_ == STOPPED || GetPlaybackRate() == 0.0f) {
     // Output silence if stopped.
     for (size_t i = 0; i < audio_data.size(); ++i)
-      memset(audio_data[i], 0, sizeof(float) * number_of_frames);
+      memset(audio_data[i], 0, sizeof(audio_data[i][0]) * number_of_frames);
     return 0;
   }
 
@@ -239,7 +265,7 @@
       int frames_to_zero = number_of_frames - filled_frames;
       memset(audio_data[channel_index] + filled_frames,
              0,
-             sizeof(float) * frames_to_zero);
+             sizeof(audio_data[channel_index][0]) * frames_to_zero);
     }
   }
   return filled_frames;
@@ -249,8 +275,31 @@
   host()->DisableAudioRenderer();
 }
 
+int64 AudioRendererImpl::OnRenderEndOfStreamDelay() {
+  audio_renderer_impl_lock_.AssertAcquired();
+  return (earliest_end_time() - base::Time::Now()).InMilliseconds();
+}
+
 void AudioRendererImpl::OnRenderEndOfStream() {
-  // TODO(enal): schedule callback instead of polling.
-  if (base::Time::Now() >= earliest_end_time_)
-    SignalEndOfStream();
+  base::AutoLock auto_lock(audio_renderer_impl_lock_);
+  if (state_ == PLAYING) {
+    state_ = ENDED_EVENT_SCHEDULED;
+    int64 delay_ms = OnRenderEndOfStreamDelay();
+    if (delay_ms <= 0) {

[scherkus (not reviewing), 2012/03/03 02:24:32]

if the delay time was calculated to be <=0 wouldn't we want to call PDT with a
time of 0? otherwise the task will get put behind whatever other tasks are
queued when using PostTask

in which case using std::max(0, delay) would work

+      pipeline_message_loop_proxy_->PostTask(
+          FROM_HERE,
+          signal_end_of_stream_callback_.callback());
+    } else {
+      pipeline_message_loop_proxy_->PostDelayedTask(
+          FROM_HERE,
+          signal_end_of_stream_callback_.callback(),
+          delay_ms);
+    }
+  }
 }
+
+void AudioRendererImpl::DoSignalEndOfStream() {
+  DCHECK_EQ(pipeline_message_loop_proxy_.get(), current_message_loop_proxy());
+  SignalEndOfStream();
+  signal_end_of_stream_callback_.Cancel();
+}