[media] Set the bounds of Sbplayer via video_painter.cc

SbPlayer needs SbPlayerSetBounds to update the bounds. This PR updates the bounds in blink via video_painter.cc for SbPlayer. Unlike Cobalt, Chromium upupdates the bounds of video frequently, resulting in multiple calls per video session for SbPlayer. We should revisit it if SbPlayerSetBounds causes performance issues. b/376320224
youtube · Nov 6, 2024 · 36d51af · 36d51af
1 parent ecc8ac9
commit 36d51af
Show file tree

Hide file tree

Showing 15 changed files with 179 additions and 26 deletions.
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
@@ -23,6 +23,11 @@
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/gfx/geometry/size.h"
 
+// For BUILDFLAG(USE_STARBOARD_MEDIA)
+#if BUILDFLAG(IS_COBALT)
+#include "starboard/build/starboard_buildflags.h"
+#endif  // BUILDFLAG(IS_COBALT)
+
 namespace media {
 
 class CdmContext;
@@ -263,6 +268,14 @@ class MEDIA_EXPORT Pipeline {
   using CdmAttachedCB = base::OnceCallback<void(bool)>;
   virtual void SetCdm(CdmContext* cdm_context,
                       CdmAttachedCB cdm_attached_cb) = 0;
+
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+  // Return SetBoundsCB if SbPlayer is used for rendering.
+  using SetBoundsCB = base::OnceCallback<bool(int x, int y, int width, int height)>;
+  virtual SetBoundsCB GetSetBoundsCB() = 0;
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
 };
 
 }  // namespace media

diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
@@ -88,6 +88,11 @@ class PipelineImpl::RendererWrapper final : public DemuxerHost,
   bool DidLoadingProgress();
   PipelineStatistics GetStatistics() const;
   void SetCdm(CdmContext* cdm_context, CdmAttachedCB cdm_attached_cb);
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+  SetBoundsCB GetSetBoundsCB();
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
 
   // |enabled_track_ids| contains track ids of enabled audio tracks.
   void OnEnabledAudioTracksChanged(
@@ -593,6 +598,16 @@ void PipelineImpl::RendererWrapper::SetCdm(CdmContext* cdm_context,
     CreateRendererInternal(std::move(create_renderer_done_cb_));
 }
 
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+Pipeline::SetBoundsCB PipelineImpl::RendererWrapper::GetSetBoundsCB() {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+  return shared_state_.renderer? shared_state_.renderer->GetSetBoundsCB() :
+      base::BindOnce(&SetBoundsNullTask);
+}
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
+
 void PipelineImpl::RendererWrapper::CreateRendererInternal(
     PipelineStatusCallback done_cb) {
   DVLOG(1) << __func__;
@@ -1576,6 +1591,15 @@ const char* PipelineImpl::GetStateString(State state) {
 
 #undef RETURN_STRING
 
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+Pipeline::SetBoundsCB PipelineImpl::GetSetBoundsCB() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return renderer_wrapper_->GetSetBoundsCB();
+}
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
+
 void PipelineImpl::AsyncCreateRenderer(
     absl::optional<RendererType> renderer_type,
     RendererCreatedCB renderer_created_cb) {

diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
@@ -19,6 +19,11 @@
 #include "media/base/renderer_factory_selector.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
+// For BUILDFLAG(USE_STARBOARD_MEDIA)
+#if BUILDFLAG(IS_COBALT)
+#include "starboard/build/starboard_buildflags.h"
+#endif  // BUILDFLAG(IS_COBALT)
+
 namespace base {
 class SingleThreadTaskRunner;
 }
@@ -119,6 +124,11 @@ class MEDIA_EXPORT PipelineImpl : public Pipeline {
   bool DidLoadingProgress() override;
   PipelineStatistics GetStatistics() const override;
   void SetCdm(CdmContext* cdm_context, CdmAttachedCB cdm_attached_cb) override;
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+  SetBoundsCB GetSetBoundsCB() override;
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
 
   // |enabled_track_ids| contains track ids of enabled audio tracks.
   void OnEnabledAudioTracksChanged(

diff --git a/media/base/renderer.cc b/media/base/renderer.cc
@@ -36,6 +36,14 @@ std::string GetRendererName(RendererType renderer_type) {
   }
 }
 
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+bool SetBoundsNullTask(int x, int y, int width, int height) {
+  return false;
+}
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
+
 Renderer::Renderer() = default;
 
 Renderer::~Renderer() = default;
@@ -72,4 +80,13 @@ void Renderer::OnExternalVideoFrameRequest() {
   // Default implementation of OnExternalVideoFrameRequest is to no-op.
 }
 
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+Renderer::SetBoundsCB Renderer::GetSetBoundsCB() {
+  // Default implementation of GetSetBoundsCB is to no-op.
+  return base::BindOnce(&SetBoundsNullTask);
+}
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
+
 }  // namespace media
diff --git a/media/base/renderer.h b/media/base/renderer.h
@@ -13,6 +13,11 @@
 #include "media/base/pipeline_status.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
+// For BUILDFLAG(USE_STARBOARD_MEDIA)
+#if BUILDFLAG(IS_COBALT)
+#include "starboard/build/starboard_buildflags.h"
+#endif  // BUILDFLAG(IS_COBALT)
+
 namespace media {
 
 class CdmContext;
@@ -43,6 +48,12 @@ enum class RendererType {
 // the actual Renderer class name or a descriptive name.
 std::string MEDIA_EXPORT GetRendererName(RendererType renderer_type);
 
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+bool MEDIA_EXPORT SetBoundsNullTask(int x, int y, int width, int height);
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
+
 class MEDIA_EXPORT Renderer {
  public:
   Renderer();
@@ -131,6 +142,14 @@ class MEDIA_EXPORT Renderer {
   // enforce RendererType registration for all Renderer implementations.
   // Note: New implementation should update RendererType.
   virtual RendererType GetRendererType() = 0;
+
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+  // Return SetBoundsCB if SbPlayer is used for rendering.
+  using SetBoundsCB = base::OnceCallback<bool(int x, int y, int width, int height)>;
+  virtual SetBoundsCB GetSetBoundsCB();
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
 };
 
 }  // namespace media

diff --git a/media/filters/pipeline_controller.cc b/media/filters/pipeline_controller.cc
@@ -448,6 +448,15 @@ void PipelineController::OnExternalVideoFrameRequest() {
   pipeline_->OnExternalVideoFrameRequest();
 }
 
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+Pipeline::SetBoundsCB PipelineController::GetSetBoundsCB() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return pipeline_->GetSetBoundsCB();
+}
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
+
 void PipelineController::FireOnTrackChangeCompleteForTesting(State set_to) {
   previous_track_change_state_ = set_to;
   OnTrackChangeComplete();

diff --git a/media/filters/pipeline_controller.h b/media/filters/pipeline_controller.h
@@ -14,6 +14,11 @@
 #include "media/base/pipeline.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
+// For BUILDFLAG(USE_STARBOARD_MEDIA)
+#if BUILDFLAG(IS_COBALT)
+#include "starboard/build/starboard_buildflags.h"
+#endif  // BUILDFLAG(IS_COBALT)
+
 namespace media {
 
 class CdmContext;
@@ -149,6 +154,11 @@ class MEDIA_EXPORT PipelineController {
   void OnSelectedVideoTrackChanged(
       absl::optional<MediaTrack::Id> selected_track_id);
   void OnExternalVideoFrameRequest();
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+  Pipeline::SetBoundsCB GetSetBoundsCB();
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
 
   // Used to fire the OnTrackChangeComplete function which is captured in a
   // OnceCallback, and doesn't play nicely with gmock.

diff --git a/media/starboard/sbplayer_bridge.cc b/media/starboard/sbplayer_bridge.cc
@@ -173,8 +173,7 @@ SbPlayerBridge::SbPlayerBridge(
       pipeline_identifier_(pipeline_identifier),
       is_url_based_(true) {
   DCHECK(host_);
-  // TODO(b/352389546): set bounds via video_painter.cc
-  // DCHECK(set_bounds_helper_);
+  DCHECK(set_bounds_helper_);
 
   output_mode_ = ComputeSbUrlPlayerOutputMode(default_output_mode);
 
@@ -244,8 +243,7 @@ SbPlayerBridge::SbPlayerBridge(
 #endif  // COBALT_MEDIA_ENABLE_DECODE_TARGET_PROVIDER
   DCHECK(audio_config.IsValidConfig() || video_config.IsValidConfig());
   DCHECK(host_);
-  // TODO(b/352389546): set bounds via video_painter.cc
-  // DCHECK(set_bounds_helper_);
+  DCHECK(set_bounds_helper_);
 #if COBALT_MEDIA_ENABLE_DECODE_TARGET_PROVIDER
   DCHECK(decode_target_provider_);
 #endif  // COBALT_MEDIA_ENABLE_DECODE_TARGET_PROVIDER
@@ -279,8 +277,7 @@ SbPlayerBridge::~SbPlayerBridge() {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
 
   callback_helper_->ResetPlayer();
-  // TODO(b/352389546): set bounds via video_painter.cc
-  // set_bounds_helper_->SetPlayerBridge(NULL);
+  set_bounds_helper_->SetPlayerBridge(NULL);
 
 #if COBALT_MEDIA_ENABLE_DECODE_TARGET_PROVIDER
   decode_target_provider_->SetOutputMode(
@@ -591,8 +588,7 @@ void SbPlayerBridge::Suspend() {
 
   sbplayer_interface_->SetPlaybackRate(player_, 0.0);
 
-  // TODO(b/352389546): set bounds via video_painter.cc
-  // set_bounds_helper_->SetPlayerBridge(NULL);
+  set_bounds_helper_->SetPlayerBridge(NULL);
 
   base::AutoLock auto_lock(lock_);
   GetInfo_Locked(&cached_video_frames_decoded_, &cached_video_frames_dropped_,
@@ -724,8 +720,7 @@ void SbPlayerBridge::CreateUrlPlayer(const std::string& url) {
   decode_target_provider_->SetOutputMode(
       ToVideoFrameProviderOutputMode(output_mode_));
 
-  // TODO(b/352389546): set bounds via video_painter.cc
-  // set_bounds_helper_->SetPlayerBridge(this);
+  set_bounds_helper_->SetPlayerBridge(this);
 
   base::AutoLock auto_lock(lock_);
   UpdateBounds_Locked();
@@ -826,8 +821,7 @@ void SbPlayerBridge::CreatePlayer() {
   decode_target_provider_->SetOutputMode(
       ToVideoFrameProviderOutputMode(output_mode_));
 #endif  // COBALT_MEDIA_ENABLE_DECODE_TARGET_PROVIDER
-  // TODO(b/352389546): set bounds via video_painter.cc
-  // set_bounds_helper_->SetPlayerBridge(this);
+  set_bounds_helper_->SetPlayerBridge(this);
 
   base::AutoLock auto_lock(lock_);
   UpdateBounds_Locked();

diff --git a/media/starboard/sbplayer_bridge.h b/media/starboard/sbplayer_bridge.h
@@ -294,8 +294,6 @@ class SbPlayerBridge {
   SbWindow window_;
   SbDrmSystem drm_system_ = kSbDrmSystemInvalid;
   Host* const host_;
-  // TODO(b/376320224): Ensure that set bounds works
-  // Consider merge |SbPlayerSetBoundsHelper| into CallbackHelper.
   SbPlayerSetBoundsHelper* const set_bounds_helper_;
   const bool allow_resume_after_suspend_;
 

diff --git a/media/starboard/starboard_renderer.cc b/media/starboard/starboard_renderer.cc
@@ -58,10 +58,12 @@ StarboardRenderer::StarboardRenderer(
     VideoRendererSink* video_renderer_sink)
     : task_runner_(task_runner),
       video_renderer_sink_(video_renderer_sink),
-      video_overlay_factory_(std::make_unique<VideoOverlayFactory>()) {
+      video_overlay_factory_(std::make_unique<VideoOverlayFactory>()),
+      set_bounds_helper_(new SbPlayerSetBoundsHelper) {
   DCHECK(task_runner_);
   DCHECK(video_renderer_sink_);
   DCHECK(video_overlay_factory_);
+  DCHECK(set_bounds_helper_);
   LOG(INFO) << "StarboardRenderer constructed.";
 }
 
@@ -275,6 +277,11 @@ base::TimeDelta StarboardRenderer::GetMediaTime() {
   return media_time;
 }
 
+Renderer::SetBoundsCB StarboardRenderer::GetSetBoundsCB() {
+  return base::BindOnce(&SbPlayerSetBoundsHelper::SetBounds,
+                        set_bounds_helper_);
+}
+
 void StarboardRenderer::CreatePlayerBridge(PipelineStatusCallback init_cb) {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
   DCHECK(init_cb);
@@ -340,9 +347,7 @@ void StarboardRenderer::CreatePlayerBridge(PipelineStatusCallback init_cb) {
         // TODO(b/326508279): Support background mode.
         kSbWindowInvalid,
         // TODO(b/328305808): Implement SbDrm support.
-        kSbDrmSystemInvalid, this,
-        // TODO(b/376320224); Verify set bounds works
-        nullptr,
+        kSbDrmSystemInvalid, this, set_bounds_helper_.get(),
         // TODO(b/326497953): Support suspend/resume.
         false,
         // TODO(b/326825450): Revisit 360 videos.

diff --git a/media/starboard/starboard_renderer.h b/media/starboard/starboard_renderer.h
@@ -33,6 +33,7 @@
 #include "media/base/video_renderer_sink.h"
 #include "media/renderers/video_overlay_factory.h"
 #include "media/starboard/sbplayer_bridge.h"
+#include "media/starboard/sbplayer_set_bounds_helper.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace media {
@@ -90,6 +91,7 @@ class MEDIA_EXPORT StarboardRenderer final : public Renderer,
     // TODO(b/375278384): Properly setup the renderer type.
     return RendererType::kRendererImpl;
   }
+  SetBoundsCB GetSetBoundsCB() override;
 
  private:
   void CreatePlayerBridge(PipelineStatusCallback init_cb);
@@ -120,6 +122,8 @@ class MEDIA_EXPORT StarboardRenderer final : public Renderer,
   // by the remote renderer.
   std::unique_ptr<VideoOverlayFactory> video_overlay_factory_;
 
+  scoped_refptr<SbPlayerSetBoundsHelper> set_bounds_helper_;
+
   DefaultSbPlayerInterface sbplayer_interface_;
   // TODO(b/326652276): Support audio write duration.
   const base::TimeDelta audio_write_duration_local_ = base::Milliseconds(500);

diff --git a/third_party/blink/public/platform/web_media_player.h b/third_party/blink/public/platform/web_media_player.h
@@ -48,6 +48,11 @@
 #include "ui/gfx/geometry/size.h"
 #include "url/gurl.h"
 
+// For BUILDFLAG(USE_STARBOARD_MEDIA)
+#if BUILDFLAG(IS_COBALT)
+#include "starboard/build/starboard_buildflags.h"
+#endif  // BUILDFLAG(IS_COBALT)
+
 namespace cc {
 class PaintCanvas;
 class PaintFlags;
@@ -303,6 +308,14 @@ class WebMediaPlayer {
   // Sets the poster image URL.
   virtual void SetPoster(const WebURL& poster) {}
 
+#if BUILDFLAG(IS_COBALT)
+#if BUILDFLAG(USE_STARBOARD_MEDIA)
+  // Return SetBoundsCB if SbPlayer is used for rendering.
+  using SetBoundsCB = base::OnceCallback<bool(int x, int y, int width, int height)>;
+  virtual SetBoundsCB GetSetBoundsCB() { return SetBoundsCB(); }
+#endif // BUILDFLAG(USE_STARBOARD_MEDIA)
+#endif // BUILDFLAG(IS_COBALT)
+
   // Inform WebMediaPlayer when the element has entered/exited fullscreen.
   virtual void EnteredFullscreen() {}
   virtual void ExitedFullscreen() {}