From 6c4333c56506a67384672bc0cb7ac05e26025f14 Mon Sep 17 00:00:00 2001 From: Dave Nicholas Date: Wed, 6 Mar 2024 20:15:38 +0000 Subject: [PATCH] feat(ABR): Additional request information for ABR Managers (#6313) This PR makes the following request information available for ABR consideration. Allowing the ABR manager to know about the request latency from the time to first byte and knowing the order order of a packet, as well as the contentType of the request. --- externs/shaka/abr_manager.js | 4 ++- externs/shaka/net.js | 14 ++++++++- lib/abr/simple_abr_manager.js | 10 +++++- lib/media/streaming_engine.js | 1 + lib/net/networking_engine.js | 29 +++++++++++++++-- lib/player.js | 8 +++-- test/net/networking_engine_unit.js | 50 ++++++++++++++++++++++++++++-- 7 files changed, 104 insertions(+), 12 deletions(-) diff --git a/externs/shaka/abr_manager.js b/externs/shaka/abr_manager.js index eb061ea076..61bb230cb2 100644 --- a/externs/shaka/abr_manager.js +++ b/externs/shaka/abr_manager.js @@ -92,9 +92,11 @@ shaka.extern.AbrManager = class { * @param {number} numBytes The total number of bytes transferred. * @param {boolean} allowSwitch Indicate if the segment is allowed to switch * to another stream. + * @param {shaka.extern.Request=} request + * A reference to the request * @exportDoc */ - segmentDownloaded(deltaTimeMs, numBytes, allowSwitch) {} + segmentDownloaded(deltaTimeMs, numBytes, allowSwitch, request) {} /** * Notifies the ABR that it is a time to suggest new streams. This is used by diff --git a/externs/shaka/net.js b/externs/shaka/net.js index c45098a652..4515a0b248 100644 --- a/externs/shaka/net.js +++ b/externs/shaka/net.js @@ -63,7 +63,11 @@ shaka.extern.RetryParameters; * drmInfo: ?shaka.extern.DrmInfo, * initData: ?Uint8Array, * initDataType: ?string, - * streamDataCallback: ?function(BufferSource):!Promise + * streamDataCallback: ?function(BufferSource):!Promise, + * requestStartTime: (?number|undefined), + * timeToFirstByte: (?number|undefined), + * packetNumber: (?number|undefined), + * contentType: (?string|undefined) * }} * * @description @@ -104,6 +108,14 @@ shaka.extern.RetryParameters; * used to initialize EME. * @property {?function(BufferSource):!Promise} streamDataCallback * A callback function to handle the chunked data of the ReadableStream. + * @property {(?number|undefined)} requestStartTime + * The time that the request started. + * @property {(?number|undefined)} timeToFirstByte + * The time taken to the first byte. + * @property {(?number|undefined)} packetNumber + * A number representing the order the packet within the request. + * @property {(?string|undefined)} contentType + * Content type (e.g. 'video', 'audio' or 'text', 'image') * @exportDoc */ shaka.extern.Request; diff --git a/lib/abr/simple_abr_manager.js b/lib/abr/simple_abr_manager.js index 99ccca75f4..992df79823 100644 --- a/lib/abr/simple_abr_manager.js +++ b/lib/abr/simple_abr_manager.js @@ -302,11 +302,19 @@ shaka.abr.SimpleAbrManager = class { /** + * @param {number} deltaTimeMs The duration, in milliseconds, that the request + * took to complete. + * @param {number} numBytes The total number of bytes transferred. + * @param {boolean} allowSwitch Indicate if the segment is allowed to switch + * to another stream. + * @param {shaka.extern.Request=} request + * A reference to the request * @override * @export */ - segmentDownloaded(deltaTimeMs, numBytes, allowSwitch) { + segmentDownloaded(deltaTimeMs, numBytes, allowSwitch, request) { shaka.log.v2('Segment downloaded:', + 'contentType=' + (request && request.contentType), 'deltaTimeMs=' + deltaTimeMs, 'numBytes=' + numBytes, 'lastTimeChosenMs=' + this.lastTimeChosenMs_, diff --git a/lib/media/streaming_engine.js b/lib/media/streaming_engine.js index a7da731857..95bcee8235 100644 --- a/lib/media/streaming_engine.js +++ b/lib/media/streaming_engine.js @@ -2482,6 +2482,7 @@ shaka.media.StreamingEngine = class { reference.endByte, retryParameters, streamDataCallback); + request.contentType = stream.type; shaka.log.v2('fetching: reference=', reference); diff --git a/lib/net/networking_engine.js b/lib/net/networking_engine.js index eb4e6ae34f..a7ec5ab7e6 100644 --- a/lib/net/networking_engine.js +++ b/lib/net/networking_engine.js @@ -46,7 +46,8 @@ goog.require('shaka.util.Timer'); */ shaka.net.NetworkingEngine = class extends shaka.util.FakeEventTarget { /** - * @param {function(number, number, boolean)=} onProgressUpdated Called when + * @param {shaka.net.NetworkingEngine.onProgressUpdated=} onProgressUpdated + * Called when * a progress event is triggered. Passed the duration, in milliseconds, * that the request took, the number of bytes transferred, and the boolean * of whether the switching is allowed. @@ -75,7 +76,7 @@ shaka.net.NetworkingEngine = class extends shaka.util.FakeEventTarget { /** @private {!Set.} */ this.responseFilters_ = new Set(); - /** @private {?function(number, number, boolean)} */ + /** @private {?shaka.net.NetworkingEngine.onProgressUpdated} */ this.onProgressUpdated_ = onProgressUpdated || null; /** @private {?shaka.net.NetworkingEngine.OnHeadersReceived} */ @@ -510,6 +511,7 @@ shaka.net.NetworkingEngine = class extends shaka.util.FakeEventTarget { startTimeMs = Date.now(); const segment = shaka.net.NetworkingEngine.RequestType.SEGMENT; + let packetNumber = 0; const progressUpdated = (time, bytes, numBytesRemaining) => { if (connectionTimer) { @@ -519,8 +521,10 @@ shaka.net.NetworkingEngine = class extends shaka.util.FakeEventTarget { stallTimer.tickAfter(stallTimeoutMs / 1000); } if (this.onProgressUpdated_ && type == segment) { + packetNumber++; + request.packetNumber = packetNumber; const allowSwitch = this.allowSwitch_(context); - this.onProgressUpdated_(time, bytes, allowSwitch); + this.onProgressUpdated_(time, bytes, allowSwitch, request); gotProgress = true; numBytesRemainingObj.setBytes(numBytesRemaining); } @@ -530,7 +534,11 @@ shaka.net.NetworkingEngine = class extends shaka.util.FakeEventTarget { this.onHeadersReceived_(headers, request, type); } headersReceivedCalled = true; + request.timeToFirstByte = Date.now() - + /** @type {number} */ (request.requestStartTime); }; + request.requestStartTime = Date.now(); + const requestPlugin = plugin( request.uris[index], request, type, progressUpdated, headersReceived); @@ -899,6 +907,21 @@ shaka.net.NetworkingEngine.ResponseAndGotProgress; */ shaka.net.NetworkingEngine.OnHeadersReceived; +/** + * @typedef {function( + * number, + * number, + * boolean, + * shaka.extern.Request=)} + * + * @description + * A callback that is passed the duration, in milliseconds, + * that the request took, the number of bytes transferred, a boolean + * representing whether the switching is allowed and a ref to the + * original request. + * @export + */ +shaka.net.NetworkingEngine.onProgressUpdated; /** * @typedef {function( diff --git a/lib/player.js b/lib/player.js index ee49d65959..2fe0da22b7 100644 --- a/lib/player.js +++ b/lib/player.js @@ -2816,14 +2816,16 @@ shaka.Player = class extends shaka.util.FakeEventTarget { this.dispatchEvent(event); } }; - /** @type {function(number, number, boolean)} */ - const onProgressUpdated_ = (deltaTimeMs, bytesDownloaded, allowSwitch) => { + /** @type {shaka.net.NetworkingEngine.onProgressUpdated} */ + const onProgressUpdated_ = (deltaTimeMs, + bytesDownloaded, allowSwitch, request) => { // In some situations, such as during offline storage, the abr manager // might not yet exist. Therefore, we need to check if abr manager has // been initialized before using it. const abrManager = getAbrManager(); if (abrManager) { - abrManager.segmentDownloaded(deltaTimeMs, bytesDownloaded, allowSwitch); + abrManager.segmentDownloaded(deltaTimeMs, bytesDownloaded, + allowSwitch, request); } }; /** @type {shaka.net.NetworkingEngine.OnHeadersReceived} */ diff --git a/test/net/networking_engine_unit.js b/test/net/networking_engine_unit.js index 0e798e4436..aae723e796 100644 --- a/test/net/networking_engine_unit.js +++ b/test/net/networking_engine_unit.js @@ -375,6 +375,23 @@ describe('NetworkingEngine', /** @suppress {accessControls} */ () => { expect(resolveScheme.calls.argsFor(0)[0]).toBe('resolve://foo'); }); + it('sets the time to 1st byte of the request when headers are available', + async () => { + const request = createRequest('resolve://foo'); + request.method = 'POST'; + + resolveScheme.and.callFake( + (uri, requestPassed, requestTypePassed, progressCallback, + headersCallback) => { + requestPassed.requestStartTime = 1; + headersCallback(); + expect(requestPassed.timeToFirstByte).toBeGreaterThan(0); + return shaka.util.AbortableOperation + .completed(createResponse()); + }); + await networkingEngine.request(requestType, request).promise; + }); + it('fills in defaults for partial request objects', async () => { const originalRequest = /** @type {shaka.extern.Request} */ ({ uris: ['resolve://foo'], @@ -1147,6 +1164,7 @@ describe('NetworkingEngine', /** @suppress {accessControls} */ () => { describe('progress events', () => { it('forwards progress events to caller', async () => { + const requestLikeObject = jasmine.objectContaining({method: 'GET'}); /** @const {!shaka.util.PublicPromise} */ const delay = new shaka.util.PublicPromise(); resolveScheme.and.callFake((uri, req, type, progress) => { @@ -1166,14 +1184,40 @@ describe('NetworkingEngine', /** @suppress {accessControls} */ () => { requestType, createRequest('resolve://')); await Util.shortDelay(); // Allow Promises to resolve. expect(onProgress).toHaveBeenCalledTimes(2); - expect(onProgress).toHaveBeenCalledWith(1, 2, true); - expect(onProgress).toHaveBeenCalledWith(4, 5, true); + expect(onProgress).toHaveBeenCalledWith(1, 2, true, requestLikeObject); + expect(onProgress).toHaveBeenCalledWith(4, 5, true, requestLikeObject); onProgress.calls.reset(); delay.resolve(); await resp.promise; expect(onProgress).toHaveBeenCalledTimes(1); - expect(onProgress).toHaveBeenCalledWith(7, 8, true); + expect(onProgress).toHaveBeenCalledWith(7, 8, true, requestLikeObject); + }); + + it('appends request packet number', async () => { + /** @const {!shaka.util.PublicPromise} */ + const delay = new shaka.util.PublicPromise(); + resolveScheme.and.callFake((uri, req, type, progress) => { + progress(1, 2, 3); + + const p = (async () => { + await delay; + progress(4, 5, 6); + return createResponse(); + })(); + return new shaka.util.AbortableOperation(p, () => {}); + }); + + /** @const {shaka.net.NetworkingEngine.PendingRequest} */ + const resp = networkingEngine.request( + requestType, createRequest('resolve://')); + await Util.shortDelay(); // Allow Promises to resolve. + expect(onProgress).toHaveBeenCalledWith(1, 2, true, + jasmine.objectContaining({packetNumber: 1})); + delay.resolve(); + await resp.promise; + expect(onProgress).toHaveBeenCalledWith(4, 5, true, + jasmine.objectContaining({packetNumber: 2})); }); it('doesn\'t forward progress events for non-SEGMENT', async () => {