fix: trim 30s back from playhead even for VOD and LIVE DVR content (#743)

Previously, if the seekable window for the content started at 0
(generally denoting a VOD or LIVE DVR playlist), the back buffer would
not be trimmed. With high bitrate and/or longer content, this could
lead to an APPEND_BUFFER_ERR due to the buffer exceeding the max
allowed buffered bytes.

In order to help alleviate this issue, particularly with the increase in
high bitrate content, the back buffer is now trimmed to a reasonable
length, even if the seekable window starts at 0.

src/segment-loader.js

@@ -67,22 +67,24 @@ export const illegalMediaSwitch = (loaderType, startingMedia, trackInfo) => {
  *         Time that is safe to remove from the back buffer without interupting playback
  */
 export const safeBackBufferTrimTime = (seekable, currentTime, targetDuration) => {
-  let removeToTime;
-
-  if (seekable.length &&
-      seekable.start(0) > 0 &&
-      seekable.start(0) < currentTime) {
-    // If we have a seekable range use that as the limit for what can be removed safely
-    removeToTime = seekable.start(0);
-  } else {
-    // otherwise remove anything older than 30 seconds before the current play head
-    removeToTime = currentTime - 30;
-  }
-
-  // Don't allow removing from the buffer within target duration of current time
-  // to avoid the possibility of removing the GOP currently being played which could
-  // cause playback stalls.
-  return Math.min(removeToTime, currentTime - targetDuration);
+  // 30 seconds before the playhead provides a safe default for trimming.
+  //
+  // Choosing a reasonable default is particularly important for high bitrate content and
+  // VOD videos/live streams with large windows, as the buffer may end up overfilled and
+  // throw an APPEND_BUFFER_ERR.
+  let trimTime = currentTime - 30;
+
+  if (seekable.length) {
+    // Some live playlists may have a shorter window of content than the full allowed back
+    // buffer. For these playlists, don't save content that's no longer within the window.
+    trimTime = Math.max(trimTime, seekable.start(0));
+  }
+
+  // Don't remove within target duration of the current time to avoid the possibility of
+  // removing the GOP currently being played, as removing it can cause playback stalls.
+  const maxTrimTime = currentTime - targetDuration;
+
+  return Math.min(maxTrimTime, trimTime);
 };
 
 const segmentInfoString = (segmentInfo) => {

test/segment-loader.test.js

@@ -149,34 +149,88 @@ QUnit.test('illegalMediaSwitch detects illegal media switches', function(assert)
   );
 });
 
-QUnit.test(
-  'safeBackBufferTrimTime determines correct safe removeToTime',
-  function(assert) {
-    const seekable = videojs.createTimeRanges([[75, 120]]);
-    const targetDuration = 10;
-    let currentTime = 70;
-
-    assert.equal(
-      safeBackBufferTrimTime(seekable, currentTime, targetDuration), 40,
-      'uses 30s before current time if currentTime is before seekable start'
-    );
-
-    currentTime = 110;
-
-    assert.equal(
-      safeBackBufferTrimTime(seekable, currentTime, targetDuration), 75,
-      'uses seekable start if currentTime is after seekable start'
-    );
-
-    currentTime = 80;
-
-    assert.equal(
-      safeBackBufferTrimTime(seekable, currentTime, targetDuration), 70,
-      'uses target duration before currentTime if currentTime is after seekable but' +
-    'within target duration'
-    );
-  }
-);
+QUnit.module('safeBackBufferTrimTime');
+
+QUnit.test('uses 30s before playhead when seekable start is 0', function(assert) {
+  const seekable = videojs.createTimeRanges([[0, 120]]);
+  const targetDuration = 10;
+  const currentTime = 70;
+
+  assert.equal(
+    safeBackBufferTrimTime(seekable, currentTime, targetDuration),
+    40,
+    'returned 30 seconds before playhead'
+  );
+});
+
+QUnit.test('uses 30s before playhead when seekable start is earlier', function(assert) {
+  const seekable = videojs.createTimeRanges([[30, 120]]);
+  const targetDuration = 10;
+  const currentTime = 70;
+
+  assert.equal(
+    safeBackBufferTrimTime(seekable, currentTime, targetDuration),
+    40,
+    'returned 30 seconds before playhead'
+  );
+});
+
+QUnit.test('uses seekable start when within 30s of playhead', function(assert) {
+  const seekable = videojs.createTimeRanges([[41, 120]]);
+  const targetDuration = 10;
+  const currentTime = 70;
+
+  assert.equal(
+    safeBackBufferTrimTime(seekable, currentTime, targetDuration),
+    41,
+    'returned 29 seconds before playhead'
+  );
+});
+
+QUnit.test('uses target duration when seekable range is within target duration', function(assert) {
+  let seekable = videojs.createTimeRanges([[0, 120]]);
+  const targetDuration = 10;
+  let currentTime = 9;
+
+  assert.equal(
+    safeBackBufferTrimTime(seekable, currentTime, targetDuration),
+    -1,
+    'returned 10 seconds before playhead'
+  );
+
+  seekable = videojs.createTimeRanges([[40, 120]]);
+  currentTime = 41;
+
+  assert.equal(
+    safeBackBufferTrimTime(seekable, currentTime, targetDuration),
+    31,
+    'returned 10 seconds before playhead'
+  );
+});
+
+QUnit.test('uses target duration when seekable range is after current time', function(assert) {
+  const seekable = videojs.createTimeRanges([[110, 120]]);
+  const targetDuration = 10;
+  const currentTime = 80;
+
+  assert.equal(
+    safeBackBufferTrimTime(seekable, currentTime, targetDuration),
+    70,
+    'returned 10 seconds before playhead'
+  );
+});
+
+QUnit.test('uses current time when seekable range is well before current time', function(assert) {
+  const seekable = videojs.createTimeRanges([[10, 20]]);
+  const targetDuration = 10;
+  const currentTime = 140;
+
+  assert.equal(
+    safeBackBufferTrimTime(seekable, currentTime, targetDuration),
+    110,
+    'returned 30 seconds before playhead'
+  );
+});
 
 QUnit.module('SegmentLoader', function(hooks) {
   hooks.beforeEach(LoaderCommonHooks.beforeEach);

test/videojs-http-streaming.test.js

@@ -3308,8 +3308,8 @@ QUnit.test('cleans up the buffer when loading live segments', function(assert) {
   });
 });
 
-QUnit.test('cleans up the buffer based on currentTime when loading a live segment ' +
-           'if seekable start is after currentTime', function(assert) {
+QUnit.test('cleans up buffer by removing targetDuration from currentTime when loading a ' +
+           'live segment if seekable start is after currentTime', function(assert) {
   let seekable = videojs.createTimeRanges([[0, 80]]);
 
   this.player.src({
@@ -3350,7 +3350,7 @@ QUnit.test('cleans up the buffer based on currentTime when loading a live segmen
 
     // Change seekable so that it starts *after* the currentTime which was set
     // based on the previous seekable range (the end of 80)
-    seekable = videojs.createTimeRanges([[100, 120]]);
+    seekable = videojs.createTimeRanges([[110, 120]]);
     this.clock.tick(1);
 
     const audioBuffer = mpc.sourceUpdater_.audioBuffer;
@@ -3388,12 +3388,12 @@ QUnit.test('cleans up the buffer based on currentTime when loading a live segmen
     // segment-loader removes at currentTime - 30
     assert.deepEqual(
       audioRemoves[0],
-      { start: 0, end: 80 - 30 },
+      { start: 0, end: 80 - 10 },
       'removed from audio buffer with right range'
     );
     assert.deepEqual(
       videoRemoves[0],
-      { start: 0, end: 80 - 30 },
+      { start: 0, end: 80 - 10 },
       'removed from video buffer with right range'
     );
   });