Notify demux stream clients when a stream is disabled.

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 116 matching lines @@
   DCHECK(demuxer_stream_provider->GetStream(DemuxerStream::AUDIO) ||
          demuxer_stream_provider->GetStream(DemuxerStream::VIDEO));
 
   client_ = client;
   demuxer_stream_provider_ = demuxer_stream_provider;
   init_cb_ = init_cb;
 
   DemuxerStream* audio_stream =
       demuxer_stream_provider->GetStream(DemuxerStream::AUDIO);
   if (audio_stream)
-    audio_stream->SetStreamRestartedCB(
+    audio_stream->SetStreamStatusChangeCB(
         base::Bind(&RendererImpl::RestartStreamPlayback, weak_this_));
   DemuxerStream* video_stream =
       demuxer_stream_provider->GetStream(DemuxerStream::VIDEO);
   if (video_stream)
-    video_stream->SetStreamRestartedCB(
+    video_stream->SetStreamStatusChangeCB(
         base::Bind(&RendererImpl::RestartStreamPlayback, weak_this_));
 
   if (HasEncryptedStream() && !cdm_context_) {
     state_ = STATE_INIT_PENDING_CDM;
     return;
   }
 
   state_ = STATE_INITIALIZING;
   InitializeAudioRenderer();
 }
@@ skipping 55 matching lines @@
 
   time_source_->SetMediaTime(time);
 
   if (audio_renderer_)
     audio_renderer_->StartPlaying();
   if (video_renderer_)
     video_renderer_->StartPlayingFrom(time);
 }
 
 void RendererImpl::RestartStreamPlayback(DemuxerStream* stream,
+                                         bool enabled,

[alokp, 2016/08/04 19:02:36]

I do not see this new argument being used. Am I missing something?

[servolk, 2016/08/04 20:31:47]

Yes, it is unused for now. I've added it per Dale suggestion in
https://codereview.chromium.org/2199913002/#msg27

                                          base::TimeDelta time) {
-  DVLOG(1) << __func__ << " stream=" << stream << " time=" << time.InSecondsF();
   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();
   if (state_ != STATE_PLAYING)
     return;
   if (stream->type() == DemuxerStream::VIDEO) {
     DCHECK(video_renderer_);
     if (restarting_video_)
       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_)
       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));
   }
 }
 
 void RendererImpl::RestartVideoRenderer(base::TimeDelta time) {
+  DVLOG(3) << __func__;

[alokp, 2016/08/04 19:02:37]

any particular reason for changing the logging level?

[servolk, 2016/08/04 20:31:48]

No particular reason. It was more useful during the initial bring up of this
functionality, and now that it's more stable I think we can reduce the priority
to make the logs less verbose.

   DCHECK(task_runner_->BelongsToCurrentThread());
-  DVLOG(2) << __func__;
+  DCHECK(video_renderer_);
+  DCHECK_EQ(state_, STATE_PLAYING);

[alokp, 2016/08/04 19:02:36]

nit: I usually prefer the expected values specified in the same order as
EXPECT_EQ

[servolk, 2016/08/04 20:31:47]

I have no strong opinion about which order is better in general, but in this
case I prefer it this way for consistency with other code in this file and
elsewhere (see
https://cs.chromium.org/search/?q=%22DCHECK_EQ(state_,+STATE_PLAYING)%22&sq=p...).

   video_ended_ = false;
-  if (state_ == STATE_PLAYING) {
-    DCHECK(video_renderer_);
-    video_renderer_->StartPlayingFrom(time);
-  }
+  video_renderer_->StartPlayingFrom(time);
 }
 
 void RendererImpl::RestartAudioRenderer(base::TimeDelta time) {
+  DVLOG(3) << __func__;

[alokp, 2016/08/04 19:02:36]

ditto

   DCHECK(task_runner_->BelongsToCurrentThread());
-  DVLOG(2) << __func__;
+  DCHECK_EQ(state_, STATE_PLAYING);
+  DCHECK(time_source_);
+  DCHECK(audio_renderer_);
   audio_ended_ = false;
-  if (state_ == STATE_PLAYING) {
-    DCHECK(time_source_);
-    DCHECK(audio_renderer_);
-    audio_renderer_->StartPlaying();
-  }
+  audio_renderer_->StartPlaying();
 }
 
 void RendererImpl::SetPlaybackRate(double playback_rate) {
   DVLOG(1) << __func__ << "(" << playback_rate << ")";
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // Playback rate changes are only carried out while playing.
   if (state_ != STATE_PLAYING)
     return;
 
@@ skipping 563 matching lines @@
   DCHECK(task_runner_->BelongsToCurrentThread());
   client_->OnVideoNaturalSizeChange(size);
 }
 
 void RendererImpl::OnVideoOpacityChange(bool opaque) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   client_->OnVideoOpacityChange(opaque);
 }
 
 }  // namespace media