-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: Render native cues using text displayer #6985
Conversation
Incremental code coverage: 52.48% |
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.
The changes here seem reasonable. The only thing I think we should have is an option to use native text track rendering. This is because on safari, if you use native pip, it will continue displaying captions if they are natively rendered. But not if they are rendered manually.
Even better if the option can be toggled at runtime so that native rendering could be toggled as part of a pip requested event handler.
I prefer that we detect if we are in PiP and switch to native mode. But this only applies if we use the PiP of the video, not the document. See: https://github.com/shaka-project/shaka-player/blob/main/ui/pip_button.js |
👍🏻, doing it automatically, sounds good to me.
💯. Though, only safari does it properly and shows captions in the video pip. Firefox will as well, but they don't implement the pip api, so, not possible to know when entering pip, i believe. |
And do you know if Firefox shows subtitles in its PiP? |
yeah, firefox shows captions in its pip. They were the first to implement that. |
Well, since Firefox does not follow the PiP standard, in this case Firefox would be left without subtitles. |
Fixes bugs introduced in #6985
Closes #2585
Ownership of text displayer has changed to player, it is created now in MSE mode and in src= mode if video element has
textTracks
property.