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: add support for multiple robustness levels in drm #7753

Merged
merged 13 commits into from
Dec 23, 2024

Conversation

gkatsev
Copy link
Contributor

@gkatsev gkatsev commented Dec 11, 2024

This changes the drm.advanced.videoRobustness and audioRobustness config options from a string to an array of strings. Internally, in the drm engine, this gets expanded into an array of DrmInfos which have those values as strings.

Best way to review this change is with whitespace turned off in the diff options.

@gkatsev gkatsev changed the title Multiple robustness feat: add support for multiple robustness levels in drm Dec 12, 2024
@shaka-bot
Copy link
Collaborator

shaka-bot commented Dec 12, 2024

Incremental code coverage: 94.15%

@tykus160 tykus160 added type: enhancement New feature or request priority: P1 Big impact or workaround impractical; resolve before feature release labels Dec 12, 2024
@tykus160 tykus160 added this to the v4.13 milestone Dec 12, 2024
Copy link
Member

@tykus160 tykus160 left a comment

Choose a reason for hiding this comment

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

Please review unit tests

lib/media/drm_engine.js Outdated Show resolved Hide resolved
lib/player.js Outdated Show resolved Hide resolved
demo/main.js Outdated
@@ -949,10 +949,10 @@ shakaDemo.Main = class {
advanced[drmSystem] = shakaDemo.Main.defaultAdvancedDrmConfig();
}
if ('videoRobustness' in params) {
advanced[drmSystem].videoRobustness = params['videoRobustness'];
advanced[drmSystem].videoRobustness = [params['videoRobustness']];
Copy link
Member

Choose a reason for hiding this comment

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

Please update the Demo in order to support an array (using ,). See preferredVideoCodecs as example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

externs/shaka/player.js Outdated Show resolved Hide resolved
demo/main.js Outdated
}
}

this.configure('drm.advanced', advanced);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

without this, it would read the advanced settings from the hash, but it would never actually set it back on the player.

@gkatsev gkatsev requested review from avelad and theodab December 16, 2024 20:32
@avelad
Copy link
Member

avelad commented Dec 20, 2024

@gkatsev can you rebase your PR? Thanks!

test/media/drm_engine_unit.js Outdated Show resolved Hide resolved
test/media/drm_engine_unit.js Outdated Show resolved Hide resolved
lib/media/drm_engine.js Outdated Show resolved Hide resolved
@gkatsev gkatsev force-pushed the multiple-robustness branch from ccfa1c4 to f4c5db1 Compare December 20, 2024 19:56
@avelad avelad added priority: P3 Useful but not urgent and removed priority: P1 Big impact or workaround impractical; resolve before feature release labels Dec 23, 2024
@avelad avelad dismissed theodab’s stale review December 23, 2024 09:19

Reviewed by Wojciech and me

@avelad avelad merged commit 88472b3 into shaka-project:main Dec 23, 2024
17 of 18 checks passed
@gkatsev gkatsev deleted the multiple-robustness branch December 23, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants