Skip to content

starboard: Replace URL player config macro with tvOS buildflag - #11606

Open
abhijeetk wants to merge 1 commit into
youtube:mainfrom
abhijeetk:cleanup-sb-has-player-with-url
Open

starboard: Replace URL player config macro with tvOS buildflag#11606
abhijeetk wants to merge 1 commit into
youtube:mainfrom
abhijeetk:cleanup-sb-has-player-with-url

Conversation

@abhijeetk

Copy link
Copy Markdown
Collaborator

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: #10698

Bug: 512045535

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Gemini Suggested Commit Message


starboard: Use BUILDFLAG for tvOS URL player

Replace the legacy SB_HAS_PLAYER_WITH_URL configuration with the
standard BUILDFLAG(IS_IOS_TVOS) to gate tvOS-specific URL player code.
This aligns Starboard with the standard Chromium architecture.

Additionally, replace the indirect SB_URL_PLAYER_INCLUDE_PATH macro
with direct header includes and remove the obsolete definitions from
the tvOS shared configuration headers.

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

Copy link
Copy Markdown
Collaborator Author

cc : @Gyuyoung @jkim-julie @rakuco

@abhijeetk
abhijeetk marked this pull request as ready for review July 30, 2026 06:18
@abhijeetk
abhijeetk requested review from a team as code owners July 30, 2026 06:18

@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 replaces the generic SB_HAS(PLAYER_WITH_URL) preprocessor macro with the platform-specific BUILDFLAG(IS_IOS_TVOS) across several media and Starboard files, and cleans up the associated configurations. Feedback suggests forward-declaring SbUrlPlayerExtraInfo in sbplayer_interface.h to avoid platform-specific headers in generic interfaces, and points out that including tvOS-specific headers in the generic NPLB test suite violates its platform-agnostic design.

Comment thread media/starboard/sbplayer_interface.h
Comment thread starboard/nplb/url_player_create_test.cc
@abhijeetk abhijeetk self-assigned this Jul 30, 2026
@abhijeetk abhijeetk changed the title starboard: Replace SB_HAS(PLAYER_WITH_URL) with BUILDFLAG(IS_IOS_TVOS) starboard: Replace URL player config macro with tvOS buildflag Jul 30, 2026
@abhijeetk

Copy link
Copy Markdown
Collaborator Author

This PR implements the changes as per review comment on PR 10698

Comment thread media/starboard/starboard_renderer_unittest.cc
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
@abhijeetk
abhijeetk force-pushed the cleanup-sb-has-player-with-url branch from 40d52c5 to bbe2429 Compare July 30, 2026 19:48
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.

2 participants