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.

Sign up for GitHub

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

Jump to bottom

feat(HLS): Add automatically keyId-key for identity format #6308

Merged
merged 15 commits into from
Feb 29, 2024
Merged

feat(HLS): Add automatically keyId-key for identity format #6308

merged 15 commits into from
Feb 29, 2024

Conversation

avelad
Copy link
Member

@avelad avelad commented Feb 28, 2024

No description provided.

@avelad avelad marked this pull request as draft February 28, 2024 08:52
@avelad avelad changed the title feat(HLS): Add automatically pair keyId-key for identity format feat(HLS): Add automatically keyId-key for identity format Feb 28, 2024
@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 labels Feb 28, 2024
@avelad avelad added this to the v5.0 milestone Feb 28, 2024
@shaka-bot
Copy link
Collaborator

shaka-bot commented Feb 28, 2024 •
edited
Loading

Incremental code coverage: 97.60%

theodab
theodab requested changes Feb 28, 2024
View reviewed changes
lib/hls/hls_parser.js Show resolved Hide resolved
lib/hls/hls_parser.js Outdated Show resolved Hide resolved
lib/hls/hls_parser.js Show resolved Hide resolved
@avelad
Copy link
Member Author

avelad commented Feb 28, 2024

@shaka-bot please test

@shaka-bot
Copy link
Collaborator

@avelad: Lab tests started with arguments:

  • pr=6308

@avelad
Copy link
Member Author

avelad commented Feb 28, 2024

@shaka-bot please test

@avelad avelad marked this pull request as ready for review February 28, 2024 16:26
@shaka-bot
Copy link
Collaborator

@avelad: Lab tests started with arguments:

  • pr=6308

@joeyparrish
Copy link
Member

Quoting the HLS spec:

An encryption method of SAMPLE-AES means that the Media Segments
are Sample Encrypted using the Advanced Encryption Standard
[AES_128]. How these media streams are encrypted and encapsulated
in a segment depends on the media encoding and the media format of
the segment. fMP4 Media Segments are encrypted using the 'cbcs'
scheme of Common Encryption [COMMON_ENC]. Encryption of other
Media Segment formats containing H.264 [H_264], AAC [ISO_14496],
AC-3 [AC_3], and Enhanced AC-3 [AC_3] media streams is described
in the HTTP Live Streaming (HLS) Sample Encryption specification
[SampleEnc]. The IV attribute MAY be present; see Section 5.2.

So I think Tizen doesn't support CBCS, and isn't checking the schm box, and it assumes CTR mode. Then it decrypts to garbage and fails to decode.

I think we need to resolve #1419 and then skip the test if CBCS is unsupported.

@avelad
Copy link
Member Author

avelad commented Feb 28, 2024

Quoting the HLS spec:

An encryption method of SAMPLE-AES means that the Media Segments
are Sample Encrypted using the Advanced Encryption Standard
[AES_128]. How these media streams are encrypted and encapsulated
in a segment depends on the media encoding and the media format of
the segment. fMP4 Media Segments are encrypted using the 'cbcs'
scheme of Common Encryption [COMMON_ENC]. Encryption of other
Media Segment formats containing H.264 [H_264], AAC [ISO_14496],
AC-3 [AC_3], and Enhanced AC-3 [AC_3] media streams is described
in the HTTP Live Streaming (HLS) Sample Encryption specification
[SampleEnc]. The IV attribute MAY be present; see Section 5.2.

So I think Tizen doesn't support CBCS, and isn't checking the schm box, and it assumes CTR mode. Then it decrypts to garbage and fails to decode.

I think we need to resolve #1419 and then skip the test if CBCS is unsupported.

I'm going to skip the test for now

@avelad
Copy link
Member Author

avelad commented Feb 28, 2024

@shaka-bot please test

@shaka-bot
Copy link
Collaborator

@avelad: Lab tests started with arguments:

  • pr=6308

@joeyparrish joeyparrish mentioned this pull request Feb 28, 2024
Closed
@joeyparrish
Copy link
Member

I'm going to skip the test for now

That's fair. I'm bumping the priority of #1419 and assigning it to myself to follow-up with a proper check on this test.

joeyparrish
joeyparrish requested changes Feb 28, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
lib/hls/hls_parser.js Outdated Show resolved Hide resolved
lib/media/manifest_filterer.js Outdated Show resolved Hide resolved
lib/player.js Outdated Show resolved Hide resolved
lib/util/mp4_box_parsers.js Outdated Show resolved Hide resolved
test/hls/hls_parser_integration.js Outdated Show resolved Hide resolved
@avelad
Copy link
Member Author

avelad commented Feb 29, 2024

@shaka-bot please test

joeyparrish
joeyparrish approved these changes Feb 29, 2024
View reviewed changes
lib/util/manifest_parser_utils.js
@@ -133,7 +135,7 @@ shaka.util.ManifestParserUtils = class {
serverCertificateUri: '',
sessionType: '',
initData: initDatas,
keyIds: new Set(keyIds),
keyIds: new Set(originalKeyIds),
Copy link
Member

Choose a reason for hiding this comment

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

Aha!

@avelad avelad merged commit d251649 into shaka-project:main Feb 29, 2024
32 checks passed
@shaka-bot shaka-bot mentioned this pull request Feb 29, 2024
Merged
@avelad avelad deleted the mp4-sample-aes-clearkey branch March 1, 2024 15:28
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 29, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Apr 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@theodab theodab theodab approved these changes

@joeyparrish joeyparrish joeyparrish approved these changes

Assignees
No one assigned
Labels
component: HLS The issue involves Apple's HLS manifest format priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Milestone
v4.8
Development

Successfully merging this pull request may close these issues.
4 participants
@avelad @shaka-bot @joeyparrish @theodab