feat(FEC-8390, FEC-8246): support 608/708 captions #67
Conversation
yairans
changed the title
src/hls-adapter.js
Outdated
| if (textTrack instanceof TextTrack && !textTrack.active) { | ||
| if (this._hls.subtitleTracks.length) { | ||
| this._hls.subtitleTrack = textTrack.id; | ||
| HlsAdapter._logger.debug('Text track changed', textTrack); |
There was a problem hiding this comment.
The structure here is a bit inconsistent.
In if clause you have a condition and then u select a track via hls.js API and call
HlsAdapter._logger.debug('Text track changed', textTrack);
this._onTrackChanged(textTrack);
```javascript
and then in else clause you have a call to method that inside it also has exact same logic and sequence but just for native tracks.
I prefer we make it consistent:
if (condition) {
//...
} else if (condition) {
//...
}
I would even say you should also return a boolean or something from both methods and then conditionally call:
if (boolean) {
HlsAdapter._logger.debug('Text track changed', textTrack);
this._onTrackChanged(textTrack);
}so you only have these two lines once and not twice like now.
src/hls-adapter.js
Outdated
| _parseNativeTextTracks(nativeTextTracks: Array<Object>): Array<TextTrack> { | ||
| let textTracks = []; | ||
| for (let i = 0; i < nativeTextTracks.length; i++) { | ||
| if (nativeTextTracks[i].cues && nativeTextTracks[i].cues.length && |
There was a problem hiding this comment.
why do you have this condition? you already check for kind so metadata tracks won't be accounted for.
It's better you clear the cues on destroy of the adapter.
Also, side note:
const track = nativeTextTracks[i];There was a problem hiding this comment.
This condition used to ignore old tracks in change media
src/hls-adapter.js
Outdated
| this._onRecoveredCallback = () => this._onRecovered(); | ||
| this._onVideoErrorCallback = (e) => this._onVideoError(e); | ||
| this._videoElement.addEventListener(EventType.ERROR, this._onVideoErrorCallback); | ||
| } | ||
|
|
||
| _onFragParsed(): void { | ||
| // parse 608/708 captions that not exposed on hls.subtitleTracks | ||
| const textTrack = this._playerTracks.find((track) => { |
There was a problem hiding this comment.
do you mean here that we stop in the following case - if we get tracks from hls.js API we check here if we already got them so we remove this listener?
If so why don't also remove the listener in the else clause if you found nativeTextTracks
There was a problem hiding this comment.
it will be removed in the next time of FRAG_PARSED
…nd (#67) * realying on loaded metadata instead of shaka loaded * change to readyState > 0 readystate=1 means metadata loaded. on the first play, it already goes to the live edge. * fix test
Description of the Changes
parse 608/708 tracks by the video element
textTrackapidepends on
kaltura/playkit-js#265
kaltura/playkit-js-ui#254
CheckLists