http-streaming
http-streaming copied to clipboard
fix: resume loading on segment timeout for `experimentalBufferBasedABR`
Previously, if there was a segment timeout and experimentalBufferBasedABR
was set to true
, then the main segment loader would stay in a READY
state and never resume loading segments. This is because the buffer based ABR path doesn't follow the same flow as non buffer based ABR, where a new playlist would be loaded immediately and it would trigger a load. Buffer based ABR may determine that no rendition change should be made, despite the timeout, leading to nothing happening. This change makes the call to load explicit, but only for buffer based ABR on timeouts.
This addresses https://github.com/videojs/http-streaming/issues/1287
Test Changes
Changed
- Updated test to show that
experimentalBufferBasedABR
does make a playlist selection onbandwidthupdate
- Updated test to remove warning logged on a
bandwidthupdate
when there's only one playlist. This warning doesn't always make sense, as a timeout that causes abandwidthupdate
would be a connection issue, rather than a stream issue.
Added
- Added test that segment-loader fires
timeout
event on timeout - Added test that master-playlist-controller reloads segment loader on
timeout
if experimentalBufferBasedABR
Testing
- Open the demo page. Enable
[EXPERIMENTAL] Use Buffer Level for ABR (reloads player)
and setpreload
tonone
. - In Chrome Devtools use a very slow connection (I manually created a 10kbps profile).
- Press play
- Without the change, after the segment request times out (16.5 seconds for the first bipbop stream, no more requests are made and the buffering spinner remains. The video never starts, even if changing the network connectivity back to something reasonable.
- With the change, a new segment is requested and playback resumes (if changing network connectivity to something reasonable).
Requirements Checklist
- [X] Feature implemented / Bug fixed
- [ ] If necessary, more likely in a feature request than a bug fix
- [X] Unit Tests updated or fixed
- [ ] Docs/guides updated
- [ ] Example created (starter template on JSBin)
- [ ] Reviewed by Two Core Contributors
Codecov Report
Merging #1333 (555cc4a) into main (66707b4) will increase coverage by
0.01%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## main #1333 +/- ##
==========================================
+ Coverage 86.33% 86.34% +0.01%
==========================================
Files 39 39
Lines 9857 9859 +2
Branches 2298 2299 +1
==========================================
+ Hits 8510 8513 +3
+ Misses 1347 1346 -1
Impacted Files | Coverage Δ | |
---|---|---|
src/master-playlist-controller.js | 94.79% <100.00%> (+0.13%) |
:arrow_up: |
src/segment-loader.js | 96.37% <100.00%> (+<0.01%) |
:arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more