feat: update ABR algorithm to use moving average and switching conditionally depending on the buffer level #886

gkatsev · 2020-06-29T01:34:05Z

We want to update our ABR algorithm to be more reliable. We already have a moving average bandwidth estimator, but we weren't using it, so, we're switching to use it with a value of 0.55 for the decay factor.
For the bitrate adaptation, we want to start switching conditionally depending on the buffer level. If the buffer level is high, and we're switching down, we want to stick with our current rendition. We also want to stick with the current rendition if we are switching up and the buffer level is low.

gkatsev · 2020-06-29T01:47:48Z

What I thought would be an easy change is making things a lot trickier. First, I saw the playback watcher kick in a couple of times for "excessive downloading" (#884).
Second, it seems like we don't try to switch every segment load? There were definitely a couple of times when it seemed like shouldSwitchMedia was called after several segments were downloaded rather than for every segment.
Finally, when testing down switching, I was experimenting with restricting the bandwidth via the chrome dev tools and it seems like we would miss a down switch opportunity and then run out of time and the early abort would kick in because it could no longer get the segment in time and it would switch down a lot more aggressively than if it caught the previous down switch opportunity. This may be related to the second item.

Tests likely don't work due to the changes in the default bandwidth estimation and bitrate adaptation algorithms.

gkatsev · 2020-07-30T18:23:30Z

I think that we need to make this behind a switch, maybe we have a separate shouldSwitchToMedia function that gets called depending on the switch?

brandonocasey · 2020-08-17T19:45:11Z

index.html

@@ -73,6 +73,10 @@ <h3>Options</h3>
      <input id=partial type="checkbox">
      Handle Partial (reloads player)
    </label>
+    <label>
+      <input id=buffer-water type="checkbox">


Adds an option to our debug page to use the new playlist selection logic.

brandonocasey · 2020-08-17T19:45:45Z

scripts/index-demo-page.js

-      window.videojs.options.vhs = window.videojs.options.vhs || {};
-      window.videojs.options.vhs.handlePartialData = event.target.checked;
+      // reload the player and scripts
+      stateEls.minified.dispatchEvent(newEvent('change'));


I tried to use this code for the buffer water line option and noticed that it didn't really work in all cases. So I updated them both.

brandonocasey · 2020-08-17T19:46:03Z

scripts/index-demo-page.js

@@ -329,7 +331,7 @@
          sources.dispatchEvent(newEvent('change'));
        }
        player.on('loadedmetadata', function() {
-          if (player.vhs) {
+          if (player.tech_.vhs) {


This was causing a deprecation warning.

brandonocasey · 2020-08-17T19:47:16Z

src/config.js

-  BUFFER_LOW_WATER_LINE_RATE: 1
+
+  // TODO: Remove this when useBufferWaterLines is removed
+  MAX_BUFFER_LOW_WATER_LINE_NEW: 16,


Not sure what we should call this, but in order to remain backwards compatible we needed a new config value or a hard coded value in shouldSwitchToMedia_. Perhaps a better name? I didn't really want to make it that much longer though as the name is already unwieldy.

I think it's probably good enough.

brandonocasey · 2020-08-17T19:47:41Z

src/master-playlist-controller.js

@@ -58,31 +60,61 @@ const shouldSwitchToMedia = function({
    return false;
  }

+  const sharedLogLine = `allowing switch ${currentPlaylist && currentPlaylist.id || 'null'} -> ${nextPlaylist.id}`;


Added tons of logging throughout this function so that we can track what is going on by turning debug logging on.

brandonocasey · 2020-08-17T19:49:14Z

src/master-playlist-controller.js

@@ -318,7 +352,12 @@ export class MasterPlaylistController extends videojs.EventTarget {
          selectedMedia = this.selectPlaylist();
        }

+        if (!selectedMedia || !this.shouldSwitchToMedia_(selectedMedia)) {


This is the one of the changes that isn't behind a feature switch, but I think it is something that we should have been doing. This prevents us from switching aborting a bunch of things and then re-requesting the same playlist and requesting them again.

This makes sense, we probably shouldn't ever load a separate playlist automatically without going through shouldSwitchToMedia_

brandonocasey · 2020-08-17T19:50:00Z

src/master-playlist-controller.js

@@ -478,6 +517,27 @@ export class MasterPlaylistController extends videojs.EventTarget {
      this.tech_.trigger({type: 'usage', name: 'hls-playlist-cue-tags'});
    }
  }
+
+  shouldSwitchToMedia_(nextPlaylist) {
+    const currentPlaylist = this.masterPlaylistLoader_.media();


We were calling this function the same way everywhere except for nextPlaylist so I just added it to the mpc object.

brandonocasey · 2020-08-17T19:50:55Z

src/master-playlist-controller.js

    this.mainSegmentLoader_.on('progress', () => {
+      if (this.bufferWaterLineSelector) {


Sometimes, especially with low bandwidth, or when bandwidth gets slower, we do not switch fast enough by waiting for an entire segment to download.

I debated using this on without the flag, but I think we want to keep the current playlist switching intact wherever possible, so I did not do it.

brandonocasey · 2020-08-17T19:56:07Z

src/master-playlist-controller.js

@@ -542,7 +598,21 @@ export class MasterPlaylistController extends videojs.EventTarget {
      this.onEndOfStream();
    });

-    this.mainSegmentLoader_.on('earlyabort', () => {
+    this.mainSegmentLoader_.on('earlyabort', (event) => {
+      if (this.bufferWaterLineSelector) {


In segmentLoader we used to abort the download during an earlyabort, but that was problematic because with the new switching algorithm at certain buffer values we do not want to force a switch. Ultimately I think that we should move most of the logic for what triggers earlyabort in here, but doing that in a backwards compatible way wasn't going to be possible without drastic effort.

So, now we exclude the current playlist, but we may not switch away from it? Should we only exclude it in this case if we do end up switching away from it?

We only temporarily exclude it below so that it isn't selected as the nextPlaylist in selectPlaylist. Then we remove the exclusion and make sure that we should switch to the next playlist before actually blacklisting it and switching. Somewhat confusing, I will add some comments here.

Oh, I totally missed that. That makes sense. Comments would definitely be helpful.

Also of note is that we always blacklist (which was the previous behavior) when not using the bufferWaterLineSelector option.

Yup, that's excpted, thanks.

brandonocasey · 2020-08-17T19:57:09Z

src/segment-loader.js

@@ -1326,22 +1326,22 @@ export default class SegmentLoader extends videojs.EventTarget {
   * @return {boolean} True if the request was aborted, false otherwise
   * @private
   */
-  abortRequestEarly_(stats) {
+  earlyAbortWhenNeeded_(stats) {


Since this function no longer aborts, the name needed a slight change. Now we only trigger earlyabort whenever we think that an earlyabort should happen and let master playlist controller handle a switch if it thinks that it should happen.

brandonocasey · 2020-08-17T19:58:54Z

test/loader-common.js

@@ -340,7 +340,7 @@ export const LoaderCommonFactory = ({
    });

    QUnit.test(
-      'aborts request at progress events if bandwidth is too low',
+      'triggers earlyabort at progress events if bandwidth is too low',


This test changed as earlyabort no longer aborts directly.

brandonocasey · 2020-08-17T19:59:20Z

test/master-playlist-controller.test.js


    this.masterPlaylistController.tech_.currentTime = () => currentTime;
    this.masterPlaylistController.tech_.buffered = () => videojs.createTimeRanges(buffered);
    this.masterPlaylistController.duration = () => duration;
    this.masterPlaylistController.selectPlaylist = () => {
      return {
+        id: id++,


This was needed as we compare playlist ids now.

src/segment-loader.js

gkatsev · 2020-09-10T12:21:45Z

src/segment-loader.js

    }

    // set the bandwidth to that of the desired playlist being sure to scale by
    // BANDWIDTH_VARIANCE and add one so the playlist selector does not exclude it
    // don't trigger a bandwidthupdate as the bandwidth is artifial
    this.bandwidth =
      switchCandidate.playlist.attributes.BANDWIDTH * Config.BANDWIDTH_VARIANCE + 1;
-    this.abort();


where does the abort happen now?

in the earlyabort handler in master-playlist-controller

I guess I don't see it? Unless I'm not understanding things. In MPC only call to segment loader's abort is inside mediachanging and setCurrentTime. Unless mediachanging ends up getting called as part of that and thus we don't need to call it explicitly here?

abort is called when we blacklist, so in the earlyabort handler we call blacklistCurrentPlaylist which will call abort and load a new playlist.

gkatsev · 2020-09-10T12:48:22Z

src/master-playlist-controller.js

+
+  // when switching down, if our buffer is lower than the high water line,
+  // we can switch down
+  if (nextBandwidth < currBandwidth && (!bufferWaterLineSelector || forwardBuffer < bufferHighWaterLine)) {


cool, this works surprisingly well and allows us not to need duplicate this method outright. Though, duplicating the method makes it exceptionally clear what the old and what the new thing is and makes it easy to remove it. I think with the comments and the earlier commit in this PR we can figure it out.

gkatsev · 2020-09-10T12:51:07Z

src/master-playlist-controller.js

@@ -318,7 +352,12 @@ export class MasterPlaylistController extends videojs.EventTarget {
          selectedMedia = this.selectPlaylist();
        }

+        if (!selectedMedia || !this.shouldSwitchToMedia_(selectedMedia)) {


This makes sense, we probably shouldn't ever load a separate playlist automatically without going through shouldSwitchToMedia_

gkatsev · 2020-09-10T13:02:12Z

src/master-playlist-controller.js

@@ -542,7 +598,21 @@ export class MasterPlaylistController extends videojs.EventTarget {
      this.onEndOfStream();
    });

-    this.mainSegmentLoader_.on('earlyabort', () => {
+    this.mainSegmentLoader_.on('earlyabort', (event) => {
+      if (this.bufferWaterLineSelector) {


So, now we exclude the current playlist, but we may not switch away from it? Should we only exclude it in this case if we do end up switching away from it?

gkatsev · 2020-09-10T13:04:15Z

src/config.js

-  BUFFER_LOW_WATER_LINE_RATE: 1
+
+  // TODO: Remove this when useBufferWaterLines is removed
+  MAX_BUFFER_LOW_WATER_LINE_NEW: 16,


I think it's probably good enough.

src/master-playlist-controller.js

…ovingAverage by default with decay of 0.55

If we are switching down, we want to stay with the current rendition if we have lots of buffer left. If we are switching up, we want to stay with the current rendition if there isn't a lot of buffer.

Co-authored-by: Gary Katsevman <git@gkatsev.com>

gkatsev · 2020-09-29T21:34:47Z

Before we merge, I think we should change the option from bufferWaterLineSelector to something that begins with experimental. Can even be experimentalBufferWaterLineSelector.
~~I have a PR incoming to here that adds a checkbox for it in the example page.~~ I see that already exists, in that case just renaming to stress it's experimental, just so that if we fiddle with things behind that flag people can't complain.

…LOW_WATER_LINE

… on an interval (#978) This is a followup to #886. There, I noticed that if we get sustained bad network we will time out because we will an opportunity for earlyabort. At first, I tried working around by delaying earlyabort in that case (#966) but as I was getting that ready I kept finding edge cases that we needed to account for. Specifically, the issue is that we only run our ABR algorithm on progress and bandwidthchange events from the segment loader. This means that if the download stalls, we stop run our algorithm and will eventually stall playback. Instead, we should remove earlyabort altogether (f897dde) and set up an interval (currently 250ms) to re-run our ABR algorithm. This means that if the network becomes bad for a sustained period, we will drop down once the buffer allows us but if the network recovers, we will switch up appropriately as well. Also, the interval is stopped while we're paused. Fixes #964.

brandonocasey force-pushed the abr-updates branch 2 times, most recently from 4d28c98 to 61cc0a5 Compare August 3, 2020 19:07

brandonocasey force-pushed the abr-updates branch from 00a8ee5 to 549713b Compare August 17, 2020 19:38

brandonocasey changed the title ~~[WIP] update ABR algorithm to use moving averge and switching conditionally depending on the buffer level~~ feat: update ABR algorithm to use moving average and switching conditionally depending on the buffer level Aug 17, 2020

brandonocasey reviewed Aug 17, 2020

View reviewed changes

gkatsev commented Sep 10, 2020

View reviewed changes

gkatsev added the needs: review label Sep 10, 2020

gkatsev added this to the ABR milestone Sep 10, 2020

gkatsev commented Sep 10, 2020

View reviewed changes

src/master-playlist-controller.js Outdated Show resolved Hide resolved

gkatsev and others added 8 commits September 29, 2020 16:21

expose lastBandwidth and movingAverageBandwidthSelector on Vhs, use m…

0426b20

…ovingAverage by default with decay of 0.55

Add high water line, update adaptation

4432850

If we are switching down, we want to stay with the current rendition if we have lots of buffer left. If we are switching up, we want to stay with the current rendition if there isn't a lot of buffer.

properly use the moving average bandwidth selector

596dfce

Put the new abr changes behind a flag

8584cbf

add shouldSwitchToMedia to mpc and use it

9614adf

fix tests, put all changes behind the option, shouldswitch earlyabort.

198594a

consistent option name across the board

99b6f37

remove unused code

04db536

brandonocasey and others added 4 commits September 29, 2020 16:22

add tests

4e0f4f6

do not trigger earlyabort with expected playlist

767c6ba

comments and move log line

08658dd

blacklisted -> excluded

db9fdc3

Co-authored-by: Gary Katsevman <git@gkatsev.com>

brandonocasey force-pushed the abr-updates branch from cb2ed9f to db9fdc3 Compare September 29, 2020 20:22

gkatsev and others added 4 commits September 30, 2020 11:43

use and expose the new max buffer low water line (#963)

77c996c

set temp exclusion to Infinity so we do not select the same one

72e846e

switch bufferWaterLineSelector to experimentalBufferBasedABR

329c26f

switch from MAX_BUFFER_LOW_WATER_LINE_NEW to EXPERIMENTAL_MAX_BUFFER_…

6715ff9

…LOW_WATER_LINE

gkatsev merged commit a05d032 into main Sep 30, 2020

gkatsev deleted the abr-updates branch September 30, 2020 23:01

gkatsev mentioned this pull request Oct 23, 2020

fix(experimentalBufferBasedABR): call selectPlaylist and change media on an interval #978

Merged

ziddey mentioned this pull request Apr 5, 2021

auto quality is switching renditions even with good internet quality #1112

Closed

gkatsev removed the needs: review label Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: update ABR algorithm to use moving average and switching conditionally depending on the buffer level #886

feat: update ABR algorithm to use moving average and switching conditionally depending on the buffer level #886

gkatsev commented Jun 29, 2020

gkatsev commented Jun 29, 2020

gkatsev commented Jul 30, 2020

brandonocasey Aug 17, 2020

brandonocasey Aug 17, 2020

brandonocasey Aug 17, 2020

brandonocasey Aug 17, 2020

gkatsev Sep 10, 2020

brandonocasey Aug 17, 2020

brandonocasey Aug 17, 2020 •

edited

Loading

gkatsev Sep 10, 2020

brandonocasey Aug 17, 2020

brandonocasey Aug 17, 2020

brandonocasey Aug 17, 2020

brandonocasey Aug 17, 2020

gkatsev Sep 10, 2020

brandonocasey Sep 10, 2020

gkatsev Sep 10, 2020

brandonocasey Sep 10, 2020

gkatsev Sep 10, 2020

brandonocasey Aug 17, 2020

brandonocasey Aug 17, 2020

brandonocasey Aug 17, 2020

gkatsev Sep 10, 2020

brandonocasey Sep 10, 2020

gkatsev Sep 10, 2020

brandonocasey Sep 21, 2020

gkatsev Sep 10, 2020

gkatsev Sep 10, 2020

gkatsev Sep 10, 2020

gkatsev Sep 10, 2020

gkatsev commented Sep 29, 2020 •

edited

Loading

		this.mainSegmentLoader_.on('progress', () => {
		if (this.bufferWaterLineSelector) {

feat: update ABR algorithm to use moving average and switching conditionally depending on the buffer level #886

feat: update ABR algorithm to use moving average and switching conditionally depending on the buffer level #886

Conversation

gkatsev commented Jun 29, 2020

gkatsev commented Jun 29, 2020

gkatsev commented Jul 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandonocasey Aug 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkatsev commented Sep 29, 2020 • edited Loading

brandonocasey Aug 17, 2020 •

edited

Loading

gkatsev commented Sep 29, 2020 •

edited

Loading