Some DRM providers require default KID in wrapped license requests by birme · Pull Request #529 · shaka-project/shaka-player

birme · 2016-09-20T13:18:55Z

Suggested fix for #528

…that a default KID is provided in the wrapped license request

joeyparrish

I left some feedback on your design. Are you using player.drmInfo() to get the new information into your filter?

joeyparrish · 2016-09-22T15:46:04Z

AUTHORS

 Sanborn Hilland <sanbornh@rogers.com>
 TalkTalk Plc <*@talktalkplc.com>
 uStudio Inc. <*@ustudio.com>
+Jonas Birmé <jonas.birme@eyevinn.se>


Please keep this list alphabetized.

joeyparrish · 2016-09-22T15:46:11Z

CONTRIBUTORS

 Timothy Drews <tdrews@google.com>
 Vasanth Polipelli <vasanthap@google.com>
 Vignesh Venkatasubramanian <vigneshv@google.com>
+Jonas Birmé <jonas.birme@eyevinn.se>


Please keep this list alphabetized.

joeyparrish · 2016-09-22T15:48:17Z

lib/media/drm_engine.js

      });
    }
+
+    if (drmInfo.defaultKeyID) {


This is inconsistent with other places where you called it defaultKID, but as mentioned earlier, please rename to keyIds.

joeyparrish · 2016-09-22T15:50:25Z

externs/shaka/manifest.js

 *   serverCertificate: Uint8Array,
- *   initData: Array.<!shakaExtern.InitDataOverride>
+ *   initData: Array.<!shakaExtern.InitDataOverride>,
+ *   defaultKID: ?string


There can be (and best practice for content authoring dictates there should be) multiple key IDs in a single manifest. If you're going to go this route, this should be an array of strings (default empty list, not null), and it should be called keyIds.

birme · 2016-09-22T16:20:16Z

Thank you for the feedback. I will correct these things

…ingle manifest

joeyparrish

Just one more change, please. The rest looks good.

joeyparrish · 2016-09-23T18:25:19Z

lib/media/drm_engine.js

 * @param {!Array.<string>} licenseServers
 * @param {!Array.<!Uint8Array>} serverCerts
 * @param {!Array.<!shakaExtern.InitDataOverride>} initDatas
+ * @param {!Array.<string>} defaultKeys


For consistency, let's give this the same name as the structure member: keyIds.

joeyparrish · 2016-09-23T18:25:55Z

lib/media/drm_engine.js

  var initDatas = [];

-  this.processDrmInfos_(drmInfos, licenseServers, serverCerts, initDatas);
+  /** @type {!Array.<!string>} */


strings are non-nullable by default, so you can just say !Array.<string>

birme · 2016-09-23T18:41:41Z

No probs, fixed that last thing now

joeyparrish

Looks good. I'll run it through the build bot to test it.

shaka-bot · 2016-09-23T22:21:47Z

Testing in progress...

shaka-bot · 2016-09-23T22:29:40Z

Failure:

Chrome 53.0.2785 (Linux 0.0.0).DrmEngine getDrmInfo includes correct info FAILED
  Expected Object({ keySystem: 'drm.abc', licenseServerUri: 'http://abc.drm/license', distinctiveIdentifierRequired: true, persistentStateRequired: true, audioRobustness: 'good', videoRobustness: 'really_really_ridiculously_good', serverCertificate: undefined, initData: [  ], keyIds: [  ] }) to equal Object({ keySystem: 'drm.abc', licenseServerUri: 'http://abc.drm/license', distinctiveIdentifierRequired: true, persistentStateRequired: true, audioRobustness: 'good', videoRobustness: 'really_really_ridiculously_good', serverCertificate: undefined, initData: [  ] }).
      at test/media/drm_engine_unit.js:1455:25
      at wrapper (lib/polyfill/promise.js:333:21)
      at Function.shaka.polyfill.Promise.flush (lib/polyfill/promise.js:440:11)
      at test/test/util/util.js:76:32


Failed 1 tests, passed 890, skipped 36

joeyparrish · 2016-09-23T23:16:52Z

Looks like you'll need to update the tests to match. You can run the tests for yourself with python build/test.py.

birme · 2016-09-24T05:39:31Z

Chrome 53.0.2785 (Mac OS X 10.10.5): Executed 891 of 927 (skipped 36) SUCCESS (6 mins 11.643 secs / 7 mins 27.425 secs)

shaka-bot · 2016-09-26T16:35:44Z

Testing in progress...

shaka-bot · 2016-09-26T16:36:49Z

Failure:

+ echo START-BUILD
START-BUILD
+ ./build/all.py
----- FILE  :  /var/lib/jenkins/jobs/Manual PR Test/workspace/test/media/drm_engine_unit.js -----
Line 1444, E:0110: Line too long (99 characters).
Found 1 errors, including 0 new errors, in 1 files (161 files OK).
Generating Closure dependencies...
Running Closure linter...
Build step 'Execute shell' marked build as failure

birme · 2016-09-26T16:56:27Z

Sorry, thought I did a lint check before pushing. Will fix

shaka-bot · 2016-09-26T18:57:19Z

Testing in progress...

shaka-bot · 2016-09-26T19:05:15Z

All tests passed!

) Makes key IDs from the manifest available to the app through DrmInfo. From there, they can be used in license request filters.

birme added 4 commits September 20, 2016 13:48

Include default KID in drm info struct as some DRM providers require …

fd38a06

…that a default KID is provided in the wrapped license request

Make defaultKID default to null

42fa9d9

Make defaultKID default to null

aa693af

Updated authors and contributors files

68f36ab

joeyparrish requested changes Sep 22, 2016

View reviewed changes

birme added 2 commits September 23, 2016 10:16

Keep list alphabetized

b9d76e3

Correction based on feedback for PR to handle multiple key IDs in a s…

72dba5c

…ingle manifest

joeyparrish requested changes Sep 23, 2016

View reviewed changes

Renaming variable defaultKeys to keyIds for consistency

de005a6

joeyparrish approved these changes Sep 23, 2016

View reviewed changes

Updated unit test for drmInfo struct to include keyIds

b4b0410

Fixed line to long error

f5f0d47

joeyparrish merged commit d05b7d5 into shaka-project:master Sep 26, 2016

joeyparrish mentioned this pull request Sep 26, 2016

Some DRM providers require default KID in wrapped license requests #528

Closed

joeyparrish pushed a commit that referenced this pull request Oct 19, 2016

Some DRM providers require default KID in wrapped license requests (#529

550a05c

) Makes key IDs from the manifest available to the app through DrmInfo. From there, they can be used in license request filters.

github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023

github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023

Conversation

birme commented Sep 20, 2016

Uh oh!

joeyparrish left a comment

Choose a reason for hiding this comment

Uh oh!

joeyparrish Sep 22, 2016

Choose a reason for hiding this comment

Uh oh!

joeyparrish Sep 22, 2016

Choose a reason for hiding this comment

Uh oh!

joeyparrish Sep 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joeyparrish Sep 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

birme commented Sep 22, 2016

Uh oh!

joeyparrish left a comment

Choose a reason for hiding this comment

Uh oh!

joeyparrish Sep 23, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joeyparrish Sep 23, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

birme commented Sep 23, 2016

Uh oh!

joeyparrish left a comment

Choose a reason for hiding this comment

Uh oh!

shaka-bot commented Sep 23, 2016

Uh oh!

shaka-bot commented Sep 23, 2016

Uh oh!

joeyparrish commented Sep 23, 2016

Uh oh!

birme commented Sep 24, 2016

Uh oh!

shaka-bot commented Sep 26, 2016

Uh oh!

shaka-bot commented Sep 26, 2016

Uh oh!

birme commented Sep 26, 2016

Uh oh!

shaka-bot commented Sep 26, 2016

Uh oh!

shaka-bot commented Sep 26, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joeyparrish Sep 22, 2016 •

edited

Loading

joeyparrish Sep 22, 2016 •

edited

Loading

joeyparrish Sep 23, 2016 •

edited

Loading

joeyparrish Sep 23, 2016 •

edited

Loading