Skip to content

Commit

Permalink
[Video, Android] Pause video only players ignoring duration and keyfr…
Browse files Browse the repository at this point in the history
…ames

My recent change https://codereview.chromium.org/2631633003 enabled
playing background video-only players on Android if they meet various
criteria like being shorter than an experimental limit or having
frequent enough keyframes.

Restore pausing logic by bypassing the irrelevant checks on Android.

BUG=None
TEST=Manual + updated unit tests

Review-Url: https://codereview.chromium.org/2643033004
Cr-Commit-Position: refs/heads/master@{#445116}
(cherry picked from commit ac1a852)

Review-Url: https://codereview.chromium.org/2655723002 .
Cr-Commit-Position: refs/branch-heads/2987@{#57}
Cr-Branched-From: ad51088-refs/heads/master@{#444943}
  • Loading branch information
Anton Vayvod committed Jan 24, 2017
1 parent 017527d commit 47a2344
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
7 changes: 6 additions & 1 deletion media/blink/webmediaplayer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2154,10 +2154,15 @@ bool WebMediaPlayerImpl::ShouldDisableVideoWhenHidden() const {
bool WebMediaPlayerImpl::IsBackgroundOptimizationCandidate() const {
DCHECK(main_task_runner_->BelongsToCurrentThread());

// Don't optimize players being Cast (Android only).
#if defined(OS_ANDROID) // WMPI_CAST
// Don't optimize players being Cast.
if (isRemote())
return false;

// Video-only players are always optimized (paused) on Android.
// Don't check the keyframe distance and duration.
if (!hasAudio() && hasVideo())
return true;
#endif // defined(OS_ANDROID)

// Don't optimize audio-only or streaming players.
Expand Down
15 changes: 14 additions & 1 deletion media/blink/webmediaplayer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -737,12 +737,25 @@ TEST_F(WebMediaPlayerImplTest, BackgroundOptimizationsFeatureDisabled) {
// Video only (pausing is enabled on Android).
SetMetadata(false, true);
EXPECT_TRUE(IsBackgroundOptimizationCandidate());
EXPECT_FALSE(ShouldDisableVideoWhenHidden());
#if defined(OS_ANDROID)
EXPECT_TRUE(ShouldPauseVideoWhenHidden());

// On Android, the duration and keyframe distance don't matter for video-only.
SetDuration(base::TimeDelta::FromSeconds(5));
EXPECT_TRUE(IsBackgroundOptimizationCandidate());
EXPECT_TRUE(ShouldPauseVideoWhenHidden());

SetVideoKeyframeDistanceAverage(base::TimeDelta::FromSeconds(100));
SetDuration(base::TimeDelta::FromSeconds(300));
EXPECT_TRUE(IsBackgroundOptimizationCandidate());
EXPECT_TRUE(ShouldPauseVideoWhenHidden());

// Restore average keyframe distance.
SetVideoKeyframeDistanceAverage(base::TimeDelta::FromSeconds(5));
#else
EXPECT_FALSE(ShouldPauseVideoWhenHidden());
#endif
EXPECT_FALSE(ShouldDisableVideoWhenHidden());

// Audio only.
SetMetadata(true, false);
Expand Down

0 comments on commit 47a2344

Please sign in to comment.