Skip to content

Commit f9c8028

Browse files
authored
fix: External text tracks in src mode related issues (#8527)
All changes were part of #8520 - Object URLs created in `addSrcTrackElement_()` are now released correctly - Use `HTMLTrackElement.track` to access its associated `TextTrack`. Searching in `HTMLMediaElement.textTracks` is not reliable - Remove only `<source>` elements during initialization. User added `<track>` elements are not harmful and they may want to keep them for dual subtitles for example.
1 parent bd97fb1 commit f9c8028

File tree

2 files changed

+43
-22
lines changed

2 files changed

+43
-22
lines changed

lib/player.js

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,9 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
918918
/** @private {boolean} */
919919
this.preloadDueAdManagerVideoEnded_ = false;
920920

921+
/** @private {!Array<HTMLTrackElement>} */
922+
this.externalSrcEqualsTextTracks_ = [];
923+
921924
/** @private {shaka.util.Timer} */
922925
this.preloadDueAdManagerTimer_ = new shaka.util.Timer(async () => {
923926
if (this.preloadDueAdManager_) {
@@ -1529,17 +1532,23 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
15291532
}
15301533

15311534
if (this.video_) {
1532-
// Remove all track nodes
1533-
shaka.util.Dom.removeAllChildren(this.video_);
1535+
// The life cycle of tracks that created by addTextTrackAsync() and
1536+
// their associated resources should be the same as the loaded video.
1537+
for (const trackNode of this.externalSrcEqualsTextTracks_) {
1538+
if (trackNode.src.startsWith('blob:')) {
1539+
URL.revokeObjectURL(trackNode.src);
1540+
}
1541+
trackNode.remove();
1542+
}
1543+
this.externalSrcEqualsTextTracks_ = [];
15341544

15351545
// In order to unload a media element, we need to remove the src
15361546
// attribute and then load again. When we destroy media source engine,
15371547
// this will be done for us, but for src=, we need to do it here.
15381548
//
15391549
// DrmEngine requires this to be done before we destroy DrmEngine
15401550
// itself.
1541-
if (this.video_.src) {
1542-
this.video_.removeAttribute('src');
1551+
if (shaka.util.Dom.clearSourceFromVideo(this.video_)) {
15431552
this.video_.load();
15441553
}
15451554
}
@@ -2518,8 +2527,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
25182527

25192528
// Remove children if we had any, i.e. from previously used src= mode.
25202529
if (this.config_.mediaSource.useSourceElements) {
2521-
this.video_.removeAttribute('src');
2522-
shaka.util.Dom.removeAllChildren(this.video_);
2530+
shaka.util.Dom.clearSourceFromVideo(this.video_);
25232531
}
25242532

25252533
this.createTextDisplayer_();
@@ -3188,7 +3196,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
31883196
await this.mediaSourceEngine_.destroy();
31893197
this.mediaSourceEngine_ = null;
31903198
}
3191-
shaka.util.Dom.removeAllChildren(mediaElement);
3199+
shaka.util.Dom.clearSourceFromVideo(mediaElement);
31923200

31933201
mediaElement.src = playbackUri;
31943202

@@ -6669,25 +6677,15 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
66696677
}
66706678

66716679
if (this.loadMode_ == shaka.Player.LoadMode.SRC_EQUALS) {
6672-
if (forced) {
6680+
if (forced && shaka.util.Platform.isApple()) {
66736681
// See: https://github.com/whatwg/html/issues/4472
66746682
kind = 'forced';
66756683
}
6676-
await this.addSrcTrackElement_(uri, language, kind, mimeType, label || '',
6677-
adCuePoints);
6678-
6679-
const LanguageUtils = shaka.util.LanguageUtils;
6680-
const languageNormalized = LanguageUtils.normalize(language);
6681-
6682-
const textTracks = this.getTextTracks();
6683-
const srcTrack = textTracks.find((t) => {
6684-
return LanguageUtils.normalize(t.language) == languageNormalized &&
6685-
t.label == (label || '') &&
6686-
t.kind == kind;
6687-
});
6688-
if (srcTrack) {
6684+
const trackNode = await this.addSrcTrackElement_(uri, language, kind,
6685+
mimeType, label || '', adCuePoints);
6686+
if (trackNode.track) {
66896687
this.onTracksChanged_();
6690-
return srcTrack;
6688+
return shaka.util.StreamUtils.html5TextTrackToTrack(trackNode.track);
66916689
}
66926690
// This should not happen, but there are browser implementations that may
66936691
// not support the Track element.
@@ -7136,6 +7134,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
71367134
}
71377135

71387136
this.video_.appendChild(trackElement);
7137+
this.externalSrcEqualsTextTracks_.push(trackElement);
71397138
return trackElement;
71407139
}
71417140

lib/util/dom_utils.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,28 @@ shaka.util.Dom = class {
5656
}
5757

5858

59+
/**
60+
* Remove all source elements and src attribute from a video element.
61+
* Returns true if any change was made.
62+
* @param {!HTMLMediaElement} video
63+
* @export
64+
* @return {boolean}
65+
*/
66+
static clearSourceFromVideo(video) {
67+
let result = false;
68+
const sources = video.getElementsByTagName('source');
69+
for (let i = sources.length - 1; i >= 0; --i) {
70+
video.removeChild(sources[i]);
71+
result = true;
72+
}
73+
if (video.src) {
74+
video.removeAttribute('src');
75+
result = true;
76+
}
77+
return result;
78+
}
79+
80+
5981
/**
6082
* Cast a Node/Element to an HTMLElement
6183
*

0 commit comments

Comments
 (0)