Skip to content

Remove #include MACRO#9279

Closed
briantting wants to merge 1 commit intoyoutube:mainfrom
briantting:main
Closed

Remove #include MACRO#9279
briantting wants to merge 1 commit intoyoutube:mainfrom
briantting:main

Conversation

@briantting
Copy link
Contributor

Removes SB_PLAYER_DMP_WRITER_INCLUDE_PATH #include MACRO as the SISO scanner has difficulty uploading required dependenies for remote builds.

Bug: 454374226

Removes SB_PLAYER_DMP_WRITER_INCLUDE_PATH #include MACRO as the SISO
scanner has difficulty uploading required dependenies for remote builds.

Bug: 454374226
@github-actions
Copy link
Contributor

🤖 Gemini Suggested Commit Message


build: Remove include macro causing scanner issues

The SB_PLAYER_DMP_WRITER_INCLUDE_PATH preprocessor macro has been
removed. This macro previously defined the include path for
video_dmp_writer.h. Its usage has been updated to directly include
the header file via a string literal.

This change resolves difficulties encountered by the SISO scanner when
uploading required dependencies for remote builds, improving the
build system's compatibility.

Bug: 454374226

💡 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

Copy link
Contributor

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

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 removes the SB_PLAYER_DMP_WRITER_INCLUDE_PATH macro, replacing its usage with a hardcoded include path to resolve a build tool issue. While this simplifies the code, it removes a layer of platform-specific configuration. I've raised a concern about the potential for this change to break builds on other platforms that might have been using this macro to provide a different path for the header file.

I am having trouble creating individual review comments. Click here to see my feedback.

starboard/linux/shared/configuration_public.h (48-51)

high

Removing the SB_PLAYER_DMP_WRITER_INCLUDE_PATH macro and hardcoding the include path in shared code removes the flexibility for platforms to specify a custom path for video_dmp_writer.h. The original comment for this macro (lines 48-49) suggests this flexibility was intentional to support platforms that might not include the video_dmp library in the same way. This change introduces a risk of breaking builds on other platforms that enable SB_PLAYER_ENABLE_VIDEO_DUMPER but require a different path for this header.

@briantting briantting closed this Feb 26, 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.

1 participant