Re-elect penalized level after configurable penalty expiry#7771
Re-elect penalized level after configurable penalty expiry#7771robwalch merged 6 commits intovideo-dev:masterfrom
Conversation
| if (levels[i].videoRange !== 'SDR') { | ||
| levels[i].fragmentError++; | ||
| levels[i].loadError++; | ||
| levels[i].loadErrorTime = self.performance.now(); |
There was a problem hiding this comment.
We were hitting
hls.js/src/controller/error-controller.ts
Line 364 in a50bb7e
loadError here too, added same change here as well, but not familiar with this code path. Thanks for double checking.
There was a problem hiding this comment.
If playback needs to fallback from HDR to SDR then it should remain there. You can leave this as-is since adaptation will remain within the same codec and video color range unless a manual switch forces the change.
| if (levels[i].videoRange !== 'SDR') { | ||
| levels[i].fragmentError++; | ||
| levels[i].loadError++; | ||
| levels[i].loadErrorTime = self.performance.now(); |
There was a problem hiding this comment.
If playback needs to fallback from HDR to SDR then it should remain there. You can leave this as-is since adaptation will remain within the same codec and video color range unless a manual switch forces the change.
| isPenaltyExpired(levelInfo, this.hls.config.errorPenaltyExpireMs) || | ||
| (levelInfo.loadError === 0 && levelInfo.fragmentError === 0)) && |
There was a problem hiding this comment.
I'd prefer if the more complex (and less likely) isPenaltyExpired check came after the 0 error counts check:
| isPenaltyExpired(levelInfo, this.hls.config.errorPenaltyExpireMs) || | |
| (levelInfo.loadError === 0 && levelInfo.fragmentError === 0)) && | |
| (levelInfo.loadError === 0 && levelInfo.fragmentError === 0) || | |
| isPenaltyExpired(levelInfo, this.hls.config.errorPenaltyExpireMs)) && |
There was a problem hiding this comment.
There is just the issue of prettier formatting needed for the build to pass (the current build task is failing on lint check: https://github.com/video-dev/hls.js/actions/runs/23777534228/job/69410199645?pr=7771).
Please run npm run prettier && npm run sanity-check and stage and commit changed files to fix. Thanks 😄
f26ab3a to
db274c2
Compare
|
Almost there: https://github.com/video-dev/hls.js/actions/runs/23823070835/job/69440861387?pr=7771 When running the command |
This PR will...
errorPenaltyExpireMsconfig option (default0= disabled) that allows a penalized level (loadError > 0) to become eligible for re-election after the configured durationisPenaltyExpiredhelper used by botherror-controller(candidate scan) andabr-controller(upswitch tolerance check)loadErrorTimealongside everyloadError++so expiry can be measured accuratelyWhy is this Pull Request needed?
A level that exhausts its retries accumulates
loadError > 0, which excludes it from ABR upswitch and error recovery candidate selection. Its errors only clear on a successful buffer append — but it can'tbuffer because it won't be selected. This leaves the player stuck at a lower quality indefinitely until
stopLoadis called.Are there any points in the code the reviewer needs to double check?
inline question about unsure change but did that for consistency.
Resolves issues:
When
errorPenaltyExpireMs > 0,isPenaltyExpiredbypasses theloadError === 0gate in both selection paths once the penalty window has elapsed, giving the level a chance to recover without requiring a fullstopLoad.Checklist