From 153dc3cfe63a7b65a9e8d56c6d4823ba068c0ebf Mon Sep 17 00:00:00 2001 From: Bo-Rong Chen Date: Tue, 19 Nov 2024 10:22:13 -0800 Subject: [PATCH] [media] Report errors via MediaError to WebApp This PR adds `StarboardRendererFactory`, which contains `media_log_` to `StarboardRenderer`. Chromium uses both `RendererClient::OnError` and `media_log_` to report error code and message to MediaError. As a result, Cobalt adapts it from C25 to report error messages to WebApp. b/375271948 b/375278384 --- content/renderer/media/media_factory.cc | 9 ++++ media/base/renderer.cc | 4 ++ media/base/renderer.h | 5 ++ media/mojo/mojom/BUILD.gn | 8 +++ media/mojo/mojom/media_types.mojom | 2 + .../mojom/media_types_enum_mojom_traits.h | 9 ++++ media/starboard/BUILD.gn | 2 + media/starboard/starboard_renderer.cc | 25 ++++++++-- media/starboard/starboard_renderer.h | 12 ++++- media/starboard/starboard_renderer_factory.cc | 41 +++++++++++++++ media/starboard/starboard_renderer_factory.h | 50 +++++++++++++++++++ .../platform/media/web_media_player_impl.cc | 23 +++------ tools/metrics/histograms/enums.xml | 2 + 13 files changed, 172 insertions(+), 20 deletions(-) create mode 100644 media/starboard/starboard_renderer_factory.cc create mode 100644 media/starboard/starboard_renderer_factory.h diff --git a/content/renderer/media/media_factory.cc b/content/renderer/media/media_factory.cc index 6e160181d052..0ecfe9253ebd 100644 --- a/content/renderer/media/media_factory.cc +++ b/content/renderer/media/media_factory.cc @@ -94,6 +94,7 @@ #include "media/mojo/clients/mojo_fuchsia_cdm_provider.h" #elif BUILDFLAG(USE_STARBOARD_MEDIA) #include "media/starboard/starboard_cdm_factory.h" +#include "media/starboard/starboard_renderer_factory.h" #elif BUILDFLAG(ENABLE_MOJO_CDM) #include "media/mojo/clients/mojo_cdm_factory.h" // nogncheck #else @@ -747,10 +748,18 @@ MediaFactory::CreateRendererFactorySelector( // this method were significantly refactored to split things up by // Android/non-Android/Cast/etc... is_base_renderer_factory_set = true; +#if BUILDFLAG(USE_STARBOARD_MEDIA) + // TODO(b/326827007): Revisit renderer to support secondary videos. + factory_selector->AddFactory( + RendererType::kStarboard, + std::make_unique(media_log)); + factory_selector->SetBaseRendererType(RendererType::kStarboard); +#else // BUILDFLAG(USE_STARBOARD_MEDIA) auto renderer_impl_factory = CreateRendererImplFactory( player_id, media_log, decoder_factory, render_thread, render_frame_); factory_selector->AddBaseFactory(RendererType::kRendererImpl, std::move(renderer_impl_factory)); +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) } return factory_selector; diff --git a/media/base/renderer.cc b/media/base/renderer.cc index 3c39c5401372..adb75dd6f1b3 100644 --- a/media/base/renderer.cc +++ b/media/base/renderer.cc @@ -33,6 +33,10 @@ std::string GetRendererName(RendererType renderer_type) { return "EmbedderDefined"; case RendererType::kTest: return "Media Renderer Implementation For Testing"; +#if BUILDFLAG(USE_STARBOARD_MEDIA) + case RendererType::kStarboard: + return "StarboardRenderer"; +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) } } diff --git a/media/base/renderer.h b/media/base/renderer.h index 4dcd53329bd2..0ea111a73c5d 100644 --- a/media/base/renderer.h +++ b/media/base/renderer.h @@ -36,7 +36,12 @@ enum class RendererType { kCastStreaming = 9, // PlaybackCommandForwardingRendererFactory kContentEmbedderDefined = 10, // Defined by the content embedder kTest = 11, // Renderer implementations used in tests +#if BUILDFLAG(USE_STARBOARD_MEDIA) + kStarboard = 12, // StarboardRendererFactory + kMaxValue = kStarboard, +#else // BUILDFLAG(USE_STARBOARD_MEDIA) kMaxValue = kTest, +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) }; // Get the name of the Renderer for `renderer_type`. The returned name could be diff --git a/media/mojo/mojom/BUILD.gn b/media/mojo/mojom/BUILD.gn index 1214f486e5c6..1a09573d1801 100644 --- a/media/mojo/mojom/BUILD.gn +++ b/media/mojo/mojom/BUILD.gn @@ -5,6 +5,10 @@ import("//media/media_options.gni") import("//mojo/public/tools/bindings/mojom.gni") +if (is_cobalt) { + import("//starboard/build/buildflags.gni") +} + mojom("mojom") { generate_java = true @@ -123,6 +127,10 @@ mojom("mojom") { enabled_features += [ "enable_cast_renderer" ] } + if (is_cobalt && use_starboard_media) { + enabled_features += [ "use_starboard_media" ] + } + shared_typemaps = [ { types = [ diff --git a/media/mojo/mojom/media_types.mojom b/media/mojo/mojom/media_types.mojom index 71336585acfb..bcce011370e7 100644 --- a/media/mojo/mojom/media_types.mojom +++ b/media/mojo/mojom/media_types.mojom @@ -535,4 +535,6 @@ enum RendererType { kCastStreaming = 9, // CastStreamingRendererFactory kContentEmbedderDefined = 10, // Defined by the content embedder kTest= 11, // Renderer implementations used in tests + [EnableIf=use_starboard_media] + kStarboard = 12, // StarboardRendererFactory }; diff --git a/media/mojo/mojom/media_types_enum_mojom_traits.h b/media/mojo/mojom/media_types_enum_mojom_traits.h index 237b91b97bba..8d3d07461b3a 100644 --- a/media/mojo/mojom/media_types_enum_mojom_traits.h +++ b/media/mojo/mojom/media_types_enum_mojom_traits.h @@ -315,6 +315,10 @@ struct EnumTraits { return media::mojom::RendererType::kContentEmbedderDefined; case ::media::RendererType::kTest: return media::mojom::RendererType::kTest; +#if BUILDFLAG(USE_STARBOARD_MEDIA) + case ::media::RendererType::kStarboard: + return media::mojom::RendererType::kStarboard; +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) } NOTREACHED(); @@ -359,6 +363,11 @@ struct EnumTraits { case media::mojom::RendererType::kTest: *output = ::media::RendererType::kTest; return true; +#if BUILDFLAG(USE_STARBOARD_MEDIA) + case media::mojom::RendererType::kStarboard: + *output = ::media::RendererType::kStarboard; + return true; +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) } NOTREACHED(); diff --git a/media/starboard/BUILD.gn b/media/starboard/BUILD.gn index 9f860dfdb446..f1d176588fd3 100644 --- a/media/starboard/BUILD.gn +++ b/media/starboard/BUILD.gn @@ -76,6 +76,8 @@ source_set("starboard") { "starboard_memory_allocator.h", "starboard_renderer.cc", "starboard_renderer.h", + "starboard_renderer_factory.cc", + "starboard_renderer_factory.h", "starboard_utils.cc", "starboard_utils.h", ] diff --git a/media/starboard/starboard_renderer.cc b/media/starboard/starboard_renderer.cc index dfd4d697764a..bdd086268a2f 100644 --- a/media/starboard/starboard_renderer.cc +++ b/media/starboard/starboard_renderer.cc @@ -57,13 +57,16 @@ bool HasRemoteAudioOutputs( StarboardRenderer::StarboardRenderer( const scoped_refptr& task_runner, - VideoRendererSink* video_renderer_sink) + VideoRendererSink* video_renderer_sink, + MediaLog* media_log) : task_runner_(task_runner), video_renderer_sink_(video_renderer_sink), + media_log_(media_log), video_overlay_factory_(std::make_unique()), set_bounds_helper_(new SbPlayerSetBoundsHelper) { DCHECK(task_runner_); DCHECK(video_renderer_sink_); + DCHECK(media_log_); DCHECK(video_overlay_factory_); DCHECK(set_bounds_helper_); LOG(INFO) << "StarboardRenderer constructed."; @@ -718,10 +721,26 @@ void StarboardRenderer::OnPlayerStatus(SbPlayerState state) { void StarboardRenderer::OnPlayerError(SbPlayerError error, const std::string& message) { - // TODO(b/375271948): Implement and verify error reporting. LOG(ERROR) << "StarboardRenderer::OnPlayerError() called with code " << error << " and message \"" << message << "\""; - NOTIMPLEMENTED(); + switch (error) { + case kSbPlayerErrorDecode: + MEDIA_LOG(ERROR, media_log_) << message; + client_->OnError(PIPELINE_ERROR_DECODE); + break; + case kSbPlayerErrorCapabilityChanged: + MEDIA_LOG(ERROR, media_log_) + << (message.empty() + ? kSbPlayerCapabilityChangedErrorMessage + : base::StringPrintf("%s: %s", + kSbPlayerCapabilityChangedErrorMessage, + message.c_str())); + client_->OnError(PIPELINE_ERROR_DECODE); + break; + case kSbPlayerErrorMax: + NOTREACHED(); + break; + } } } // namespace media diff --git a/media/starboard/starboard_renderer.h b/media/starboard/starboard_renderer.h index 65bfc1b8ba73..e89507a74a3d 100644 --- a/media/starboard/starboard_renderer.h +++ b/media/starboard/starboard_renderer.h @@ -26,6 +26,7 @@ #include "media/base/cdm_context.h" #include "media/base/decoder_buffer.h" #include "media/base/demuxer_stream.h" +#include "media/base/media_log.h" #include "media/base/media_resource.h" #include "media/base/pipeline_status.h" #include "media/base/renderer.h" @@ -44,7 +45,8 @@ class MEDIA_EXPORT StarboardRenderer final : public Renderer, private SbPlayerBridge::Host { public: StarboardRenderer(const scoped_refptr& task_runner, - VideoRendererSink* video_renderer_sink); + VideoRendererSink* video_renderer_sink, + MediaLog* media_log); ~StarboardRenderer() final; @@ -103,8 +105,8 @@ class MEDIA_EXPORT StarboardRenderer final : public Renderer, void OnPlayerError(SbPlayerError error, const std::string& message) final; scoped_refptr task_runner_; - const raw_ptr video_renderer_sink_; + const raw_ptr media_log_; raw_ptr audio_stream_ = nullptr; raw_ptr video_stream_ = nullptr; @@ -149,6 +151,12 @@ class MEDIA_EXPORT StarboardRenderer final : public Renderer, uint32_t last_video_frames_decoded_ = 0; uint32_t last_video_frames_dropped_ = 0; + // Message to signal a capability changed error. + // "MEDIA_ERR_CAPABILITY_CHANGED" must be in the error message to be + // understood as a capability changed error. Do not change this message. + static inline constexpr const char* kSbPlayerCapabilityChangedErrorMessage = + "MEDIA_ERR_CAPABILITY_CHANGED"; + base::WeakPtrFactory weak_factory_{this}; base::WeakPtr weak_this_{weak_factory_.GetWeakPtr()}; diff --git a/media/starboard/starboard_renderer_factory.cc b/media/starboard/starboard_renderer_factory.cc new file mode 100644 index 000000000000..d819c63fa133 --- /dev/null +++ b/media/starboard/starboard_renderer_factory.cc @@ -0,0 +1,41 @@ +// Copyright 2024 The Cobalt Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "media/starboard/starboard_renderer_factory.h" + +#include "base/check.h" +#include "base/task/sequenced_task_runner.h" +#include "media/starboard/starboard_renderer.h" + +namespace media { + +StarboardRendererFactory::StarboardRendererFactory(MediaLog* media_log) + : media_log_(media_log) {} + +StarboardRendererFactory::~StarboardRendererFactory() = default; + +std::unique_ptr StarboardRendererFactory::CreateRenderer( + const scoped_refptr& media_task_runner, + const scoped_refptr& worker_task_runner, + AudioRendererSink* audio_renderer_sink, + VideoRendererSink* video_renderer_sink, + RequestOverlayInfoCB request_overlay_info_cb, + const gfx::ColorSpace& target_color_space) { + DCHECK(video_renderer_sink); + DCHECK(media_log_); + return std::make_unique( + media_task_runner, video_renderer_sink, media_log_); +} + +} // namespace media diff --git a/media/starboard/starboard_renderer_factory.h b/media/starboard/starboard_renderer_factory.h new file mode 100644 index 000000000000..4f474fdd62f2 --- /dev/null +++ b/media/starboard/starboard_renderer_factory.h @@ -0,0 +1,50 @@ +// Copyright 2024 The Cobalt Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef MEDIA_STARBOARD_STARBOARD_RENDERER_FACTORY_H_ +#define MEDIA_STARBOARD_STARBOARD_RENDERER_FACTORY_H_ + +#include "base/memory/raw_ptr.h" +#include "base/task/sequenced_task_runner.h" +#include "media/base/media_log.h" +#include "media/base/renderer_factory.h" + +namespace media { + +// Creates Renderers using Starboard. +class MEDIA_EXPORT StarboardRendererFactory final : public RendererFactory { + public: + StarboardRendererFactory(MediaLog* media_log); + + StarboardRendererFactory(const StarboardRendererFactory&) = delete; + StarboardRendererFactory& operator=(const StarboardRendererFactory&) = delete; + + ~StarboardRendererFactory() final; + + // RendererFactory implementation. + std::unique_ptr CreateRenderer( + const scoped_refptr& media_task_runner, + const scoped_refptr& worker_task_runner, + AudioRendererSink* audio_renderer_sink, + VideoRendererSink* video_renderer_sink, + RequestOverlayInfoCB request_overlay_info_cb, + const gfx::ColorSpace& target_color_space) final; + + private: + raw_ptr media_log_; +}; + +} // namespace media + +#endif // MEDIA_STARBOARD_STARBOARD_RENDERER_FACTORY_H_ 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 2750e4de13d1..7b9e29eaff13 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 @@ -2807,21 +2807,6 @@ std::unique_ptr WebMediaPlayerImpl::CreateRenderer( &WebMediaPlayerImpl::OnOverlayInfoRequested, weak_this_)); #endif -#if BUILDFLAG(USE_STARBOARD_MEDIA) - // TODO(b/375278384): Select the StarboardRenderer properly instead of - // hard coding. - - // StarboardRenderer always uses full screen with overlay video mode. - overlay_info_.is_fullscreen = true; - - // `media_task_runner_` is always true, use an if statement to avoid - // potential build warning on unreachable code. - if (media_task_runner_) { - return std::make_unique(media_task_runner_, - compositor_.get()); - } -#endif // BUILDFLAG(USE_STARBOARD_MEDIA) - if (renderer_type) { DVLOG(1) << __func__ << ": renderer_type=" << static_cast(renderer_type.value()); @@ -2838,6 +2823,14 @@ std::unique_ptr WebMediaPlayerImpl::CreateRenderer( media_metrics_provider_->SetRendererType(renderer_type_); media_log_->SetProperty(renderer_type_); +#if BUILDFLAG(USE_STARBOARD_MEDIA) + LOG(INFO) << "Renderer Type is " << GetRendererName(renderer_type_) << "."; + if (renderer_type_ == media::RendererType::kStarboard) { + // StarboardRenderer always uses full screen with overlay video mode. + overlay_info_.is_fullscreen = true; + } +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + return renderer_factory_selector_->GetCurrentFactory()->CreateRenderer( media_task_runner_, worker_task_runner_, audio_source_provider_.get(), compositor_.get(), std::move(request_overlay_info_cb), diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index cf788e89061e..f456cc502309 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -68501,6 +68501,8 @@ Called by update_use_counter_css.py.--> + +