Skip to content

media: Add InitializeWithUrl transport and URL player renderer - #10989

Closed
abhijeetk wants to merge 2 commits into
youtube:mainfrom
abhijeetk:url-player-1
Closed

media: Add InitializeWithUrl transport and URL player renderer#10989
abhijeetk wants to merge 2 commits into
youtube:mainfrom
abhijeetk:url-player-1

Conversation

@abhijeetk

Copy link
Copy Markdown
Collaborator

Add mojom::Renderer::InitializeWithUrl(client, source_url) to deliver
the URL on the renderer Mojo pipe, keeping URL delivery and
initialization ordered on the same pipe.

Add UrlPlayerRenderer for tvOS URL-based playback via SbPlayerBridge
and UrlPlayerRendererWrapper as the Mojo service bridge. The wrapper's
InitializeWithUrl() sets the URL on the renderer internally then calls
Initialize().

MojoRendererService validates the renderer type and URL before
forwarding via static_cast, guarded by GetRendererType() == kUrlPlayer
and ReportBadMessage().

Bug: 512045535

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Gemini Suggested Commit Message


media: Add URL player renderer for tvOS

Introduce InitializeWithUrl to the mojom::Renderer interface and
implement UrlPlayerRenderer to support URL-based media playback.
This change is primarily targeted at tvOS to enable native HLS
playback via SbPlayerBridge.

By delivering the source URL directly on the renderer Mojo pipe,
delivery and initialization remain ordered. This also includes
the necessary Mojo service wrappers and build system flags to
enable this functionality for tvOS builds.

Bug: 512045535

💡 Pro Tips for a Better Commit Message:

  1. Influence the Result: Want to change the output? You can write custom prompts or instructions directly in the Pull Request description. The model uses that text to generate the message.
  2. Re-run the Generator: Post a comment with: /generate-commit-message

@abhijeetk
abhijeetk force-pushed the url-player-1 branch 3 times, most recently from ac56964 to 30d4223 Compare June 22, 2026 19:06
@abhijeetk
abhijeetk marked this pull request as ready for review June 25, 2026 18:00
@abhijeetk
abhijeetk requested review from a team as code owners June 25, 2026 18:00

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces UrlPlayerRenderer and its Mojo wrapper UrlPlayerRendererWrapper to support URL-based media playback on tvOS using Starboard. The code review feedback focuses on improving robustness, safety, and style guide compliance. Key recommendations include replacing unsafe CHECK and DCHECK statements with graceful runtime checks, initializing primitive local variables to prevent undefined behavior, adding defensive null checks for client_ and player_bridge_ to avoid crashes, and removing unused private member variables. Additionally, the reviewer notes that both new classes require detailed class comments explaining their lifetime, ownership, and threading models to adhere to the repository style guide.

Comment thread media/mojo/services/mojo_renderer_service.cc Outdated
Comment thread media/starboard/url_player_renderer.cc Outdated
Comment thread media/starboard/url_player_renderer.cc
Comment thread media/starboard/url_player_renderer.cc
Comment thread media/mojo/clients/mojo_renderer.cc Outdated
Comment thread media/starboard/url_player_renderer.cc
Comment thread media/starboard/url_player_renderer.cc Outdated
Comment thread media/starboard/url_player_renderer.h
Comment thread media/mojo/services/starboard/url_player_renderer_wrapper.h
Comment thread media/starboard/url_player_renderer.h Outdated
@borongc

borongc commented Jun 25, 2026

Copy link
Copy Markdown
Member

Since url player is only on tvOS, just want to check if checking is_tvos is sufficient?

Comment thread media/mojo/clients/mojo_renderer.h Outdated
Comment thread media/mojo/clients/mojo_renderer_wrapper.h Outdated
Comment thread media/mojo/services/starboard/url_player_renderer_wrapper_unittest.cc Outdated
Comment thread media/mojo/services/starboard/url_player_renderer_wrapper.h Outdated
Comment thread media/mojo/services/mojo_renderer_service.h Outdated
@abhijeetk

Copy link
Copy Markdown
Collaborator Author

Since url player is only on tvOS, just want to check if checking is_tvos is sufficient?

No, is_tvos alone isn't sufficient and doesn't exist as a standalone buildflag in the build system. tvOS is identified as is_ios && target_platform == "tvos".

use_starboard_url_player is defined as use_starboard_media && is_ios && target_platform == "tvos" in starboard/build/buildflags.gni. The use_starboard_media dependency is required because the URL player stack is built entirely on top of Starboard infrastructure. It uses SbPlayerBridge for player creation, StarboardRendererExtension/StarboardRendererClientExtension Mojo interfaces for SbWindow and video geometry, and SbDrmSystem for FairPlay DRM. Without use_starboard_media, none of these would be compiled in.

Add use_starboard_url_player GN capability flag, derived from
use_starboard_media and tvOS platform detection. Expose it as
BUILDFLAG(USE_STARBOARD_URL_PLAYER) through build_config.h and
register matching Mojo enabled_features entry.

Add RendererType::kUrlPlayer = 13 to the C++ enum, Mojom enum,
traits mapping, and histogram. The new enum is gated behind
USE_STARBOARD_URL_PLAYER so non-tvOS builds are unaffected.

This establishes the capability boundary for all subsequent
URL-player CLs.

Bug: 512045535
@abhijeetk

Copy link
Copy Markdown
Collaborator Author

cc : @jkim-julie @Gyuyoung @rakuco

Comment thread media/mojo/services/starboard/url_player_renderer_wrapper_unittest.cc Outdated
Introduce InitializeWithUrl(client, source_url) on the Mojo renderer
pipe to carry the media URL from the renderer process to the GPU
process, keeping URL delivery and initialization ordered on the same
pipe.

On the GPU side, add UrlPlayerRenderer for tvOS URL-based playback
via SbPlayerBridge in punch-out mode, and UrlPlayerRendererWrapper
as the Mojo service bridge. The wrapper sets the URL on the renderer
then calls Initialize().

On the renderer-process side, MojoRenderer and MojoRendererWrapper
forward InitializeWithUrl over the Mojo pipe. MojoRendererService
validates the renderer type and URL before forwarding, with runtime
guards for unexpected state and invalid URLs.

Bug: 512045535
@abhijeetk
abhijeetk requested a review from borongc June 26, 2026 13:24
@abhijeetk abhijeetk self-assigned this Jun 26, 2026
@borongc

borongc commented Jun 26, 2026

Copy link
Copy Markdown
Member

Since url player is only on tvOS, just want to check if checking is_tvos is sufficient?

No, is_tvos alone isn't sufficient and doesn't exist as a standalone buildflag in the build system. tvOS is identified as is_ios && target_platform == "tvos".

use_starboard_url_player is defined as use_starboard_media && is_ios && target_platform == "tvos" in starboard/build/buildflags.gni. The use_starboard_media dependency is required because the URL player stack is built entirely on top of Starboard infrastructure. It uses SbPlayerBridge for player creation, StarboardRendererExtension/StarboardRendererClientExtension Mojo interfaces for SbWindow and video geometry, and SbDrmSystem for FairPlay DRM. Without use_starboard_media, none of these would be compiled in.

In this case, I think the gating logic should be like this:

if (use_starboard_media) {
  if (is_ios && target_platform == "tvos") {
    url_player code...
  }
}

Starboard should contain multiple platforms (android, tvOS, and 3P devices), so the flag use_starboard_url_player doesn't make sense to me, as it should be a subset of use_starboard_media as you define use_starboard_url_player as use_starboard_media && is_ios && target_platform == "tvos".

@abhijeetk

Copy link
Copy Markdown
Collaborator Author

Thanks for all review comments. We are moving ahead with approach in PR #10698 (UrlPlayer using StarboardRenderer).
Closing this PR.

@abhijeetk abhijeetk closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants