Skip to content

Commit

Permalink
Allow passing durations from scheme plugins.
Browse files Browse the repository at this point in the history
This allows the networking plugins to pass the duration it took for
the request.  For example, if a plugin were pre-fetching the segment,
this allows the plugin to tell NetworkingEngine about how long it
really took to download.

Closes #621

Change-Id: Ie67e3f99389cf02d5b4a345bde06ccc418b6e91c
  • Loading branch information
TheModMaker committed Dec 20, 2016
1 parent 0082844 commit b1711c3
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 69 deletions.
8 changes: 3 additions & 5 deletions externs/shaka/abr_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,13 @@ shakaExtern.AbrManager.prototype.disable = function() {};
* Notifies the AbrManager that a segment has been downloaded (includes MP4
* SIDX data, WebM Cues data, initialization segments, and media segments).
*
* @param {number} startTimeMs The wall-clock time, in milliseconds, when the
* request began (before any outbound request filters).
* @param {number} endTimeMs The wall-clock time, in milliseconds, when the
* response ended (after all retries and inbound response filters).
* @param {number} deltaTimeMs The duration, in milliseconds, that the request
* took to complete.
* @param {number} numBytes The total number of bytes transferred.
* @exportDoc
*/
shakaExtern.AbrManager.prototype.segmentDownloaded = function(
startTimeMs, endTimeMs, numBytes) {};
deltaTimeMs, numBytes) {};


/**
Expand Down
6 changes: 5 additions & 1 deletion externs/shaka/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ shakaExtern.Request;
* @typedef {{
* uri: string,
* data: ArrayBuffer,
* headers: !Object.<string, string>
* headers: !Object.<string, string>,
* timeMs: (number|undefined)
* }}
*
* @description
Expand All @@ -106,6 +107,9 @@ shakaExtern.Request;
* A map of response headers, if supported by the underlying protocol.
* All keys should be lowercased.
* For HTTP/HTTPS, may not be available cross-origin.
* @property {(number|undefined)} timeMs
* Optional. The time it took to get the response, in miliseconds. If not
* given, NetworkingEngine will calculate it using Date.now.
*
* @exportDoc
*/
Expand Down
10 changes: 4 additions & 6 deletions lib/abr/simple_abr_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,12 @@ shaka.abr.SimpleAbrManager.prototype.disable = function() {
* @export
*/
shaka.abr.SimpleAbrManager.prototype.segmentDownloaded = function(
startTimeMs, endTimeMs, numBytes) {
deltaTimeMs, numBytes) {
shaka.log.v2('Segment downloaded:',
'startTimeMs=' + startTimeMs,
'endTimeMs=' + endTimeMs,
'deltaTimeMs=' + deltaTimeMs,
'numBytes=' + numBytes);
goog.asserts.assert(endTimeMs >= startTimeMs,
'expected a non-negative duration');
this.bandwidthEstimator_.sample(endTimeMs - startTimeMs, numBytes);
goog.asserts.assert(deltaTimeMs >= 0, 'expected a non-negative duration');
this.bandwidthEstimator_.sample(deltaTimeMs, numBytes);

if ((this.lastTimeChosenMs_ != null) && this.enabled_)
this.suggestStreams_();
Expand Down
39 changes: 23 additions & 16 deletions lib/net/networking_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,9 @@ goog.require('shaka.util.PublicPromise');
* handle the actual request. A plugin is registered using registerScheme.
* Each scheme has at most one plugin to handle the request.
*
* @param {function(number, number, number)=} opt_onSegmentDownloaded Called
* when a segment is downloaded. Passed the wall-clock time, in
* milliseconds, when the request began (before any outbound request
* filters); the wall-clock time, in milliseconds, when the response ended
* (after all retries and inbound response filters); and the total number
* of bytes transferred.
* @param {function(number, number)=} opt_onSegmentDownloaded Called
* when a segment is downloaded. Passed the duration, in milliseconds, that
* the request took; and the total number of bytes transferred.
*
* @struct
* @constructor
Expand All @@ -56,7 +53,7 @@ shaka.net.NetworkingEngine = function(opt_onSegmentDownloaded) {
/** @private {!Array.<shakaExtern.ResponseFilter>} */
this.responseFilters_ = [];

/** @private {?function(number, number, number)} */
/** @private {?function(number, number)} */
this.onSegmentDownloaded_ = opt_onSegmentDownloaded || null;
};

Expand Down Expand Up @@ -254,7 +251,7 @@ shaka.net.NetworkingEngine.prototype.request = function(type, request) {
goog.asserts.assert(request.uris && request.uris.length,
'Request without URIs!');

var startTimeMs = Date.now();
var filterStartMs = Date.now();

// Send to the filter first, in-case they change the URI.
var requestFilters = this.requestFilters_;
Expand All @@ -265,28 +262,28 @@ shaka.net.NetworkingEngine.prototype.request = function(type, request) {
return Promise.reject(error);
}
}
var filterTimeMs = (Date.now() - filterStartMs);

var retry = request.retryParameters || {};
var maxAttempts = retry.maxAttempts || 1;
var backoffFactor = retry.backoffFactor || 2.0;
var delay = (retry.baseDelay == null ? 1000 : retry.baseDelay);

var p = this.send_(type, request, 0);
var p = this.send_(type, request, 0, filterTimeMs);
for (var i = 1; i < maxAttempts; i++) {
var index = i % request.uris.length;
p = p.catch(this.resend_.bind(this, type, request, delay, index));
p = p.catch(
this.resend_.bind(this, type, request, delay, index, filterTimeMs));
delay *= backoffFactor;
}

// Add the request to the array.
this.requests_.push(p);
return p.then(function(response) {
this.requests_.splice(this.requests_.indexOf(p), 1);
var endTimeMs = Date.now();
if (this.onSegmentDownloaded_ &&
type == shaka.net.NetworkingEngine.RequestType.SEGMENT) {
this.onSegmentDownloaded_(
startTimeMs, endTimeMs, response.data.byteLength);
this.onSegmentDownloaded_(response.timeMs, response.data.byteLength);
}
return response;
}.bind(this)).catch(function(e) {
Expand All @@ -302,10 +299,12 @@ shaka.net.NetworkingEngine.prototype.request = function(type, request) {
* @param {shaka.net.NetworkingEngine.RequestType} type
* @param {shakaExtern.Request} request
* @param {number} index
* @param {number} requestFilterTime
* @return {!Promise.<shakaExtern.Response>}
* @private
*/
shaka.net.NetworkingEngine.prototype.send_ = function(type, request, index) {
shaka.net.NetworkingEngine.prototype.send_ = function(
type, request, index, requestFilterTime) {
if (this.destroyed_)
return Promise.reject();

Expand Down Expand Up @@ -333,13 +332,20 @@ shaka.net.NetworkingEngine.prototype.send_ = function(type, request, index) {
uri));
}

var startTimeMs = Date.now();
return plugin(request.uris[index], request).then(function(response) {
if (response.timeMs === undefined)
response.timeMs = Date.now() - startTimeMs;

// Since we are inside a promise, no need to catch errors; they will result
// in a failed promise.
var filterStartMs = Date.now();
var responseFilters = this.responseFilters_;
for (var i = 0; i < responseFilters.length; i++) {
responseFilters[i](type, response);
}
response.timeMs += Date.now() - filterStartMs;
response.timeMs += requestFilterTime;

return response;
}.bind(this));
Expand All @@ -353,11 +359,12 @@ shaka.net.NetworkingEngine.prototype.send_ = function(type, request, index) {
* @param {shakaExtern.Request} request
* @param {number} delayMs The current base delay.
* @param {number} index
* @param {number} requestFilterTime
* @return {!Promise.<shakaExtern.Response>}
* @private
*/
shaka.net.NetworkingEngine.prototype.resend_ =
function(type, request, delayMs, index) {
function(type, request, delayMs, index, requestFilterTime) {
var p = new shaka.util.PublicPromise();

// Fuzz the delay to avoid tons of clients hitting the server at once
Expand All @@ -369,7 +376,7 @@ shaka.net.NetworkingEngine.prototype.resend_ =
var fuzzedDelay = delayMs * (1.0 + negToPosFuzzFactor);
shaka.net.NetworkingEngine.setTimeout_(p.resolve, fuzzedDelay);

return p.then(this.send_.bind(this, type, request, index));
return p.then(this.send_.bind(this, type, request, index, requestFilterTime));
};


Expand Down
8 changes: 3 additions & 5 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -1362,14 +1362,12 @@ shaka.Player.prototype.updateStats_ = function() {
/**
* Callback from NetworkingEngine.
*
* @param {number} startTimeMs
* @param {number} endTimeMs
* @param {number} deltaTimeMs
* @param {number} numBytes
* @private
*/
shaka.Player.prototype.onSegmentDownloaded_ = function(
startTimeMs, endTimeMs, numBytes) {
this.config_.abr.manager.segmentDownloaded(startTimeMs, endTimeMs, numBytes);
shaka.Player.prototype.onSegmentDownloaded_ = function(deltaTimeMs, numBytes) {
this.config_.abr.manager.segmentDownloaded(deltaTimeMs, numBytes);
};


Expand Down
66 changes: 33 additions & 33 deletions test/abr/simple_abr_manager_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,14 @@ describe('SimpleAbrManager', function() {
it(description, function() {
abrManager.chooseStreams(streamSetsByType);

abrManager.segmentDownloaded(0, 1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, 2000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

abrManager.enable();

// Make another call to segmentDownloaded() so switchCallback() is
// called.
abrManager.segmentDownloaded(3000, 4000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

expect(switchCallback).toHaveBeenCalled();
expect(switchCallback.calls.argsFor(0)[0]).toEqual({
Expand All @@ -165,14 +165,14 @@ describe('SimpleAbrManager', function() {
it(description, function() {
abrManager.chooseStreams(streamSetsByType);

abrManager.segmentDownloaded(0, 1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, 2000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

abrManager.enable();

// Make another call to segmentDownloaded() so switchCallback() is
// called.
abrManager.segmentDownloaded(3000, 4000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

expect(switchCallback).toHaveBeenCalled();
expect(switchCallback.calls.argsFor(0)[0]).toEqual({
Expand All @@ -191,12 +191,12 @@ describe('SimpleAbrManager', function() {
abrManager.chooseStreams(streamSetsByType);

// 0 duration segment shouldn't cause us to get stuck on the lowest variant
abrManager.segmentDownloaded(1000, 1000, bytesPerSecond);
abrManager.segmentDownloaded(2000, 3000, bytesPerSecond);
abrManager.segmentDownloaded(0, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

abrManager.enable();

abrManager.segmentDownloaded(4000, 5000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

expect(switchCallback).toHaveBeenCalled();
expect(switchCallback.calls.argsFor(0)[0]).toEqual({
Expand All @@ -219,14 +219,14 @@ describe('SimpleAbrManager', function() {

abrManager.chooseStreams(streamSetsByType);

abrManager.segmentDownloaded(0, 1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, 2000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

abrManager.enable();

// Make another call to segmentDownloaded() so switchCallback() is
// called.
abrManager.segmentDownloaded(3000, 4000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

expect(switchCallback).toHaveBeenCalled();
expect(switchCallback.calls.argsFor(0)[0]).toEqual({
Expand All @@ -244,9 +244,9 @@ describe('SimpleAbrManager', function() {
abrManager.chooseStreams(streamSetsByType);

// Don't enable AbrManager.
abrManager.segmentDownloaded(0, 1000, bytesPerSecond);
abrManager.segmentDownloaded(2000, 3000, bytesPerSecond);
abrManager.segmentDownloaded(4000, 5000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
expect(switchCallback).not.toHaveBeenCalled();
});

Expand All @@ -258,12 +258,12 @@ describe('SimpleAbrManager', function() {

abrManager.chooseStreams(streamSetsByType);

abrManager.segmentDownloaded(0, 1000, bytesPerSecond);
abrManager.segmentDownloaded(2000, 3000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

abrManager.enable();

abrManager.segmentDownloaded(3000, 4000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
expect(switchCallback).toHaveBeenCalled();
switchCallback.calls.reset();

Expand All @@ -273,24 +273,24 @@ describe('SimpleAbrManager', function() {
bytesPerSecond =
sufficientBWMultiplier * (audioBandwidth + videoBandwidth) / 8.0;

abrManager.segmentDownloaded(4000, 5000, bytesPerSecond);
abrManager.segmentDownloaded(5000, 6000, bytesPerSecond);
abrManager.segmentDownloaded(6000, 7000, bytesPerSecond);
abrManager.segmentDownloaded(7000, 8000, bytesPerSecond);
abrManager.segmentDownloaded(8000, 9000, bytesPerSecond);
abrManager.segmentDownloaded(9000, 10000, bytesPerSecond);
abrManager.segmentDownloaded(10000, 11000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

// Stay inside switch interval.
shaka.test.Util.fakeEventLoop(
(shaka.abr.SimpleAbrManager.SWITCH_INTERVAL_MS / 1000.0) - 2);
abrManager.segmentDownloaded(10000, 11000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

expect(switchCallback).not.toHaveBeenCalled();

// Move outside switch interval.
shaka.test.Util.fakeEventLoop(3);
abrManager.segmentDownloaded(12000, 13000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

expect(switchCallback).toHaveBeenCalled();
expect(switchCallback.calls.argsFor(0)[0]).toEqual({
Expand All @@ -309,14 +309,14 @@ describe('SimpleAbrManager', function() {

abrManager.chooseStreams(streamSetsByType);

abrManager.segmentDownloaded(0, 1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, 2000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

abrManager.enable();

// Make another call to segmentDownloaded(). switchCallback() will be
// called to upgrade.
abrManager.segmentDownloaded(3000, 4000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

// The second parameter is missing to indicate that the buffer should not be
// cleared.
Expand All @@ -335,14 +335,14 @@ describe('SimpleAbrManager', function() {
abrManager.setDefaultEstimate(4e6);
abrManager.chooseStreams(streamSetsByType);

abrManager.segmentDownloaded(0, 1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, 2000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

abrManager.enable();

// Make another call to segmentDownloaded(). switchCallback() will be
// called to downgrade.
abrManager.segmentDownloaded(3000, 4000, bytesPerSecond);
abrManager.segmentDownloaded(1000, bytesPerSecond);

// The second parameter is missing to indicate that the buffer should not be
// cleared.
Expand Down
6 changes: 3 additions & 3 deletions test/net/networking_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ describe('NetworkingEngine', /** @suppress {accessControls} */ function() {
resolveScheme.and.callFake(function(uri, request) {
expect(uri).toBe(request.uris[0]);
expect(request).toEqual(request);
return Promise.resolve();
return Promise.resolve({});
});
networkingEngine.request(requestType, request).catch(fail).then(done);
});
Expand Down Expand Up @@ -497,7 +497,7 @@ describe('NetworkingEngine', /** @suppress {accessControls} */ function() {

Util.delay(0.1).then(function() {
expect(d.status).toBe('pending');
p.resolve();
p.resolve({});
return d;
}).then(function() {
return Util.delay(0.1);
Expand Down Expand Up @@ -563,7 +563,7 @@ describe('NetworkingEngine', /** @suppress {accessControls} */ function() {
expect(r1.status).toBe('pending');
expect(r2.status).toBe('rejected');
expect(d.status).toBe('pending');
p.resolve();
p.resolve({});
return d;
}).then(function() {
return Util.delay(0.1);
Expand Down

0 comments on commit b1711c3

Please sign in to comment.