-
Notifications
You must be signed in to change notification settings - Fork 425
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: Add Airplay support when overriding native HLS in Safari/iOS #1543
Conversation
…support airplay, add test
// If we are playing HLS with MSE in Safari, add source elements for both the blob and manifest URLs. | ||
// The latter will enable Airplay playback on receiver devices. | ||
if ((videojs.browser.IS_ANY_SAFARI || videojs.browser.IS_IOS) && this.options_.overrideNative && this.options_.sourceType === 'hls') { | ||
this.tech_.addSourceElement(this.mediaSourceUrl_); |
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.
Is it possible that addSourceElement
will not exist on some techs? Do we want to check for it's existence first?
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.
MSE is an extension of HTML5 so we should be able to safely assume that all tech
references are for the HTML5 tech.
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.
Though unlikely, doing a check here adds safety in case someone is using a later version of vhs with the current version of videojs.
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.
Added a check
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.
Though unlikely, doing a check here adds safety in case someone is using a later version of vhs with the current version of videojs.
I guess if someone tries to use it this way, npm should warn that peer dependency is incompatible
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1543 +/- ##
==========================================
+ Coverage 86.36% 86.40% +0.04%
==========================================
Files 43 43
Lines 11148 11155 +7
Branches 2545 2550 +5
==========================================
+ Hits 9628 9639 +11
+ Misses 1520 1516 -4 ☔ View full report in Codecov by Sentry. |
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.
+1 once the changes from video.js are pulled in
Description
Note: This depends on videojs/video.js#8886. Before we merge this, we will need a new Video.js release and then have to bump the peer dependency version in this package.
This PR adds Airplay support when overriding native HLS in Safari and/or iOS by using two
source
elements, one for the MediaSource URL object and one for the Airplay-compatible manifest URL.From the Webkit guide on Airplay and MSE:
Specific Changes proposed
If we are playing HLS with MSE in Safari or iOS, set the source by adding two
<source>
elements-- one for the Media Source blob and one for the HLS manifest that will be used for Airplay. Preserve the existing behavior in all other cases.Requirements Checklist