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] Turn SbPlayerPrivate into an interface #4257

Closed

Conversation

xiaomings
Copy link
Contributor

@xiaomings xiaomings commented Oct 14, 2024

This allows us to move the implementation to a sub-class named SbPlayerPrivateImpl in namespace starboard::shared::starboard::player, which has the following benefits:

  1. It allows the implementation to use symbols inside the starboard namespace directly.
  2. By moving concrete implementation from the global namespace into a nested namespace, a binary can make use of two implementations (each in their own namespaces), e.g. experimenting on them.

b/327287075

@xiaomings xiaomings force-pushed the refine_global_media_symbols branch 3 times, most recently from 7ca8668 to cde0db3 Compare October 15, 2024 18:34
@xiaomings xiaomings changed the title (WIP) [media] Reduce implementation in global namespace [media] Turn SbPlayerPrivate into an interface Oct 15, 2024
@xiaomings xiaomings force-pushed the refine_global_media_symbols branch 6 times, most recently from deb9e9f to a62de4c Compare October 15, 2024 19:49
This allows us to move the implementation to a sub-class named
SbPlayerPrivateImpl in namespace starboard::shared::starboard::player,
which has the following benefits:

1. It allows the implementation to use symbols inside the starboard
namespace directly.
2. By moving concrete implementation from the global namespace into a
nested namespace, a binary can make use of two implementations (each in
their own namespaces), e.g. experimenting on them.

b/327287075
@xiaomings xiaomings force-pushed the refine_global_media_symbols branch from a62de4c to 9a2e9b8 Compare October 15, 2024 19:49
@xiaomings xiaomings marked this pull request as ready for review October 15, 2024 20:23
jasonzhangxx
jasonzhangxx previously approved these changes Oct 15, 2024
@xiaomings xiaomings dismissed jasonzhangxx’s stale review October 17, 2024 23:20

I am going to merge it to main branch instead

@xiaomings
Copy link
Contributor Author

Moved to Chrobalt as #4575.

@xiaomings xiaomings closed this Dec 12, 2024
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