From 845649b001e4dac4ed09d1a832b8213c8c56555d Mon Sep 17 00:00:00 2001 From: baconz Date: Tue, 15 Aug 2023 08:36:06 -0700 Subject: [PATCH] fix: Typing of PeriodCombiner.Period was incorrect (#5442) When we exported `PeriodCombiner` in https://github.com/shaka-project/shaka-player/pull/5324 we added an `@export` to `PeriodCombiner.Period`, and since then we've been testing in our dogfood builds using shaka-player.compiled.debug, because we like to get logs from dogfood. Everything was working great. When we went to switch over to the production build, we realized that `@export` on a `typedef` doesn't really work because the type gets minified internally!! This moves `Period` over to `extern` so that it does not get minified and can be used externally. --- externs/shaka/manifest.js | 27 +++++++++++++++++++++++++ lib/dash/dash_parser.js | 4 ++-- lib/util/periods.js | 36 +++++---------------------------- test/util/periods_unit.js | 42 +++++++++++++++++++-------------------- 4 files changed, 55 insertions(+), 54 deletions(-) diff --git a/externs/shaka/manifest.js b/externs/shaka/manifest.js index 7fb1c03693..af879b79bd 100644 --- a/externs/shaka/manifest.js +++ b/externs/shaka/manifest.js @@ -99,6 +99,33 @@ shaka.extern.Manifest; +/** + * @typedef {{ +* id: string, +* audioStreams: !Array., +* videoStreams: !Array., +* textStreams: !Array., +* imageStreams: !Array. +* }} +* +* @description Contains the streams from one DASH period. +* For use in {@link shaka.util.PeriodCombiner}. +* +* @property {string} id +* The Period ID. +* @property {!Array.} audioStreams +* The audio streams from one Period. +* @property {!Array.} videoStreams +* The video streams from one Period. +* @property {!Array.} textStreams +* The text streams from one Period. +* @property {!Array.} imageStreams +* The image streams from one Period. +* +* @exportDoc +*/ +shaka.extern.Period; + /** * @typedef {{ * initData: !Uint8Array, diff --git a/lib/dash/dash_parser.js b/lib/dash/dash_parser.js index 3a2780b085..81df0dc42e 100644 --- a/lib/dash/dash_parser.js +++ b/lib/dash/dash_parser.js @@ -590,7 +590,7 @@ shaka.dash.DashParser = class { * @param {!Array.} baseUris * @param {!Element} mpd * @return {{ - * periods: !Array., + * periods: !Array., * duration: ?number, * durationDerivedFromPeriods: boolean * }} @@ -742,7 +742,7 @@ shaka.dash.DashParser = class { * @param {shaka.dash.DashParser.Context} context * @param {!Array.} baseUris * @param {shaka.dash.DashParser.PeriodInfo} periodInfo - * @return {shaka.util.PeriodCombiner.Period} + * @return {shaka.extern.Period} * @private */ parsePeriod_(context, baseUris, periodInfo) { diff --git a/lib/util/periods.js b/lib/util/periods.js index ef976297b6..ab0921cf63 100644 --- a/lib/util/periods.js +++ b/lib/util/periods.js @@ -104,7 +104,7 @@ shaka.util.PeriodCombiner = class { } /** - * @param {!Array.} periods + * @param {!Array.} periods * @param {boolean} isDynamic * @return {!Promise} * @@ -262,7 +262,7 @@ shaka.util.PeriodCombiner = class { } /** - * @param {!Array.} periods + * @param {!Array.} periods * @private */ static filterOutAudioStreamDuplicates_(periods) { @@ -299,7 +299,7 @@ shaka.util.PeriodCombiner = class { } /** - * @param {!Array.} periods + * @param {!Array.} periods * @private */ static filterOutTextStreamDuplicates_(periods) { @@ -333,7 +333,7 @@ shaka.util.PeriodCombiner = class { } /** - * @param {!Array.} periods + * @param {!Array.} periods * @private */ static filterOutVideoStreamDuplicates_(periods) { @@ -370,7 +370,7 @@ shaka.util.PeriodCombiner = class { } /** - * @param {!Array.} periods + * @param {!Array.} periods * @private */ static filterOutImageStreamDuplicates_(periods) { @@ -1639,32 +1639,6 @@ shaka.util.PeriodCombiner = class { } }; -/** - * @typedef {{ - * id: string, - * audioStreams: !Array., - * videoStreams: !Array., - * textStreams: !Array., - * imageStreams: !Array. - * }} - * - * @description Contains the streams from one DASH period. - * - * @property {string} id - * The Period ID. - * @property {!Array.} audioStreams - * The audio streams from one Period. - * @property {!Array.} videoStreams - * The video streams from one Period. - * @property {!Array.} textStreams - * The text streams from one Period. - * @property {!Array.} imageStreams - * The image streams from one Period. - * - * @export - */ -shaka.util.PeriodCombiner.Period; - /** * @enum {number} */ diff --git a/test/util/periods_unit.js b/test/util/periods_unit.js index e3ab2e5c4c..c18eec3eb4 100644 --- a/test/util/periods_unit.js +++ b/test/util/periods_unit.js @@ -29,7 +29,7 @@ describe('PeriodCombiner', () => { }); it('Ad insertion - join during main content', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: 'main', @@ -112,7 +112,7 @@ describe('PeriodCombiner', () => { }); it('Ad insertion - join during ad', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: 'ad', @@ -190,7 +190,7 @@ describe('PeriodCombiner', () => { }); it('Ad insertion - smaller ad, res not found in main content', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -240,7 +240,7 @@ describe('PeriodCombiner', () => { }); it('Ad insertion - larger ad, res not found in main content', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -288,7 +288,7 @@ describe('PeriodCombiner', () => { }); it('Language changes during and after an ad', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: 'show1', @@ -368,7 +368,7 @@ describe('PeriodCombiner', () => { }); it('VOD playlist of completely unrelated periods', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: 'show1', @@ -421,7 +421,7 @@ describe('PeriodCombiner', () => { /** @type {shaka.extern.Stream} */ const video2 = makeVideoStream(480); video2.bandwidth = 2; - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -538,7 +538,7 @@ describe('PeriodCombiner', () => { const i3 = makeImageStream(240); i3.originalId = 'i3'; - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -606,7 +606,7 @@ describe('PeriodCombiner', () => { // Regression test for #3383, where we failed on multi-period content with // multiple image streams per period. it('Can handle multiple image streams', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -647,7 +647,7 @@ describe('PeriodCombiner', () => { }); it('handles text track gaps', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -714,7 +714,7 @@ describe('PeriodCombiner', () => { }); it('handles image track gaps', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -778,7 +778,7 @@ describe('PeriodCombiner', () => { }); it('Disjoint audio channels', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -826,7 +826,7 @@ describe('PeriodCombiner', () => { return stream; }; - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -874,7 +874,7 @@ describe('PeriodCombiner', () => { return stream; }; - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -918,7 +918,7 @@ describe('PeriodCombiner', () => { const newCodec = makeVideoStream(720); newCodec.codecs = 'foo.abcd'; - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -970,7 +970,7 @@ describe('PeriodCombiner', () => { stream4.codecs = 'mp4a.40.2'; stream4.roles = ['description']; - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '0', @@ -1052,7 +1052,7 @@ describe('PeriodCombiner', () => { stream8.bandwidth = 120000; stream8.codecs = 'vp09.01.20.08.01'; - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '0', @@ -1110,7 +1110,7 @@ describe('PeriodCombiner', () => { const stream4 = makeAudioStreamWithRoles(['role1']); stream4.originalId = 'stream4'; - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -1152,7 +1152,7 @@ describe('PeriodCombiner', () => { }); it('Matches streams with roles in common', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1', @@ -1204,7 +1204,7 @@ describe('PeriodCombiner', () => { }); it('Matches streams with mismatched roles', async () => { - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '0', @@ -1381,7 +1381,7 @@ describe('PeriodCombiner', () => { v20.frameRate = 24000/1001; v20.bandwidth = 570005990000; - /** @type {!Array.} */ + /** @type {!Array.} */ const periods = [ { id: '1',