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

perf: simplify and improve performance of parsing initData when deduping #5775

Conversation

vanyaxk
Copy link
Contributor

@vanyaxk vanyaxk commented Oct 16, 2023

This change simplifies code that parses initData in the getCommonDrmInfos method

@avelad avelad changed the title refactor(drm): simplify and improve performance of parsing initData when deduping perf: simplify and improve performance of parsing initData when deduping Oct 16, 2023
@avelad avelad added type: performance A performance issue priority: P1 Big impact or workaround impractical; resolve before feature release labels Oct 16, 2023
@avelad avelad added this to the v4.6 milestone Oct 16, 2023
@vanyaxk vanyaxk force-pushed the refactor/avoid-unnecessary-complexity-when-comparing-init-data branch from 59b705a to d365049 Compare October 16, 2023 11:41
@shaka-bot
Copy link
Collaborator

Incremental code coverage: 100.00%

for (const d of bothInitDatas) {
if (typeof d !== 'string' && !initDataMap.has(d.keyId)) {
initDataMap.set(d.keyId, d);
} else if (typeof d === 'string' && !initDataMap.has(d)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps I'm forgetting something. In what situations are we expecting the initData array of the drm to contain a string instead of a shaka.extern.InitDataOverride? The previous version of this code did not seem to account for that.

Copy link
Contributor Author

@vanyaxk vanyaxk Oct 21, 2023

Choose a reason for hiding this comment

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

I had to take this into account as not doing so breaks one of the tests where there is indeed a string in initData which I reckoned there is a reason behind

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. That's annoying. We shouldn't have tests like that.
In that case I'll approve this, and then just quickly go and make a PR to change those tests.

@theodab theodab merged commit 041a08f into shaka-project:main Oct 22, 2023
15 checks passed
Robloche pushed a commit to Robloche/shaka-player that referenced this pull request Nov 30, 2023
…ing (shaka-project#5775)

This change simplifies code that parses initData in the `getCommonDrmInfos` method

Co-authored-by: Ivan Kohut <ivan.kohut@lamin.ar>
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Dec 21, 2023
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: performance A performance issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants