Fix time correction.. some more (#712)

* Fudge segments that are reported as having a zero-second duration
  * The fetcher logic basically ignores segments with a duration of zero. Give them a tiny duration so that the fetcher will "see" these segments.
* Added tests for and fixed getSegmentBufferedPercent_ calculations
* Now returns the percent buffered of the entire segment duration instead of the "adjusted" duration
* Handles segments reported as having a zero-duration
* Reduced the number of segments that we will attempt to "timeCorrect" when the segment chosen by `checkBuffer_` is already more than 90% buffered to 1
* No longer trigger errors from `timeCorrection_` handling, returning to the previous behavior
* Use the tech's setCurrentTime function in segment loaders
* Moved getSegmentBufferedPercent to `Ranges` module
* Moved correction for zero-duration segments from parse-stream to parser proper

src/m3u8/parser.js

@@ -108,10 +108,17 @@ export default class Parser extends Stream {
                   message: 'defaulting discontinuity sequence to zero'
                 });
               }
-              if (entry.duration >= 0) {
+              if (entry.duration > 0) {
                 currentUri.duration = entry.duration;
               }
 
+              if (entry.duration === 0) {
+                currentUri.duration = 0.01;
+                this.trigger('info', {
+                  message: 'updating zero segment duration to a small value'
+                });
+              }
+
               this.manifest.segments = uris;
             },
             key() {

src/master-playlist-controller.js

@@ -72,7 +72,7 @@ export default class MasterPlaylistController extends videojs.EventTarget {
       withCredentials: this.withCredentials,
       seekable: () => this.seekable(),
       seeking: () => this.tech_.seeking(),
-      setCurrentTime: (a) => this.setCurrentTime(a),
+      setCurrentTime: (a) => this.tech_.setCurrentTime(a),
       hasPlayed: () => this.tech_.played().length !== 0,
       bandwidth
     };

src/ranges.js

@@ -10,6 +10,16 @@ import videojs from 'video.js';
 // Fudge factor to account for TimeRanges rounding
 const TIME_FUDGE_FACTOR = 1 / 30;
 
+/**
+ * Clamps a value to within a range
+ * @param {Number} num - the value to clamp
+ * @param {Number} start - the start of the range to clamp within, inclusive
+ * @param {Number} end - the end of the range to clamp within, inclusive
+ * @return {Number}
+ */
+const clamp = function(num, [start, end]) {
+  return Math.min(Math.max(start, num), end);
+};
 const filterRanges = function(timeRanges, predicate) {
   let results = [];
   let i;
@@ -184,27 +194,81 @@ const bufferIntersection = function(bufferA, bufferB) {
 /**
  * Calculates the percentage of `segmentRange` that overlaps the
  * `buffered` time ranges.
- * @param {TimeRanges} segmentRange - the time range that the segment covers
+ * @param {TimeRanges} segmentRange - the time range that the segment
+ * covers adjusted according to currentTime
+ * @param {TimeRanges} referenceRange - the original time range that the
+ * segment covers
  * @param {TimeRanges} buffered - the currently buffered time ranges
  * @returns {Number} percent of the segment currently buffered
  */
-const calculateBufferedPercent = function(segmentRange, buffered) {
+const calculateBufferedPercent = function(segmentRange, referenceRange, buffered) {
+  let referenceDuration = referenceRange.end(0) - referenceRange.start(0);
   let segmentDuration = segmentRange.end(0) - segmentRange.start(0);
   let intersection = bufferIntersection(segmentRange, buffered);
-  let overlapDuration = 0;
   let count = intersection.length;
 
   while (count--) {
-    overlapDuration += intersection.end(count) - intersection.start(count);
+    segmentDuration -= intersection.end(count) - intersection.start(count);
+  }
+  return (referenceDuration - segmentDuration) / referenceDuration * 100;
+};
+
+/**
+ * Return the amount of a segment specified by the mediaIndex overlaps
+ * the current buffered content.
+ *
+ * @param {Number} startOfSegment - the time where the segment begins
+ * @param {Number} segmentDuration - the duration of the segment in seconds
+ * @param {TimeRanges} buffered - the state of the buffer
+ * @returns {Number} percentage of the segment's time range that is
+ * already in `buffered`
+ */
+const getSegmentBufferedPercent = function(startOfSegment,
+                                           segmentDuration,
+                                           currentTime,
+                                           buffered) {
+  let endOfSegment = startOfSegment + segmentDuration;
+
+  // The entire time range of the segment
+  let originalSegmentRange = videojs.createTimeRanges([[
+    startOfSegment,
+    endOfSegment
+  ]]);
+
+  // The adjusted segment time range that is setup such that it starts
+  // no earlier than currentTime
+  // Flash has no notion of a back-buffer so adjustedSegmentRange adjusts
+  // for that and the function will still return 100% if a only half of a
+  // segment is actually in the buffer as long as the currentTime is also
+  // half-way through the segment
+  let adjustedSegmentRange = videojs.createTimeRanges([[
+    clamp(startOfSegment, [currentTime, endOfSegment]),
+    endOfSegment
+  ]]);
+
+  // This condition happens when the currentTime is beyond the segment's
+  // end time
+  if (adjustedSegmentRange.start(0) === adjustedSegmentRange.end(0)) {
+    return 0;
+  }
+
+  let percent = calculateBufferedPercent(adjustedSegmentRange,
+                                         originalSegmentRange,
+                                         buffered);
+
+  // If the segment is reported as having a zero duration, return 0%
+  // since it is likely that we will need to fetch the segment
+  if (isNaN(percent) || percent === Infinity || percent === -Infinity) {
+    return 0;
   }
 
-  return (overlapDuration / segmentDuration) * 100;
+  return percent;
 };
 
 export default {
   findRange,
   findNextRange,
   findSoleUncommonTimeRangesEnd,
-  calculateBufferedPercent,
+  getSegmentBufferedPercent,
   TIME_FUDGE_FACTOR
 };

src/segment-loader.js

@@ -312,29 +312,6 @@ export default class SegmentLoader extends videojs.EventTarget {
                                                  CHECK_BUFFER_DELAY);
   }
 
-  /**
-   * Return the amount of a segment specified by the mediaIndex overlaps
-   * the current buffered content.
-   *
-   * @param {Object} playlist the playlist object to fetch segments from
-   * @param {Number} mediaIndex the index of the segment in the playlist
-   * @param {TimeRanges} buffered the state of the buffer
-   * @returns {Number} percentage of the segment's time range that is
-   * already in `buffered`
-   */
-  getSegmentBufferedPercent_(playlist, mediaIndex, currentTime, buffered) {
-    let segment = playlist.segments[mediaIndex];
-    let startOfSegment = duration(playlist,
-                                  playlist.mediaSequence + mediaIndex,
-                                  this.expired_);
-    let segmentRange = videojs.createTimeRanges([[
-      Math.max(currentTime, startOfSegment),
-      startOfSegment + segment.duration
-    ]]);
-
-    return Ranges.calculateBufferedPercent(segmentRange, buffered);
-  }
-
   /**
    * Determines what segment request should be made, given current
    * playback state.
@@ -432,7 +409,9 @@ export default class SegmentLoader extends videojs.EventTarget {
       // to the source buffer
       timestampOffset,
       // The timeline that the segment is in
-      timeline: segment.timeline
+      timeline: segment.timeline,
+      // The expected duration of the segment in seconds
+      duration: segment.duration
     };
   }
 
@@ -471,12 +450,23 @@ export default class SegmentLoader extends videojs.EventTarget {
       return;
     }
 
+    if (request.mediaIndex === this.playlist_.segments.length - 1 &&
+        this.mediaSource_.readyState === 'ended' &&
+        !this.seeking_()) {
+      return;
+    }
+
+    let segment = this.playlist_.segments[request.mediaIndex];
+    let startOfSegment = duration(this.playlist_,
+                                  this.playlist_.mediaSequence + request.mediaIndex,
+                                  this.expired_);
+
     // 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,
+    let percentBuffered = Ranges.getSegmentBufferedPercent(startOfSegment,
+                                                          segment.duration,
                                                           this.currentTime_(),
                                                           this.sourceUpdater_.buffered());
 
@@ -484,9 +474,9 @@ export default class SegmentLoader extends videojs.EventTarget {
       // 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);
+      let correctionApplied = this.incrementTimeCorrection_(this.playlist_.targetDuration, 1);
 
-      if (!this.paused()) {
+      if (correctionApplied && !this.paused()) {
         this.fillBuffer_();
       }
 
@@ -761,7 +751,8 @@ export default class SegmentLoader extends videojs.EventTarget {
 
     // add segment metadata if it we have gained information during the
     // last append
-    this.updateTimeline_(segmentInfo);
+    let timelineUpdated = this.updateTimeline_(segmentInfo);
+
     this.trigger('progress');
 
     let currentMediaIndex = segmentInfo.mediaIndex;
@@ -805,7 +796,22 @@ export default class SegmentLoader extends videojs.EventTarget {
 
     this.state = 'READY';
 
-    if (!this.paused()) {
+    if (timelineUpdated) {
+      this.timeCorrection_ = 0;
+      if (!this.paused()) {
+        this.fillBuffer_();
+      }
+      return;
+    }
+
+    // the last segment append must have been entirely in the
+    // already buffered time ranges. adjust the timeCorrection
+    // offset to fetch forward until we find a segment that adds
+    // to the buffered time ranges and improves subsequent media
+    // index calculations.
+    let correctionApplied = this.incrementTimeCorrection_(segmentInfo.duration, 4);
+
+    if (correctionApplied && !this.paused()) {
       this.fillBuffer_();
     }
   }
@@ -820,8 +826,7 @@ export default class SegmentLoader extends videojs.EventTarget {
   updateTimeline_(segmentInfo) {
     let segment;
     let segmentEnd;
-    let timelineUpdated;
-    let segmentLength = this.playlist_.targetDuration;
+    let timelineUpdated = false;
     let playlist = segmentInfo.playlist;
     let currentMediaIndex = segmentInfo.mediaIndex;
 
@@ -837,19 +842,9 @@ export default class SegmentLoader extends videojs.EventTarget {
       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
-    // offset to fetch forward until we find a segment that adds
-    // to the buffered time ranges and improves subsequent media
-    // index calculations.
-    if (!timelineUpdated) {
-      this.incrementTimeCorrection_(segmentLength);
-    } else {
-      this.timeCorrection_ = 0;
-    }
+    return timelineUpdated;
   }
 
   /**
@@ -862,17 +857,18 @@ export default class SegmentLoader extends videojs.EventTarget {
    * @private
    * @param {Number} secondsToIncrement number of seconds to add to the
    * timeCorrection_ variable
+   * @param {Number} maxSegmentsToWalk maximum number of times we allow this
+   * function to walk forward
    */
-  incrementTimeCorrection_(secondsToIncrement) {
+  incrementTimeCorrection_(secondsToIncrement, maxSegmentsToWalk) {
     // 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) {
+    // stop searching for a segment and reset timeCorrection_
+    if (this.timeCorrection_ >= this.playlist_.targetDuration * maxSegmentsToWalk) {
       this.timeCorrection_ = 0;
-      this.pause();
-      return this.trigger('error');
+      return false;
     }
 
     this.timeCorrection_ += secondsToIncrement;
+    return true;
   }
 }

test/ranges.test.js

@@ -91,3 +91,78 @@ QUnit.test('detects time range end-point changed by updates', function() {
   edge = Ranges.findSoleUncommonTimeRangesEnd(createTimeRanges([[0, 11]]), null);
   QUnit.strictEqual(edge, null, 'treat null update buffer as an empty TimeRanges object');
 });
+
+QUnit.module('Segment Percent Buffered Calculations');
+
+QUnit.test('calculates the percent buffered for segments', function() {
+  let segmentStart = 10;
+  let segmentDuration = 10;
+  let currentTime = 0;
+  let buffered = createTimeRanges([[15, 19]]);
+  let percentBuffered = Ranges.getSegmentBufferedPercent(
+    segmentStart,
+    segmentDuration,
+    currentTime,
+    buffered);
+
+  QUnit.equal(percentBuffered, 40, 'calculated the buffered amount correctly');
+});
+
+QUnit.test('calculates the percent buffered for segments taking into account ' +
+           'currentTime', function() {
+  let segmentStart = 10;
+  let segmentDuration = 10;
+  let currentTime = 15;
+  let buffered = createTimeRanges([[15, 19]]);
+  let percentBuffered = Ranges.getSegmentBufferedPercent(
+    segmentStart,
+    segmentDuration,
+    currentTime,
+    buffered);
+
+  QUnit.equal(percentBuffered, 90, 'calculated the buffered amount correctly');
+});
+
+QUnit.test('calculates the percent buffered for segments with multiple buffered ' +
+           'regions', function() {
+  let segmentStart = 10;
+  let segmentDuration = 10;
+  let currentTime = 0;
+  let buffered = createTimeRanges([[0, 11], [12, 19]]);
+  let percentBuffered = Ranges.getSegmentBufferedPercent(
+    segmentStart,
+    segmentDuration,
+    currentTime,
+    buffered);
+
+  QUnit.equal(percentBuffered, 80, 'calculated the buffered amount correctly');
+});
+
+QUnit.test('calculates the percent buffered for segments with multiple buffered ' +
+           'regions taking into account currentTime', function() {
+  let segmentStart = 10;
+  let segmentDuration = 10;
+  let currentTime = 12;
+  let buffered = createTimeRanges([[0, 11], [12, 19]]);
+  let percentBuffered = Ranges.getSegmentBufferedPercent(
+    segmentStart,
+    segmentDuration,
+    currentTime,
+    buffered);
+
+  QUnit.equal(percentBuffered, 90, 'calculated the buffered amount correctly');
+});
+
+QUnit.test('calculates the percent buffered as 0 for zero-length segments', function() {
+  let segmentStart = 10;
+  let segmentDuration = 0;
+  let currentTime = 0;
+  let buffered = createTimeRanges([[0, 19]]);
+  let percentBuffered = Ranges.getSegmentBufferedPercent(
+    segmentStart,
+    segmentDuration,
+    currentTime,
+    buffered);
+
+  QUnit.equal(percentBuffered, 0, 'calculated the buffered amount correctly');
+});

test/segment-loader.test.js

@@ -2,43 +2,8 @@ import QUnit from 'qunit';
 import SegmentLoader from '../src/segment-loader';
 import videojs from 'video.js';
 import xhrFactory from '../src/xhr';
-import {useFakeEnvironment, useFakeMediaSource} from './test-helpers.js';
 import Config from '../src/config';
-
-const playlistWithDuration = function(time, conf) {
-  let result = {
-    targetDuration: 10,
-    mediaSequence: conf && conf.mediaSequence ? conf.mediaSequence : 0,
-    discontinuityStarts: [],
-    segments: [],
-    endList: true
-  };
-  let count = Math.floor(time / 10);
-  let remainder = time % 10;
-  let i;
-  let isEncrypted = conf && conf.isEncrypted;
-
-  for (i = 0; i < count; i++) {
-    result.segments.push({
-      uri: i + '.ts',
-      resolvedUri: i + '.ts',
-      duration: 10
-    });
-    if (isEncrypted) {
-      result.segments[i].key = {
-        uri: i + '-key.php',
-        resolvedUri: i + '-key.php'
-      };
-    }
-  }
-  if (remainder) {
-    result.segments.push({
-      uri: i + '.ts',
-      duration: remainder
-    });
-  }
-  return result;
-};
+import { playlistWithDuration, useFakeEnvironment, useFakeMediaSource } from './test-helpers.js';
 
 let currentTime;
 let mediaSource;
@@ -428,8 +393,8 @@ QUnit.test('adjusts the playlist offset even when segment.end is set if no' +
   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() {
+QUnit.test('adjusts the playlist offset if no buffering progress is made after ' +
+           'several consecutive attempts', function() {
   let sourceBuffer;
   let playlist;
   let errors = 0;
@@ -452,7 +417,7 @@ QUnit.test('adjusts the playlist offset if no buffering progress is made after'
   sourceBuffer.buffered = videojs.createTimeRanges([[0, 10]]);
   sourceBuffer.trigger('updateend');
 
-  for (let i = 1; i <= 6; i++) {
+  for (let i = 1; i <= 5; 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);
@@ -460,10 +425,8 @@ QUnit.test('adjusts the playlist offset if no buffering progress is made after'
     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');
+  this.clock.tick(1);
+  QUnit.equal(this.requests.length, 0, 'no more requests are made');
 });
 
 QUnit.test('cancels outstanding requests on abort', function() {

test/test-helpers.js

@@ -292,3 +292,38 @@ export const absoluteUrl = function(relativeUrl) {
         .join('/')
     );
 };
+
+export const playlistWithDuration = function(time, conf) {
+  let result = {
+    targetDuration: 10,
+    mediaSequence: conf && conf.mediaSequence ? conf.mediaSequence : 0,
+    discontinuityStarts: [],
+    segments: [],
+    endList: true
+  };
+  let count = Math.floor(time / 10);
+  let remainder = time % 10;
+  let i;
+  let isEncrypted = conf && conf.isEncrypted;
+
+  for (i = 0; i < count; i++) {
+    result.segments.push({
+      uri: i + '.ts',
+      resolvedUri: i + '.ts',
+      duration: 10
+    });
+    if (isEncrypted) {
+      result.segments[i].key = {
+        uri: i + '-key.php',
+        resolvedUri: i + '-key.php'
+      };
+    }
+  }
+  if (remainder) {
+    result.segments.push({
+      uri: i + '.ts',
+      duration: remainder
+    });
+  }
+  return result;
+};

utils/manifest/zeroDuration.js

@@ -0,0 +1,16 @@
+{
+  "allowCache": true,
+  "mediaSequence": 0,
+  "playlistType": "VOD",
+  "segments": [
+    {
+      "duration": 0.01,
+      "timeline": 0,
+      "uri": "http://example.com/00001.ts"
+    }
+  ],
+  "targetDuration": 10,
+  "endList": true,
+  "discontinuitySequence": 0,
+  "discontinuityStarts": []
+}

utils/manifest/zeroDuration.m3u8

@@ -0,0 +1,8 @@
+#EXTM3U
+#EXT-X-PLAYLIST-TYPE:VOD
+#EXT-X-TARGETDURATION:10
+
+#EXTINF:0,
+http://example.com/00001.ts
+#ZEN-TOTAL-DURATION:57.9911
+#EXT-X-ENDLIST