Fix processing of multiple stream status changes by renderer

media/renderers/renderer_impl.cc

Index: media/renderers/renderer_impl.cc
diff --git a/media/renderers/renderer_impl.cc b/media/renderers/renderer_impl.cc
index 210e5cc574f2437b4368c498f56b02facf5c1e61..c04e93819be8b60de21e35456357be937e25444a 100644
--- a/media/renderers/renderer_impl.cc
+++ b/media/renderers/renderer_impl.cc
@@ -208,28 +208,40 @@ void RendererImpl::StartPlayingFrom(base::TimeDelta time) {
     video_renderer_->StartPlayingFrom(time);
 }
 
-void RendererImpl::RestartStreamPlayback(DemuxerStream* stream,
+void RendererImpl::OnStreamStatusChanged(DemuxerStream* stream,
                                          bool enabled,
                                          base::TimeDelta time) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(stream);
   bool video = (stream->type() == DemuxerStream::VIDEO);
   DVLOG(1) << __func__ << (video ? " video" : " audio") << " stream=" << stream
-           << " enabled=" << stream->enabled() << " time=" << time.InSecondsF();
+           << " enabled=" << enabled << " time=" << time.InSecondsF();
   if ((state_ != STATE_PLAYING) || (audio_ended_ && video_ended_))
     return;
   if (stream->type() == DemuxerStream::VIDEO) {
     DCHECK(video_renderer_);
-    if (restarting_video_)
+    if (restarting_video_) {

[DaleCurtis, 2017/01/10 21:14:48]

Can restarting_video_ and restarting_audio_ be pulled outside of this
conditional and coalesced into a single pending_status_notifications_ queue?
Also, just use a vector for simplicity. You always insert at the end and when
removing you remove everything. See also recent chromium-dev threads on avoiding
dequeue/queue due to memory overhead.

[servolk, 2017/01/10 23:12:10]

Yes, we can do this (uploaded a new patchset), although we do need to keep the
two separate flags for restarting audio and video, in order to handle buffering
status changes correctly (e.g. if we are restarting an audio stream, then a
HAVE_NOTHING buffering status change on audio stream is expected. On the other
hand if we get HAVE_NOTHING for video stream while restarting an audio stream,
then we do want to report underflow).
Re std::queue: I guess we can use std::list here instead. We do insert always at
the end, but we remove items one-by-one from the front as we process all pending
notifications.

+      DVLOG(3) << __func__ << ": postponed stream " << stream
+               << " status change handling.";
+      pending_video_status_notifications_.push(
+          base::Bind(&RendererImpl::OnStreamStatusChanged, weak_this_, stream,
+                     enabled, time));
       return;
+    }
     restarting_video_ = true;
     video_renderer_->Flush(
         base::Bind(&RendererImpl::RestartVideoRenderer, weak_this_, time));
   } else if (stream->type() == DemuxerStream::AUDIO) {
     DCHECK(audio_renderer_);
     DCHECK(time_source_);
-    if (restarting_audio_)
+    if (restarting_audio_) {
+      DVLOG(3) << __func__ << ": postponed stream " << stream
+               << " status change handling.";
+      pending_audio_status_notifications_.push(
+          base::Bind(&RendererImpl::OnStreamStatusChanged, weak_this_, stream,
+                     enabled, time));
       return;
+    }
     restarting_audio_ = true;
     // Stop ticking (transition into paused state) in audio renderer before
     // calling Flush, since after Flush we are going to restart playback by
@@ -380,7 +392,7 @@ void RendererImpl::InitializeAudioRenderer() {
   }
 
   audio_stream->SetStreamStatusChangeCB(base::Bind(
-      &RendererImpl::RestartStreamPlayback, weak_this_, audio_stream));
+      &RendererImpl::OnStreamStatusChanged, weak_this_, audio_stream));
 
   audio_renderer_client_.reset(
       new RendererClientInternal(DemuxerStream::AUDIO, this));
@@ -429,7 +441,7 @@ void RendererImpl::InitializeVideoRenderer() {
   }
 
   video_stream->SetStreamStatusChangeCB(base::Bind(
-      &RendererImpl::RestartStreamPlayback, weak_this_, video_stream));
+      &RendererImpl::OnStreamStatusChanged, weak_this_, video_stream));
 
   video_renderer_client_.reset(
       new RendererClientInternal(DemuxerStream::VIDEO, this));
@@ -564,6 +576,7 @@ const char* BufferingStateStr(BufferingState state) {
 bool RendererImpl::HandleRestartedStreamBufferingChanges(
     DemuxerStream::Type type,
     BufferingState new_buffering_state) {
+  DCHECK(task_runner_->BelongsToCurrentThread());
   // When restarting playback we want to defer the BUFFERING_HAVE_NOTHING for
   // the stream being restarted, to allow continuing uninterrupted playback on
   // the other stream.
@@ -571,7 +584,9 @@ bool RendererImpl::HandleRestartedStreamBufferingChanges(
     if (new_buffering_state == BUFFERING_HAVE_ENOUGH) {
       DVLOG(1) << __func__ << " Got BUFFERING_HAVE_ENOUGH for video stream,"
                               " resuming playback.";
-      restarting_video_ = false;
+      task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&RendererImpl::OnVideoRestartCompleted, weak_this_));
       if (state_ == STATE_PLAYING &&
           !deferred_video_underflow_cb_.IsCancelled()) {
         // If deferred_video_underflow_cb_ wasn't triggered, then audio should
@@ -616,12 +631,32 @@ bool RendererImpl::HandleRestartedStreamBufferingChanges(
       // Now that we have decoded enough audio, pause playback momentarily to
       // ensure video renderer is synchronised with audio.
       PausePlayback();
-      restarting_audio_ = false;
+      task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&RendererImpl::OnAudioRestartCompleted, weak_this_));
     }
   }
   return false;
 }
 
+void RendererImpl::OnAudioRestartCompleted() {
+  DCHECK(restarting_audio_);
+  restarting_audio_ = false;
+  if (!pending_audio_status_notifications_.empty()) {
+    pending_audio_status_notifications_.front().Run();
+    pending_audio_status_notifications_.pop();
+  }
+}
+
+void RendererImpl::OnVideoRestartCompleted() {
+  DCHECK(restarting_video_);
+  restarting_video_ = false;
+  if (!pending_video_status_notifications_.empty()) {
+    pending_video_status_notifications_.front().Run();
+    pending_video_status_notifications_.pop();
+  }
+}
+
 void RendererImpl::OnBufferingStateChange(DemuxerStream::Type type,
                                           BufferingState new_buffering_state) {
   DCHECK((type == DemuxerStream::AUDIO) || (type == DemuxerStream::VIDEO));