-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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(Xbox): drop incompatible variants for XBOX early #5777
perf(Xbox): drop incompatible variants for XBOX early #5777
Conversation
Incremental code coverage: 83.33% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good otherwise. Thanks!
lib/util/stream_utils.js
Outdated
// See: https://github.com/shaka-project/shaka-player/issues/3380 | ||
// Note: it makes sense to drop early | ||
if (isXboxOne && video && | ||
((video.width && video.width > 1920) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If width is null or undefined, it will not be > 1920. Please drop the "truthy" checks on width and height and combine width > 1920 || height > 1080 on one line if possible, to make this long sequence of booleans more readable.
Also, these continuations should be indented +2 from where they are now. That will also make them easier to differentiate from the log inside the if(). Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately, the compiler had a problem with that change, but I added dimension variables to make sure the type is right - hope that's OK
…5777) This change: - drops incompatible variants for XBOX before parsing codes and checking them against media capabilities API - avoids redeclaring variables each time in the loop Co-authored-by: Ivan Kohut <ivan.kohut@lamin.ar>
This change: