-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Improve EME Controller: Add PlayReady support #1918
Conversation
Would be nice to have public test content! Awesome job 👍 |
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.
Just a quick look over, but in general looks good.
I will have more comments after a more in depth look at it,.
#1915 can be closed if start looking at this? |
Yeah, that works for me. |
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.
IE11 doesn't support the navigator.requestMediaKeySystemAccess
api. You need to do something like:
let mediaKeys;
try {
mediaKeys = new window.MSMediaKeys(keyName);
} catch (error) {
return;
}
@ssreed A test stream would be nice to have so that we can automate testing of this. |
src/controller/eme-controller.js
Outdated
@@ -67,6 +72,7 @@ const createWidevineMediaKeySystemConfigurations = function (audioCodecs, videoC | |||
const getSupportedMediaKeySystemConfigurations = function (keySystem, audioCodecs, videoCodecs) { | |||
switch (keySystem) { | |||
case KeySystems.WIDEVINE: | |||
case KeySystems.PLAYREADY: |
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.
if this the same proc, we could rename createWidevineMediaKeySystemConfigurations
to reflect it is not widevine specific?
src/controller/eme-controller.js
Outdated
@@ -404,28 +419,16 @@ class EMEController extends EventHandler { | |||
let challenge; | |||
|
|||
if (keysListItem.mediaKeySystemDomain === KeySystems.PLAYREADY) { | |||
logger.error('PlayReady is not supported (yet)'); |
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.
was firmly believing we'd remove this line one day 👍
@mikrohard Thanks for making that asset! It took a couple tries, but I am getting playback in the demo page you put up. It took a minute or so to start playback, but now it is starting quickly on subsequent plays. I've tried on 2 versions of Edge (41.16299.726.0 and 42.17134.1.0). I'm not getting playback on a local build of this branch of hls.js and a couple demo players ( http://playready.azurewebsites.net/Home/TestPlayer and https://bitmovin.com/demos/drm). On the local build, I'm getting no errors, and I see loading, but no playback. For my config, I have
Is there anything I'm missing? I will pull this down and continue looking locally. Thanks again! |
You're missing these two: |
Brilliant! That works and I have playback in my local build. Thanks! |
That's why I suggested that |
That is how it actually works for Widevine but I never updated the demo page to properly support PlayReady. 😅 |
@mikrohard Do you have any instructions handy for using that KeySeed to do content for that MSFT Playready Public Test license server? We're trying to generate a fragmented playready file in addition to the bytearray asset you provided |
Using microsofts test keyseed only means using the right keyId/contentKey combination while encrypting the content (you can calculate the contentKey using the keySeed & keyId). You can find the instructions here: https://brokenpipe.wordpress.com/2016/10/06/generating-a-playready-content-key-using-a-key-seed-and-key-id/ Actually I didn't bother calculating this and just used the sample values provided on this website. If you do the same, you can also reuse PS: Later I also found out that https://test.playready.microsoft.com/service/rightsmanager.asmx implements the |
@mikrohard Great, thanks! We'll give that a try. |
Added key rotation support but there are some issues and work that needs to be addressed.
Would love feedback or advice on tackling some of these issues! |
@ssreed Thanks for the work you've done here ! I haven't had time to review it yet, but it is on my plate. In the meantime as part of our effort to bring more TypeScript into the project we just merged in a PR that converts / adds typings to the EME controller in its existing state causing conflicts with your changes here. Just an FYI, that's all. Here's the related PR for reference: #2076 |
Hope those changes make sense, I'm pretty new to TypeScript. |
Is this pull request still active? I would love to see playready support getting merged. We're planning to go into production with this patch... but I would very much prefer seeing this merged. Otherwise we'll have to maintain the patch by ourself. |
I'm not sure how much bandwidth I will have but I wouldn't mind some assistance in seeing this one through! |
@ssreed @mikrohard Sorry for sleeping on this PR for so long. I'm looking at fixing up the EME controller and would like to get these changes in. The current blockers for me are:
Our Widevine functional tests fail quite often and it looks to me like the controller is not handling async EME operations correctly. I'm working on a quick fix or at least better logging, but also thinking about doing some refactoring here. @ssreed if you could address the conflicts that would be great. Any test stream contributions (apologies if I missed the window of opportunity on the stream/test demo above). Creating DRM streams of my own would take away from the time I can allocate to actually fixing and testing these issues, so connecting me with providers (Azure, EZDRM for example) and test streams would be really helpful. I read through the conversation and it also looks like more could be done on the API config side. drm: {
widevine: {
url: licenseServerUrl,
licenseRequestFilter: function (request, drmInfo) {
request.headers["Content-Type"] = ["application/json"];
request.body = JSON.stringify({ desiredPayload: goesHere });
},
licenseResponseFilter: function (response, drmInfo) {
response.data = ArrayBufferfromBase64(wrappedLicense);
},
},
playready: {
// same
}
}
Totally agree. Usually the solution is just finding the intersection between Most of the concerns around configuration and docs were addressed in this PR that never made it in https://github.com/video-dev/hls.js/pull/2194/files#diff-96cc378dcdef180641176501f42183c7R79-R82 I'm keeping eyes on this change set because anyone wanting to integrate with different DRM solutions is going to need this level of configuration. |
Replaced by #4930 |
This PR will...
Depends on: #1915
Why is this Pull Request needed?
Are there any points in the code the reviewer needs to double check?
Resolves issues:
Checklist