diff --git a/lib/media/media_source_engine.js b/lib/media/media_source_engine.js
index 0a23404047..dc078a6f93 100644
--- a/lib/media/media_source_engine.js
+++ b/lib/media/media_source_engine.js
@@ -407,9 +407,6 @@ shaka.media.MediaSourceEngine = class {
if (this.textEngine_) {
cleanup.push(this.textEngine_.destroy());
}
- if (this.textDisplayer_) {
- cleanup.push(this.textDisplayer_.destroy());
- }
for (const contentType in this.transmuxers_) {
cleanup.push(this.transmuxers_[contentType].destroy());
@@ -1746,12 +1743,7 @@ shaka.media.MediaSourceEngine = class {
* @param {!shaka.extern.TextDisplayer} textDisplayer
*/
setTextDisplayer(textDisplayer) {
- const oldTextDisplayer = this.textDisplayer_;
this.textDisplayer_ = textDisplayer;
- if (oldTextDisplayer) {
- textDisplayer.setTextVisibility(oldTextDisplayer.isTextVisible());
- oldTextDisplayer.destroy();
- }
if (this.textEngine_) {
this.textEngine_.setDisplayer(textDisplayer);
}
diff --git a/lib/player.js b/lib/player.js
index fcf23fa66e..a041c888eb 100644
--- a/lib/player.js
+++ b/lib/player.js
@@ -38,6 +38,7 @@ goog.require('shaka.net.NetworkingUtils');
goog.require('shaka.text.SimpleTextDisplayer');
goog.require('shaka.text.StubTextDisplayer');
goog.require('shaka.text.TextEngine');
+goog.require('shaka.text.Utils');
goog.require('shaka.text.UITextDisplayer');
goog.require('shaka.text.WebVttGenerator');
goog.require('shaka.util.BufferUtils');
@@ -50,6 +51,7 @@ goog.require('shaka.util.Error');
goog.require('shaka.util.EventManager');
goog.require('shaka.util.FakeEvent');
goog.require('shaka.util.FakeEventTarget');
+goog.require('shaka.util.Functional');
goog.require('shaka.util.IDestroyable');
goog.require('shaka.util.LanguageUtils');
goog.require('shaka.util.ManifestParserUtils');
@@ -889,6 +891,9 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
'use the attach method instead.');
this.attach(mediaElement, /* initializeMediaSource= */ true);
}
+
+ /** @private {?shaka.extern.TextDisplayer} */
+ this.textDisplayer_ = null;
}
/**
@@ -1412,6 +1417,11 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
this.cmsdManager_.reset();
}
+ if (this.textDisplayer_) {
+ await this.textDisplayer_.destroy();
+ this.textDisplayer_ = null;
+ }
+
if (this.video_) {
// Remove all track nodes
shaka.util.Dom.removeAllChildren(this.video_);
@@ -2307,6 +2317,29 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
return false;
}
+ /**
+ * @private
+ */
+ createTextDisplayer_() {
+ // When changing text visibility we need to update both the text displayer
+ // and streaming engine because we don't always stream text. To ensure
+ // that the text displayer and streaming engine are always in sync, wait
+ // until they are both initialized before setting the initial value.
+ const textDisplayerFactory = this.config_.textDisplayFactory;
+ if (textDisplayerFactory === this.lastTextFactory_) {
+ return;
+ }
+ this.textDisplayer_ = textDisplayerFactory();
+ if (this.textDisplayer_.configure) {
+ this.textDisplayer_.configure(this.config_.textDisplayer);
+ } else {
+ shaka.Deprecate.deprecateFeature(5,
+ 'Text displayer w/ configure',
+ 'Text displayer should have a "configure" method!');
+ }
+ this.lastTextFactory_ = textDisplayerFactory;
+ }
+
/**
* Initializes the media source engine.
*
@@ -2327,24 +2360,12 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
this.makeStateChangeEvent_('media-source');
- // When changing text visibility we need to update both the text displayer
- // and streaming engine because we don't always stream text. To ensure
- // that the text displayer and streaming engine are always in sync, wait
- // until they are both initialized before setting the initial value.
- const textDisplayerFactory = this.config_.textDisplayFactory;
- const textDisplayer = textDisplayerFactory();
- if (textDisplayer.configure) {
- textDisplayer.configure(this.config_.textDisplayer);
- } else {
- shaka.Deprecate.deprecateFeature(5,
- 'Text displayer w/ configure',
- 'Text displayer should have a "configure" method!');
- }
- this.lastTextFactory_ = textDisplayerFactory;
-
+ this.createTextDisplayer_();
+ goog.asserts.assert(this.textDisplayer_,
+ 'Text displayer should be created already');
const mediaSourceEngine = this.createMediaSourceEngine(
this.video_,
- textDisplayer,
+ this.textDisplayer_,
{
getKeySystem: () => this.keySystem(),
onMetadata: (metadata, offset, endTime) => {
@@ -2851,6 +2872,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
}
if (mediaElement.textTracks) {
+ this.createTextDisplayer_();
this.loadEventManager_.listen(
mediaElement.textTracks, 'addtrack', (e) => {
const trackEvent = /** @type {!TrackEvent} */(e);
@@ -2881,6 +2903,22 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
this.loadEventManager_.listen(
mediaElement.textTracks, 'change',
() => this.onTracksChanged_());
+ this.loadEventManager_.listen(
+ mediaElement, 'enterpictureinpicture', () => {
+ const track = this.getFilteredTextTracks_()
+ .find((t) => t.mode !== 'disabled');
+ if (track) {
+ track.mode = 'showing';
+ }
+ });
+ this.loadEventManager_.listen(
+ mediaElement, 'leavepictureinpicture', () => {
+ const track = this.getFilteredTextTracks_()
+ .find((t) => t.mode !== 'disabled');
+ if (track) {
+ track.mode = 'hidden';
+ }
+ });
}
// By setting |src| we are done "loading" with src=. We don't need to set
@@ -2952,6 +2990,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
});
} else if (textTracks.length > 0) {
this.isTextVisible_ = true;
+ this.textDisplayer_.setTextVisibility(true);
}
// If we have moved on to another piece of content while waiting for
@@ -2959,6 +2998,20 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
if (unloaded) {
return;
}
+ let enabledNativeTrack = false;
+ for (const track of textTracks) {
+ if (track.mode !== 'disabled') {
+ if (!enabledNativeTrack) {
+ this.enableNativeTrack_(track);
+ enabledNativeTrack = true;
+ } else {
+ track.mode = 'disabled';
+ shaka.log.alwaysWarn(
+ 'Found more than one enabled text track, disabling it',
+ track);
+ }
+ }
+ }
this.setupPreferredTextOnSrc_();
});
@@ -3967,18 +4020,23 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
const {segmentRelativeVttTiming} = this.config_.manifest;
this.mediaSourceEngine_.setSegmentRelativeVttTiming(
segmentRelativeVttTiming);
+ }
+ if (this.textDisplayer_) {
const textDisplayerFactory = this.config_.textDisplayFactory;
if (this.lastTextFactory_ != textDisplayerFactory) {
- const displayer = textDisplayerFactory();
- if (displayer.configure) {
- displayer.configure(this.config_.textDisplayer);
+ const oldDisplayer = this.textDisplayer_;
+ this.textDisplayer_ = textDisplayerFactory();
+ if (this.textDisplayer_.configure) {
+ this.textDisplayer_.configure(this.config_.textDisplayer);
} else {
shaka.Deprecate.deprecateFeature(5,
'Text displayer w/ configure',
'Text displayer should have a "configure" method!');
}
- this.mediaSourceEngine_.setTextDisplayer(displayer);
+ this.textDisplayer_.setTextVisibility(oldDisplayer.isTextVisible());
+ oldDisplayer.destroy();
+ this.mediaSourceEngine_.setTextDisplayer(this.textDisplayer_);
this.lastTextFactory_ = textDisplayerFactory;
if (this.streamingEngine_) {
@@ -3986,12 +4044,12 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
this.streamingEngine_.reloadTextStream();
}
} else {
- const displayer = this.mediaSourceEngine_.getTextDisplayer();
- if (displayer.configure) {
- displayer.configure(this.config_.textDisplayer);
+ if (this.textDisplayer_.configure) {
+ this.textDisplayer_.configure(this.config_.textDisplayer);
}
}
}
+
if (this.abrManager_) {
this.abrManager_.configure(this.config_.abr);
// Simply enable/disable ABR with each call, since multiple calls to these
@@ -4837,20 +4895,42 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
this.currentTextLanguage_ = stream.language;
} else if (this.video_ && this.video_.src && this.video_.textTracks) {
const textTracks = this.getFilteredTextTracks_();
- for (const textTrack of textTracks) {
- if (shaka.util.StreamUtils.html5TrackId(textTrack) == track.id) {
- // Leave the track in 'hidden' if it's selected but not showing.
- textTrack.mode = this.isTextVisible_ ? 'showing' : 'hidden';
- } else {
- // Safari allows multiple text tracks to have mode == 'showing', so be
- // explicit in resetting the others.
- textTrack.mode = 'disabled';
+ const oldTrack = textTracks.find((textTrack) =>
+ textTrack.mode !== 'disabled');
+ const newTrack = textTracks.find((textTrack) =>
+ shaka.util.StreamUtils.html5TrackId(textTrack) === track.id);
+ if (oldTrack !== newTrack) {
+ if (oldTrack) {
+ oldTrack.mode = 'disabled';
+ this.loadEventManager_.unlisten(oldTrack, 'cuechange');
+ this.textDisplayer_.remove(0, Infinity);
+ }
+ if (newTrack) {
+ this.enableNativeTrack_(newTrack);
}
}
this.onTextChanged_();
}
}
+ /**
+ * @param {!TextTrack} track
+ * @private
+ */
+ enableNativeTrack_(track) {
+ this.loadEventManager_.listen(track, 'cuechange', () => {
+ // Always remove cues from the past to avoid memory grow.
+ const removeEnd = Math.max(0,
+ this.video_.currentTime - this.config_.streaming.bufferBehind);
+ this.textDisplayer_.remove(0, removeEnd);
+ const cues = Array.from(track.activeCues || [])
+ .map(shaka.text.Utils.mapNativeCueToShakaCue)
+ .filter(shaka.util.Functional.isNotNull);
+ this.textDisplayer_.append(cues);
+ });
+ track.mode = document.pictureInPictureElement ? 'showing' : 'hidden';
+ }
+
/**
* Select a specific variant track to play. track should come
* from a call to getVariantTracks. If track cannot
@@ -5167,20 +5247,14 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
*/
isTextTrackVisible() {
const expected = this.isTextVisible_;
-
- if (this.mediaSourceEngine_ &&
- this.loadMode_ == shaka.Player.LoadMode.MEDIA_SOURCE) {
- // Make sure our values are still in-sync.
- const actual = this.mediaSourceEngine_.getTextDisplayer().isTextVisible();
+ if (this.textDisplayer_) {
+ const actual = this.textDisplayer_.isTextVisible();
goog.asserts.assert(
actual == expected, 'text visibility has fallen out of sync');
// Always return the actual value so that the app has the most accurate
// information (in the case that the values come out of sync in prod).
return actual;
- } else if (this.video_ && this.video_.src && this.video_.textTracks) {
- const textTracks = this.getFilteredTextTracks_();
- return textTracks.some((t) => t.mode == 'showing');
}
return expected;
@@ -5309,8 +5383,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
// Hold of on setting the text visibility until we have all the components
// we need. This ensures that they stay in-sync.
if (this.loadMode_ == shaka.Player.LoadMode.MEDIA_SOURCE) {
- this.mediaSourceEngine_.getTextDisplayer()
- .setTextVisibility(newVisibility);
+ this.textDisplayer_.setTextVisibility(newVisibility);
// When the user wants to see captions, we stream captions. When the user
// doesn't want to see captions, we don't stream captions. This is to
@@ -5340,15 +5413,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
}
}
} else if (this.video_ && this.video_.src && this.video_.textTracks) {
- const textTracks = this.getFilteredTextTracks_();
- // Find the active track by looking for one which is not disabled. This
- // is the only way to identify the track which is currently displayed.
- // Set it to 'showing' or 'hidden' based on newVisibility.
- for (const textTrack of textTracks) {
- if (textTrack.mode != 'disabled') {
- textTrack.mode = newVisibility ? 'showing' : 'hidden';
- }
- }
+ this.textDisplayer_.setTextVisibility(newVisibility);
}
// We need to fire the event after we have updated everything so that
@@ -6962,7 +7027,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
if (this.isTextVisible_) {
// If the cached value says to show text, then update the text displayer
// since it defaults to not shown.
- this.mediaSourceEngine_.getTextDisplayer().setTextVisibility(true);
+ this.textDisplayer_.setTextVisibility(true);
goog.asserts.assert(this.shouldStreamText_(),
'Should be streaming text');
}
diff --git a/test/media/media_source_engine_unit.js b/test/media/media_source_engine_unit.js
index a72dbf3337..7fdd817d57 100644
--- a/test/media/media_source_engine_unit.js
+++ b/test/media/media_source_engine_unit.js
@@ -1467,12 +1467,6 @@ describe('MediaSourceEngine', () => {
expect(mockTextEngine).toBeTruthy();
expect(mockTextEngine.destroy).toHaveBeenCalled();
});
-
- // Regression test for https://github.com/shaka-project/shaka-player/issues/984
- it('destroys TextDisplayer on destroy', async () => {
- await mediaSourceEngine.destroy();
- expect(mockTextDisplayer.destroySpy).toHaveBeenCalled();
- });
});
function createMockMediaSource() {
diff --git a/test/player_unit.js b/test/player_unit.js
index 380d7de575..ae58612c8c 100644
--- a/test/player_unit.js
+++ b/test/player_unit.js
@@ -228,6 +228,7 @@ describe('Player', () => {
expect(playhead.release).toHaveBeenCalled();
expect(mediaSourceEngine.destroy).toHaveBeenCalled();
expect(streamingEngine.destroy).toHaveBeenCalled();
+ expect(textDisplayer.destroySpy).toHaveBeenCalled();
for (const segmentIndex of segmentIndexes) {
if (segmentIndex) {