Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(68)

Unified Diff: media/blink/webmediaplayer_impl.cc

Issue 2631633003: [Video] Only start tracking foreground time if the video was resumed. (Closed)
Patch Set: Split the histogram into paused and disabled track Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: media/blink/webmediaplayer_impl.cc
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index aabb04079f5861bd4dd8c600c9dab1a20ae6ada7..4f54463bee1c07deebf22f4b39f45306eaa60f4d 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -128,6 +128,14 @@ bool IsBackgroundVideoTrackOptimizationEnabled() {
return base::FeatureList::IsEnabled(kBackgroundVideoTrackOptimization);
}
+bool IsBackgroundVideoPauseEnabled() {
+#if defined(OS_ANDROID)
+ return true;
+#else
+ return IsBackgroundVideoTrackOptimizationEnabled();
+#endif
+}
+
bool IsNetworkStateError(blink::WebMediaPlayer::NetworkState state) {
bool result = state == blink::WebMediaPlayer::NetworkStateFormatError ||
state == blink::WebMediaPlayer::NetworkStateNetworkError ||
@@ -1410,7 +1418,7 @@ void WebMediaPlayerImpl::OnFrameHidden() {
if (watch_time_reporter_)
watch_time_reporter_->OnHidden();
- if (ShouldPauseWhenHidden()) {
+ if (IsBackgroundVideoPauseEnabled() && ShouldPauseWhenHidden()) {
if (!paused_when_hidden_) {
// OnPause() will set |paused_when_hidden_| to false and call
// UpdatePlayState(), so set the flag to true after and then return.
@@ -1441,10 +1449,23 @@ void WebMediaPlayerImpl::OnFrameShown() {
if (watch_time_reporter_)
watch_time_reporter_->OnShown();
- compositor_task_runner_->PostTask(
- FROM_HERE,
- base::Bind(&VideoFrameCompositor::SetForegroundTime,
- base::Unretained(compositor_), base::TimeTicks::Now()));
+ // Only track the time to the first frame if playing or about to play because
+ // of being shown and only for videos we would optimize background playback
+ // for.
+ if (!paused_ && ShouldDisableVideoWhenHidden()) {
+ compositor_task_runner_->PostTask(
+ FROM_HERE,
+ base::Bind(
+ &VideoFrameCompositor::SetForegroundTime,
+ base::Unretained(compositor_), base::TimeTicks::Now(),
+ VideoFrameCompositor::BackgroundVideoOptimization::DisabledTrack));
+ } else if (paused_when_hidden_ && ShouldPauseWhenHidden()) {
+ compositor_task_runner_->PostTask(
+ FROM_HERE,
+ base::Bind(&VideoFrameCompositor::SetForegroundTime,
+ base::Unretained(compositor_), base::TimeTicks::Now(),
+ VideoFrameCompositor::BackgroundVideoOptimization::Paused));
+ }
if (paused_when_hidden_) {
paused_when_hidden_ = false;
@@ -2123,15 +2144,10 @@ void WebMediaPlayerImpl::ActivateViewportIntersectionMonitoring(bool activate) {
}
bool WebMediaPlayerImpl::ShouldPauseWhenHidden() const {
- DCHECK(IsHidden());
-// Don't pause videos being Cast (Android only) or if the background video
-// optimizations are off (desktop only).
+// Don't pause videos being Cast (Android only).
#if defined(OS_ANDROID) // WMPI_CAST
if (isRemote())
return false;
-#else // defined(OS_ANDROID)
- if (!IsBackgroundVideoTrackOptimizationEnabled())
- return false;
#endif // defined(OS_ANDROID)
return hasVideo() && !hasAudio();
@@ -2139,12 +2155,15 @@ bool WebMediaPlayerImpl::ShouldPauseWhenHidden() const {
bool WebMediaPlayerImpl::ShouldDisableVideoWhenHidden() const {
DCHECK(main_task_runner_->BelongsToCurrentThread());
- DCHECK(IsHidden());
- if (!IsBackgroundVideoTrackOptimizationEnabled() || !hasVideo() ||
- !hasAudio() || IsStreaming()) {
+// Don't disable video track for videos being Cast (Android only).
+#if defined(OS_ANDROID) // WMPI_CAST
+ if (isRemote())
+ return false;
+#endif // defined(OS_ANDROID)
+
+ if (!hasVideo() || !hasAudio() || IsStreaming())
return false;
- }
PipelineStatistics stats = GetPipelineStatistics();
return stats.video_keyframe_distance_average <
@@ -2174,7 +2193,8 @@ void WebMediaPlayerImpl::DisableVideoTrackIfNeeded() {
if (is_pipeline_resuming_ || seeking_)
return;
- if (!video_track_disabled_ && ShouldDisableVideoWhenHidden()) {
+ if (!video_track_disabled_ && IsBackgroundVideoTrackOptimizationEnabled() &&
+ ShouldDisableVideoWhenHidden()) {
video_track_disabled_ = true;
selectedVideoTrackChanged(nullptr);
}

Powered by Google App Engine
This is Rietveld 408576698