Skip to content

Commit

Permalink
fix(FEC-11697): 2 captions are selected in the menu (#633)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@JonathanTGold
JonathanTGold authored Feb 20, 2022
1 parent d23026b commit bf28e11
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 82 deletions.
29 changes: 17 additions & 12 deletions src/engines/html5/media-source/adapters/native-adapter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export default class NativeAdapter extends BaseMediaSourceAdapter {
* @type {number}
* @private
*/
_nativeTextTracksMap = [];
_nativeTextTracksMap = {};
/**
*
* @type {number}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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])) {
Expand All @@ -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];
}
}
}
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@ export default class Player extends FakeEventTarget {
this._activeTextCues = [];
this._updateTextDisplay([]);
this._tracks = [];
TextTrack.reset();
this._resetStateFlags();
this._engineType = '';
this._streamType = '';
Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions src/track/external-captions-handler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
Expand Down Expand Up @@ -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,
Expand Down
23 changes: 23 additions & 0 deletions src/track/text-track.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
}
};

Expand Down
95 changes: 55 additions & 40 deletions test/src/engines/html5/media-source/adapters/native-adapter.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -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 => {
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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);
});
});
});

Expand Down
Loading

0 comments on commit bf28e11

Please sign in to comment.