experimentalBufferBasedABR pauses loading on request timeout

[andreasgangso]

Description

HLS stream not switching rendition when network condition changes. This only happens with experimentalBufferBasedABR set to true. Screen recording: https://user-images.githubusercontent.com/727125/169267092-86277870-9d3a-4179-98a0-679aded8fbaf.mp4

Sources

Can reproduce with https://d2zihajmogu5jn.cloudfront.net/bipbop-advanced/bipbop_16x9_variant.m3u8, see codesandbox below.

Steps to reproduce

Explain in detail the exact steps necessary to reproduce the issue.

1. Open https://codesandbox.io/s/gallant-grass-o97t5c?file=/src/index.js
2. Open devtools network tab
3. Start the stream with no network throttling, playing one of the high bandwidth renditions.
4. Switch to "Slow 3G". The video request will likely time out.

Results

Expected

Quickly switch to a rendition requiring lower bandwidth.

Actual

Completely stops requesting new segments.

Additional Information

videojs-http-streaming version

what version of videojs-http-streaming does this occur with? videojs-http-streaming 2.13.1

videojs version

what version of videojs does this occur with? video.js 7.18.1

Browsers

Microsoft Edge (Chromium-based) Version 101.0.1210.47 (Official build) (arm64)

Platforms

MacOS

[video-archivist-bot]

Hey! We've detected some video files in a comment on this issue. If you'd like to permanently archive these videos and tie them to this project, a maintainer of the project can reply to this issue with the following commands:

• for https://user-images.githubusercontent.com/727125/169267092-86277870-9d3a-4179-98a0-679aded8fbaf.mp4: say @video-archivist-bot save nwRprE
• for https://d2zihajmogu5jn.cloudfront.net/bipbop-advanced/bipbop_16x9_variant.m3u8: say @video-archivist-bot save 5wqgWE

[andreasgangso]

Updated. Forgot to mention that this only happens with experimentalBufferBasedABR set to true.

[gesinger]

This is a great catch @andreasgan ! Thank you for submitting, and for the reproducible test case!

It looks like what's happening is the request times out, leading to the segment loader pausing: https://github.com/videojs/http-streaming/blob/5223427eac208e50af6285562767111c865d05ac/src/segment-loader.js#L2681-L2689

The handleTimeout_ function triggers bandwidthupdate: https://github.com/videojs/http-streaming/blob/5223427eac208e50af6285562767111c865d05ac/src/segment-loader.js#L2636

A bandwidthupdate should trigger a new quality selection, but experimentalBufferBasedABR disables that quality selection, as it's running on a timer instead: https://github.com/videojs/http-streaming/blob/5223427eac208e50af6285562767111c865d05ac/src/master-playlist-controller.js#L773-L779

This pause shouldn't happen for experimentalBufferBasedABR, and we'll look into fixing it.

Thank you again!

[gesinger]

After discussing with @gkatsev , the easiest approach may be to call checkABR_ on bandwidthupdate for experimentalBufferBasedABR as well.

[phloxic]

As per #1295 the likelyhood of triggering this can be increased by adding this to the steps to reproduce:

5. Seek to an unbuffered position directly after throttling the connection

[phloxic]

FWIW, this happens both with HLS and Dash.

[phloxic]

@gesinger - I'm testing this patch. It covers the scenario of the added test.

However, it does not fix the problem I was describing in #1295 and above. The behaviour after start playback - throttle - seek is different: where before the player just hang or got stuck, it now continues to load video segments of the current resolution, but does not play. That's because it continues to load, but it does not switch down (which it does when buffer based ABR is turned off).

[gesinger]

Good catch @phloxic ! Thank you for testing.

I took a look at that case and saw what's happening. It stems from a change made here: https://github.com/videojs/http-streaming/pull/1137

What's happening is that the bandwidth value is not being updated despite the timeouts, so the rendition is never changed.

I think two changes make sense here:

1. Right now the moving average selector is checking against the last bandwidth value and only updating the average if it's different. However, a new bandwidth update can have the same bandwidth value. The intention is to change on bandwidth updates, even if the new value is the same as the old. So I'll add a timestamp for the bandwidth value and check to see if it's a different bandwidth update, even if it's the same value.
2. Right now the moving average selector is checking against a systemBandwidth of 0 and ignoring it if it is. Since a timeout leads to a systemBandwidth value of 0, and aborts are not factored into the bandwidth value, I'll remove this check.

[phloxic]

@gesinger - in case you were wondering: I still have to leave experimentalBufferBasedABR turned off ;-)

[phloxic]

@gesinger - ah, it seems that a fix is in v8. - Any chance of it being backported to v7?