Skip to content

Commit

Permalink
fix(FEC-7935): fix text selection logic with hlsjs (#65)
Browse files Browse the repository at this point in the history
this is basically the same as before, but hls.js run different code paths when we use it's API for selecting text track.
Before this change we used to set the showing/hidden mode attribute of the text track in playkit-js html5 engine. we now set this in HLS.JS which internally does the same but also it manages internal state for this and so it's better to use its API and not access the textTrack.mode directly.
This fix depends on a fix in kaltura/playkit-js#263
  • Loading branch information
OrenMe authored Jul 12, 2018
1 parent eec593d commit 3074d19
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 38 deletions.
21 changes: 6 additions & 15 deletions src/hls-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ export default class HlsAdapter extends BaseMediaSourceAdapter {
adapterConfig.hlsConfig.startPosition = config.playback.startTime;
}
}
if (Utils.Object.hasPropertyPath(config, 'playback.useNativeTextTrack')) {
adapterConfig.subtitleDisplay = Utils.Object.getPropertyPath(config, 'playback.useNativeTextTrack');
}
return new this(videoElement, source, adapterConfig);
}

Expand Down Expand Up @@ -202,6 +205,7 @@ export default class HlsAdapter extends BaseMediaSourceAdapter {
this._config.hlsConfig['pLoader'] = pLoader;
}
this._hls = new Hlsjs(this._config.hlsConfig);
this._hls.subtitleDisplay = this._config.subtitleDisplay;
this._addBindings();
}

Expand Down Expand Up @@ -479,8 +483,7 @@ export default class HlsAdapter extends BaseMediaSourceAdapter {
*/
selectTextTrack(textTrack: TextTrack): void {
if (textTrack instanceof TextTrack && !textTrack.active && this._videoElement.textTracks) {
this._disableAllTextTracks();
this._videoElement.textTracks[textTrack.index].mode = 'hidden';
this._hls.subtitleTrack = textTrack.index;
HlsAdapter._logger.debug('Text track changed', textTrack);
this._onTrackChanged(textTrack);
}
Expand All @@ -492,7 +495,7 @@ export default class HlsAdapter extends BaseMediaSourceAdapter {
* @public
*/
hideTextTrack(): void {
this._disableAllTextTracks();
this._hls.subtitleTrack = -1;
}

/**
Expand Down Expand Up @@ -644,18 +647,6 @@ export default class HlsAdapter extends BaseMediaSourceAdapter {
}
}

/**
* Disables all the video tag text tracks.
* @returns {void}
* @private
*/
_disableAllTextTracks() {
let vidTextTracks = this._videoElement.textTracks;
for (let i = 0; i < vidTextTracks.length; i++) {
vidTextTracks[i].mode = 'disabled';
}
}

/**
* Handles hls errors.
* @param {any} data - The event data object.
Expand Down
23 changes: 0 additions & 23 deletions test/src/hls-adapter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,29 +210,6 @@ describe('HlsAdapter Instance - Unit', function () {
JSON.parse(JSON.stringify(tracks)).should.deep.equal(allTracks);
});

it('should disable all text tracks', function () {
hlsAdapterInstance._videoElement = {
textTracks: hls_tracks.subtitles,
removeEventListener: () => {}
};
hlsAdapterInstance._disableAllTextTracks();
for (let i = 0; i < hlsAdapterInstance._videoElement.textTracks.length; i++) {
hlsAdapterInstance._videoElement.textTracks[i].mode.should.equal('disabled');
}
});

it('should hide the active text track', function () {
hlsAdapterInstance._videoElement = {
textTracks: hls_tracks.subtitles,
removeEventListener: () => {}
};
hlsAdapterInstance._videoElement.textTracks[0].mode = 'showing';
hlsAdapterInstance.hideTextTrack();
for (let i = 0; i < hlsAdapterInstance._videoElement.textTracks.length; i++) {
hlsAdapterInstance._videoElement.textTracks[i].mode.should.equal('disabled');
}
});

it('should enable adaptive bitrate', function () {
hlsAdapterInstance._hls = {
on: function () {
Expand Down

0 comments on commit 3074d19

Please sign in to comment.