Enable SurfaceLayerForVideo for PIP only.

This CL adds a new flag, UseSurfaceLayerForVideoPIP, which will
switch to SurfaceLayer only when entering PIP mode.  The player does
not switch back when exiting PIP.

When doing this, the VideoFrameCompositor, VFS, etc. all run on
the compositor impl thread rather than a dedicated media thread.
This is to simplify the switchover.

This CL also changes the default from Use...Video to Use...PIP.

This is based on
https://chromium-review.googlesource.com/c/chromium/src/+/1219948

Bug: 883931

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ibb612d83df721712929aeab42aba790e32c1a8ea
Reviewed-on: https://chromium-review.googlesource.com/1220562
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594459}

android_webview/lib/aw_main_delegate.cc

@@ -173,9 +173,11 @@ bool AwMainDelegate::BasicStartupComplete(int* exit_code) {
   CommandLineHelper::AddDisabledFeature(*cl, media::kUseAndroidOverlay.name);
 
   // WebView doesn't support embedding CompositorFrameSinks which is needed for
-  // UseSurfaceLayerForVideo feature. https://crbug.com/853832
+  // UseSurfaceLayerForVideo[PIP] feature. https://crbug.com/853832
   CommandLineHelper::AddDisabledFeature(*cl,
                                         media::kUseSurfaceLayerForVideo.name);
+  CommandLineHelper::AddDisabledFeature(
+      *cl, media::kUseSurfaceLayerForVideoPIP.name);
 
   // WebView does not support EME persistent license yet, because it's not
   // clear on how user can remove persistent media licenses from UI.

content/renderer/media/media_factory.cc

@@ -129,18 +129,27 @@ void PostContextProviderToCallback(
 namespace content {
 
 // static
-bool MediaFactory::VideoSurfaceLayerEnabled() {
+media::WebMediaPlayerParams::SurfaceLayerMode
+MediaFactory::GetVideoSurfaceLayerMode() {
   // LayoutTests do not support SurfaceLayer by default at the moment.
   // See https://crbug.com/838128
   content::RenderThreadImpl* render_thread =
       content::RenderThreadImpl::current();
   if (render_thread && render_thread->layout_test_mode() &&
       !render_thread->LayoutTestModeUsesDisplayCompositorPixelDump()) {
-    return false;
+    return media::WebMediaPlayerParams::SurfaceLayerMode::kNever;
   }
 
-  return base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo) &&
-         !features::IsMultiProcessMash();
+  if (features::IsMultiProcessMash())
+    return media::WebMediaPlayerParams::SurfaceLayerMode::kNever;
+
+  if (base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo))
+    return media::WebMediaPlayerParams::SurfaceLayerMode::kAlways;
+
+  if (base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideoPIP))
+    return media::WebMediaPlayerParams::SurfaceLayerMode::kOnDemand;
+
+  return media::WebMediaPlayerParams::SurfaceLayerMode::kNever;
 }
 
 bool MediaFactory::VideoSurfaceLayerEnabledForMS() {
@@ -282,23 +291,36 @@ blink::WebMediaPlayer* MediaFactory::CreateMediaPlayer(
   scoped_refptr<base::SingleThreadTaskRunner>
       video_frame_compositor_task_runner;
   std::unique_ptr<blink::WebVideoFrameSubmitter> submitter;
-  bool use_surface_layer_for_video = VideoSurfaceLayerEnabled();
-  if (use_surface_layer_for_video) {
+  media::WebMediaPlayerParams::SurfaceLayerMode use_surface_layer_for_video =
+      GetVideoSurfaceLayerMode();
+  bool use_sync_primitives = false;
+  if (use_surface_layer_for_video ==
+      media::WebMediaPlayerParams::SurfaceLayerMode::kAlways) {
+    // Run the compositor / frame submitter on its own thread.
     video_frame_compositor_task_runner =
         render_thread->CreateVideoFrameCompositorTaskRunner();
-    submitter = blink::WebVideoFrameSubmitter::Create(
-        base::BindRepeating(
-            &PostContextProviderToCallback,
-            RenderThreadImpl::current()->GetCompositorMainThreadTaskRunner()),
-        settings);
+    // We must use sync primitives on this thread.
+    use_sync_primitives = true;
   } else {
+    // Run on the cc thread, even if we may switch to SurfaceLayer mode later
+    // if we're in kOnDemand mode.  We do this to avoid switching threads when
+    // switching to SurfaceLayer.
     video_frame_compositor_task_runner =
         render_thread->compositor_task_runner()
             ? render_thread->compositor_task_runner()
             : render_frame_->GetTaskRunner(
                   blink::TaskType::kInternalMediaRealTime);
   }
 
+  if (use_surface_layer_for_video !=
+      media::WebMediaPlayerParams::SurfaceLayerMode::kNever) {
+    submitter = blink::WebVideoFrameSubmitter::Create(
+        base::BindRepeating(
+            &PostContextProviderToCallback,
+            RenderThreadImpl::current()->GetCompositorMainThreadTaskRunner()),
+        settings, use_sync_primitives);
+  }
+
   DCHECK(layer_tree_view);
   scoped_refptr<base::SingleThreadTaskRunner> media_task_runner =
       render_thread->GetMediaThreadTaskRunner();
@@ -530,7 +552,7 @@ blink::WebMediaPlayer* MediaFactory::CreateWebMediaPlayerForMediaStream(
           base::BindRepeating(
               &PostContextProviderToCallback,
               RenderThreadImpl::current()->GetCompositorMainThreadTaskRunner()),
-          settings),
+          settings, true),
       VideoSurfaceLayerEnabledForMS());
 }
 

content/renderer/media/media_factory.h

@@ -12,6 +12,7 @@
 #include "media/base/renderer_factory_selector.h"
 #include "media/base/routing_token_callback.h"
 #include "media/blink/url_index.h"
+#include "media/blink/webmediaplayer_params.h"
 #include "media/media_buildflags.h"
 #include "media/mojo/buildflags.h"
 #include "media/mojo/interfaces/remoting.mojom.h"
@@ -72,7 +73,8 @@ class RendererMediaPlayerManager;
 class MediaFactory {
  public:
   // Helper function returning whether VideoSurfaceLayer should be enabled.
-  static bool VideoSurfaceLayerEnabled();
+  static media::WebMediaPlayerParams::SurfaceLayerMode
+  GetVideoSurfaceLayerMode();
 
   // Helper function returning whether VideoSurfaceLayer should be enabled for
   // MediaStreams.

content/renderer/render_view_impl.cc

@@ -934,7 +934,8 @@ void RenderView::ApplyWebPreferences(const WebPreferences& prefs,
 
   settings->SetPictureInPictureEnabled(
       prefs.picture_in_picture_enabled &&
-      MediaFactory::VideoSurfaceLayerEnabled());
+      MediaFactory::GetVideoSurfaceLayerMode() !=
+          media::WebMediaPlayerParams::SurfaceLayerMode::kNever);
 
   settings->SetDataSaverHoldbackWebApi(
       prefs.data_saver_holdback_web_api_enabled);

media/base/media_switches.cc

@@ -320,14 +320,21 @@ const base::Feature kUseR16Texture{"use-r16-texture",
 const base::Feature kUnifiedAutoplay{"UnifiedAutoplay",
                                      base::FEATURE_ENABLED_BY_DEFAULT};
 
-// Use SurfaceLayer instead of VideoLayer.
+// If enabled, use SurfaceLayer instead of VideoLayer for all playbacks that
+// aren't MediaStream.
 const base::Feature kUseSurfaceLayerForVideo{"UseSurfaceLayerForVideo",
                                              base::FEATURE_ENABLED_BY_DEFAULT};
 
 // Use SurfaceLayer instead of VideoLayer for MediaStream.
 const base::Feature kUseSurfaceLayerForVideoMS{
     "UseSurfaceLayerForVideoMS", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Use SurfaceLayer instead of VideoLayer when entering Picture-in-Picture mode.
+// Does nothing if UseSurfaceLayerForVideo is enabled.  Does not affect
+// MediaStream playbacks.
+const base::Feature kUseSurfaceLayerForVideoPIP{
+    "UseSurfaceLayerForVideoPIP", base::FEATURE_ENABLED_BY_DEFAULT};
+
 // Enable VA-API hardware encode acceleration for VP8.
 const base::Feature kVaapiVP8Encoder{"VaapiVP8Encoder",
                                      base::FEATURE_DISABLED_BY_DEFAULT};

media/base/media_switches.h

@@ -137,6 +137,7 @@ MEDIA_EXPORT extern const base::Feature kUseNewMediaCache;
 MEDIA_EXPORT extern const base::Feature kUseR16Texture;
 MEDIA_EXPORT extern const base::Feature kUseSurfaceLayerForVideo;
 MEDIA_EXPORT extern const base::Feature kUseSurfaceLayerForVideoMS;
+MEDIA_EXPORT extern const base::Feature kUseSurfaceLayerForVideoPIP;
 MEDIA_EXPORT extern const base::Feature kVaapiVP8Encoder;
 MEDIA_EXPORT extern const base::Feature kVideoBlitColorAccuracy;
 

media/blink/video_frame_compositor.cc

@@ -84,11 +84,18 @@ void VideoFrameCompositor::EnableSubmission(
     bool is_opaque,
     blink::WebFrameSinkDestroyedCallback frame_sink_destroyed_callback) {
   DCHECK(task_runner_->BelongsToCurrentThread());
+
+  // If we're switching to |submitter_| from some other client, then tell it.
+  if (client_ && client_ != submitter_.get())
+    client_->StopUsingProvider();
+
   submitter_->SetRotation(rotation);
   submitter_->SetForceSubmit(force_submit);
   submitter_->SetIsOpaque(is_opaque);
   submitter_->EnableSubmission(id, std::move(frame_sink_destroyed_callback));
   client_ = submitter_.get();
+  if (rendering_)
+    client_->StartRendering();
 }
 
 bool VideoFrameCompositor::IsClientSinkAvailable() {

media/blink/webmediaplayer_impl.cc

@@ -270,7 +270,7 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
           params->enable_instant_source_buffer_gc()),
       embedded_media_experience_enabled_(
           params->embedded_media_experience_enabled()),
-      surface_layer_for_video_enabled_(params->use_surface_layer_for_video()),
+      surface_layer_mode_(params->use_surface_layer_for_video()),
       create_bridge_callback_(params->create_bridge_callback()),
       request_routing_token_cb_(params->request_routing_token_cb()),
       overlay_routing_token_(OverlayInfo::RoutingToken()),
@@ -818,6 +818,9 @@ void WebMediaPlayerImpl::SetVolume(double volume) {
 
 void WebMediaPlayerImpl::EnterPictureInPicture(
     blink::WebMediaPlayer::PipWindowOpenedCallback callback) {
+  if (!surface_layer_for_video_enabled_)
+    ActivateSurfaceLayerForVideo();
+
   DCHECK(bridge_);
 
   const viz::SurfaceId& surface_id = bridge_->GetSurfaceId();
@@ -1669,43 +1672,16 @@ void WebMediaPlayerImpl::OnMetadata(PipelineMetadata metadata) {
         DisableOverlay();
     }
 
-    if (!surface_layer_for_video_enabled_) {
+    if (surface_layer_mode_ !=
+        WebMediaPlayerParams::SurfaceLayerMode::kAlways) {
       DCHECK(!video_layer_);
       video_layer_ = cc::VideoLayer::Create(
           compositor_.get(),
           pipeline_metadata_.video_decoder_config.video_rotation());
       video_layer_->SetContentsOpaque(opaque_);
       client_->SetCcLayer(video_layer_.get());
     } else {
-      DCHECK(!bridge_);
-
-      bridge_ = std::move(create_bridge_callback_)
-                    .Run(this, compositor_->GetUpdateSubmissionStateCallback());
-      bridge_->CreateSurfaceLayer();
-
-      vfc_task_runner_->PostTask(
-          FROM_HERE,
-          base::BindOnce(
-              &VideoFrameCompositor::EnableSubmission,
-              base::Unretained(compositor_.get()), bridge_->GetSurfaceId(),
-              pipeline_metadata_.video_decoder_config.video_rotation(),
-              IsInPictureInPicture(), opaque_,
-              BindToCurrentLoop(base::BindRepeating(
-                  &WebMediaPlayerImpl::OnFrameSinkDestroyed, AsWeakPtr()))));
-      bridge_->SetContentsOpaque(opaque_);
-
-      // If the element is already in Picture-in-Picture mode, it means that it
-      // was set in this mode prior to this load, with a different
-      // WebMediaPlayerImpl. The new player needs to send its id, size and
-      // surface id to the browser process to make sure the states are properly
-      // updated.
-      // TODO(872056): the surface should be activated but for some reasons, it
-      // does not. It is possible that this will no longer be neded after 872056
-      // is fixed.
-      if (client_->DisplayType() ==
-          WebMediaPlayer::DisplayType::kPictureInPicture) {
-        OnSurfaceIdUpdated(bridge_->GetSurfaceId());
-      }
+      ActivateSurfaceLayerForVideo();
     }
   }
 
@@ -1721,6 +1697,47 @@ void WebMediaPlayerImpl::OnMetadata(PipelineMetadata metadata) {
   UpdatePlayState();
 }
 
+void WebMediaPlayerImpl::ActivateSurfaceLayerForVideo() {
+  // Note that we might or might not already be in VideoLayer mode.
+  DCHECK(!bridge_);
+
+  surface_layer_for_video_enabled_ = true;
+
+  // If we're in VideoLayer mode, then get rid of the layer.
+  if (video_layer_) {
+    client_->SetCcLayer(nullptr);
+    video_layer_ = nullptr;
+  }
+
+  bridge_ = std::move(create_bridge_callback_)
+                .Run(this, compositor_->GetUpdateSubmissionStateCallback());
+  bridge_->CreateSurfaceLayer();
+
+  vfc_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &VideoFrameCompositor::EnableSubmission,
+          base::Unretained(compositor_.get()), bridge_->GetSurfaceId(),
+          pipeline_metadata_.video_decoder_config.video_rotation(),
+          IsInPictureInPicture(), opaque_,
+          BindToCurrentLoop(base::BindRepeating(
+              &WebMediaPlayerImpl::OnFrameSinkDestroyed, AsWeakPtr()))));
+  bridge_->SetContentsOpaque(opaque_);
+
+  // If the element is already in Picture-in-Picture mode, it means that it
+  // was set in this mode prior to this load, with a different
+  // WebMediaPlayerImpl. The new player needs to send its id, size and
+  // surface id to the browser process to make sure the states are properly
+  // updated.
+  // TODO(872056): the surface should be activated but for some reasons, it
+  // does not. It is possible that this will no longer be needed after 872056
+  // is fixed.
+  if (client_->DisplayType() ==
+      WebMediaPlayer::DisplayType::kPictureInPicture) {
+    OnSurfaceIdUpdated(bridge_->GetSurfaceId());
+  }
+}
+
 void WebMediaPlayerImpl::OnFrameSinkDestroyed() {
   bridge_->ClearSurfaceId();
 }

media/blink/webmediaplayer_impl.h

@@ -594,6 +594,9 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   // Sets the UKM container name if needed.
   void MaybeSetContainerName();
 
+  // Switch to SurfaceLayer, either initially or from VideoLayer.
+  void ActivateSurfaceLayerForVideo();
+
   blink::WebLocalFrame* const frame_;
 
   // The playback state last reported to |delegate_|, to avoid setting duplicate
@@ -882,7 +885,11 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
   // Whether embedded media experience is currently enabled.
   bool embedded_media_experience_enabled_ = false;
 
-  // Whether the use of a surface layer instead of a video layer is enabled.
+  // When should we use SurfaceLayer for video?
+  WebMediaPlayerParams::SurfaceLayerMode surface_layer_mode_ =
+      WebMediaPlayerParams::SurfaceLayerMode::kNever;
+
+  // Whether surface layer is currently in use to display frames.
   bool surface_layer_for_video_enabled_ = false;
 
   CreateSurfaceLayerBridgeCB create_bridge_callback_;

media/blink/webmediaplayer_impl_unittest.cc

@@ -397,7 +397,9 @@ class WebMediaPlayerImplTest : public testing::Test {
         base::BindOnce(&WebMediaPlayerImplTest::CreateMockSurfaceLayerBridge,
                        base::Unretained(this)),
         viz::TestContextProvider::Create(),
-        base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo));
+        base::FeatureList::IsEnabled(media::kUseSurfaceLayerForVideo)
+            ? WebMediaPlayerParams::SurfaceLayerMode::kAlways
+            : WebMediaPlayerParams::SurfaceLayerMode::kNever);
 
     auto compositor = std::make_unique<StrictMock<MockVideoFrameCompositor>>(
         params->video_frame_compositor_task_runner());

media/blink/webmediaplayer_params.cc

@@ -28,7 +28,7 @@ WebMediaPlayerParams::WebMediaPlayerParams(
     mojom::MediaMetricsProviderPtr metrics_provider,
     CreateSurfaceLayerBridgeCB create_bridge_callback,
     scoped_refptr<viz::ContextProvider> context_provider,
-    bool use_surface_layer_for_video)
+    SurfaceLayerMode use_surface_layer_for_video)
     : defer_load_cb_(defer_load_cb),
       audio_renderer_sink_(audio_renderer_sink),
       media_log_(std::move(media_log)),

media/blink/webmediaplayer_params.h

@@ -66,6 +66,18 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerParams {
   // not the WebMediaPlayer!
   typedef base::Callback<int64_t(int64_t)> AdjustAllocatedMemoryCB;
 
+  // Describes when we use SurfaceLayer for video instead of VideoLayer.
+  enum class SurfaceLayerMode {
+    // Always use VideoLayer
+    kNever,
+
+    // Use SurfaceLayer only when we switch to Picture-in-Picture.
+    kOnDemand,
+
+    // Always use SurfaceLayer for video.
+    kAlways,
+  };
+
   // |defer_load_cb|, |audio_renderer_sink|, |compositor_task_runner|, and
   // |context_3d_cb| may be null.
   WebMediaPlayerParams(
@@ -86,7 +98,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerParams {
       mojom::MediaMetricsProviderPtr metrics_provider,
       CreateSurfaceLayerBridgeCB bridge_callback,
       scoped_refptr<viz::ContextProvider> context_provider,
-      bool use_surface_layer_for_video);
+      SurfaceLayerMode use_surface_layer_for_video);
 
   ~WebMediaPlayerParams();
 
@@ -153,7 +165,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerParams {
     return context_provider_;
   }
 
-  bool use_surface_layer_for_video() const {
+  SurfaceLayerMode use_surface_layer_for_video() const {
     return use_surface_layer_for_video_;
   }
 
@@ -176,7 +188,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerParams {
   mojom::MediaMetricsProviderPtr metrics_provider_;
   CreateSurfaceLayerBridgeCB create_bridge_callback_;
   scoped_refptr<viz::ContextProvider> context_provider_;
-  bool use_surface_layer_for_video_;
+  SurfaceLayerMode use_surface_layer_for_video_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(WebMediaPlayerParams);
 };

third_party/blink/public/platform/web_video_frame_submitter.h

@@ -38,7 +38,8 @@ class BLINK_PLATFORM_EXPORT WebVideoFrameSubmitter
  public:
   static std::unique_ptr<WebVideoFrameSubmitter> Create(
       WebContextProviderCallback,
-      const cc::LayerTreeSettings&);
+      const cc::LayerTreeSettings&,
+      bool use_sync_primitives);
   ~WebVideoFrameSubmitter() override = default;
 
   // Intialize must be called before submissions occur, pulled out of