Implement disabling and enabling media tracks

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 6de238c68ce39ea96f3a74813e535b21bb177605..ce2f49215862dea12e4954047f964e35725ca29d 100644
--- a/media/renderers/renderer_impl.cc
+++ b/media/renderers/renderer_impl.cc
@@ -201,6 +201,51 @@ void RendererImpl::StartPlayingFrom(base::TimeDelta time) {
     video_renderer_->StartPlayingFrom(time);
 }
 
+void RendererImpl::RestartStreamPlayback(DemuxerStream* stream,
+                                         base::TimeDelta time) {
+  DVLOG(1) << __FUNCTION__ << " stream=" << stream
+           << " time=" << time.InSecondsF();
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  if (stream->type() == DemuxerStream::VIDEO) {
+    DCHECK(video_renderer_);
+    DCHECK(video_ended_);
+    video_ended_ = false;
+    restarting_video_ = true;
+    video_renderer_->Flush(base::Bind(
+        &RendererImpl::OnVideoRendererFlushDone_ForRestart, weak_this_, time));
+  } else if (stream->type() == DemuxerStream::AUDIO) {
+    DCHECK(audio_renderer_);
+    DCHECK(time_source_);
+    DCHECK(audio_ended_);
+    audio_ended_ = false;
+    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.
+    time_ticking_ = false;
+    time_source_->StopTicking();
+    audio_renderer_->Flush(base::Bind(
+        &RendererImpl::OnAudioRendererFlushDone_ForRestart, weak_this_, time));
+  }
+}
+
+void RendererImpl::OnVideoRendererFlushDone_ForRestart(base::TimeDelta time) {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  if (state_ == STATE_PLAYING) {
+    DCHECK(video_renderer_);
+    video_renderer_->StartPlayingFrom(time);
+  }
+}
+
+void RendererImpl::OnAudioRendererFlushDone_ForRestart(base::TimeDelta time) {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  if (state_ == STATE_PLAYING) {
+    DCHECK(time_source_);
+    DCHECK(audio_renderer_);
+    audio_renderer_->StartPlaying();
+  }
+}
+
 void RendererImpl::SetPlaybackRate(double playback_rate) {
   DVLOG(1) << __FUNCTION__ << "(" << playback_rate << ")";
   DCHECK(task_runner_->BelongsToCurrentThread());
@@ -502,6 +547,48 @@ void RendererImpl::OnBufferingStateChange(DemuxerStream::Type type,
 
   bool was_waiting_for_enough_data = WaitingForEnoughData();
 
+  // When restarting video stream playback we want to ignore the
+  // BUFFERING_HAVE_NOTHING notification so that audio keeps playing while
+  // video decoder is catching up.
+  if (type == DemuxerStream::VIDEO && restarting_video_) {
+    if (new_buffering_state == BUFFERING_HAVE_NOTHING) {
+      DVLOG(1) << __FUNCTION__ << " Ignoring BUFFERING_HAVE_NOTHING for video "
+                                  "stream which is being restarted.";
+    } else if (new_buffering_state == BUFFERING_HAVE_ENOUGH) {

[chcunningham, 2016/06/03 01:38:49]

What if the new buffering state never reaches HAVE_ENOUGH? If MSE apps decide to
not bother appending video while the video track is disabled, but then later
enable the track, we should probably signal that we don't have any video data at
the current time. The same argument could be made for audio.

I probably haven't thought about this enough, but one option would be to
recognize the HAVE_NOTHING as potentially legit, but defer surfacing this via
deferred post task (like we do when video underflows below). Cancel the posting
of the task if the buffering state comes in as HAVE_ENOUGH under the threshold.

[servolk, 2016/06/03 02:53:15]

If MSE apps decide to not bother appending video then they can already do that
today without even disabling the video track explicitly. That would result in
stalled playback of course, but that's an application bug and we can't prevent
them from doing that. So I think we don't need to be worried about this here.

[chcunningham, 2016/06/03 20:54:59]

I don't think its a matter of worrying just to help the app devs - its more that
I think we need to do this to meet the spec (which hopefully helps app devs
too). The spec would have us change our readyState to less-than HAVE_ENOUGH
whenever we don't HAVE_ENOUGH. 

IIUC, the current code would actually *not* stall playback in the scenario where
the track is re-enabled without having data buffered. This is asymmetric
behavior from the case you described where they leave the track enabled the
whole time but never append. I think the behavior of just-enabled-tracks should
match that of always-enabled-tracks.

+      DVLOG(1)
+          << __FUNCTION__
+          << " Got BUFFERING_HAVE_ENOUGH for video stream, resuming playback.";
+      restarting_video_ = false;
+      if (playback_rate_ > 0)
+        video_renderer_->OnTimeStateChanged(true);

[chcunningham, 2016/06/03 01:38:49]

Not really related to this line, but it triggered a thought...

I think we should make video renderer call OnTimeStateChanged(false) on itself
when it observes an EOS. Or we could do it in the renderer_impl when video
renderer signals its playback has ended. As it stands, I think the video sink's
timer will continue firing in spite of the track being disabled.

[servolk, 2016/06/03 02:53:15]

Yes, it sounds reasonable to me. I'm not sure why that hasn't been done yet,
perhaps there are reasons why we need to keep ticking. I'll let Xiaohan or Matt
comment on this.

+    }
+    return;
+  }
+
+  if (type == DemuxerStream::AUDIO && restarting_audio_) {
+    if (new_buffering_state == BUFFERING_HAVE_NOTHING) {
+      DVLOG(1) << __FUNCTION__ << " Ignoring BUFFERING_HAVE_NOTHING for "
+               << (type == DemuxerStream::AUDIO ? "audio" : "video")
+               << " stream";
+    } else if (new_buffering_state == BUFFERING_HAVE_ENOUGH) {
+      DVLOG(1) << __FUNCTION__ << " Got BUFFERING_HAVE_ENOUGH for "
+               << (type == DemuxerStream::AUDIO ? "audio" : "video")
+               << " stream";
+
+      restarting_audio_ = false;
+
+      // Now that we have decoded enough audio, pause and unpause playback
+      // momentarily to ensure video renderer is synchronised with audio.
+      double curr_playback_rate = playback_rate_;
+      SetPlaybackRate(0);

[chcunningham, 2016/06/03 01:38:49]

Why does this help sync? I think you're already paused because you stopped
ticking prior to flush. I think you just need to call
time_source_->SetMediaTime() in OnAudioRendererFlushDone_ForRestart, and then
here you just need to StartTicking. 

[chcunningham, 2016/06/03 20:55:37]

(reminder about this one in case you missed it)

+      SetPlaybackRate(curr_playback_rate);
+
+      time_ticking_ = true;
+      time_source_->StartTicking();
+    }
+    return;
+  }
+
   // When audio is present and has enough data, defer video underflow callbacks
   // for some time to avoid unnecessary glitches in audio; see
   // http://crbug.com/144683#c53.
@@ -612,7 +699,8 @@ void RendererImpl::StartPlayback() {
 }
 
 void RendererImpl::OnRendererEnded(DemuxerStream::Type type) {
-  DVLOG(1) << __FUNCTION__;
+  DVLOG(1) << __FUNCTION__
+           << (type == DemuxerStream::AUDIO ? " audio" : " video");
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK((type == DemuxerStream::AUDIO) || (type == DemuxerStream::VIDEO));