From fba14f7355acfe94e5261194727271d79cbc5364 Mon Sep 17 00:00:00 2001 From: Bo-Rong Chen Date: Wed, 6 Nov 2024 14:13:10 -0800 Subject: [PATCH] [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 --- media/base/pipeline.h | 6 ++++++ media/base/pipeline_impl.cc | 18 ++++++++++++++++++ media/base/pipeline_impl.h | 3 +++ media/base/renderer.cc | 13 +++++++++++++ media/base/renderer.h | 10 ++++++++++ media/filters/pipeline_controller.cc | 7 +++++++ media/filters/pipeline_controller.h | 3 +++ media/starboard/sbplayer_bridge.cc | 18 ++++++------------ media/starboard/sbplayer_bridge.h | 2 -- media/starboard/starboard_renderer.cc | 13 +++++++++---- media/starboard/starboard_renderer.h | 4 ++++ .../blink/public/platform/web_media_player.h | 6 ++++++ .../blink/renderer/core/paint/video_painter.cc | 12 ++++++++++++ .../platform/media/web_media_player_impl.cc | 10 +++++++++- .../platform/media/web_media_player_impl.h | 8 ++++++++ 15 files changed, 114 insertions(+), 19 deletions(-) diff --git a/media/base/pipeline.h b/media/base/pipeline.h index 3359a7d904b3..39a42f3d8d92 100644 --- a/media/base/pipeline.h +++ b/media/base/pipeline.h @@ -263,6 +263,12 @@ class MEDIA_EXPORT Pipeline { using CdmAttachedCB = base::OnceCallback; virtual void SetCdm(CdmContext* cdm_context, CdmAttachedCB cdm_attached_cb) = 0; + +#if BUILDFLAG(USE_STARBOARD_MEDIA) + // Return SetBoundsCB if SbPlayer is used for rendering. + using SetBoundsCB = base::OnceCallback; + virtual SetBoundsCB GetSetBoundsCB() = 0; +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) }; } // namespace media diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc index 93f15566d72f..3c13ee6fd364 100644 --- a/media/base/pipeline_impl.cc +++ b/media/base/pipeline_impl.cc @@ -88,6 +88,9 @@ class PipelineImpl::RendererWrapper final : public DemuxerHost, bool DidLoadingProgress(); PipelineStatistics GetStatistics() const; void SetCdm(CdmContext* cdm_context, CdmAttachedCB cdm_attached_cb); +#if BUILDFLAG(USE_STARBOARD_MEDIA) + SetBoundsCB GetSetBoundsCB(); +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) // |enabled_track_ids| contains track ids of enabled audio tracks. void OnEnabledAudioTracksChanged( @@ -593,6 +596,14 @@ void PipelineImpl::RendererWrapper::SetCdm(CdmContext* cdm_context, CreateRendererInternal(std::move(create_renderer_done_cb_)); } +#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) + void PipelineImpl::RendererWrapper::CreateRendererInternal( PipelineStatusCallback done_cb) { DVLOG(1) << __func__; @@ -1576,6 +1587,13 @@ const char* PipelineImpl::GetStateString(State state) { #undef RETURN_STRING +#if BUILDFLAG(USE_STARBOARD_MEDIA) +Pipeline::SetBoundsCB PipelineImpl::GetSetBoundsCB() { + DCHECK(thread_checker_.CalledOnValidThread()); + return renderer_wrapper_->GetSetBoundsCB(); +} +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + void PipelineImpl::AsyncCreateRenderer( absl::optional renderer_type, RendererCreatedCB renderer_created_cb) { diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h index 418ff17cb857..603ddcaed907 100644 --- a/media/base/pipeline_impl.h +++ b/media/base/pipeline_impl.h @@ -119,6 +119,9 @@ 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(USE_STARBOARD_MEDIA) + SetBoundsCB GetSetBoundsCB() override; +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) // |enabled_track_ids| contains track ids of enabled audio tracks. void OnEnabledAudioTracksChanged( diff --git a/media/base/renderer.cc b/media/base/renderer.cc index 3a2d6a3096c7..3c39c5401372 100644 --- a/media/base/renderer.cc +++ b/media/base/renderer.cc @@ -36,6 +36,12 @@ std::string GetRendererName(RendererType renderer_type) { } } +#if BUILDFLAG(USE_STARBOARD_MEDIA) +bool SetBoundsNullTask(int x, int y, int width, int height) { + return false; +} +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + Renderer::Renderer() = default; Renderer::~Renderer() = default; @@ -72,4 +78,11 @@ void Renderer::OnExternalVideoFrameRequest() { // Default implementation of OnExternalVideoFrameRequest is to no-op. } +#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) + } // namespace media diff --git a/media/base/renderer.h b/media/base/renderer.h index 65d3036fbb5a..4dcd53329bd2 100644 --- a/media/base/renderer.h +++ b/media/base/renderer.h @@ -43,6 +43,10 @@ enum class RendererType { // the actual Renderer class name or a descriptive name. std::string MEDIA_EXPORT GetRendererName(RendererType renderer_type); +#if BUILDFLAG(USE_STARBOARD_MEDIA) +bool MEDIA_EXPORT SetBoundsNullTask(int x, int y, int width, int height); +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + class MEDIA_EXPORT Renderer { public: Renderer(); @@ -131,6 +135,12 @@ class MEDIA_EXPORT Renderer { // enforce RendererType registration for all Renderer implementations. // Note: New implementation should update RendererType. virtual RendererType GetRendererType() = 0; + +#if BUILDFLAG(USE_STARBOARD_MEDIA) + // Return SetBoundsCB if SbPlayer is used for rendering. + using SetBoundsCB = base::OnceCallback; + virtual SetBoundsCB GetSetBoundsCB(); +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) }; } // namespace media diff --git a/media/filters/pipeline_controller.cc b/media/filters/pipeline_controller.cc index 35dc4f32dc78..e19d3a726345 100644 --- a/media/filters/pipeline_controller.cc +++ b/media/filters/pipeline_controller.cc @@ -448,6 +448,13 @@ void PipelineController::OnExternalVideoFrameRequest() { pipeline_->OnExternalVideoFrameRequest(); } +#if BUILDFLAG(USE_STARBOARD_MEDIA) +Pipeline::SetBoundsCB PipelineController::GetSetBoundsCB() { + DCHECK(thread_checker_.CalledOnValidThread()); + return pipeline_->GetSetBoundsCB(); +} +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + 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 index d50290a0acdd..1bccbe80c957 100644 --- a/media/filters/pipeline_controller.h +++ b/media/filters/pipeline_controller.h @@ -149,6 +149,9 @@ class MEDIA_EXPORT PipelineController { void OnSelectedVideoTrackChanged( absl::optional selected_track_id); void OnExternalVideoFrameRequest(); +#if BUILDFLAG(USE_STARBOARD_MEDIA) + Pipeline::SetBoundsCB GetSetBoundsCB(); +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) // 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 index 7a8316dbd22d..909042dbb599 100644 --- 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 index c71eed5525b2..602bdae8d26f 100644 --- 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 index ff1443967ad9..23eb98961534 100644 --- 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()) { + video_overlay_factory_(std::make_unique()), + 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 index d0e5584c3ecb..512446c68de7 100644 --- 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 video_overlay_factory_; + scoped_refptr 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 index eb92552cb343..8b9dec4058d7 100644 --- a/third_party/blink/public/platform/web_media_player.h +++ b/third_party/blink/public/platform/web_media_player.h @@ -303,6 +303,12 @@ class WebMediaPlayer { // Sets the poster image URL. virtual void SetPoster(const WebURL& poster) {} +#if BUILDFLAG(USE_STARBOARD_MEDIA) + // Return SetBoundsCB if SbPlayer is used for rendering. + using SetBoundsCB = base::OnceCallback; + virtual SetBoundsCB GetSetBoundsCB() { return SetBoundsCB(); } +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + // Inform WebMediaPlayer when the element has entered/exited fullscreen. virtual void EnteredFullscreen() {} virtual void ExitedFullscreen() {} diff --git a/third_party/blink/renderer/core/paint/video_painter.cc b/third_party/blink/renderer/core/paint/video_painter.cc index 8b7a7b694d25..bdb5c55c182e 100644 --- a/third_party/blink/renderer/core/paint/video_painter.cc +++ b/third_party/blink/renderer/core/paint/video_painter.cc @@ -35,6 +35,9 @@ void VideoPainter::PaintReplaced(const PaintInfo& paint_info, force_video_poster; if (!should_display_poster && !media_player) return; +#if BUILDFLAG(USE_STARBOARD_MEDIA) + WebMediaPlayer::SetBoundsCB set_bounds_cb = media_player->GetSetBoundsCB(); +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) PhysicalRect replaced_rect = layout_video_.ReplacedContentRect(); replaced_rect.Move(paint_offset); @@ -77,6 +80,15 @@ void VideoPainter::PaintReplaced(const PaintInfo& paint_info, layer->SetBounds(snapped_replaced_rect.size()); layer->SetIsDrawable(true); layer->SetHitTestable(true); +#if BUILDFLAG(USE_STARBOARD_MEDIA) + if (!set_bounds_cb.is_null()) { + // TODO(b/377754564): revisit it for the impact of performance of SbPlayerSetBounds. + std::move(set_bounds_cb).Run(layout_video_.AbsoluteBoundingBoxRect().x(), + layout_video_.AbsoluteBoundingBoxRect().y(), + layout_video_.AbsoluteBoundingBoxRect().width(), + layout_video_.AbsoluteBoundingBoxRect().height()); + } +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) RecordForeignLayer(context, layout_video_, DisplayItem::kForeignLayerVideo, layer, snapped_replaced_rect.origin()); diff --git a/third_party/blink/renderer/platform/media/web_media_player_impl.cc b/third_party/blink/renderer/platform/media/web_media_player_impl.cc index 0c8211b9a03d..7629590c3630 100644 --- a/third_party/blink/renderer/platform/media/web_media_player_impl.cc +++ b/third_party/blink/renderer/platform/media/web_media_player_impl.cc @@ -690,6 +690,14 @@ void WebMediaPlayerImpl::DisableOverlay() { MaybeSendOverlayInfoToDecoder(); } +#if BUILDFLAG(USE_STARBOARD_MEDIA) +WebMediaPlayer::SetBoundsCB WebMediaPlayerImpl::GetSetBoundsCB() { + // |pipeline_| is always valid during WebMediaPlayerImpl's life time. It is + // also reference counted so it lives after WebMediaPlayerImpl is destroyed. + return pipeline_controller_->GetSetBoundsCB(); +} +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + void WebMediaPlayerImpl::EnteredFullscreen() { overlay_info_.is_fullscreen = true; @@ -2813,7 +2821,7 @@ std::unique_ptr WebMediaPlayerImpl::CreateRenderer( return std::make_unique(media_task_runner_, compositor_.get()); } -#endif // BUILDFLAG(USE_STARBOARD_MEDIA) +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) if (renderer_type) { DVLOG(1) << __func__ diff --git a/third_party/blink/renderer/platform/media/web_media_player_impl.h b/third_party/blink/renderer/platform/media/web_media_player_impl.h index 00159b375c77..50f9f1ceebf9 100644 --- a/third_party/blink/renderer/platform/media/web_media_player_impl.h +++ b/third_party/blink/renderer/platform/media/web_media_player_impl.h @@ -59,6 +59,10 @@ #include "third_party/blink/renderer/platform/platform_export.h" #include "url/gurl.h" +#if BUILDFLAG(USE_STARBOARD_MEDIA) +#include "media/starboard/starboard_renderer.h" +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + namespace base { class SingleThreadTaskRunner; class TaskRunner; @@ -254,6 +258,10 @@ class PLATFORM_EXPORT WebMediaPlayerImpl WebContentDecryptionModule* cdm, WebContentDecryptionModuleResult result) override; +#if BUILDFLAG(USE_STARBOARD_MEDIA) + SetBoundsCB GetSetBoundsCB() override; +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + void EnteredFullscreen() override; void ExitedFullscreen() override; void BecameDominantVisibleContent(bool is_dominant) override;