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

feat(hls): expose manifest skd uri on drmInfo #6857

Merged
merged 5 commits into from
Jun 19, 2024

Conversation

gkatsev
Copy link
Contributor

@gkatsev gkatsev commented Jun 18, 2024

No description provided.

@@ -203,6 +203,7 @@ shaka.extern.ServiceDescription;
* @typedef {{
* keySystem: string,
* encryptionScheme: string,
* keySystemUris: (Set.<string>|undefined),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was made a Set to match keyIds. I originally named it keySystemUris to be more generic, since DrmInfo is used both for hls/fairplay and DASH, but its type is possibly undefined so that the property could be unavailable when the data isn't present.

@avelad avelad requested a review from joeyparrish June 18, 2024 19:25
@avelad avelad added type: enhancement New feature or request component: HLS The issue involves Apple's HLS manifest format priority: P3 Useful but not urgent component: FairPlay The issue involves the FairPlay DRM labels Jun 18, 2024
@avelad avelad added this to the v4.10 milestone Jun 18, 2024
@shaka-bot
Copy link
Collaborator

shaka-bot commented Jun 18, 2024

Incremental code coverage: 78.95%

Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good otherwise. Thanks!

@@ -224,6 +225,9 @@ shaka.extern.ServiceDescription;
* @property {string} encryptionScheme
* <i>Required.</i> <br>
* The encryption scheme, e.g., "cenc", "cbcs", "cbcs-1-9".
* @property {Set.<string>} keySystemUris
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type (in docs) should match the above type (in compiler)

Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, looks like you have a test failure:

TOTAL: 1 FAILED, 2686 SUCCESS


1) for FairPlay
     HlsParser constructs DrmInfo with EXT-X-SESSION-KEY
     Expected $.variants[0].video.drmInfos[0] not to have properties
    keySystemUris: Set( 'skd://f93d4e700d7ddde90529a27735d9e7cb' )
Error: Expected $.variants[0].video.drmInfos[0] not to have properties
    keySystemUris: Set( 'skd://f93d4e700d7ddde90529a27735d9e7cb' )
    at <Jasmine>
    at _callee76$ (test/hls/hls_parser_unit.js:3934:22 <- test/hls/hls_parser_unit.js:2966:30)
    at tryCatch (node_modules/@babel/polyfill/dist/polyfill.js:6473:40)
    at Generator.invoke [as _invoke] (node_modules/@babel/polyfill/dist/polyfill.js:6702:22)

@gkatsev
Copy link
Contributor Author

gkatsev commented Jun 18, 2024

I'll take a look, probably missed something because I was cherry-picking the changes.

@avelad avelad merged commit 644677c into shaka-project:main Jun 19, 2024
14 of 15 checks passed
@tykus160 tykus160 deleted the keysystemuris branch June 19, 2024 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: FairPlay The issue involves the FairPlay DRM component: HLS The issue involves Apple's HLS manifest format priority: P3 Useful but not urgent type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants