[Media, Video] Enable the video track for a new renderer.

media/blink/webmediaplayer_impl.cc

 // Copyright 2013 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/blink/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <string>
@@ skipping 182 matching lines @@
       media_task_runner_(params.media_task_runner()),
       worker_task_runner_(params.worker_task_runner()),
       media_log_(params.media_log()),
       pipeline_(media_task_runner_, media_log_.get()),
       pipeline_controller_(
           &pipeline_,
           base::Bind(&WebMediaPlayerImpl::CreateRenderer,
                      base::Unretained(this)),
           base::Bind(&WebMediaPlayerImpl::OnPipelineSeeked, AsWeakPtr()),
           base::Bind(&WebMediaPlayerImpl::OnPipelineSuspended, AsWeakPtr()),
+          base::Bind(&WebMediaPlayerImpl::OnPipelineResumed, AsWeakPtr()),
           base::Bind(&WebMediaPlayerImpl::OnError, AsWeakPtr())),
       load_type_(LoadTypeURL),
       opaque_(false),
       playback_rate_(0.0),
       paused_(true),
       paused_when_hidden_(false),
       seeking_(false),
       pending_suspend_resume_cycle_(false),
       ended_(false),
       should_notify_time_changed_(false),
@@ skipping 254 matching lines @@
   // incorrectly discard what it thinks is a seek to the existing time.
   paused_time_ =
       ended_ ? pipeline_.GetMediaDuration() : pipeline_.GetMediaTime();
 
   if (observer_)
     observer_->OnPaused();
 
   DCHECK(watch_time_reporter_);
   watch_time_reporter_->OnPaused();
   media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::PAUSE));
+
+  // Enable the background video track back so that when the new renderer is
+  // initialized upon resume, it registers itself with the video track.
+  // TODO(avayvod): Remove when disabling video tracks works correctly. See
+  //                https://crbug.com/678374.
+  if (video_track_disabled_) {

[sandersd (OOO until July 31), 2017/01/09 20:12:49]

I think this would be better in an OnBeforeResume() callback, it's not trivially
obvious that doing it in pause() is always enough.

[whywhat, 2017/01/10 01:04:48]

Sure. I guess the fix would be reduced to enabling the video track back in
OnBeforeResume (OnResuming? OnResumeRequested?) and disabling in OnResumed()

+    video_track_disabled_ = false;
+    if (client_->hasSelectedVideoTrack()) {
+      WebMediaPlayer::TrackId trackId = client_->getSelectedVideoTrackId();
+      selectedVideoTrackChanged(&trackId);
+    }
+  }
+
   UpdatePlayState();
 }
 
 bool WebMediaPlayerImpl::supportsSave() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   return supports_save_;
 }
 
 void WebMediaPlayerImpl::seek(double seconds) {
   DVLOG(1) << __func__ << "(" << seconds << "s)";
@@ skipping 177 matching lines @@
                               << "]";
   pipeline_.OnEnabledAudioTracksChanged(enabledMediaTrackIds);
 }
 
 void WebMediaPlayerImpl::selectedVideoTrackChanged(
     blink::WebMediaPlayer::TrackId* selectedTrackId) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   std::ostringstream logstr;
   std::vector<MediaTrack::Id> selectedVideoMediaTrackId;
-  bool canAddVideoTrack =
-      !IsBackgroundVideoTrackOptimizationEnabled() || !IsHidden();
-  if (selectedTrackId && canAddVideoTrack) {
+  if (selectedTrackId && !video_track_disabled_) {
     selectedVideoMediaTrackId.push_back(selectedTrackId->utf8().data());
     logstr << selectedVideoMediaTrackId[0];
   }
   MEDIA_LOG(INFO, media_log_) << "Selected video track: [" << logstr.str()
                               << "]";
   pipeline_.OnSelectedVideoTrackChanged(selectedVideoMediaTrackId);
 }
 
 blink::WebSize WebMediaPlayerImpl::naturalSize() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
@@ skipping 366 matching lines @@
     set_cdm_result_->completeWithError(
         blink::WebContentDecryptionModuleExceptionNotSupportedError, 0,
         "Unable to set ContentDecryptionModule object");
     set_cdm_result_.reset();
   }
 }
 
 void WebMediaPlayerImpl::OnPipelineSeeked(bool time_updated) {
   seeking_ = false;
   seek_time_ = base::TimeDelta();
+
   if (paused_) {
 #if defined(OS_ANDROID)  // WMPI_CAST
     if (isRemote()) {
       paused_time_ = base::TimeDelta::FromSecondsD(cast_impl_.currentTime());
     } else {
       paused_time_ = pipeline_.GetMediaTime();
     }
 #else
     paused_time_ = pipeline_.GetMediaTime();
 #endif
@@ skipping 27 matching lines @@
   }
 
   ReportMemoryUsage();
 
   if (pending_suspend_resume_cycle_) {
     pending_suspend_resume_cycle_ = false;
     UpdatePlayState();
   }
 }
 
+void WebMediaPlayerImpl::OnPipelineResumed() {
+  // Disable video track if we start playing the video in the background and
+  // haven't disabled the decoder yet.
+  // TODO(avayvod): Remove this when disabling and enabling video tracks in
+  // non-playing state works correctly. See https://crbug.com/678374.
+  if (IsBackgroundVideoTrackOptimizationEnabled() && IsHidden() &&
+      !video_track_disabled_) {
+    video_track_disabled_ = true;
+    selectedVideoTrackChanged(nullptr);
+  }
+}
+
 void WebMediaPlayerImpl::OnDemuxerOpened() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   client_->mediaSourceOpened(
       new WebMediaSourceImpl(chunk_demuxer_, media_log_));
 }
 
 void WebMediaPlayerImpl::OnError(PipelineStatus status) {
   DVLOG(1) << __func__;
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   DCHECK_NE(status, PIPELINE_OK);
@@ skipping 230 matching lines @@
     watch_time_reporter_->OnHidden();
 
   if (IsBackgroundVideoTrackOptimizationEnabled()) {
     if (ShouldPauseWhenHidden()) {
       // OnPause() will set |paused_when_hidden_| to false and call
       // UpdatePlayState(), so set the flag to true after and then return.
       OnPause();
       paused_when_hidden_ = true;
       return;
     }
-
-    selectedVideoTrackChanged(nullptr);
   }
 
   UpdatePlayState();
 
+  // Only disable video tracks for background videos when they're playing.
+  // Otherwise the video track will get detached from the renderer.
+  // TODO(avayvod): Remove extra check once this is no longer happening.
+  //                See https://crbug.com/678374.
+  if (IsBackgroundVideoTrackOptimizationEnabled() &&
+      delegate_state_ == DelegateState::PLAYING) {

[sandersd (OOO until July 31), 2017/01/09 20:12:49]

Is |delegate_state_| the best thing to check for this? Do we even need this with
OnBeforeResume()?

[whywhat, 2017/01/10 01:04:48]

Will try. I saw this member is gone in your refactor :)

+    video_track_disabled_ = true;
+    selectedVideoTrackChanged(nullptr);
+  }
+
   // Schedule suspended playing media to be paused if the user doesn't come back
   // to it within some timeout period to avoid any autoplay surprises.
   ScheduleIdlePauseTimer();
 }
 
 void WebMediaPlayerImpl::OnShown() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   if (watch_time_reporter_)
     watch_time_reporter_->OnShown();
 
   compositor_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&VideoFrameCompositor::SetForegroundTime,
                  base::Unretained(compositor_), base::TimeTicks::Now()));
 
-  if (IsBackgroundVideoTrackOptimizationEnabled()) {
-    if (paused_when_hidden_) {
-      paused_when_hidden_ = false;
-      OnPlay();  // Calls UpdatePlayState() so return afterwards.
-      return;
-    }
+  must_suspend_ = false;
+  background_pause_timer_.Stop();
 
+  if (paused_when_hidden_) {
+    paused_when_hidden_ = false;
+    OnPlay();  // Calls UpdatePlayState() so return afterwards.
+    return;
+  }
+
+  if (video_track_disabled_) {
+    video_track_disabled_ = false;
     if (client_->hasSelectedVideoTrack()) {
       WebMediaPlayer::TrackId trackId = client_->getSelectedVideoTrackId();
       selectedVideoTrackChanged(&trackId);
     }
   }
 
-  must_suspend_ = false;
-  background_pause_timer_.Stop();
-
   UpdatePlayState();
 }
 
 bool WebMediaPlayerImpl::OnSuspendRequested(bool must_suspend) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   if (must_suspend) {
     must_suspend_ = true;
     UpdatePlayState();
     return true;
@@ skipping 667 matching lines @@
 bool WebMediaPlayerImpl::ShouldPauseWhenHidden() const {
 #if defined(OS_ANDROID)  // WMPI_CAST
   if (isRemote())
     return false;
 #endif  // defined(OS_ANDROID)  // WMPI_CAST
 
   return hasVideo() && !hasAudio();
 }
 
 }  // namespace media