Fix processing of multiple stream status changes by renderer

media/renderers/renderer_impl.cc

 // Copyright 2014 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/renderers/renderer_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 190 matching lines @@
   }
 
   time_source_->SetMediaTime(time);
 
   if (audio_renderer_)
     audio_renderer_->StartPlaying();
   if (video_renderer_)
     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
     // calling audio renderer StartPlaying which would fail in playing state.
     if (time_ticking_) {
       time_ticking_ = false;
       time_source_->StopTicking();
     }
     audio_renderer_->Flush(
         base::Bind(&RendererImpl::RestartAudioRenderer, weak_this_, time));
@@ skipping 130 matching lines @@
 
   DemuxerStream* audio_stream =
       demuxer_stream_provider_->GetStream(DemuxerStream::AUDIO);
   if (!audio_stream) {
     audio_renderer_.reset();
     task_runner_->PostTask(FROM_HERE, base::Bind(done_cb, PIPELINE_OK));
     return;
   }
 
   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));
   // Note: After the initialization of a renderer, error events from it may
   // happen at any time and all future calls must guard against STATE_ERROR.
   audio_renderer_->Initialize(audio_stream, cdm_context_,
                               audio_renderer_client_.get(), done_cb);
 }
 
 void RendererImpl::OnAudioRendererInitializeDone(PipelineStatus status) {
@@ skipping 28 matching lines @@
 
   DemuxerStream* video_stream =
       demuxer_stream_provider_->GetStream(DemuxerStream::VIDEO);
   if (!video_stream) {
     video_renderer_.reset();
     task_runner_->PostTask(FROM_HERE, base::Bind(done_cb, PIPELINE_OK));
     return;
   }
 
   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));
   video_renderer_->Initialize(
       video_stream, cdm_context_, video_renderer_client_.get(),
       base::Bind(&RendererImpl::GetWallClockTimes, base::Unretained(this)),
       done_cb);
 }
 
 void RendererImpl::OnVideoRendererInitializeDone(PipelineStatus status) {
@@ skipping 114 matching lines @@
       return "HAVE_ENOUGH";
   }
   NOTREACHED();
   return "";
 }
 }
 
 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.
   if (type == DemuxerStream::VIDEO && restarting_video_) {
     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
         // still be playing, we only need to unpause the video stream.
         DVLOG(4) << "deferred_video_underflow_cb_.Cancel()";
         deferred_video_underflow_cb_.Cancel();
         video_buffering_state_ = new_buffering_state;
         if (playback_rate_ > 0)
           video_renderer_->OnTimeProgressing();
         return true;
@@ skipping 24 matching lines @@
       // Cancel the deferred callback and report the underflow immediately.
       DVLOG(4) << "deferred_audio_restart_underflow_cb_.Cancel()";
       deferred_audio_restart_underflow_cb_.Cancel();
     } else if (new_buffering_state == BUFFERING_HAVE_ENOUGH) {
       DVLOG(1) << __func__ << " Got BUFFERING_HAVE_ENOUGH for audio stream,"
                               " resuming playback.";
       deferred_audio_restart_underflow_cb_.Cancel();
       // 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));
   BufferingState* buffering_state = type == DemuxerStream::AUDIO
                                         ? &audio_buffering_state_
                                         : &video_buffering_state_;
 
   DVLOG(1) << __func__ << (type == DemuxerStream::AUDIO ? " audio " : " video ")
            << BufferingStateStr(*buffering_state) << " -> "
            << BufferingStateStr(new_buffering_state);
@@ skipping 199 matching lines @@
   DCHECK(task_runner_->BelongsToCurrentThread());
   client_->OnVideoNaturalSizeChange(size);
 }
 
 void RendererImpl::OnVideoOpacityChange(bool opaque) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   client_->OnVideoOpacityChange(opaque);
 }
 
 }  // namespace media