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

Add ability to uninstall PatchedMediaKeysApple polyfill #4469

Closed
martinstark opened this issue Sep 6, 2022 · 10 comments · Fixed by #4471 or #4424
Closed

Add ability to uninstall PatchedMediaKeysApple polyfill #4469

martinstark opened this issue Sep 6, 2022 · 10 comments · Fixed by #4471 or #4424
Labels
component: FairPlay The issue involves the FairPlay DRM flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone

Comments

@martinstark
Copy link
Contributor

Have you read the FAQ and checked for duplicate open issues?
Yes

Is your feature request related to a problem? Please describe.

Yes. We want to be able to install and uninstall shaka.polyfill.PatchedMediaKeysApple depending on the type of content we're playing, since we want to be able to swap between native and non-native Safari Fairplay playback. This is due to "startover" live content not being playable using Shaka Fairplay HLS (see #4431).

Describe the solution you'd like

A shaka.polyfill.PatchedMediaKeysApple.uninstall() method, that uninstalls the polyfill. Synchronously if possible.

Describe alternatives you've considered

We're currently using native Fairplay playback, but it has more quality issues that playing using Shaka, so we would like to use Shaka playback as much as possible.

One alternative solution is to fix #4431.

Additional context

@martinstark martinstark added the type: enhancement New feature or request label Sep 6, 2022
@github-actions github-actions bot added this to the Backlog milestone Sep 6, 2022
@avelad avelad added priority: P3 Useful but not urgent component: FairPlay The issue involves the FairPlay DRM flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this labels Sep 6, 2022
@joeyparrish
Copy link
Member

I don't understand why this is necessary. Admittedly, I don't have a lot of hands-on experience with FairPlay. But removing the polyfill is a bit odd to me. In general, I would expect you simply not to install it, instead. Can you help me understand what the underlying issue is?

@martinstark
Copy link
Contributor Author

martinstark commented Sep 13, 2022

If there's a way to do this that we've not found, please correct us. This is where we're running into issues:

We use com.apple.fps.1_0 with initDataTransform to add contentId when using the polyfill and playing natively, and com.apple.fps without initDataTransform or the polyfill when using shaka.

If we don't uninstall the polyfill before playing using shaka, it throws HLS_MSE_ENCRYPTED_LEGACY_APPLE_MEDIA_KEYS_NOT_SUPPORTED.

If we use com.apple.fps to play natively (with or without initDataTransform), we get a number of different errors from different materials: FAILED_TO_GENERATE_LICENSE_REQUEST MEDIA_ERR_SRC_NOT_SUPPORTED MEDIA_ERR_DECODE.

Since Shaka is able to play using com.apple.fps I assume our provider supports modern EME.

Is there a difference in the default handling of the initData when playing natively vs. playing using shaka with modern EME?

We're using DrmToday by Castlabs.

Thank you for taking your time.

To clarify the issue:

We would like to use both native and non-native shaka playback depending on content, but we haven't been able to get a single keysystem to work for both. That is why the question of being able to uninstall the polyfill in order to use both keysystems came up. It'd be great if we would be able to use modern EME for everything, in which case this ticket can and should be closed.

@avelad
Copy link
Collaborator

avelad commented Sep 13, 2022

@joeyparrish , I think the same as @martinstark , because I have found the same error, that's why I have approved the PR.

@martinstark
Copy link
Contributor Author

just FYI, the PR in its current state is non-functional. I will update it to the working version if there's no comment on my question

@joeyparrish
Copy link
Member

I feel like the best thing to do would be to align Fairplay support such that the same key system IDs and configurations work whether you're using native HLS or not. Is that not possible?

@martinstark
Copy link
Contributor Author

@avelad @joeyparrish I agree, the root cause seems to be that modern EME support is broken when playing natively through Shaka.

I don't have ready access to an Apple laptop in order to attempt this myself. I could maybe try with some pointers? But it's probably better if someone who does could look at it.

@joeyparrish
Copy link
Member

@martinstark, does the fix for #4431 relieve the need to switch?

I'm not deeply familiar with FairPlay myself, and I'm going to be away until Monday. I apologize for the inconvenience.

@avelad, if you have any insights, please let me know. I can take a look at this again when I return, but I'd be starting from scratch.

@avelad
Copy link
Collaborator

avelad commented Sep 14, 2022

I have looked it up and my conclusions are:

  • Modern EME, only supports MP4 with MSE and with src= supports both TS and MP4. The problem is that the DRM provider has to support this new implementation and not all support it (in my experience only 50%)
  • The Legacy API does not support MSE, and with src= it supports TS as MP4. It is the API with which all DRM providers are integrated.

Using MSE has many advantages and if a provider supports MP4 and the Modern EME is the way to go.

This issue makes a lot of sense if you have to support two DRM providers at the same time and one supports Modern EME and the other doesn't.

Since the PR does not add a large amount of code, I would approve it, since there is no good solution unless the code handles both APIs at the same time (which has never been done).

What I would like to add to the PR is to document in the FairPlay tutorial that this new method exists.

@joeyparrish
Copy link
Member

Okay. I'm convinced. Thank you both for taking the time to explain it.

just FYI, the PR in its current state is non-functional. I will update it to the working version if there's no comment on my question

@avelad, please coordinate with @martinstark and merge the PR when you both feel it's ready. @martinstark, I left a comment on the PR with a tip that may help with the prototype getter.

I would prefer to see a test added for this new uninstall() method if possible. You can limit it to run only on Safari and use jasmine's pending() method (terrible name) to skip the test otherwise. @avelad, if you judge it's too difficult to add a test, you can merge the PR without it. I'll trust your decision.

Thank you both for your patience with me.

@martinstark
Copy link
Contributor Author

martinstark commented Sep 15, 2022

does the fix for #4431 relieve the need to switch?

Yes, for our particular use case it does, thank you very much for fixing it (testing it right now on the main branch). Since it seems this feature may be needed by others I'll keep working on the PR.

@avelad avelad modified the milestones: Backlog, v4.3 Sep 23, 2022
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Nov 22, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: FairPlay The issue involves the FairPlay DRM flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
3 participants