diff --git a/src/segment-loader.js b/src/segment-loader.js index b0818f8d7..061c16363 100644 --- a/src/segment-loader.js +++ b/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 diff --git a/src/vtt-segment-loader.js b/src/vtt-segment-loader.js index 8edca0a57..5097999bd 100644 --- a/src/vtt-segment-loader.js +++ b/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) { diff --git a/test/segment-loader.test.js b/test/segment-loader.test.js index aaa6bb391..e964152e6 100644 --- a/test/segment-loader.test.js +++ b/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'); + }); }); });