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(dash): improve readability and reduce number of loops in dash parser #5768

Conversation

vanyaxk
Copy link
Contributor

@vanyaxk vanyaxk commented Oct 13, 2023

This change reduces the amount of loops in dash parser, and improves code readability, as checking config and creating an array of sets is in a separate method now.

@shaka-bot
Copy link
Collaborator

Incremental code coverage: 100.00%

@avelad avelad changed the title refactor(dash): improve readability and reduce number of loops in dash parser perf(dash): improve readability and reduce number of loops in dash parser Oct 13, 2023
@avelad avelad added type: performance A performance issue priority: P1 Big impact or workaround impractical; resolve before feature release component: DASH The issue involves the MPEG DASH manifest format labels Oct 13, 2023
@avelad avelad added this to the v4.6 milestone Oct 13, 2023
@vanyaxk vanyaxk force-pushed the refactor/improve-readability-in-dash-period-parser branch from 3de6bfd to 685ec39 Compare October 18, 2023 18:44
@vanyaxk vanyaxk force-pushed the refactor/improve-readability-in-dash-period-parser branch from 685ec39 to 9bd1040 Compare October 18, 2023 18:46
return [];
}

return adaptationSets.reduce((acc, currSet) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also use shaka.util.Functional.collapseArrays for this.
Something like...

return adaptationSets.map((set) => {
  return set.contentType == contentType ? set.streams : [];
}).reduce(shaka.util.Functional.collapseArrays, []);

If you think that doing it this way is better, though, instead I'll ask that you rename the variables on the method to all and part, as that is what we usually call them when reducing arrays.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to (all, part)

@theodab theodab merged commit 17a4363 into shaka-project:main Oct 20, 2023
15 checks passed
Robloche pushed a commit to Robloche/shaka-player that referenced this pull request Nov 30, 2023
…rser (shaka-project#5768)

This change reduces the amount of loops in dash parser, and improves
code readability, as checking config and creating an array of sets is in
a separate method now.

---------

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 19, 2023
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Dec 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: DASH The issue involves the MPEG DASH manifest format 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.

4 participants