Add requireKeySystemAccessOnStart (fixes Chrome PIPELINE_DECODE_ERROR on clear to encrypted transition)
#7216
Conversation
…ack logic to key loader
|
@grabofus I'm seeing very mixed results with https://sample-videos-zyrkp2nj.s3-eu-west-1.amazonaws.com/test-20250422/master_clear_to_cbcs_preset_media_keys_vod.html Occasionally, Chrome successfully plays through and allows seeking between the discontinuities. At other times, it still fails with a pipeline error: Media error in dev console: Media tab logs: On the occasion that I observed it play through without error, it detected the audio decoder config change first: |
|
@robwalch this branch was missing an earlier commit from our fork that attempts to set media keys as soon as they are created, instead of waiting for the https://sample-videos-zyrkp2nj.s3-eu-west-1.amazonaws.com/test-20250422/master_clear_to_cbcs_preset_media_keys.html |
There was a problem hiding this comment.
Thanks for the update. It works quite nicely.
Can you look at why this unit test is failing?
https://github.com/video-dev/hls.js/actions/runs/14861488418/job/41727400833?pr=7216
AssertionError: expected setMediaKeys to have been called exactly 'once', but it was called twice
I'm taking a look now. To troubleshoot tests, add only to the test (it.only('should request keysystem...) and then start the test runner in Chrome in debuggable mode:
$ npm run test:unit:debug
src/controller/eme-controller.ts
Outdated
| @@ -292,6 +294,7 @@ class EMEController extends Logger implements ComponentAPI { | |||
| certificate, | |||
| ); | |||
| } | |||
| this.attemptSetMediaKeys(keySystem, mediaKeys); | |||
There was a problem hiding this comment.
This change is resulting in a second call to setMediaKeys (failing unit test assertion that setMediaKeys is only called once).
Is it necessary? (perhaps so based on your last comment)
If there is a certificate, however, they will not be set. getMediaKeysPromise is only supposed to retrieve the media keys. They shouldn't be set here.
I would recommend this be done in selectKeySystem but that is now used in two places. We only want to do the attempt from key-loader, so a new public method or key-loader would be approapriate:
public getKeySystemAccess(keySystemsToAttempt: KeySystems[]) {
return this.getKeySystemSelectionPromise(keySystemsToAttempt).then(
({ keySystem, mediaKeys }) => {
return this.attemptSetMediaKeys(keySystem, mediaKeys);
},
);
}
private selectKeySystem(| } | ||
| } | ||
| } | ||
| return null; |
There was a problem hiding this comment.
loadClear should more or less do the same thing whether or not the playlist has encrypted segments and requireKeySystemAccessOnStart is enabled.
Here I've modified it to only follow either path when there is not a selected format (this.emeController.getSelectedKeySystemFormats().length) and to call getKeySystemAccess, defined above, in either path when requireKeySystemAccessOnStart is enabled.
loadClear(
loadingFrag: Fragment,
encryptedFragments: Fragment[],
): null | Promise<void> {
if (
this.emeController &&
this.config.emeEnabled &&
!this.emeController.getSelectedKeySystemFormats().length
) {
// access key-system with nearest key on start (loading frag is unencrypted)
if (encryptedFragments.length) {
const { sn, cc } = loadingFrag;
for (let i = 0; i < encryptedFragments.length; i++) {
const frag = encryptedFragments[i];
if (
cc <= frag.cc &&
(sn === 'initSegment' || frag.sn === 'initSegment' || sn < frag.sn)
) {
return this.emeController
.selectKeySystemFormat(frag)
.then((keySystemFormat) => {
frag.setKeyFormat(keySystemFormat);
if (
this.emeController &&
this.config.requireKeySystemAccessOnStart
) {
const keySystem =
keySystemFormatToKeySystemDomain(keySystemFormat);
if (keySystem) {
return this.emeController.getKeySystemAccess([keySystem]);
}
}
});
}
}
} else if (this.config.requireKeySystemAccessOnStart) {
const keySystemsInConfig = getKeySystemsForConfig(this.config);
if (keySystemsInConfig.length) {
return this.emeController.getKeySystemAccess(keySystemsInConfig);
}
}
}
return null;
}I'm still not 100% on how useful getSelectedKeySystemFormats() is and whether or not there is a simpler way to test if a format needs to be selected and set when we have encrypted fragments.
|
@robwalch Thanks for the review! I've incorporated the changes you've suggested and updated the test pages. On a separate note, should the |
|
Hi @grabofus. Thanks for making those changes.
Any thoughts on deduping media-keys setting?
I think it should for v1.6.3. File an issue for defaulting it to enabling it in v1.7. |
requireKeySystemAccessOnStart (fixes Chrome PIPELINE_DECODE_ERROR on clear to encrypted transition)
|
@robwalch thank you! |
This PR will...
Request key system access based on the eme configuration (with
config.requireKeySystemAccessOnStartenabled) and block loading clear content until the media keys are set.This resolves an issue on Chrome where if clear content is encrypted into the SourceBuffer followed by setting media keys then transitioning to encrypted content, Chrome will die with
PIPELINE_DECODE_ERROR:Why is this Pull Request needed?
Are there any points in the code the reviewer needs to double check?
Resolves issues:
#4230
#5753
Checklist