Allow media track switching.

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 f03b2da865c7f2b02b748b4f9f7f26f2627ebd50..9ff87fbd7c46b728343d8810a96a107e9f2400c9 100644
--- a/media/renderers/renderer_impl.cc
+++ b/media/renderers/renderer_impl.cc
@@ -80,6 +80,8 @@ RendererImpl::RendererImpl(
       task_runner_(task_runner),
       audio_renderer_(std::move(audio_renderer)),
       video_renderer_(std::move(video_renderer)),
+      current_audio_stream_(nullptr),
+      current_video_stream_(nullptr),
       time_source_(NULL),
       time_ticking_(false),
       playback_rate_(0.0),
@@ -126,6 +128,8 @@ RendererImpl::~RendererImpl() {
     FinishInitialization(PIPELINE_ERROR_ABORT);
   } else if (!flush_cb_.is_null()) {
     base::ResetAndReturn(&flush_cb_).Run();
+  } else if (pending_flush_cb_) {
+    base::ResetAndReturn(&pending_flush_cb_).Run();
   }
 }
 
@@ -189,6 +193,25 @@ void RendererImpl::Flush(const base::Closure& flush_cb) {
     return;
   }
 
+  // If we are currently handling a media stream status change, then postpone
+  // Flush until after that's done (because stream status changes also flush
+  // audio_renderer_/video_renderer_ and they need to be restarted before they
+  // can be flushed again). OnStreamRestartCompleted will resume Flush
+  // processing after audio/video restart has completed and there are no other
+  // pending stream status changes.
+  if (restarting_audio_ || restarting_video_) {
+    DCHECK(!pending_flush_cb_);
+    pending_flush_cb_ = flush_cb;
+    pending_actions_.push_back(
+        base::Bind(&RendererImpl::Flush, weak_this_, pending_flush_cb_));
+    return;
+  }
+
+  if (pending_flush_cb_) {

[DaleCurtis, 2017/04/03 19:17:59]

Is this necessary? Is something calling with the same flush cb twice?

[servolk, 2017/04/03 20:27:46]

External clients aren't calling Flush twice. But we can now get to line 210 in
two slightly different ways:
1. External caller called Flush and there were no other pending actions, so we
are just going to perform Flush right away (pending_flush_cb_ will be empty
then).
2. If an external caller calls Flush while we are processing another media track
change, then Flush will be postponed (see line 205 above) and it will be
reinvoked with the same flush_cb (also saved as pending_flush_cb_ in order to be
able to access it in the destructor) after the media track status processing is
done.

The pending_flush_cb_ is non-null only for case #2. The DCHECK is just a sanity
check that we are going to perform the same Flush request that we've previously
saved at line 205, and since now we know that there are no other pending actions
and we are going to do Flush immediately, we need to reset the
|pending_flush_cb_| in order to not call Flush twice if the RendererImpl is
destroyed immediately after returning from this function.

[DaleCurtis, 2017/04/03 21:32:41]

Ah, in that case I think this should call something like FlushInternal() which
doesn't set flush_cb_. It'd be clearer if this was a base::OnceCallback() but
that can be done another time.

[servolk, 2017/04/03 22:38:33]

Wait, I don't follow. Where would we set flush_cb_ then and what would the
FlushInternal be like - just the lines 218-221 below?

[DaleCurtis, 2017/04/03 22:50:35]

I was first thinking you'd post to a Flush that doesn't set the cb, but then I
was thinking you'd never save pending_flush_cb_ and just post Flush() without
saving it. But actually (some more thinking!), the renderers and decoders are
already flushed after a track switch, so why not just clear and run
pending_flush_cb_ when the switch completes?

[servolk, 2017/04/04 00:04:13]

What I think you missed here is the fact that only ONE of renderers will be in a
'recently flushed' state after the track switch, but the Flush needs to ensure
BOTH audio AND video renderer are flushed. So we still need to flush the other
one. And initially I wanted to avoid modifying the existing code in the
RendererImpl::FlushAudio/VideoRenderer, that's why I did it like this.
We just had a discussion about this with Xiaohan, we'll see if we can simplify
this logic.

+    DCHECK(flush_cb.Equals(pending_flush_cb_));
+    base::ResetAndReturn(&pending_flush_cb_);
+  }
+
   flush_cb_ = flush_cb;
   state_ = STATE_FLUSHING;
 
@@ -223,21 +246,24 @@ void RendererImpl::OnStreamStatusChanged(DemuxerStream* stream,
   bool video = (stream->type() == DemuxerStream::VIDEO);
   DVLOG(1) << __func__ << (video ? " video" : " audio") << " stream=" << stream
            << " enabled=" << enabled << " time=" << time.InSecondsF();
-  if ((state_ != STATE_PLAYING) || (audio_ended_ && video_ended_))
+  if ((state_ != STATE_PLAYING && state_ != STATE_FLUSHING) ||
+      (audio_ended_ && video_ended_))
     return;
-  if (restarting_audio_ || restarting_video_) {
+  if (restarting_audio_ || restarting_video_ || flush_cb_) {
     DVLOG(3) << __func__ << ": postponed stream " << stream
              << " status change handling.";
-    pending_stream_status_notifications_.push_back(
-        base::Bind(&RendererImpl::OnStreamStatusChanged, weak_this_, stream,
-                   enabled, time));
+    pending_actions_.push_back(base::Bind(&RendererImpl::OnStreamStatusChanged,
+                                          weak_this_, stream, enabled, time));
     return;
   }
   if (stream->type() == DemuxerStream::VIDEO) {
     DCHECK(video_renderer_);
     restarting_video_ = true;
     video_renderer_->Flush(
-        base::Bind(&RendererImpl::RestartVideoRenderer, weak_this_, time));
+        base::Bind(stream == current_video_stream_
+                       ? &RendererImpl::RestartVideoRenderer
+                       : &RendererImpl::ReinitializeVideoRenderer,
+                   weak_this_, stream, time));
   } else if (stream->type() == DemuxerStream::AUDIO) {
     DCHECK(audio_renderer_);
     DCHECK(time_source_);
@@ -250,25 +276,88 @@ void RendererImpl::OnStreamStatusChanged(DemuxerStream* stream,
       time_source_->StopTicking();
     }
     audio_renderer_->Flush(
-        base::Bind(&RendererImpl::RestartAudioRenderer, weak_this_, time));
+        base::Bind(stream == current_audio_stream_
+                       ? &RendererImpl::RestartAudioRenderer
+                       : &RendererImpl::ReinitializeAudioRenderer,
+                   weak_this_, stream, time));
+  }
+}
+
+void RendererImpl::ReinitializeVideoRenderer(DemuxerStream* stream,
+                                             base::TimeDelta time) {
+  DVLOG(2) << __func__ << " stream=" << stream << " time=" << time.InSecondsF();
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK_NE(stream, current_video_stream_);
+
+  current_video_stream_ = stream;
+  video_renderer_->OnTimeStopped();
+  video_renderer_->Initialize(
+      stream, cdm_context_, video_renderer_client_.get(),
+      base::Bind(&RendererImpl::GetWallClockTimes, base::Unretained(this)),
+      base::Bind(&RendererImpl::OnVideoRendererReinitialized, weak_this_,
+                 stream, time));
+}
+
+void RendererImpl::OnVideoRendererReinitialized(DemuxerStream* stream,
+                                                base::TimeDelta time,
+                                                PipelineStatus status) {
+  DVLOG(2) << __func__ << ": status=" << status;
+  DCHECK_EQ(stream, current_video_stream_);
+
+  if (status != PIPELINE_OK) {
+    OnError(status);
+    return;
   }
+  RestartVideoRenderer(stream, time);
 }
 
-void RendererImpl::RestartVideoRenderer(base::TimeDelta time) {
-  DVLOG(3) << __func__;
+void RendererImpl::RestartVideoRenderer(DemuxerStream* stream,
+                                        base::TimeDelta time) {
+  DVLOG(2) << __func__ << " stream=" << stream << " time=" << time.InSecondsF();
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(video_renderer_);
-  DCHECK_EQ(state_, STATE_PLAYING);
+  DCHECK(state_ == STATE_PLAYING || state_ == STATE_FLUSHING);
+  DCHECK_EQ(stream, current_video_stream_);
+
   video_ended_ = false;
   video_renderer_->StartPlayingFrom(time);
 }
 
-void RendererImpl::RestartAudioRenderer(base::TimeDelta time) {
-  DVLOG(3) << __func__;
+void RendererImpl::ReinitializeAudioRenderer(DemuxerStream* stream,
+                                             base::TimeDelta time) {
+  DVLOG(2) << __func__ << " stream=" << stream << " time=" << time.InSecondsF();
   DCHECK(task_runner_->BelongsToCurrentThread());
-  DCHECK_EQ(state_, STATE_PLAYING);
+  DCHECK_NE(stream, current_audio_stream_);
+
+  current_audio_stream_ = stream;
+  audio_renderer_->Initialize(
+      stream, cdm_context_, audio_renderer_client_.get(),
+      base::Bind(&RendererImpl::OnAudioRendererReinitialized, weak_this_,
+                 stream, time));
+}
+
+void RendererImpl::OnAudioRendererReinitialized(DemuxerStream* stream,
+                                                base::TimeDelta time,
+                                                PipelineStatus status) {
+  DVLOG(2) << __func__ << ": status=" << status;
+  DCHECK_EQ(stream, current_audio_stream_);
+
+  if (status != PIPELINE_OK) {
+    OnError(status);
+    return;
+  }
+  RestartAudioRenderer(stream, time);
+}
+
+void RendererImpl::RestartAudioRenderer(DemuxerStream* stream,
+                                        base::TimeDelta time) {
+  DVLOG(2) << __func__ << " stream=" << stream << " time=" << time.InSecondsF();
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(state_ == STATE_PLAYING || state_ == STATE_FLUSHING);
   DCHECK(time_source_);
   DCHECK(audio_renderer_);
+  DCHECK_EQ(stream, current_audio_stream_);
+
   audio_ended_ = false;
   audio_renderer_->StartPlaying();
 }
@@ -394,6 +483,8 @@ void RendererImpl::InitializeAudioRenderer() {
     return;
   }
 
+  current_audio_stream_ = audio_stream;
+
   audio_renderer_client_.reset(
       new RendererClientInternal(DemuxerStream::AUDIO, this));
   // Note: After the initialization of a renderer, error events from it may
@@ -442,6 +533,8 @@ void RendererImpl::InitializeVideoRenderer() {
     return;
   }
 
+  current_video_stream_ = video_stream;
+
   video_renderer_client_.reset(
       new RendererClientInternal(DemuxerStream::VIDEO, this));
   video_renderer_->Initialize(
@@ -554,6 +647,12 @@ void RendererImpl::OnVideoRendererFlushDone() {
   video_ended_ = false;
   state_ = STATE_PLAYING;
   base::ResetAndReturn(&flush_cb_).Run();
+
+  if (!pending_actions_.empty()) {
+    base::Closure closure = pending_actions_.front();
+    pending_actions_.pop_front();
+    closure.Run();
+  }
 }
 
 void RendererImpl::OnStatisticsUpdate(const PipelineStatistics& stats) {
@@ -628,12 +727,15 @@ bool RendererImpl::HandleRestartedStreamBufferingChanges(
 }
 
 void RendererImpl::OnStreamRestartCompleted() {
+  DVLOG(3) << __func__ << " restarting_audio_=" << restarting_audio_
+           << " restarting_video_=" << restarting_video_;
   DCHECK(restarting_audio_ || restarting_video_);
   restarting_audio_ = false;
   restarting_video_ = false;
-  if (!pending_stream_status_notifications_.empty()) {
-    pending_stream_status_notifications_.front().Run();
-    pending_stream_status_notifications_.pop_front();
+  if (!pending_actions_.empty()) {
+    base::Closure closure = pending_actions_.front();
+    pending_actions_.pop_front();
+    closure.Run();
   }
 }