From bf28e111aed3ec992728a2e34529c95eeec974aa Mon Sep 17 00:00:00 2001 From: JonathanTGold <62672270+JonathanTGold@users.noreply.github.com> Date: Sun, 20 Feb 2022 16:05:55 +0200 Subject: [PATCH] fix(FEC-11697): 2 captions are selected in the menu (#633) issue: the create _addTextTrackOffOption method of the player creates duplicated indexes every time duplicated languages are detected (and warning created); fix: right now the index property of TextTrack is used as a unique identifier but in the other hand we don't mange it as such (the index is created in different places), so , the index management all moved to one place under the TextTrack class related pr: kaltura/playkit-js-dash#182 kaltura/playkit-js-hls#161 solves: FEC-11697 & FEC-11893 & FEC-11973 --- .../media-source/adapters/native-adapter.js | 29 +++--- src/player.js | 2 +- src/track/external-captions-handler.js | 7 +- src/track/text-track.js | 23 +++++ .../adapters/native-adapter.spec.js | 95 +++++++++++-------- test/src/player.spec.js | 66 ++++++++----- 6 files changed, 140 insertions(+), 82 deletions(-) diff --git a/src/engines/html5/media-source/adapters/native-adapter.js b/src/engines/html5/media-source/adapters/native-adapter.js index 202426b26..950437afb 100644 --- a/src/engines/html5/media-source/adapters/native-adapter.js +++ b/src/engines/html5/media-source/adapters/native-adapter.js @@ -149,7 +149,7 @@ export default class NativeAdapter extends BaseMediaSourceAdapter { * @type {number} * @private */ - _nativeTextTracksMap = []; + _nativeTextTracksMap = {}; /** * * @type {number} @@ -589,7 +589,7 @@ export default class NativeAdapter extends BaseMediaSourceAdapter { this._waitingEventTriggered = false; this._progressiveSources = []; this._loadPromise = null; - this._nativeTextTracksMap = []; + this._nativeTextTracksMap = {}; this._loadPromiseReject = null; this._liveEdge = 0; this._lastTimeUpdate = 0; @@ -721,7 +721,6 @@ export default class NativeAdapter extends BaseMediaSourceAdapter { const captionsTextTrackLanguageCodes = [this._config.captionsTextTrack1LanguageCode, this._config.captionsTextTrack2LanguageCode]; const textTracks = this._videoElement.textTracks; const parsedTracks = []; - let internalTrackIndex = 0; if (textTracks) { for (let i = 0; i < textTracks.length; i++) { if (!PKTextTrack.isExternalTrack(textTracks[i])) { @@ -730,18 +729,19 @@ export default class NativeAdapter extends BaseMediaSourceAdapter { active: textTracks[i].mode === PKTextTrack.MODE.SHOWING, label: textTracks[i].label, language: textTracks[i].language, - available: true, - index: internalTrackIndex++ + available: true }; if (settings.kind === PKTextTrack.KIND.SUBTITLES) { - parsedTracks.push(new PKTextTrack(settings)); - this._nativeTextTracksMap[settings.index] = textTracks[i]; + const newTrack: PKTextTrack = new PKTextTrack(settings); + parsedTracks.push(newTrack); + this._nativeTextTracksMap[newTrack.index] = textTracks[i]; } else if (settings.kind === PKTextTrack.KIND.CAPTIONS && this._config.enableCEA708Captions) { settings.label = settings.label || captionsTextTrackLabels.shift(); settings.language = settings.language || captionsTextTrackLanguageCodes.shift(); settings.available = this._captionsHidden; - parsedTracks.push(new PKTextTrack(settings)); - this._nativeTextTracksMap[settings.index] = textTracks[i]; + const newTrack: PKTextTrack = new PKTextTrack(settings); + parsedTracks.push(newTrack); + this._nativeTextTracksMap[newTrack.index] = textTracks[i]; } } } @@ -976,10 +976,15 @@ export default class NativeAdapter extends BaseMediaSourceAdapter { _onNativeTextTrackChange(): void { const pkTextTracks = this._getPKTextTracks(); const pkOffTrack = pkTextTracks.find(track => track.language === 'off'); - const getActiveVidTextTrackIndex = () => { - return this._nativeTextTracksMap.findIndex(textTrack => textTrack && this._getDisplayTextTrackModeString() === textTrack.mode); + const getActiveVidTextTrackIndex = (): number => { + for (const textTrackId in this._nativeTextTracksMap) { + if (this._getDisplayTextTrackModeString() === this._nativeTextTracksMap[textTrackId].mode) { + return Number(textTrackId); + } + } + return -1; }; - const vidIndex = getActiveVidTextTrackIndex(); + const vidIndex: number = getActiveVidTextTrackIndex(); const activePKtextTrack = this._getActivePKTextTrack(); const pkIndex = activePKtextTrack ? activePKtextTrack.index : -1; if (vidIndex !== pkIndex) { diff --git a/src/player.js b/src/player.js index a10b84b46..10b78b9de 100644 --- a/src/player.js +++ b/src/player.js @@ -657,6 +657,7 @@ export default class Player extends FakeEventTarget { this._activeTextCues = []; this._updateTextDisplay([]); this._tracks = []; + TextTrack.reset(); this._resetStateFlags(); this._engineType = ''; this._streamType = ''; @@ -2527,7 +2528,6 @@ export default class Player extends FakeEventTarget { this._tracks.push( new TextTrack({ active: false, - index: textTracks.length, kind: TextTrack.KIND.SUBTITLES, label: 'Off', language: OFF diff --git a/src/track/external-captions-handler.js b/src/track/external-captions-handler.js index e05296222..c3b846249 100644 --- a/src/track/external-captions-handler.js +++ b/src/track/external-captions-handler.js @@ -124,14 +124,13 @@ class ExternalCaptionsHandler extends FakeEventTarget { this._addNativeTextTrack(); } const playerTextTracks = tracks.filter(track => track instanceof TextTrack); - let textTracksLength = playerTextTracks.length || 0; const newTextTracks = []; captions.forEach(caption => { if (!caption.language) { const error = new Error(Error.Severity.RECOVERABLE, Error.Category.TEXT, Error.Code.UNKNOWN_LANGUAGE, {caption: caption}); this.dispatchEvent(new FakeEvent(Html5EventType.ERROR, error)); } else { - const track = this._createTextTrack(caption, textTracksLength++); + const track = this._createTextTrack(caption); this._maybeAddTrack(track, caption, playerTextTracks, newTextTracks); } }); @@ -160,14 +159,12 @@ class ExternalCaptionsHandler extends FakeEventTarget { /** * creates a new text track * @param {PKExternalCaptionObject} caption - caption to create the text track with - * @param {number} index - index of the text track * @returns {TextTrack} - new text track * @private */ - _createTextTrack(caption: PKExternalCaptionObject, index: number): TextTrack { + _createTextTrack(caption: PKExternalCaptionObject): TextTrack { return new TextTrack({ active: !!caption.default, - index: index, kind: TextTrack.KIND.SUBTITLES, label: caption.label, language: caption.language, diff --git a/src/track/text-track.js b/src/track/text-track.js index d43a4535b..ad2c17b83 100644 --- a/src/track/text-track.js +++ b/src/track/text-track.js @@ -16,6 +16,28 @@ const TextTrack: TextTrack = class TextTrack extends Track { isNativeTextTrack: Function; isExternalTrack: Function; + /** + * use as a uniq identifier of the track. + * @static + * @type {number} + * @private + */ + static _tracksCount: number = 0; + /** + * index generator. + * @returns {number} - the next track index. + */ + static _generateIndex(): number { + return TextTrack._tracksCount++; + } + /** + * reset the track count. + * @returns {void} + */ + static reset(): void { + TextTrack._tracksCount = 0; + } + /** * The kind of the text track: * subtitles/captions/metadata. @@ -60,6 +82,7 @@ const TextTrack: TextTrack = class TextTrack extends Track { this._label = this.label || this.language; this._kind = settings.kind; this._external = settings.external; + this._index = TextTrack._generateIndex(); } }; diff --git a/test/src/engines/html5/media-source/adapters/native-adapter.spec.js b/test/src/engines/html5/media-source/adapters/native-adapter.spec.js index a0aee46cc..cb703df99 100644 --- a/test/src/engines/html5/media-source/adapters/native-adapter.spec.js +++ b/test/src/engines/html5/media-source/adapters/native-adapter.spec.js @@ -634,20 +634,25 @@ describe('NativeAdapter: selectTextTrack', function () { event.payload.selectedTextTrack.language.should.equal('fr'); done(); }; - nativeInstance.load().then(() => { - if (nativeInstance._videoElement.textTracks) { - nativeInstance.addEventListener(CustomEventType.TEXT_TRACK_CHANGED, onTextTrackChanged); - nativeInstance._videoElement.textTracks[0].mode.should.be.equal('showing'); - nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); - nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); - nativeInstance.selectTextTrack(new TextTrack({index: 2, language: 'fr', kind: 'subtitles'})); - nativeInstance._videoElement.textTracks[0].mode.should.be.equal('disabled'); - nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); - nativeInstance._videoElement.textTracks[2].mode.should.be.equal('hidden'); - } else { + nativeInstance + .load() + .then(() => { + if (nativeInstance._videoElement.textTracks) { + nativeInstance.addEventListener(CustomEventType.TEXT_TRACK_CHANGED, onTextTrackChanged); + nativeInstance._videoElement.textTracks[0].mode.should.be.equal('showing'); + nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); + nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); + nativeInstance.selectTextTrack(nativeInstance._playerTracks[1]); + nativeInstance._videoElement.textTracks[0].mode.should.be.equal('disabled'); + nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); + nativeInstance._videoElement.textTracks[2].mode.should.be.equal('hidden'); + } else { + done(); + } + }) + .catch(() => { done(); - } - }); + }); }); it('should select a new captions track', done => { @@ -656,20 +661,25 @@ describe('NativeAdapter: selectTextTrack', function () { event.payload.selectedTextTrack.language.should.equal('fr'); done(); }; - nativeInstance.load().then(() => { - if (nativeInstance._videoElement.textTracks) { - nativeInstance.addEventListener(CustomEventType.TEXT_TRACK_CHANGED, onTextTrackChanged); - nativeInstance._videoElement.textTracks[0].mode.should.be.equal('showing'); - nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); - nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); - nativeInstance.selectTextTrack(new TextTrack({index: 2, language: 'fr', kind: 'captions'})); - nativeInstance._videoElement.textTracks[0].mode.should.be.equal('disabled'); - nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); - nativeInstance._videoElement.textTracks[2].mode.should.be.equal('hidden'); - } else { + nativeInstance + .load() + .then(() => { + if (nativeInstance._videoElement.textTracks) { + nativeInstance.addEventListener(CustomEventType.TEXT_TRACK_CHANGED, onTextTrackChanged); + nativeInstance._videoElement.textTracks[0].mode.should.be.equal('showing'); + nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); + nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); + nativeInstance.selectTextTrack(nativeInstance._playerTracks[1]); + nativeInstance._videoElement.textTracks[0].mode.should.be.equal('disabled'); + nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); + nativeInstance._videoElement.textTracks[2].mode.should.be.equal('hidden'); + } else { + done(); + } + }) + .catch(() => { done(); - } - }); + }); }); it('should not change the selected text track', done => { @@ -680,7 +690,7 @@ describe('NativeAdapter: selectTextTrack', function () { nativeInstance._videoElement.textTracks[0].mode.should.be.equal('showing'); nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); - nativeInstance.selectTextTrack(new TextTrack({index: 0, language: 'en', kind: 'subtitles'})); + nativeInstance.selectTextTrack(nativeInstance._playerTracks[0]); nativeInstance._videoElement.textTracks[0].mode.should.be.equal('hidden'); nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); @@ -700,7 +710,7 @@ describe('NativeAdapter: selectTextTrack', function () { nativeInstance._videoElement.textTracks[0].mode.should.be.equal('showing'); nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); - nativeInstance.selectTextTrack(new TextTrack({index: 3, kind: 'subtitles'})); + nativeInstance.selectTextTrack(new TextTrack({kind: 'subtitles'})); nativeInstance._videoElement.textTracks[0].mode.should.be.equal('showing'); nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); @@ -713,18 +723,23 @@ describe('NativeAdapter: selectTextTrack', function () { }); it('should not change the selected for metadata text track', done => { - nativeInstance.load().then(() => { - if (nativeInstance._videoElement.textTracks) { - nativeInstance._videoElement.textTracks[0].mode.should.be.equal('showing'); - nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); - nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); - nativeInstance.selectTextTrack(new TextTrack({index: 1, kind: 'subtitles'})); - nativeInstance._videoElement.textTracks[0].mode.should.be.equal('showing'); - nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); - nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); - } - done(); - }); + nativeInstance + .load() + .then(() => { + if (nativeInstance._videoElement.textTracks) { + nativeInstance._videoElement.textTracks[0].mode.should.be.equal('showing'); + nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); + nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); + nativeInstance.selectTextTrack(new TextTrack({kind: 'subtitles'})); + nativeInstance._videoElement.textTracks[0].mode.should.be.equal('showing'); + nativeInstance._videoElement.textTracks[1].mode.should.be.equal('disabled'); + nativeInstance._videoElement.textTracks[2].mode.should.be.equal('disabled'); + } + done(); + }) + .catch(e => { + done(e); + }); }); }); diff --git a/test/src/player.spec.js b/test/src/player.spec.js index 5081a1edd..398de73cf 100644 --- a/test/src/player.spec.js +++ b/test/src/player.spec.js @@ -19,6 +19,7 @@ import {EngineProvider} from '../../src/engines/engine-provider'; import FakeEvent from '../../src/event/fake-event'; import Html5AutoPlayCapability from '../../src/engines/html5/capabilities/html5-autoplay'; import * as Utils from '../../src/utils/util'; +import {TrackType} from '../../src/track/track-type'; const targetId = 'player-placeholder_player.spec'; let sourcesConfig = PKObject.copyDeep(SourcesConfig); @@ -1110,6 +1111,7 @@ describe('Player', function () { video = player._engine.getVideoElement(); video.addTextTrack('subtitles', 'English', 'en'); video.addTextTrack('subtitles', 'French', 'fr'); + video.addTextTrack('captions', 'German', 'de'); }); afterEach(() => { @@ -1141,7 +1143,8 @@ describe('Player', function () { video.textTracks[1].mode.should.be.equal('disabled'); tracks[0].active.should.be.true; tracks[1].active.should.be.false; - player.selectTrack(new TextTrack({language: 'fr', kind: 'subtitles', index: 1})); + const track = player.getTracks(TrackType.TEXT).find(track => track.language === 'fr'); + player.selectTrack(track); }) .catch(e => { done(e); @@ -1150,26 +1153,39 @@ describe('Player', function () { }); it('should select a new captions track', done => { - player.load(); - player.ready().then(() => { - player.addEventListener(CustomEventType.TEXT_TRACK_CHANGED, event => { - (event.payload.selectedTextTrack instanceof TextTrack).should.be.true; - event.payload.selectedTextTrack.index.should.equal(1); - video.textTracks[0].mode.should.be.equal('disabled'); - video.textTracks[1].mode.should.be.equal('hidden'); - tracks[0].active.should.be.false; - tracks[1].active.should.be.true; - done(); - }); - let tracks = player._tracks.filter(track => { - return track instanceof TextTrack; + player + .ready() + .then(() => { + let selectedTrackIndex; + player.addEventListener(CustomEventType.TEXT_TRACK_CHANGED, event => { + try { + (event.payload.selectedTextTrack instanceof TextTrack).should.be.true; + event.payload.selectedTextTrack.index.should.equal(selectedTrackIndex); + video.textTracks[0].mode.should.be.equal('disabled'); + video.textTracks[2].mode.should.be.equal('hidden'); + tracks[0].active.should.be.false; + tracks[2].active.should.be.true; + done(); + } catch (e) { + done(e); + } + }); + let tracks = player._tracks.filter(track => { + return track instanceof TextTrack; + }); + video.textTracks[0].mode.should.be.equal('hidden'); + video.textTracks[1].mode.should.be.equal('disabled'); + tracks[0].active.should.be.true; + tracks[1].active.should.be.false; + const track = player._tracks.find(track => track.language === 'de'); + track.available = true; + selectedTrackIndex = track.index; + player.selectTrack(track); + }) + .catch(e => { + done(e); }); - video.textTracks[0].mode.should.be.equal('hidden'); - video.textTracks[1].mode.should.be.equal('disabled'); - tracks[0].active.should.be.true; - tracks[1].active.should.be.false; - player.selectTrack(new TextTrack({index: 1, kind: 'captions', language: 'fr'})); - }); + player.load(); }); it('should not change the selected text track', done => { @@ -1181,7 +1197,7 @@ describe('Player', function () { video.textTracks[1].mode.should.be.equal('disabled'); tracks[0].active.should.be.true; tracks[1].active.should.be.false; - player.selectTrack(new TextTrack({index: 0, kind: 'subtitles'})); + player.selectTrack(player.getTracks(TrackType.TEXT).find(track => track.language === 'en')); video.textTracks[0].mode.should.be.equal('hidden'); video.textTracks[1].mode.should.be.equal('disabled'); tracks[0].active.should.be.true; @@ -1201,7 +1217,7 @@ describe('Player', function () { video.textTracks[1].mode.should.be.equal('disabled'); tracks[0].active.should.be.true; tracks[1].active.should.be.false; - player.selectTrack(new TextTrack({index: 3, kind: 'subtitles'})); + player.selectTrack(new TextTrack({language: 'asp'})); video.textTracks[0].mode.should.be.equal('hidden'); video.textTracks[1].mode.should.be.equal('disabled'); tracks[0].active.should.be.true; @@ -1211,6 +1227,7 @@ describe('Player', function () { }); it('should not change the selected for metadata text track', done => { + player.addTextTrack('metadata', 'metadata', 'metadata'); player.ready().then(() => { let tracks = player._tracks.filter(track => { return track instanceof TextTrack; @@ -1219,7 +1236,8 @@ describe('Player', function () { video.textTracks[1].mode.should.be.equal('disabled'); tracks[0].active.should.be.true; tracks[1].active.should.be.false; - player.selectTrack(new TextTrack({index: 1, kind: 'metadata'})); + const track = Object.values(player.getVideoElement().textTracks).find(track => track.kind === 'metadata'); + player.selectTrack(track); video.textTracks[0].mode.should.be.equal('hidden'); video.textTracks[1].mode.should.be.equal('disabled'); tracks[0].active.should.be.true; @@ -1297,7 +1315,7 @@ describe('Player', function () { if (audioTracks.length) { player.getActiveTracks().audio.should.deep.equals(audioTracks[0]); } - player.selectTrack(new TextTrack({index: 1, kind: 'subtitles'})); + player.selectTrack(player.getTracks(TrackType.TEXT).find(track => track.language === 'fr')); }); player.load(); });