tvos: Implement UrlPlayer for native HLS playback - #10698
Conversation
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
bdfff13 to
63765d6
Compare
2735b1c to
e8b32c7
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces support for URL-based native media playback (such as HLS on tvOS) by implementing a placeholder UrlPlayerDemuxer and updating the Starboard renderer components to delegate demuxing, buffering, and playback to the platform player. The code review feedback identifies critical preprocessor guard mismatches that could cause compilation failures when USE_STARBOARD_MEDIA is disabled, and suggests adding a defensive fallback for video resolution rather than relying solely on a DCHECK. Additionally, the newly introduced classes lack required documentation on lifetime, ownership, and threading models as specified by the style guide, and a missing header include for raw_ptr was noted.
e8b32c7 to
46b0401
Compare
|
cc : @Gyuyoung @jkim-julie @rakuco |
|
@jasonzhangxx is out, this PR needs his review. |
|
I wanted to share an update on the URL player (AVPlayer/HLS) work for tvOS. I've closed this PR #10698 ("media: Implement UrlPlayer for native HLS on tvOS"). This PR added URL player support by extending the existing Starboard renderer and wrapper classes inline. The new approach introduces dedicated renderer, client, and factory classes (UrlPlayerRenderer UrlPlayerRendererClient, UrlPlayerRendererWrapper) with a clean factory-level split, rather than piggybacking on the Starboard code path. This gives us better separation of concerns and makes the code easier to review and maintain. Here is the new PR stack. Please review in order, as each PR builds on the previous one:
All PRs have been build-verified and tested on tvOS simulator. |
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
80c6e7f to
af7c613
Compare
Add URL player support to StarboardRenderer for native HLS playback. UrlPlayerDemuxer provides placeholder streams for pipeline init while the platform player handles actual playback via SetSourceUrl. Bug: 512045535
af7c613 to
c280e82
Compare
through StarboardRenderer, StarboardRendererWrapper, Mojo IPC, and UrlPlayerDemuxer. This ensures that the JavaScript video.duration and video.buffered APIs correctly reflect the underlying platform player state on tvOS. Duration is reported once when the player reaches the presenting state. Buffered ranges are polled via GetMediaTime on each timer tick to provide continuous updates to the web application. Blocked by youtube#10698 Bug: 512045535
|
Let's change the to "tvos: Implement UrlPlayer for native HLS playback", as the change is specifically to tvos. |
|
Left some comments in #11548, which should be addressed in this PR. |
jasonzhangxx
left a comment
There was a problem hiding this comment.
Let's change the title to start with "tvos:" as it's specifically for tvos.
| // AVSBVideoRenderer::SetBounds() when necessary. | ||
| UIView* playerContainerView = [[UIView alloc] init]; | ||
| UIView* playerContainerView = | ||
| [[UIView alloc] initWithFrame:[UIScreen mainScreen].bounds]; |
There was a problem hiding this comment.
@abhijeetk, could you help me understand this change? I'm curious why we need it, and if it could have problem when player is non full screen.
There was a problem hiding this comment.
I checked this against C25. In C25, SBDApplicationView was initialized with UIScreen.mainScreen.bounds in application_window.mm, and the player
container was initialized with that application view frame in application_view.mm. So using [UIScreen mainScreen].bounds here keeps the new chrobalt shell path closer to the old C25 behavior.
That said, I also verified that reverting this to [[UIView alloc] init] does not break playback. The URL player view (SBDPlayerView) is created with its own fullscreen frame in
application_player.mm. It is later resized via setBoundsX:y:width:height: in application_player.mm.
So the initial frame of the chrobalt shell playerContainerView is not what determines the final video size. Initializing it with [UIScreen mainScreen].bounds is mainly for C25 layout parity and not because final video bounds depend on it.
| is_url_player = url.is_valid(); | ||
| #endif // BUILDFLAG(IS_IOS_TVOS) | ||
|
|
||
| if (!is_url_player && |
There was a problem hiding this comment.
@abhijeetk, can you help to confirm if url player and mojo bypass mechanisms mutually exclusive?
My understanding is we can still bypass mojo with url player. @borongc to review this piece of code.
There was a problem hiding this comment.
Hi @jasonzhangxx , borong has reviewed same changes in PR : #11548
As per this comment, I can infer that bypass mojo is not applicable to tvOS.
| }; | ||
|
|
||
| // Returns true when the renderer is operating in URL player mode. | ||
| bool IsUrlPlayer() const; |
There was a problem hiding this comment.
IsUrlPlayer and OnUrlPlayerPresenting are for url player on tvos, we can guard them behind SB_HAS(PLAYER_WITH_URL) or BUILDFLAG(IS_IOS_TVOS).
There was a problem hiding this comment.
IsUrlPlayer() is called unconditionally in Initialize(), CreatePlayerBridge(), OnNeedData() etc. Guarding the declaration would require wrapping every call site with #if BUILDFLAG(IS_IOS_TVOS), adding noise and making rebasing harder.
Currently it just returns false on non-tvOS via #if/#else in the body, keeping call sites clean. Happy to change if you prefer though.
| if (width > 0 && height > 0) { | ||
| gfx::Size size(width, height); | ||
| client_->OnVideoNaturalSizeChange(size); | ||
| paint_video_hole_frame_cb_.Run(size); |
There was a problem hiding this comment.
The current implementation triggers the video natural size update and the paint_video_hole_frame_cb_ only once here. As hls content is adaptive, will this work properly if the video resolution changes mid-playback?
There was a problem hiding this comment.
You're right, OnUrlPlayerPresenting() is called once from OnPlayerStatus() when the player transitions to kSbPlayerStatePresenting. There's currently no mechanism to detect resolution changes mid-playback.
Will add this in a follow-up PR if preferred.
| } | ||
|
|
||
| // URL player handles all buffering natively so ignore OnNeedData. | ||
| if (IsUrlPlayer()) { |
There was a problem hiding this comment.
Will OnNeedData be called by url player?
There was a problem hiding this comment.
No, it should not be called for the URL-player path. OnNeedData() is reached through SbPlayerBridge::OnDecoderStatus(), which is wired from the decoder-status callback passed to normal SbPlayerCreate(). The URL-player path uses SbUrlPlayerCreate(), whose API does not take a decoder-status callback, so URL player should not request demuxed data through OnNeedData().
The early return in OnNeedData is just a defensive guard as this function deals with stream which we want to bypass for URL-Player. I’ll remove this guard unless we want to keep it only as defensive protection.
| )); | ||
| )); | ||
| } | ||
| if (player_bridge_->IsValid()) { |
There was a problem hiding this comment.
I made a cleanup pr go/cobalt-pr/11583 to simply the code. Please rebase the change after that pr is merged. That pr can help to simplify the code here.
| } | ||
|
|
||
| AudioDecoderConfig UrlPlayerDemuxerStream::audio_decoder_config() { | ||
| return AudioDecoderConfig(AudioCodec::kAAC, kSampleFormatS16, |
There was a problem hiding this comment.
Will this function and video_decoder_config be actually used?
There was a problem hiding this comment.
Yes. These are placeholder configs, not configs used for URL-player decoding. These placeholder configs are required for media pipeline initialization, which is stream-based.
UrlPlayerDemuxer exposes audio/video DemuxerStreams via UrlPlayerDemuxer::GetAllStreams(), and the Mojo demuxer stream initialization reads the matching audio_decoder_config()/video_decoder_config() for initial stream metadata.
These configs need to be valid because they are sent over Mojo IPC during stream initialization, and the Mojo traits reject invalid configs via IsValidConfig() (media/mojo/mojom/audio_decoder_config_mojom_traits.cc, media/mojo/mojom/
video_decoder_config_mojom_traits.cc:73). AAC/H264 are only valid placeholder configs for the stream-based pipeline handshake; the native URL player does not use them for actual decoding.
SB_HAS_PLAYER_WITH_URL is legacy Cobalt configuration that was only defined for tvOS in configuration_public.h. In the Chromium architecture, BUILDFLAG(IS_IOS_TVOS) is the standard way to gate tvOS-specific code. Replace all SB_HAS(PLAYER_WITH_URL) guards with BUILDFLAG(IS_IOS_TVOS) across sbplayer_bridge, sbplayer_interface, starboard_renderer, and url_player_create_test. Replace the indirect SB_URL_PLAYER_INCLUDE_PATH include with the direct header path. Remove both definitions from starboard/tvos/shared/configuration_public.h. Required-For: youtube#10698 Bug: 512045535
Introduce native HLS support for tvOS by integrating the platform's
AVPlayer into the media pipeline. This allows Cobalt to leverage
native hardware acceleration and platform-specific HLS features
while maintaining the standard Chromium control interface.
A UrlPlayerDemuxer is injected via the demuxer_override mechanism
and exposes placeholder streams to satisfy Chromium's stream-based
initialization. Actual playback is delegated to the platform player.
The HLS URL is carried from the demuxer through
MediaResource::GetMediaUrl() in the renderer process, then bridged
to the GPU process via the StarboardRendererExtension Mojo interface.
Bug: 512045535