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

use explicit check instead of implicit checking the part for truthy as to… #5714

Merged
merged 3 commits into from Aug 3, 2023

Conversation

jhonalino
Copy link
Contributor

@jhonalino jhonalino commented Aug 1, 2023

… not skip part 0

This PR will...

fix the part check conditional for the logic that overrides the level/track timeouts for LL-HLS blocking playlist reload request

Why is this Pull Request needed?

the level/track timeouts override is required for all parts, but its skipping part 0 erroneously because 0 is falsy,

Are there any points in the code the reviewer needs to double check?

skipping part 0 maybe on purpose?

Resolves issues:

not overriding the level/track timeouts for part 0 for LL-HLS

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

src/loader/playlist-loader.ts Outdated Show resolved Hide resolved
@robwalch robwalch added this to the 1.5.0 milestone Aug 2, 2023
jhonalino and others added 2 commits August 1, 2023 17:19
@jhonalino jhonalino changed the title use null check instead of implicit checking the part for truthy as to… use explicit check instead of implicit checking the part for truthy as to… Aug 2, 2023
@robwalch
Copy link
Collaborator

robwalch commented Aug 2, 2023

Thanks @jhonalino!

I'll merge this shortly after the build checks are complete.

Note: the player is overriding config.playlistLoadPolicy.default which is expected to be tuned for VOD and standard Live playlist loading. If you think it would be a useful feature for hls.js to define a policy specifically for Low-Latency playlist loading that developers could override, please consider filing a feature request. Adding config.playlistLoadPolicy.lowLatency might be a good addition, and would allow this code to be removed. Static defaults might not work as well as these dynamic settings for every part target duration, but 1 second is the norm, and it would be fully configurable.

@robwalch robwalch merged commit 6a67a96 into video-dev:master Aug 3, 2023
12 checks passed
@jhonalino jhonalino deleted the bugfix/LL-HLS-part-0-falsy branch August 3, 2023 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants