fix: don't save bandwidth and throughput for really small 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
@@ -2182,14 +2186,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_() {
@@ -2261,11 +2271,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;
 
@@ -2662,6 +2672,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

@@ -3286,6 +3286,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');
+ });
  });
 });