-
Notifications
You must be signed in to change notification settings - Fork 207
tvos: Implement UrlPlayer for native HLS playback #10698
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
| #include "base/functional/bind.h" | ||
| #include "base/time/time.h" | ||
| #include "base/unguessable_token.h" | ||
| #include "build/build_config.h" | ||
| #include "media/base/media_log.h" | ||
| #include "media/base/media_resource.h" | ||
| #include "media/base/media_switches.h" | ||
|
|
@@ -30,6 +31,10 @@ | |
| #include "mojo/public/cpp/bindings/callback_helpers.h" | ||
| #include "mojo/public/cpp/bindings/pending_receiver.h" | ||
|
|
||
| #if BUILDFLAG(IS_IOS_TVOS) | ||
| #include "url/gurl.h" | ||
| #endif // BUILDFLAG(IS_IOS_TVOS) | ||
|
abhijeetk marked this conversation as resolved.
|
||
|
|
||
| #if BUILDFLAG(IS_ANDROID) | ||
| #include "base/task/bind_post_task.h" | ||
| #endif // BUILDFLAG(IS_ANDROID) | ||
|
|
@@ -351,8 +356,18 @@ void StarboardRendererClient::InitializeMojoRenderer( | |
| DCHECK(media_task_runner_->RunsTasksInCurrentSequence()); | ||
| DCHECK(AreMojoPipesConnected()); | ||
|
|
||
| if (base::FeatureList::IsEnabled(kCobaltBypassMojoForMedia) || | ||
| bypass_mojo_for_media_) { | ||
| // URL player is incompatible with the bypass bridge because the bypass | ||
| // bridge proxies demuxer streams, while the URL player delegates playback | ||
| // entirely to the native platform player. | ||
| bool is_url_player = false; | ||
| #if BUILDFLAG(IS_IOS_TVOS) | ||
| GURL url = media_resource->GetMediaUrl(); | ||
| is_url_player = url.is_valid(); | ||
| #endif // BUILDFLAG(IS_IOS_TVOS) | ||
|
|
||
| if (!is_url_player && | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| (base::FeatureList::IsEnabled(kCobaltBypassMojoForMedia) || | ||
| bypass_mojo_for_media_)) { | ||
| bypass_bridge_ = base::MakeRefCounted<MojoRendererBypassBridge>( | ||
| media_task_runner_, | ||
| base::BindRepeating(&StarboardRendererClient::OnTimeUpdateFromBridge, | ||
|
|
@@ -378,6 +393,12 @@ void StarboardRendererClient::InitializeMojoRenderer( | |
| return; | ||
| } | ||
|
|
||
| #if BUILDFLAG(IS_IOS_TVOS) | ||
| if (is_url_player) { | ||
| renderer_extension_->SetSourceUrl(url.spec()); | ||
| } | ||
| #endif // BUILDFLAG(IS_IOS_TVOS) | ||
|
abhijeetk marked this conversation as resolved.
|
||
|
|
||
| MojoRendererWrapper::Initialize(media_resource, client, std::move(init_cb)); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked this against C25. In C25,
SBDApplicationViewwas initialized withUIScreen.mainScreen.boundsin application_window.mm, and the playercontainer was initialized with that application view frame in application_view.mm. So using
[UIScreen mainScreen].boundshere 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 inapplication_player.mm. It is later resized via
setBoundsX:y:width:height:in application_player.mm.So the initial frame of the chrobalt shell
playerContainerViewis not what determines the final video size. Initializing it with[UIScreen mainScreen].boundsis mainly for C25 layout parity and not because final video bounds depend on it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather revert this part of the change to avoid confusion. The comment above this bit mentions how the size is actually set via
AVSBVideoRenderer::SetBounds().