Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[media] Report errors via MediaError to WebApp #4453

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions content/renderer/media/media_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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::StarboardRendererFactory>(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;
Expand Down
4 changes: 4 additions & 0 deletions media/base/renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
5 changes: 5 additions & 0 deletions media/base/renderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions media/mojo/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -123,6 +127,10 @@ mojom("mojom") {
enabled_features += [ "enable_cast_renderer" ]
}

if (is_cobalt && use_starboard_media) {
andrewsavage1 marked this conversation as resolved.
Show resolved Hide resolved
enabled_features += [ "use_starboard_media" ]
}

shared_typemaps = [
{
types = [
Expand Down
2 changes: 2 additions & 0 deletions media/mojo/mojom/media_types.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
9 changes: 9 additions & 0 deletions media/mojo/mojom/media_types_enum_mojom_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,10 @@ struct EnumTraits<media::mojom::RendererType, ::media::RendererType> {
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();
Expand Down Expand Up @@ -359,6 +363,11 @@ struct EnumTraits<media::mojom::RendererType, ::media::RendererType> {
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();
Expand Down
2 changes: 2 additions & 0 deletions media/starboard/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
Expand Down
25 changes: 22 additions & 3 deletions media/starboard/starboard_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,16 @@ bool HasRemoteAudioOutputs(

StarboardRenderer::StarboardRenderer(
const scoped_refptr<base::SequencedTaskRunner>& 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<VideoOverlayFactory>()),
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.";
Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have a fallback for other cases not specified now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For SbPlayer, we only have kSbPlayerErrorDecode and kSbPlayerErrorCapabilityChanged for errors. It should be fine here for not specifying a default case.

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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this kSbPlayerErrorMax ever appear?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so.

break;
}
}

} // namespace media
12 changes: 10 additions & 2 deletions media/starboard/starboard_renderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -44,7 +45,8 @@ class MEDIA_EXPORT StarboardRenderer final : public Renderer,
private SbPlayerBridge::Host {
public:
StarboardRenderer(const scoped_refptr<base::SequencedTaskRunner>& task_runner,
VideoRendererSink* video_renderer_sink);
VideoRendererSink* video_renderer_sink,
MediaLog* media_log);

~StarboardRenderer() final;

Expand Down Expand Up @@ -103,8 +105,8 @@ class MEDIA_EXPORT StarboardRenderer final : public Renderer,
void OnPlayerError(SbPlayerError error, const std::string& message) final;

scoped_refptr<base::SequencedTaskRunner> task_runner_;

const raw_ptr<VideoRendererSink> video_renderer_sink_;
const raw_ptr<MediaLog> media_log_;

raw_ptr<DemuxerStream> audio_stream_ = nullptr;
raw_ptr<DemuxerStream> video_stream_ = nullptr;
Expand Down Expand Up @@ -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<StarboardRenderer> weak_factory_{this};
base::WeakPtr<StarboardRenderer> weak_this_{weak_factory_.GetWeakPtr()};

Expand Down
41 changes: 41 additions & 0 deletions media/starboard/starboard_renderer_factory.cc
Original file line number Diff line number Diff line change
@@ -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<Renderer> StarboardRendererFactory::CreateRenderer(
const scoped_refptr<base::SequencedTaskRunner>& media_task_runner,
const scoped_refptr<base::TaskRunner>& 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::StarboardRenderer>(
media_task_runner, video_renderer_sink, media_log_);
}

} // namespace media
50 changes: 50 additions & 0 deletions media/starboard/starboard_renderer_factory.h
Original file line number Diff line number Diff line change
@@ -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<Renderer> CreateRenderer(
const scoped_refptr<base::SequencedTaskRunner>& media_task_runner,
const scoped_refptr<base::TaskRunner>& 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<MediaLog> media_log_;
};

} // namespace media

#endif // MEDIA_STARBOARD_STARBOARD_RENDERER_FACTORY_H_
Original file line number Diff line number Diff line change
Expand Up @@ -2807,21 +2807,6 @@ std::unique_ptr<media::Renderer> 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::StarboardRenderer>(media_task_runner_,
compositor_.get());
}
#endif // BUILDFLAG(USE_STARBOARD_MEDIA)

if (renderer_type) {
DVLOG(1) << __func__
<< ": renderer_type=" << static_cast<int>(renderer_type.value());
Expand All @@ -2838,6 +2823,14 @@ std::unique_ptr<media::Renderer> WebMediaPlayerImpl::CreateRenderer(
media_metrics_provider_->SetRendererType(renderer_type_);
media_log_->SetProperty<MediaLogProperty::kRendererName>(renderer_type_);

#if BUILDFLAG(USE_STARBOARD_MEDIA)
LOG(INFO) << "Renderer Type is " << GetRendererName(renderer_type_) << ".";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already logged in Media DevTools in l.2824, is this extra LOG() needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may not always use DevTools for media. This can ensure we can have this in adb bugreport.

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),
Expand Down
2 changes: 2 additions & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -68501,6 +68501,8 @@ Called by update_use_counter_css.py.-->
<int value="9" label="kCastStreaming"/>
<int value="10" label="kContentEmbedderDefined"/>
<int value="11" label="kTest"/>
<!-- Cobalt: Renderer for Starboard (SbPlayer) -->
<int value="12" label="kStarboard"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\o/

</enum>

<enum name="MediaResponseCacheType">
Expand Down
Loading