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

WebkitAddKey argument mismatch while playing EME v0.1b widevine stream #2149

Closed
pavankn opened this issue Aug 30, 2017 · 12 comments
Closed

WebkitAddKey argument mismatch while playing EME v0.1b widevine stream #2149

pavankn opened this issue Aug 30, 2017 · 12 comments
Labels

Comments

@pavankn
Copy link

pavankn commented Aug 30, 2017

Hi ,
I am Trying to load the stream below and getting an error from Dash reference player v2.5.0 .
Stream : https://storage.googleapis.com/shaka-demo-assets/sintel-widevine/dash.mpd
DRM KeySystem : com.widevine.alpha
License Server : https://widevine-proxy.appspot.com/proxy

Platform :
Browser : WPEWebKit master branch
Hardware : ARM based SOC
EME : EME v0.1b (prefixed EME)

The Error is as follows :
dash.all.debug.js:40688:37: CONSOLE ERROR TypeError: Argument 3 ('initData') to HTMLMediaElement.webkitAddKey must be an instance of Uint8Array .

The Call that resulted in the error is as follows :
videoElement[api.addKey](keySystem.systemString, new Uint8Array(message), sessionToken.initData, sessionID);

here sessionToken.initData is an ArrayBuffer and not UInt8Array which is causing the issue .

I would like to know if there is a fix for this already .

@davemevans
Copy link
Contributor

// Some browsers return initData as Uint8Array (IE), some as ArrayBuffer (Chrome).
converts all needkey-derived initdata to ArrayBuffer, which was introduced in 5dc5bc1.

It looks like the initData needs to be wrapped in a new UInt8Array somewhere in the protection model (probably updateKeySession as you note). Does that fix the problem? I note that initData handling in 3Feb2014 protection model does convert the initdata in updateKeySession correctly so this certainly looks like a bug.

I don't know if there are implementations which rely on this broken behavior.

@epiclabsDASH
Copy link
Contributor

epiclabsDASH commented Aug 30, 2017

According to EME specification (https://w3c.github.io/encrypted-media/#mediakeysession-interface) update method of MediaKeySession should receive an argument whose type should be BufferSource. This is, ArrayBuffer or any TypedArray (Int8Array, Uint8Array, etc..). So this seems to be fine with dash.js implementation and the reason behind this didn't raise any error until now.

However, in the practice, I guess there are browser/devices implementations that require the key
type to be Uint8Array. This could be for historical reasons as for previous specifications of EME, key type was always Uint8Array.

So, to guarantee compatibility across different devices I think make sense modifying dash.js to ensure key type is Uint8Array. This will work with everything: I right tested both scenarios, ArrayBuffer and Uint8Array, in Chrome.

@epiclabsDASH
Copy link
Contributor

epiclabsDASH commented Aug 30, 2017

Sorry, right notice I was wrong. Browser that raised this issue is using ProtectionModel_01b so it uses addKey method (first version of EME spec) and it always requires the key to be passed as an Uint8Array.

Line

new Uint8Array(message), sessionToken.initData, sessionID);
should be:
new Uint8Array(message), new Uint8Array(sessionToken.initData), sessionID);

@davemevans
Copy link
Contributor

Presumably the ClearKey branch of the surrounding if would need updating (and testing) too.

@epiclabsDASH
Copy link
Contributor

yep, you are right.

@pavankn, given you have a quick way of reproducing the issue, would it be possible for you to check if above recommendation fix your problem?

@pavankn
Copy link
Author

pavankn commented Aug 31, 2017

Hi @epiclabsDASH and @bbcrddave ,
Thanks for looking into the issue .
Now, I have wrapped intData under uint8Array and the logic works fine for me now .

videoElement[api.addKey](keySystem.systemString, new Uint8Array(message), new Uint8Array(sessionToken.initData), sessionID);

I will update the bug again after testing the ClearKey branch of the surrounding if . The 3rd argument in the case of clearkey is message.keyPairs[i].keyID which seems to me like a Uint8Array itself but i will test and update

Thanks

@epiclabsDASH
Copy link
Contributor

Great, many thanks for the feedback

@epiclabsDASH
Copy link
Contributor

Hi @pavankn,

Do you have any info to share regarding ClearKey and the surrounding if?

Thanks!

@pavankn
Copy link
Author

pavankn commented Sep 11, 2017

Hi,
Sorry for the late reply . I have tested the clearkey implementation and this works with the existing code without any changes . So, no need to change that for now

Pavan

@epiclabsDASH
Copy link
Contributor

Perfect. I will send a PR with the modification that works for the non ClearKey branch of the if.

Thanks for your help.

@epiclabsDASH
Copy link
Contributor

Closing this one. @pavankn, thanks for your support.

@pavankn
Copy link
Author

pavankn commented Sep 11, 2017

Thanks @epiclabsDASH .

Pavan

dsparacio pushed a commit that referenced this issue Sep 29, 2017
siropeich pushed a commit to siropeich/dash.js that referenced this issue Oct 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants