Skip to content

fix: External text tracks in src mode related issues #8527

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 29, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 23 additions & 24 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,9 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
/** @private {boolean} */
this.preloadDueAdManagerVideoEnded_ = false;

/** @private {!Array<HTMLTrackElement>} */
this.externalSrcEqualsTextTracks_ = [];

/** @private {shaka.util.Timer} */
this.preloadDueAdManagerTimer_ = new shaka.util.Timer(async () => {
if (this.preloadDueAdManager_) {
Expand Down Expand Up @@ -1529,17 +1532,23 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
}

if (this.video_) {
// Remove all track nodes
shaka.util.Dom.removeAllChildren(this.video_);
// The life cycle of tracks that created by addTextTrackAsync() and
// their associated resources should be the same as the loaded video.
for (const trackNode of this.externalSrcEqualsTextTracks_) {
if (trackNode.src.startsWith('blob:')) {
URL.revokeObjectURL(trackNode.src);
}
trackNode.remove();
}
this.externalSrcEqualsTextTracks_ = [];

// In order to unload a media element, we need to remove the src
// attribute and then load again. When we destroy media source engine,
// this will be done for us, but for src=, we need to do it here.
//
// DrmEngine requires this to be done before we destroy DrmEngine
// itself.
if (this.video_.src) {
this.video_.removeAttribute('src');
if (shaka.util.Dom.clearSourceFromVideo(this.video_)) {
this.video_.load();
}
}
Expand Down Expand Up @@ -2518,8 +2527,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {

// Remove children if we had any, i.e. from previously used src= mode.
if (this.config_.mediaSource.useSourceElements) {
this.video_.removeAttribute('src');
shaka.util.Dom.removeAllChildren(this.video_);
shaka.util.Dom.clearSourceFromVideo(this.video_);
}

this.createTextDisplayer_();
Expand Down Expand Up @@ -3188,7 +3196,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
await this.mediaSourceEngine_.destroy();
this.mediaSourceEngine_ = null;
}
shaka.util.Dom.removeAllChildren(mediaElement);
shaka.util.Dom.clearSourceFromVideo(mediaElement);

mediaElement.src = playbackUri;

Expand Down Expand Up @@ -6668,25 +6676,15 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
}

if (this.loadMode_ == shaka.Player.LoadMode.SRC_EQUALS) {
if (forced) {
if (forced && shaka.util.Platform.isApple()) {
// See: https://github.com/whatwg/html/issues/4472
kind = 'forced';
}
await this.addSrcTrackElement_(uri, language, kind, mimeType, label || '',
adCuePoints);

const LanguageUtils = shaka.util.LanguageUtils;
const languageNormalized = LanguageUtils.normalize(language);

const textTracks = this.getTextTracks();
const srcTrack = textTracks.find((t) => {
return LanguageUtils.normalize(t.language) == languageNormalized &&
t.label == (label || '') &&
t.kind == kind;
});
if (srcTrack) {
const trackNode = await this.addSrcTrackElement_(uri, language, kind,
mimeType, label || '', adCuePoints);
if (trackNode.track) {
this.onTracksChanged_();
return srcTrack;
return shaka.util.StreamUtils.html5TextTrackToTrack(trackNode.track);
}
// This should not happen, but there are browser implementations that may
// not support the Track element.
Expand Down Expand Up @@ -7117,8 +7115,8 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
mimeType = 'text/vtt';
}

const trackElement =
/** @type {!HTMLTrackElement} */(document.createElement('track'));
const trackElement = /** @type {!HTMLTrackElement} */
(this.video_.ownerDocument.createElement('track'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use ownerDocument ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see: #8520 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avelad
This change has been reverted

trackElement.src = this.cmcdManager_.appendTextTrackData(uri);
trackElement.label = label;
trackElement.kind = kind;
Expand All @@ -7135,6 +7133,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
}

this.video_.appendChild(trackElement);
this.externalSrcEqualsTextTracks_.push(trackElement);
return trackElement;
}

Expand Down
22 changes: 22 additions & 0 deletions lib/util/dom_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,28 @@ shaka.util.Dom = class {
}


/**
* Remove all source elements and src attribute from a video element.
* Returns true if any change was made.
* @param {!HTMLMediaElement} video
* @export
* @return {boolean}
*/
static clearSourceFromVideo(video) {
let result = false;
const sources = video.getElementsByTagName('source');
for (let i = sources.length - 1; i >= 0; --i) {
video.removeChild(sources[i]);
result = true;
}
if (video.src) {
video.removeAttribute('src');
result = true;
}
return result;
}


/**
* Cast a Node/Element to an HTMLElement
*
Expand Down
Loading