fix: don't save bandwidth and throughput for really small segments (#…

…1024)

Really small segments can mess with our ABR algorithm by not reflecting our bandwidth and throughput accurately for the majority of segments.

src/segment-loader.js

@@ -26,6 +26,10 @@ import shallowEqual from './util/shallow-equal.js';
 // in ms
 const CHECK_BUFFER_DELAY = 500;
 const finite = (num) => typeof num === 'number' && isFinite(num);
+// With most content hovering around 30fps, if a segment has a duration less than a half
+// frame at 30fps or one frame at 60fps, the bandwidth and throughput calculations will
+// not accurately reflect the rest of the content.
+const MIN_SEGMENT_DURATION_TO_SAVE_STATS = 1 / 60;
 
 export const illegalMediaSwitch = (loaderType, startingMedia, trackInfo) => {
   // Although these checks should most likely cover non 'main' types, for now it narrows
@@ -2210,14 +2214,20 @@ export default class SegmentLoader extends videojs.EventTarget {
     }
   }
 
-  saveBandwidthRelatedStats_(stats) {
-    this.bandwidth = stats.bandwidth;
-    this.roundTrip = stats.roundTripTime;
-
+  saveBandwidthRelatedStats_(duration, stats) {
     // byteLength will be used for throughput, and should be based on bytes receieved,
     // which we only know at the end of the request and should reflect total bytes
     // downloaded rather than just bytes processed from components of the segment
     this.pendingSegment_.byteLength = stats.bytesReceived;
+
+    if (duration < MIN_SEGMENT_DURATION_TO_SAVE_STATS) {
+      this.logger_(`Ignoring segment's bandwidth because its duration of ${duration}` +
+        ` is less than the min to record ${MIN_SEGMENT_DURATION_TO_SAVE_STATS}`);
+      return;
+    }
+
+    this.bandwidth = stats.bandwidth;
+    this.roundTrip = stats.roundTripTime;
   }
 
   handleTimeout_() {
@@ -2289,11 +2299,11 @@ export default class SegmentLoader extends videojs.EventTarget {
       return;
     }
 
+    const segmentInfo = this.pendingSegment_;
+
     // the response was a success so set any bandwidth stats the request
     // generated for ABR purposes
-    this.saveBandwidthRelatedStats_(simpleSegment.stats);
-
-    const segmentInfo = this.pendingSegment_;
+    this.saveBandwidthRelatedStats_(segmentInfo.duration, simpleSegment.stats);
 
     segmentInfo.endOfAllRequests = simpleSegment.endOfAllRequests;
 
@@ -2690,6 +2700,12 @@ export default class SegmentLoader extends videojs.EventTarget {
    * @param {Object} segmentInfo the object returned by loadSegment
    */
   recordThroughput_(segmentInfo) {
+    if (segmentInfo.duration < MIN_SEGMENT_DURATION_TO_SAVE_STATS) {
+      this.logger_(`Ignoring segment's throughput because its duration of ${segmentInfo.duration}` +
+        ` is less than the min to record ${MIN_SEGMENT_DURATION_TO_SAVE_STATS}`);
+      return;
+    }
+
     const rate = this.throughput.rate;
     // Add one to the time to ensure that we don't accidentally attempt to divide
     // by zero in the case where the throughput is ridiculously high

src/vtt-segment-loader.js

@@ -281,16 +281,17 @@ export default class VTTSegmentLoader extends SegmentLoader {
       return;
     }
 
+    const segmentInfo = this.pendingSegment_;
+
     // although the VTT segment loader bandwidth isn't really used, it's good to
     // maintain functionality between segment loaders
-    this.saveBandwidthRelatedStats_(simpleSegment.stats);
+    this.saveBandwidthRelatedStats_(segmentInfo.duration, simpleSegment.stats);
 
     this.state = 'APPENDING';
 
     // used for tests
     this.trigger('appending');
 
-    const segmentInfo = this.pendingSegment_;
     const segment = segmentInfo.segment;
 
     if (segment.map) {

test/segment-loader.test.js

@@ -3300,6 +3300,73 @@ QUnit.module('SegmentLoader', function(hooks) {
         );
       });
     });
+
+    QUnit.test('saves bandwidth when segment duration is >= min to record', function(assert) {
+      const stats = {
+        bytesReceived: 100,
+        bandwidth: 101,
+        roundTrip: 102
+      };
+
+      loader.bandwidth = 999;
+      // used for updating byte length
+      loader.pendingSegment_ = {};
+      loader.saveBandwidthRelatedStats_(0.04, stats);
+
+      assert.equal(loader.bandwidth, 101, 'saved bandwidth');
+    });
+
+    QUnit.test('does not save bandwidth when segment duration is < min to record', function(assert) {
+      const stats = {
+        bytesReceived: 100,
+        bandwidth: 101,
+        roundTrip: 102
+      };
+
+      loader.bandwidth = 999;
+      // used for updating byte length
+      loader.pendingSegment_ = {};
+      loader.saveBandwidthRelatedStats_(0.01, stats);
+
+      assert.equal(loader.bandwidth, 999, 'did not save bandwidth');
+    });
+
+    QUnit.test('saves throughput when segment duration is >= min to record', function(assert) {
+      const segmentInfo = {
+        duration: 0.04,
+        rate: 101,
+        endOfAllRequests: Date.now(),
+        byteLength: 100
+      };
+
+      loader.throughput = {
+        rate: 1000,
+        count: 1
+      };
+      loader.recordThroughput_(segmentInfo);
+
+      // easier to assert not equal than deal with mocking dates
+      assert.notEqual(loader.throughput.rate, 1000, 'saved throughput');
+      assert.equal(loader.throughput.count, 2, 'saved throughput');
+    });
+
+    QUnit.test('does not save throughput when segment duration is < min to record', function(assert) {
+      const segmentInfo = {
+        duration: 0.01,
+        rate: 101,
+        endOfAllRequests: Date.now(),
+        byteLength: 100
+      };
+
+      loader.throughput = {
+        rate: 1000,
+        count: 1
+      };
+      loader.recordThroughput_(segmentInfo);
+
+      assert.equal(loader.throughput.rate, 1000, 'did not save throughput');
+      assert.equal(loader.throughput.count, 1, 'did not save throughput');
+    });
   });
 });