Fix time correction (#706)

* Fix how and when timeCorrection_ was being applied so that it works for Flash * Always apply timeCorrection_ when we haven't learned anything even if playlists have changed * Apply timeCorrection_ directly to the currentTime when calling getMediaIndexForTime so that we can work around bad segment.end metadata in the playlist * Emit an error if we have been stuck in a "timeCorrection" loop for more than 5 fetch attempts * Make sure to clear the timeout before setting another in monitorBuffer_ * Clearer naming of variables * Use timeCorrection_ when a chosen segment is more than 90% buffered * Break out timeCorrection_ functionality into a reusable piece * Added tests of the new timeCorrection_ functionality * Test that timeCorrection is applied even when the segment.end data is misleading * Verify that an error is emitted when we have failed to make progress after 5 time-corrections * Ensure that we only have 1 timer if monitorBuffer_ is called multiple times * Moved percent-buffered check out of checkBuffer_ to fillBuffer_

Fix time correction (#706)
* Fix how and when timeCorrection_ was being applied so that it works for Flash * Always apply timeCorrection_ when we haven't learned anything even if playlists have changed * Apply timeCorrection_ directly to the currentTime when calling getMediaIndexForTime so that we can work around bad segment.end metadata in the playlist * Emit an error if we have been stuck in a "timeCorrection" loop for more than 5 fetch attempts * Make sure to clear the timeout before setting another in monitorBuffer_ * Clearer naming of variables * Use timeCorrection_ when a chosen segment is more than 90% buffered * Break out timeCorrection_ functionality into a reusable piece * Added tests of the new timeCorrection_ functionality * Test that timeCorrection is applied even when the segment.end data is misleading * Verify that an error is emitted when we have failed to make progress after 5 time-corrections * Ensure that we only have 1 timer if monitorBuffer_ is called multiple times * Moved percent-buffered check out of checkBuffer_ to fillBuffer_
Jon-Carlos Rivera
Showing 2 changed files with 176 additions and 61 deletions
src/segment-loader.js
test/segment-loader.test.js
--- a/src/segment-loader.js
View file @8da2b07
+++ b/src/segment-loader.js
View file @8da2b07
@@ -303,6 +303,11 @@ export default class SegmentLoader extends videojs.EventTarget {
    if (this.state === 'READY') {
      this.fillBuffer_();
    }
+    if (this.checkBufferTimeout_) {
+      window.clearTimeout(this.checkBufferTimeout_);
+    }
    this.checkBufferTimeout_ = window.setTimeout(this.monitorBuffer_.bind(this),
                                                 CHECK_BUFFER_DELAY);
  }
@@ -364,8 +369,8 @@ export default class SegmentLoader extends videojs.EventTarget {
    if (currentBuffered.length === 0) {
      // find the segment containing currentTime
      mediaIndex = getMediaIndexForTime(playlist,
-                                        currentTime,
+                                        currentTime + this.timeCorrection_,
-                                        this.expired_ + this.timeCorrection_);
+                                        this.expired_);
    } else {
      // find the segment adjacent to the end of the current
      // buffered region
@@ -384,42 +389,14 @@ export default class SegmentLoader extends videojs.EventTarget {
        return null;
      }
      mediaIndex = getMediaIndexForTime(playlist,
-                                        currentBufferedEnd,
+                                        currentBufferedEnd + this.timeCorrection_,
-                                        this.expired_ + this.timeCorrection_);
+                                        this.expired_);
    }
    if (mediaIndex < 0 || mediaIndex === playlist.segments.length) {
      return null;
    }
-    // Sanity check the segment-index determining logic above but calcuating
-    // the percentage of the chosen segment that is buffered. If more than 90%
-    // of the segment is buffered then fetching it will likely not help in any
-    // way
-    let percentBuffered = this.getSegmentBufferedPercent_(playlist,
-                                                          mediaIndex,
-                                                          currentTime,
-                                                          buffered);
-    if (percentBuffered >= 90) {
-      // Retry the buffered calculation with the next segment if there is another
-      // segment after the currently selected segment
-      if (mediaIndex + 1 < playlist.segments.length) {
-        percentBuffered = this.getSegmentBufferedPercent_(playlist,
-                                                          mediaIndex + 1,
-                                                          currentTime,
-                                                          buffered);
-      }
-      // If both checks failed return and don't load anything
-      if (percentBuffered >= 90) {
-        return;
-      }
-      // Otherwise, continue with the next segment
-      mediaIndex += 1;
-    }
    segment = playlist.segments[mediaIndex];
    let startOfSegment = duration(playlist,
                                  playlist.mediaSequence + mediaIndex,
@@ -490,9 +467,33 @@ export default class SegmentLoader extends videojs.EventTarget {
                                    this.currentTime_(),
                                    this.timestampOffset_);
-    if (request) {
+    if (!request) {
-      this.loadSegment_(request);
+      return;
    }
+    // Sanity check the segment-index determining logic by calcuating the
+    // percentage of the chosen segment that is buffered. If more than 90%
+    // of the segment is buffered then fetching it will likely not help in
+    // any way
+    let percentBuffered = this.getSegmentBufferedPercent_(this.playlist_,
+                                                          request.mediaIndex,
+                                                          this.currentTime_(),
+                                                          this.sourceUpdater_.buffered());
+    if (percentBuffered >= 90) {
+      // Increment the timeCorrection_ variable to push the fetcher forward
+      // in time and hopefully skip any gaps or flaws in our understanding
+      // of the media
+      this.incrementTimeCorrection_(this.playlist_.targetDuration);
+      if (!this.paused()) {
+        this.fillBuffer_();
+      }
+      return;
+    }
+    this.loadSegment_(request);
  }
  /**
@@ -757,12 +758,11 @@ export default class SegmentLoader extends videojs.EventTarget {
    let currentTime = this.currentTime_();
    this.pendingSegment_ = null;
-    // add segment timeline information if we're still using the
-    // same playlist
+    // add segment metadata if it we have gained information during the
-    if (segmentInfo && segmentInfo.playlist.uri === this.playlist_.uri) {
+    // last append
-      this.updateTimeline_(segmentInfo);
+    this.updateTimeline_(segmentInfo);
-      this.trigger('progress');
+    this.trigger('progress');
-    }
    let currentMediaIndex = segmentInfo.mediaIndex;
@@ -819,24 +819,26 @@ export default class SegmentLoader extends videojs.EventTarget {
   */
  updateTimeline_(segmentInfo) {
    let segment;
-    let timelineUpdate;
+    let segmentEnd;
+    let timelineUpdated;
+    let segmentLength = this.playlist_.targetDuration;
    let playlist = segmentInfo.playlist;
    let currentMediaIndex = segmentInfo.mediaIndex;
    currentMediaIndex += playlist.mediaSequence - this.playlist_.mediaSequence;
    segment = playlist.segments[currentMediaIndex];
-    if (!segment) {
-      return;
-    }
-    timelineUpdate = Ranges.findSoleUncommonTimeRangesEnd(segmentInfo.buffered,
-                                                          this.sourceUpdater_.buffered());
    // Update segment meta-data (duration and end-point) based on timeline
-    let timelineUpdated = updateSegmentMetadata(playlist,
+    if (segment &&
-                                                currentMediaIndex,
+        segmentInfo &&
-                                                timelineUpdate);
+        segmentInfo.playlist.uri === this.playlist_.uri) {
+      segmentEnd = Ranges.findSoleUncommonTimeRangesEnd(segmentInfo.buffered,
+                                                        this.sourceUpdater_.buffered());
+      timelineUpdated = updateSegmentMetadata(playlist,
+                                              currentMediaIndex,
+                                              segmentEnd);
+      segmentLength = segment.duration;
+    }
    // the last segment append must have been entirely in the
    // already buffered time ranges. adjust the timeCorrection
@@ -844,9 +846,33 @@ export default class SegmentLoader extends videojs.EventTarget {
    // to the buffered time ranges and improves subsequent media
    // index calculations.
    if (!timelineUpdated) {
-      this.timeCorrection_ -= segment.duration;
+      this.incrementTimeCorrection_(segmentLength);
    } else {
      this.timeCorrection_ = 0;
    }
  }
+  /**
+   * add a number of seconds to the currentTime when determining which
+   * segment to fetch in order to force the fetcher to advance in cases
+   * where it may get stuck on the same segment due to buffer gaps or
+   * missing segment annotation after a rendition switch (especially
+   * during a live stream)
+   *
+   * @private
+   * @param {Number} secondsToIncrement number of seconds to add to the
+   * timeCorrection_ variable
+   */
+  incrementTimeCorrection_(secondsToIncrement) {
+    // If we have already incremented timeCorrection_ beyond the limit,
+    // then stop trying to find a segment, pause fetching, and emit an
+    // error event
+    if (this.timeCorrection_ >= this.playlist_.targetDuration * 5) {
+      this.timeCorrection_ = 0;
+      this.pause();
+      return this.trigger('error');
+    }
+    this.timeCorrection_ += secondsToIncrement;
+  }
 }
--- a/test/segment-loader.test.js
View file @8da2b07
+++ b/test/segment-loader.test.js
View file @8da2b07
@@ -6,6 +6,7 @@ import { useFakeEnvironment, useFakeMediaSource } from './test-helpers.js';
 const playlistWithDuration = function(time, conf) {
  let result = {
+    targetDuration: 10,
    mediaSequence: conf && conf.mediaSequence ? conf.mediaSequence : 0,
    discontinuityStarts: [],
    segments: [],
@@ -350,14 +351,7 @@ QUnit.test('never attempt to load a segment that ' +
  sourceBuffer.buffered = videojs.createTimeRanges([[0, 9.2]]);
  sourceBuffer.trigger('updateend');
-  // the next segment doesn't increase the buffer at all
+  // the loader should move on to the next segment
-  QUnit.equal(this.requests[0].url, '1.ts', 'requested the next segment');
-  this.clock.tick(1);
-  this.requests[0].response = new Uint8Array(10).buffer;
-  this.requests.shift().respond(200, null, '');
-  sourceBuffer.trigger('updateend');
-  // so the loader should try the next segment
  QUnit.equal(this.requests[0].url, '1.ts', 'moved ahead a segment');
 });
@@ -398,6 +392,79 @@ QUnit.test('adjusts the playlist offset if no buffering progress is made', funct
  QUnit.equal(this.requests[0].url, '1.ts', 'moved ahead a segment');
 });
+QUnit.test('adjusts the playlist offset even when segment.end is set if no' +
+           ' buffering progress is made', function() {
+  let sourceBuffer;
+  let playlist;
+  playlist = playlistWithDuration(40);
+  playlist.endList = false;
+  loader.playlist(playlist);
+  loader.mimeType(this.mimeType);
+  loader.load();
+  sourceBuffer = mediaSource.sourceBuffers[0];
+  // buffer some content and switch playlists on progress
+  this.clock.tick(1);
+  this.requests[0].response = new Uint8Array(10).buffer;
+  this.requests.shift().respond(200, null, '');
+  sourceBuffer.buffered = videojs.createTimeRanges([[0, 5]]);
+  loader.one('progress', function f() {
+    QUnit.equal(playlist.segments[0].end, 5, 'segment.end was set based on the buffer');
+    playlist.segments[0].end = 10;
+  });
+  sourceBuffer.trigger('updateend');
+  // the next segment doesn't increase the buffer at all
+  QUnit.equal(this.requests[0].url, '0.ts', 'requested the same segment');
+  this.clock.tick(1);
+  this.requests[0].response = new Uint8Array(10).buffer;
+  this.requests.shift().respond(200, null, '');
+  sourceBuffer.trigger('updateend');
+  // so the loader should try the next segment
+  QUnit.equal(this.requests[0].url, '1.ts', 'moved ahead a segment');
+});
+QUnit.test('adjusts the playlist offset if no buffering progress is made after' +
+           ' five consecutive attempts', function() {
+  let sourceBuffer;
+  let playlist;
+  let errors = 0;
+  loader.on('error', () => {
+    errors++;
+  });
+  playlist = playlistWithDuration(120);
+  playlist.endList = false;
+  loader.playlist(playlist);
+  loader.mimeType(this.mimeType);
+  loader.load();
+  sourceBuffer = mediaSource.sourceBuffers[0];
+  // buffer some content
+  this.clock.tick(1);
+  this.requests[0].response = new Uint8Array(10).buffer;
+  this.requests.shift().respond(200, null, '');
+  sourceBuffer.buffered = videojs.createTimeRanges([[0, 10]]);
+  sourceBuffer.trigger('updateend');
+  for (let i = 1; i <= 6; i++) {
+    // the next segment doesn't increase the buffer at all
+    QUnit.equal(this.requests[0].url, (i + '.ts'), 'requested the next segment');
+    this.clock.tick(1);
+    this.requests[0].response = new Uint8Array(10).buffer;
+    this.requests.shift().respond(200, null, '');
+    sourceBuffer.trigger('updateend');
+  }
+  // so the loader should try the next segment
+  QUnit.equal(errors, 1, 'emitted error');
+  QUnit.ok(loader.paused(), 'loader is paused');
+});
 QUnit.test('cancels outstanding requests on abort', function() {
  loader.playlist(playlistWithDuration(20));
  loader.mimeType(this.mimeType);
@@ -855,6 +922,7 @@ QUnit.module('Segment Loading Calculation', {
    this.env = useFakeEnvironment();
    this.mse = useFakeMediaSource();
    this.hasPlayed = true;
+    this.clock = this.env.clock;
    currentTime = 0;
    loader = new SegmentLoader({
@@ -911,11 +979,14 @@ QUnit.test('does not download the next segment if the buffer is full', function(
 QUnit.test('downloads the next segment if the buffer is getting low', function() {
  let buffered;
  let segmentInfo;
+  let playlist = playlistWithDuration(30);
  loader.mimeType(this.mimeType);
+  loader.playlist(playlist);
+  playlist.segments[1].end = 19.999;
  buffered = videojs.createTimeRanges([[0, 19.999]]);
-  segmentInfo = loader.checkBuffer_(buffered, playlistWithDuration(30), 15);
+  segmentInfo = loader.checkBuffer_(buffered, playlist, 15);
  QUnit.ok(segmentInfo, 'made a request');
  QUnit.equal(segmentInfo.uri, '2.ts', 'requested the third segment');
@@ -1001,3 +1072,21 @@ QUnit.test('adjusts calculations based on expired time', function() {
  QUnit.ok(segmentInfo, 'fetched a segment');
  QUnit.equal(segmentInfo.uri, '2.ts', 'accounted for expired time');
 });
+QUnit.test('doesn\'t allow more than one monitor buffer timer to be set', function() {
+  let timeoutCount = this.clock.methods.length;
+  loader.mimeType(this.mimeType);
+  loader.monitorBuffer_();
+  QUnit.equal(this.clock.methods.length, timeoutCount, 'timeout count remains the same');
+  loader.monitorBuffer_();
+  QUnit.equal(this.clock.methods.length, timeoutCount, 'timeout count remains the same');
+  loader.monitorBuffer_();
+  loader.monitorBuffer_();
+  QUnit.equal(this.clock.methods.length, timeoutCount, 'timeout count remains the same');
+});