-
Notifications
You must be signed in to change notification settings - Fork 72
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
fix: legacy fairplay #204
fix: legacy fairplay #204
Conversation
@@ -228,70 +228,22 @@ const onPlayerReady = (player, emeError) => { | |||
|
|||
setupSessions(player); | |||
|
|||
if (window.MediaKeys) { | |||
const playerOptions = getOptions(player); | |||
const isLegacyFairplay = playerOptions.keySystem && playerOptions.keySystem[LEGACY_FAIRPLAY_KEY_SYSTEM]; |
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.
Can we add some documentation regarding legacy/current fairplay for users?
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.
Yeah good call, will add some doc changes to this PR.
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 some usage docs to the readme and a comment defining legacy fairplay.
src/plugin.js
Outdated
const handleFn = (event) => { | ||
// TODO convert to videojs.log.debug and add back in | ||
// https://github.com/videojs/video.js/pull/4780 | ||
// videojs.log('eme', 'Received a \'webkitneedkey\' event'); |
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.
should it be removed?
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.
Yeah I'm thinking maybe we just remove the whole comment as this pertains only to legacy fairplay. Good catch.
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.
or uncommented and changed to videojs.log.debug()
?
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 suppose a bit of extra logging especially around legacy fairplay isn't a bad thing. My vote is now for extra logging. 🪚 🌲
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 am definitely for extra logging. I wonder why it was not included before :)
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 log.debug
for each event that was missing it and removed the TODOs.
Due to the way legacy fairplay
com.apple.fps.1_0
was initialized in the plugin, it was broken in v5. This is because Safari has MediaKeys as well as WebkitMediaKeys capabilities, so users attempting to play legacy fairplay content in modern Safari would fall into theif (window.MediaKeys)
logic.To avoid this, we check the playerOptions for the legacy keysystem
com.apple.fps.1_0
, if it exists we need to not initialize the in spec EME listeners and instead fall to theelse if (window.WebkitMediaKeys)
legacy fairplay initialization.Additionally, due to the way some players may initialize the options object the legacy fairplay initialization code was added to the API to allow for a player to initialize after the keysystem has been identified.