Prepare for src= video background track switch finch experimentation.

Previously, the BackgroundVideoTrackOptimization flag was used to control both src= and MSE videos. MSE videos has been fully decoupled from the flag now. This CL also uses max_keyframe_distance as a fixed value, determined by the previous finch experiment on MSE videos. The chrome://flags entry is also being removed in preparation for the experiment and (most likely) subsequent enabling of the feature. Bug: 709302 Change-Id: I3875fc700ceb569da7a7c4fa39b28d938235e6a8 Reviewed-on: https://chromium-review.googlesource.com/1194794 Commit-Queue: Ted Meyer <[email protected]> Reviewed-by: Ken Buchanan <[email protected]> Reviewed-by: Nasko Oskov <[email protected]> Reviewed-by: Mounir Lamouri <[email protected]> Reviewed-by: Jochen Eisinger <[email protected]> Reviewed-by: Dale Curtis <[email protected]> Cr-Commit-Position: refs/heads/master@{#589728}
feiyue1206 · Sep 8, 2018 · a918e59 · a918e59
1 parent 14af52f
commit a918e59
Show file tree

Hide file tree

Showing 17 changed files with 30 additions and 146 deletions.
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
@@ -2959,10 +2959,6 @@ const FeatureEntry kFeatureEntries[] = {
      flag_descriptions::kNewAudioRenderingMixingStrategyDescription,
      kOsWin | kOsMac | kOsLinux | kOsAndroid,
      FEATURE_VALUE_TYPE(media::kNewAudioRenderingMixingStrategy)},
-    {"disable-background-video-track",
-     flag_descriptions::kBackgroundVideoTrackOptimizationName,
-     flag_descriptions::kBackgroundVideoTrackOptimizationDescription, kOsAll,
-     FEATURE_VALUE_TYPE(media::kBackgroundVideoTrackOptimization)},
     {"enable-new-remote-playback-pipeline",
      flag_descriptions::kNewRemotePlaybackPipelineName,
      flag_descriptions::kNewRemotePlaybackPipelineDescription, kOsAll,

diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc
@@ -148,12 +148,6 @@ const char kAv1DecoderName[] = "Enable AV1 video decoding.";
 const char kAv1DecoderDescription[] =
     "Allow decoding of files with the AV1 video codec.";
 
-const char kBackgroundVideoTrackOptimizationName[] =
-    "Optimize background video playback.";
-const char kBackgroundVideoTrackOptimizationDescription[] =
-    "Disable video tracks when the video is played in the background to "
-    "optimize performance.";
-
 const char kBleAdvertisingInExtensionsName[] = "BLE Advertising in Chrome Apps";
 const char kBleAdvertisingInExtensionsDescription[] =
     "Enables BLE Advertising in Chrome Apps. BLE Advertising might interfere "

diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h
@@ -116,9 +116,6 @@ extern const char kAutoplayPolicyDocumentUserActivation[];
 extern const char kAv1DecoderName[];
 extern const char kAv1DecoderDescription[];
 
-extern const char kBackgroundVideoTrackOptimizationName[];
-extern const char kBackgroundVideoTrackOptimizationDescription[];
-
 extern const char kBleAdvertisingInExtensionsName[];
 extern const char kBleAdvertisingInExtensionsDescription[];
 

diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
@@ -584,9 +584,6 @@ WebPreferences RenderViewHostImpl::ComputeWebkitPrefs() {
   if (delegate_ && delegate_->HasPersistentVideo())
     prefs.media_controls_enabled = false;
 
-  prefs.background_video_track_optimization_enabled =
-      base::FeatureList::IsEnabled(media::kBackgroundVideoTrackOptimization);
-
   GetContentClient()->browser()->OverrideWebkitPrefs(this, &prefs);
   return prefs;
 }

diff --git a/content/public/common/common_param_traits_macros.h b/content/public/common/common_param_traits_macros.h
@@ -232,7 +232,6 @@ IPC_STRUCT_TRAITS_BEGIN(content::WebPreferences)
   IPC_STRUCT_TRAITS_MEMBER(default_minimum_page_scale_factor)
   IPC_STRUCT_TRAITS_MEMBER(default_maximum_page_scale_factor)
   IPC_STRUCT_TRAITS_MEMBER(hide_download_ui)
-  IPC_STRUCT_TRAITS_MEMBER(background_video_track_optimization_enabled)
   IPC_STRUCT_TRAITS_MEMBER(presentation_receiver)
   IPC_STRUCT_TRAITS_MEMBER(media_controls_enabled)
   IPC_STRUCT_TRAITS_MEMBER(do_not_update_selection_on_mutating_selection_range)

diff --git a/content/public/common/web_preferences.cc b/content/public/common/web_preferences.cc
@@ -230,7 +230,6 @@ WebPreferences::WebPreferences()
       default_maximum_page_scale_factor(4.f),
 #endif
       hide_download_ui(false),
-      background_video_track_optimization_enabled(false),
       presentation_receiver(false),
       media_controls_enabled(true),
       do_not_update_selection_on_mutating_selection_range(false),

diff --git a/content/public/common/web_preferences.h b/content/public/common/web_preferences.h
@@ -281,9 +281,6 @@ struct CONTENT_EXPORT WebPreferences {
   // Whether download UI should be hidden on this page.
   bool hide_download_ui;
 
-  // If enabled, disabled video track when the video is in the background.
-  bool background_video_track_optimization_enabled;
-
   // Whether it is a presentation receiver.
   bool presentation_receiver;
 

diff --git a/content/renderer/media/media_factory.cc b/content/renderer/media/media_factory.cc
@@ -245,15 +245,6 @@ blink::WebMediaPlayer* MediaFactory::CreateMediaPlayer(
       webkit_preferences.embedded_media_experience_enabled;
 #endif  // defined(OS_ANDROID)
 
-  // Enable background optimizations based on field trial for src= content, but
-  // always enable for MSE content. See http://crbug.com/709302.
-  base::TimeDelta max_keyframe_distance_to_disable_background_video =
-      base::TimeDelta::FromMilliseconds(base::GetFieldTrialParamByFeatureAsInt(
-          media::kBackgroundVideoTrackOptimization, "max_keyframe_distance_ms",
-          0));
-  base::TimeDelta max_keyframe_distance_to_disable_background_video_mse =
-      base::TimeDelta::FromSeconds(5);
-
   // When memory pressure based garbage collection is enabled for MSE, the
   // |enable_instant_source_buffer_gc| flag controls whether the GC is done
   // immediately on memory pressure notification or during the next SourceBuffer
@@ -336,8 +327,6 @@ blink::WebMediaPlayer* MediaFactory::CreateMediaPlayer(
           base::Bind(&v8::Isolate::AdjustAmountOfExternalAllocatedMemory,
                      base::Unretained(blink::MainThreadIsolate())),
           initial_cdm, request_routing_token_cb_, media_observer,
-          max_keyframe_distance_to_disable_background_video,
-          max_keyframe_distance_to_disable_background_video_mse,
           enable_instant_source_buffer_gc, embedded_media_experience_enabled,
           std::move(metrics_provider),
           base::BindOnce(&blink::WebSurfaceLayerBridge::Create,

diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
@@ -918,8 +918,6 @@ void RenderView::ApplyWebPreferences(const WebPreferences& prefs,
   settings->SetSmoothScrollForFindEnabled(prefs.smooth_scroll_for_find_enabled);
 
   settings->SetHideDownloadUI(prefs.hide_download_ui);
-  WebRuntimeFeatures::EnableBackgroundVideoTrackOptimization(
-      prefs.background_video_track_optimization_enabled);
   WebRuntimeFeatures::EnableNewRemotePlaybackPipeline(
       base::FeatureList::IsEnabled(media::kNewRemotePlaybackPipeline));
 

diff --git a/media/base/media_switches.cc b/media/base/media_switches.cc
@@ -245,8 +245,8 @@ const base::Feature kAv1Decoder{"Av1Decoder",
                                 base::FEATURE_ENABLED_BY_DEFAULT};
 
 // Let video track be unselected when video is playing in the background.
-const base::Feature kBackgroundVideoTrackOptimization{
-    "BackgroundVideoTrackOptimization", base::FEATURE_ENABLED_BY_DEFAULT};
+const base::Feature kBackgroundSrcVideoTrackOptimization{
+    "BackgroundSrcVideoTrackOptimization", base::FEATURE_DISABLED_BY_DEFAULT};
 
 // Let video without audio be paused when it is playing in the background.
 const base::Feature kBackgroundVideoPauseOptimization{

diff --git a/media/base/media_switches.h b/media/base/media_switches.h
@@ -102,7 +102,7 @@ MEDIA_EXPORT extern const base::Feature kAutoplayIgnoreWebAudio;
 MEDIA_EXPORT extern const base::Feature kAutoplaySoundSettings;
 MEDIA_EXPORT extern const base::Feature kAv1Decoder;
 MEDIA_EXPORT extern const base::Feature kBackgroundVideoPauseOptimization;
-MEDIA_EXPORT extern const base::Feature kBackgroundVideoTrackOptimization;
+MEDIA_EXPORT extern const base::Feature kBackgroundSrcVideoTrackOptimization;
 MEDIA_EXPORT extern const base::Feature kD3D11VideoDecoder;
 MEDIA_EXPORT extern const base::Feature kExternalClearKeyForTesting;
 MEDIA_EXPORT extern const base::Feature kHardwareSecureDecryption;

diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
@@ -114,8 +114,11 @@ bool IsResumeBackgroundVideosEnabled() {
   return base::FeatureList::IsEnabled(kResumeBackgroundVideo);
 }
 
-bool IsBackgroundVideoTrackOptimizationEnabled() {
-  return base::FeatureList::IsEnabled(kBackgroundVideoTrackOptimization);
+bool IsBackgroundVideoTrackOptimizationEnabled(
+    WebMediaPlayer::LoadType load_type) {
+  // Background video track optimization is always enabled for MSE videos.
+  return load_type == WebMediaPlayer::LoadType::kLoadTypeMediaSource ||
+         base::FeatureList::IsEnabled(kBackgroundSrcVideoTrackOptimization);
 }
 
 bool IsBackgroundVideoPauseOptimizationEnabled() {
@@ -152,6 +155,9 @@ constexpr base::TimeDelta kPrerollAttemptTimeout =
 // Maximum number, per-WMPI, of media logs of playback rate changes.
 constexpr int kMaxNumPlaybackRateLogs = 10;
 
+constexpr base::TimeDelta kMaxKeyframeDistance =
+    base::TimeDelta::FromSeconds(5);
+
 blink::WebLocalizedString::Name GetSwitchToLocalMessage(
     MediaObserverClient::ReasonToSwitchToLocal reason) {
   switch (reason) {
@@ -248,10 +254,6 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
 #endif
       renderer_factory_selector_(std::move(renderer_factory_selector)),
       observer_(params->media_observer()),
-      max_keyframe_distance_to_disable_background_video_(
-          params->max_keyframe_distance_to_disable_background_video()),
-      max_keyframe_distance_to_disable_background_video_mse_(
-          params->max_keyframe_distance_to_disable_background_video_mse()),
       enable_instant_source_buffer_gc_(
           params->enable_instant_source_buffer_gc()),
       embedded_media_experience_enabled_(
@@ -2995,8 +2997,11 @@ bool WebMediaPlayerImpl::ShouldPauseVideoWhenHidden() const {
 }
 
 bool WebMediaPlayerImpl::ShouldDisableVideoWhenHidden() const {
-  // This optimization is behind the flag on all platforms.
-  if (!IsBackgroundVideoTrackOptimizationEnabled())
+  // This optimization is behind the flag on all platforms, only for non-mse
+  // video. MSE video track switching on hide has gone through a field test.
+  // TODO(tmathmeyer): Passing load_type_ won't be needed after src= field
+  // testing is finished. see: http://crbug.com/709302
+  if (!IsBackgroundVideoTrackOptimizationEnabled(load_type_))
     return false;
 
   // Disable video track only for players with audio that match the criteria for
@@ -3028,16 +3033,13 @@ bool WebMediaPlayerImpl::IsBackgroundOptimizationCandidate() const {
 
   // Videos shorter than the maximum allowed keyframe distance can be optimized.
   base::TimeDelta duration = GetPipelineMediaDuration();
-  base::TimeDelta max_keyframe_distance =
-      (load_type_ == kLoadTypeMediaSource)
-          ? max_keyframe_distance_to_disable_background_video_mse_
-          : max_keyframe_distance_to_disable_background_video_;
-  if (duration < max_keyframe_distance)
+
+  if (duration < kMaxKeyframeDistance)
     return true;
 
   // Otherwise, only optimize videos with shorter average keyframe distance.
   PipelineStatistics stats = GetPipelineStatistics();
-  return stats.video_keyframe_distance_average < max_keyframe_distance;
+  return stats.video_keyframe_distance_average < kMaxKeyframeDistance;
 }
 
 void WebMediaPlayerImpl::UpdateBackgroundVideoOptimizationState() {

diff --git a/media/blink/webmediaplayer_impl_unittest.cc b/media/blink/webmediaplayer_impl_unittest.cc
@@ -80,10 +80,8 @@ namespace media {
 constexpr char kAudioOnlyTestFile[] = "sfx-opus-441.webm";
 
 // Specify different values for testing.
-const base::TimeDelta kMaxKeyframeDistanceToDisableBackgroundVideo =
+constexpr base::TimeDelta kMaxKeyframeDistanceToDisableBackgroundVideo =
     base::TimeDelta::FromSeconds(5);
-const base::TimeDelta kMaxKeyframeDistanceToDisableBackgroundVideoMSE =
-    base::TimeDelta::FromSeconds(10);
 
 MATCHER(WmpiDestroyed, "") {
   return CONTAINS_STRING(arg, "WEBMEDIAPLAYER_DESTROYED {}");
@@ -392,9 +390,7 @@ class WebMediaPlayerImplTest : public testing::Test {
         base::ThreadTaskRunnerHandle::Get(), media_thread_.task_runner(),
         base::BindRepeating(&WebMediaPlayerImplTest::OnAdjustAllocatedMemory,
                             base::Unretained(this)),
-        nullptr, RequestRoutingTokenCallback(), nullptr,
-        kMaxKeyframeDistanceToDisableBackgroundVideo,
-        kMaxKeyframeDistanceToDisableBackgroundVideoMSE, false, false,
+        nullptr, RequestRoutingTokenCallback(), nullptr, false, false,
         std::move(provider),
         base::BindOnce(&WebMediaPlayerImplTest::CreateMockSurfaceLayerBridge,
                        base::Unretained(this)),
@@ -1452,9 +1448,9 @@ class WebMediaPlayerImplBackgroundBehaviorTest
     std::string enabled_features;
     std::string disabled_features;
     if (IsBackgroundOptimizationOn()) {
-      enabled_features += kBackgroundVideoTrackOptimization.name;
+      enabled_features += kBackgroundSrcVideoTrackOptimization.name;
     } else {
-      disabled_features += kBackgroundVideoTrackOptimization.name;
+      disabled_features += kBackgroundSrcVideoTrackOptimization.name;
     }
 
     if (IsBackgroundPauseOn()) {
@@ -1531,11 +1527,7 @@ class WebMediaPlayerImplBackgroundBehaviorTest
   }
 
   int GetMaxKeyframeDistanceSec() const {
-    base::TimeDelta max_keyframe_distance =
-        std::get<kIsMediaSource>(GetParam())
-            ? kMaxKeyframeDistanceToDisableBackgroundVideoMSE
-            : kMaxKeyframeDistanceToDisableBackgroundVideo;
-    return max_keyframe_distance.InSeconds();
+    return kMaxKeyframeDistanceToDisableBackgroundVideo.InSeconds();
   }
 
   bool IsAndroid() {

diff --git a/media/blink/webmediaplayer_params.cc b/media/blink/webmediaplayer_params.cc
@@ -23,8 +23,6 @@ WebMediaPlayerParams::WebMediaPlayerParams(
     blink::WebContentDecryptionModule* initial_cdm,
     RequestRoutingTokenCallback request_routing_token_cb,
     base::WeakPtr<MediaObserver> media_observer,
-    base::TimeDelta max_keyframe_distance_to_disable_background_video,
-    base::TimeDelta max_keyframe_distance_to_disable_background_video_mse,
     bool enable_instant_source_buffer_gc,
     bool embedded_media_experience_enabled,
     mojom::MediaMetricsProviderPtr metrics_provider,
@@ -42,10 +40,6 @@ WebMediaPlayerParams::WebMediaPlayerParams(
       initial_cdm_(initial_cdm),
       request_routing_token_cb_(std::move(request_routing_token_cb)),
       media_observer_(media_observer),
-      max_keyframe_distance_to_disable_background_video_(
-          max_keyframe_distance_to_disable_background_video),
-      max_keyframe_distance_to_disable_background_video_mse_(
-          max_keyframe_distance_to_disable_background_video_mse),
       enable_instant_source_buffer_gc_(enable_instant_source_buffer_gc),
       embedded_media_experience_enabled_(embedded_media_experience_enabled),
       metrics_provider_(std::move(metrics_provider)),

diff --git a/media/blink/webmediaplayer_params.h b/media/blink/webmediaplayer_params.h
@@ -81,8 +81,6 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerParams {
       blink::WebContentDecryptionModule* initial_cdm,
       RequestRoutingTokenCallback request_routing_token_cb,
       base::WeakPtr<MediaObserver> media_observer,
-      base::TimeDelta max_keyframe_distance_to_disable_background_video,
-      base::TimeDelta max_keyframe_distance_to_disable_background_video_mse,
       bool enable_instant_source_buffer_gc,
       bool embedded_media_experience_enabled,
       mojom::MediaMetricsProviderPtr metrics_provider,
@@ -135,15 +133,6 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerParams {
     return media_observer_;
   }
 
-  base::TimeDelta max_keyframe_distance_to_disable_background_video() const {
-    return max_keyframe_distance_to_disable_background_video_;
-  }
-
-  base::TimeDelta max_keyframe_distance_to_disable_background_video_mse()
-      const {
-    return max_keyframe_distance_to_disable_background_video_mse_;
-  }
-
   bool enable_instant_source_buffer_gc() const {
     return enable_instant_source_buffer_gc_;
   }
@@ -182,8 +171,6 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerParams {
   blink::WebContentDecryptionModule* initial_cdm_;
   RequestRoutingTokenCallback request_routing_token_cb_;
   base::WeakPtr<MediaObserver> media_observer_;
-  base::TimeDelta max_keyframe_distance_to_disable_background_video_;
-  base::TimeDelta max_keyframe_distance_to_disable_background_video_mse_;
   bool enable_instant_source_buffer_gc_;
   const bool embedded_media_experience_enabled_;
   mojom::MediaMetricsProviderPtr metrics_provider_;

diff --git a/testing/variations/fieldtrial_testing_config.json b/testing/variations/fieldtrial_testing_config.json
@@ -745,74 +745,20 @@
                 "chromeos",
                 "linux",
                 "mac",
-                "windows"
-            ],
-            "experiments": [
-                {
-                    "name": "BackgroundOptimizationEnabled1sOrLessMediaSource",
-                    "params": {
-                        "max_keyframe_distance_media_source_ms": "1000",
-                        "max_keyframe_distance_ms": "0"
-                    },
-                    "enable_features": [
-                        "BackgroundVideoTrackOptimization"
-                    ]
-                },
-                {
-                    "name": "BackgroundOptimizationEnabled3sOrLessMediaSource",
-                    "params": {
-                        "max_keyframe_distance_media_source_ms": "3000",
-                        "max_keyframe_distance_ms": "0"
-                    },
-                    "enable_features": [
-                        "BackgroundVideoTrackOptimization"
-                    ]
-                },
-                {
-                    "name": "BackgroundOptimizationEnabled5sOrLessMediaSource",
-                    "params": {
-                        "max_keyframe_distance_media_source_ms": "5000",
-                        "max_keyframe_distance_ms": "0"
-                    },
-                    "enable_features": [
-                        "BackgroundVideoTrackOptimization"
-                    ]
-                }
-            ]
-        },
-        {
-            "platforms": [
+                "windows",
                 "android"
             ],
             "experiments": [
                 {
-                    "name": "BackgroundOptimizationEnabled1sOrLessMediaSource",
-                    "params": {
-                        "max_keyframe_distance_media_source_ms": "1000",
-                        "max_keyframe_distance_ms": "1000"
-                    },
-                    "enable_features": [
-                        "BackgroundVideoTrackOptimization"
-                    ]
-                },
-                {
-                    "name": "BackgroundOptimizationEnabled3sOrLessMediaSource",
-                    "params": {
-                        "max_keyframe_distance_media_source_ms": "3000",
-                        "max_keyframe_distance_ms": "1000"
-                    },
+                    "name": "Enabled",
                     "enable_features": [
-                        "BackgroundVideoTrackOptimization"
+                        "BackgroundSrcVideoTrackOptimization"
                     ]
                 },
                 {
-                    "name": "BackgroundOptimizationEnabled5sOrLessMediaSource",
-                    "params": {
-                        "max_keyframe_distance_media_source_ms": "5000",
-                        "max_keyframe_distance_ms": "1000"
-                    },
-                    "enable_features": [
-                        "BackgroundVideoTrackOptimization"
+                    "name": "Control",
+                    "disable_features": [
+                        "BackgroundSrcVideoTrackOptimization"
                     ]
                 }
             ]

diff --git a/third_party/blink/renderer/core/html/media/html_media_element.cc b/third_party/blink/renderer/core/html/media/html_media_element.cc
@@ -3350,13 +3350,10 @@ void HTMLMediaElement::RemotePlaybackCompatibilityChanged(const WebURL& url,
 }
 
 bool HTMLMediaElement::HasSelectedVideoTrack() {
-  DCHECK(RuntimeEnabledFeatures::BackgroundVideoTrackOptimizationEnabled());
-
   return video_tracks_ && video_tracks_->selectedIndex() != -1;
 }
 
 WebMediaPlayer::TrackId HTMLMediaElement::GetSelectedVideoTrackId() {
-  DCHECK(RuntimeEnabledFeatures::BackgroundVideoTrackOptimizationEnabled());
   DCHECK(HasSelectedVideoTrack());
 
   int selected_track_index = video_tracks_->selectedIndex();