-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 support for com.apple.fps.3_0 #2630
Conversation
src/controller/eme-controller.ts
Outdated
@@ -344,6 +412,10 @@ class EMEController extends EventHandler { | |||
logger.log(`Generating key-session request for "${initDataType}" init data type`); | |||
keysListItem.mediaKeysSessionInitialized = true; | |||
|
|||
if (initDataType === 'sinf') { | |||
keysListItem.mediaKeyId = this._findKeyInSinf(initData); |
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.
❓ Don't know if we should pass the whole tenc box here so any integrator could retrieve all the info (IV & encrypted info + ...) + could do their own transformation on the encrypted info from binary data
In my case that's the content keyId, so I guess I'll need to change the name because it depends on the integrator ?
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 this a generic thing? getting the mediaKeyId from the tenc in order to send it in the XHR headers?
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 think it depends on each DRM Provider because some would support sinf
initDataType while others don't... In my use case, I need it but others might not need it at all.
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.
Honestly, I'm getting to the point with a DRM solution for a video-engine layer like hls.js, if the best solution would be to get out of everyone's way, and simply offer a callback that a user can return a Promise from which we can use when it resolves to setMediaKeys
on the video element. You can build on top of this incredibly simple design for each different DRM provider, but the core logic doesn't need to concern itself with this information.
We detect when the browser has signalled through the encrypted event, or the manifest has signalled through tags that it is encrypted, and provide the raw information to the user. They become responsible for providing the completed handshake which we can setMediaKeys
with. We could still do the navigator key system check and provide this metadata, or not. I'm not convinced if we should or not, given that the priority of which key system gets used can change based on business arrangements.
I feel like we can offer built-in pure functions for the simpler DRM cases, but then there is nothing preventing a vendor from getting their custom need met.
I think this is inline with what Oren is suggesting about abstracting, but we could go even further and at the most "core" hls logic, completely abstract how a media key is resolved, and kick that out as a user-provided interface. This will then allow DRM support across the board, without blocking anyone, and we can continue to iterate on support for individual DRM vendors / systems to reduce duplication of effort, and complexity.
@robwalch @OrenMe @valotvince Thoughts on this as an approach?
src/controller/eme-controller.ts
Outdated
const xhr = new XMLHttpRequest(); | ||
const licenseXhrSetup = this._licenseXhrSetup; | ||
const xhrSetupData : LicenseXHRAdditionalData = { keyId: keysListItem.mediaKeyId }; |
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 allow integrators to populate xhr headers / post info with correct data depending of their DRM providers
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 keyId in header a generic FPS 3.0 requirement? If it is not then this is specific per a DRM service and should be abstracted.
Also the type LicenseXHRAdditionalData
is for Fairplay but this is a method that handles all DRM systems so not sure this is fitting.
In general having a getter in every function that needs to know about the type of DRM system might get dirty as more implementation details will surface.
Might be better to rethink the whole stack - for instance:
- have a generic interface to handle DRM and then write adapters for each DRM system, which should be quite easy as they all adhere to CDM interface and differ in internal non spec-ed per DRM system configs.
- Use the network stack already built - xhr-loader - this will give access to same retry config and logic and performance metrics and is better suited to future work for adding fetch support.
3.maybe now it is a good time to expose a some sort of mechanism to add middleware like to request/response instead of todays xhrSetup or loaders config. Shaka are doing it and it is quite nice. Of course it will require to use same loader as mentioned in handle xhr timeout and i/o errors #2
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.
See my answer above, i think it's more of a provider constraint than a requirement for FPS 3.0
src/controller/eme-controller.ts
Outdated
this._attemptKeySystemAccess(KeySystems.WIDEVINE, audioCodecs, videoCodecs); | ||
console.log(data.levels); | ||
|
||
this._attemptKeySystemAccess(KeySystems.FAIRPLAY, audioCodecs, videoCodecs); |
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.
🆘 From there, I don't really know how to select the good keySystem, based on what gives the manifest, any ideas on this ?
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 think based on manifest metadata is best as you will have the key system type there.
You will have to add parser support for that.
Do you have a sample manifest to use?
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 have one sample that works but I should rethink of #2606 so the encrypt information is still flagged but not loaded as done today
@@ -4,6 +4,7 @@ | |||
export enum KeySystems { | |||
WIDEVINE = 'com.widevine.alpha', | |||
PLAYREADY = 'com.microsoft.playready', | |||
FAIRPLAY = 'com.apple.fps.3_0' |
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 don't really know the differences between 2_0 et 3_0 (only that from 2_0 we can use MSE)
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.
do you have a sample that uses com.apple.fps.3_0 and that is actually supported on Safari for any given domain?
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 have a working sample with our DRM provider (DRMToday), but it only works with assets having one encryption key. Assets with multiple encryption keys doesn't work yet but it seems to be a provider limitation rather than a Safari one.
Hi @robwalch :) |
src/controller/eme-controller.ts
Outdated
const xhr = new XMLHttpRequest(); | ||
const licenseXhrSetup = this._licenseXhrSetup; | ||
const xhrSetupData : LicenseXHRAdditionalData = { keyId: keysListItem.mediaKeyId }; |
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 keyId in header a generic FPS 3.0 requirement? If it is not then this is specific per a DRM service and should be abstracted.
Also the type LicenseXHRAdditionalData
is for Fairplay but this is a method that handles all DRM systems so not sure this is fitting.
In general having a getter in every function that needs to know about the type of DRM system might get dirty as more implementation details will surface.
Might be better to rethink the whole stack - for instance:
- have a generic interface to handle DRM and then write adapters for each DRM system, which should be quite easy as they all adhere to CDM interface and differ in internal non spec-ed per DRM system configs.
- Use the network stack already built - xhr-loader - this will give access to same retry config and logic and performance metrics and is better suited to future work for adding fetch support.
3.maybe now it is a good time to expose a some sort of mechanism to add middleware like to request/response instead of todays xhrSetup or loaders config. Shaka are doing it and it is quite nice. Of course it will require to use same loader as mentioned in handle xhr timeout and i/o errors #2
src/controller/eme-controller.ts
Outdated
this._attemptKeySystemAccess(KeySystems.WIDEVINE, audioCodecs, videoCodecs); | ||
console.log(data.levels); | ||
|
||
this._attemptKeySystemAccess(KeySystems.FAIRPLAY, audioCodecs, videoCodecs); |
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 think based on manifest metadata is best as you will have the key system type there.
You will have to add parser support for that.
Do you have a sample manifest to use?
@@ -4,6 +4,7 @@ | |||
export enum KeySystems { | |||
WIDEVINE = 'com.widevine.alpha', | |||
PLAYREADY = 'com.microsoft.playready', | |||
FAIRPLAY = 'com.apple.fps.3_0' |
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.
do you have a sample that uses com.apple.fps.3_0 and that is actually supported on Safari for any given domain?
8077c9e
to
5f3a715
Compare
@valotvince Hla fair play support is completed? Can you give some demo source code? |
@singhcool Hi :) This was complete, I was able to make it work with a single-key but not with a multi-key encrypted video... It should only be a limitation from my DRM provider.
Where contentIdFromInitData is a function retrieving the keyId from the Hope that helps ! Best, |
5f3a715
to
560bfb9
Compare
@valotvince Thank you. If i need any support during integration, please help me. Advance Thanks ! :) |
This PR will...
Add support for Fairplay w/ MSE
Why is this Pull Request needed?
Based on a feature request @BedrockStreaming needed to capture EXT-X tags in the manifest so we could do Client-side Ad Insertion on Mac / Safari
Are there any points in the code the reviewer needs to double check?
Resolves issues:
Checklist