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

fix: legacy fairplay #204

Merged
merged 5 commits into from
Feb 17, 2024
Merged

fix: legacy fairplay #204

merged 5 commits into from
Feb 17, 2024

Conversation

adrums86
Copy link
Contributor

@adrums86 adrums86 commented Feb 14, 2024

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 the if (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 the else 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.

@adrums86 adrums86 changed the title Fix legacy fairplay fix: legacy fairplay Feb 15, 2024
@adrums86 adrums86 marked this pull request as ready for review February 15, 2024 22:30
@@ -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];
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it be removed?

Copy link
Contributor Author

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.

Copy link
Contributor

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()?

Copy link
Contributor Author

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. 🪚 🌲

Copy link
Contributor

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 :)

Copy link
Contributor Author

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.

src/fairplay.js Outdated Show resolved Hide resolved
src/plugin.js Show resolved Hide resolved
@adrums86 adrums86 merged commit ee6e512 into main Feb 17, 2024
4 checks passed
@adrums86 adrums86 deleted the fix-legacy-fairplay branch February 17, 2024 19:17
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.

None yet

4 participants