From 97ad0635f708da8e53c74fa92b44ee5266026b2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Thu, 4 Jul 2024 14:19:59 +0200 Subject: [PATCH 01/14] feat: Render native cues using text displayer --- lib/media/media_source_engine.js | 8 -- lib/player.js | 136 +++++++++++++++---------- lib/text/text_utils.js | 40 ++++++++ test/media/media_source_engine_unit.js | 6 -- test/player_unit.js | 1 + test/text/text_utils_unit.js | 74 ++++++++++++++ 6 files changed, 200 insertions(+), 65 deletions(-) create mode 100644 test/text/text_utils_unit.js diff --git a/lib/media/media_source_engine.js b/lib/media/media_source_engine.js index 87a8431eca..a1ab1ea486 100644 --- a/lib/media/media_source_engine.js +++ b/lib/media/media_source_engine.js @@ -392,9 +392,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()); @@ -1687,12 +1684,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 83d461079e..99753b584f 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'); @@ -887,6 +888,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; } /** @@ -1410,6 +1414,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_); @@ -2305,6 +2314,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. * @@ -2325,24 +2357,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) => { @@ -2849,6 +2869,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); @@ -2950,6 +2971,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 @@ -2957,6 +2979,11 @@ shaka.Player = class extends shaka.util.FakeEventTarget { if (unloaded) { return; } + for (const track of textTracks) { + if (track.mode !== 'disabled') { + this.enableNativeTrack_(track); + } + } this.setupPreferredTextOnSrc_(); }); @@ -3956,18 +3983,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_) { @@ -3975,12 +4007,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 @@ -4826,20 +4858,37 @@ 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', () => { + const cues = Array.from(track.activeCues || []).map( + shaka.text.Utils.mapNativeCue); + this.textDisplayer_.append(cues); + }); + track.mode = 'hidden'; + } + /** * Select a specific variant track to play. track should come * from a call to getVariantTracks. If track cannot @@ -5152,20 +5201,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; @@ -5294,8 +5337,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 @@ -5325,15 +5367,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 @@ -6947,7 +6981,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/lib/text/text_utils.js b/lib/text/text_utils.js index eebb7a8871..458632d654 100644 --- a/lib/text/text_utils.js +++ b/lib/text/text_utils.js @@ -7,6 +7,7 @@ goog.provide('shaka.text.Utils'); goog.require('shaka.text.Cue'); +goog.require('shaka.text.CueRegion'); shaka.text.Utils = class { @@ -178,4 +179,43 @@ shaka.text.Utils = class { } return uniqueCues; } + + /** + * @param {!VTTCue} vttCue + * @return {!shaka.text.Cue} + */ + static mapNativeCue(vttCue) { + const cue = new shaka.text.Cue(vttCue.startTime, vttCue.endTime, + vttCue.text); + cue.line = typeof vttCue.line === 'number' ? vttCue.line : null; + cue.lineAlign = /** @type {shaka.text.Cue.lineAlign} */ (vttCue.lineAlign); + cue.lineInterpretation = vttCue.snapToLines ? + shaka.text.Cue.lineInterpretation.LINE_NUMBER : + shaka.text.Cue.lineInterpretation.PERCENTAGE; + cue.position = typeof vttCue.position === 'number' ? + vttCue.position : null; + cue.positionAlign = /** @type {shaka.text.Cue.positionAlign} */ + (vttCue.positionAlign); + cue.size = vttCue.size; + cue.textAlign = /** @type {shaka.text.Cue.textAlign} */ (vttCue.align); + if (vttCue.vertical === 'lr') { + cue.writingMode = shaka.text.Cue.writingMode.VERTICAL_LEFT_TO_RIGHT; + } else if (vttCue.vertical === 'rl') { + cue.writingMode = shaka.text.Cue.writingMode.VERTICAL_RIGHT_TO_LEFT; + } + if (vttCue.region) { + cue.region.id = vttCue.region.id; + cue.region.height = vttCue.region.lines; + cue.region.heightUnits = shaka.text.CueRegion.units.LINES; + cue.region.regionAnchorX = vttCue.region.regionAnchorX; + cue.region.regionAnchorY = vttCue.region.regionAnchorY; + cue.region.scroll = /** @type {shaka.text.CueRegion.scrollMode} */ + (vttCue.region.scroll); + cue.region.viewportAnchorX = vttCue.region.viewportAnchorX; + cue.region.viewportAnchorY = vttCue.region.viewportAnchorY; + cue.region.width = vttCue.region.width; + } + + return cue; + } }; 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 13d8e9f623..091faef5c3 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) { diff --git a/test/text/text_utils_unit.js b/test/text/text_utils_unit.js new file mode 100644 index 0000000000..cef266b845 --- /dev/null +++ b/test/text/text_utils_unit.js @@ -0,0 +1,74 @@ +/*! @license + * Shaka Player + * Copyright 2016 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +describe('TextUtils', () => { + describe('mapNativeCue', () => { + beforeEach(() => { + if (!window.VTTCue) { + pending('VTTCue not available'); + } + }); + + it('should map VTTCue to shaka cue', () => { + const vttCue = new VTTCue(10, 20, 'foo'); + vttCue.line = 8; + vttCue.lineAlign = 'center'; + vttCue.snapToLines = false; + vttCue.position = 'auto'; + vttCue.positionAlign = 'line-right'; + vttCue.size = 2; + vttCue.align = 'left'; + vttCue.vertical = 'rl'; + + const cue = shaka.text.Utils.mapNativeCue(vttCue); + expect(cue.startTime).toBe(10); + expect(cue.endTime).toBe(20); + expect(cue.payload).toBe('foo'); + expect(cue.line).toBe(8); + expect(cue.lineAlign).toBe(shaka.text.Cue.lineAlign.CENTER); + expect(cue.lineInterpretation) + .toBe(shaka.text.Cue.lineInterpretation.PERCENTAGE); + expect(cue.position).toBe(null); + expect(cue.positionAlign).toBe(shaka.text.Cue.positionAlign.RIGHT); + expect(cue.size).toBe(2); + expect(cue.textAlign).toBe('left'); + expect(cue.writingMode) + .toBe(shaka.text.Cue.writingMode.VERTICAL_RIGHT_TO_LEFT); + }); + + it('should map VTTRegion to shaka region', () => { + if (!window.VTTRegion) { + pending('VTTRegion not available'); + } + const vttRegion = new VTTRegion(); + vttRegion.id = 'bar'; + vttRegion.lines = 3; + vttRegion.regionAnchorX = 0; + vttRegion.regionAnchorY = 100; + vttRegion.scroll = 'up'; + vttRegion.viewportAnchorX = 50; + vttRegion.viewportAnchorY = 0; + vttRegion.width = 100; + + const vttCue = new VTTCue(10, 20, 'foo'); + vttCue.region = vttRegion; + + const region = shaka.text.Utils.mapNativeCue(vttCue).region; + expect(region.id).toBe('bar'); + expect(region.width).toBe(100); + expect(region.widthUnits).toBe(shaka.text.CueRegion.units.PERCENTAGE); + expect(region.height).toBe(3); + expect(region.heightUnits).toBe(shaka.text.CueRegion.units.LINES); + expect(region.scroll).toBe(shaka.text.CueRegion.scrollMode.UP); + expect(region.regionAnchorX).toBe(0); + expect(region.regionAnchorY).toBe(100); + expect(region.viewportAnchorX).toBe(50); + expect(region.viewportAnchorY).toBe(0); + expect(region.viewportAnchorUnits) + .toBe(shaka.text.CueRegion.units.PERCENTAGE); + }); + }); +}); From 3bfc71466d2e5f94a30c7936a257cee6c1c0d929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Thu, 4 Jul 2024 14:41:45 +0200 Subject: [PATCH 02/14] disable additional enabled tracks --- lib/player.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/player.js b/lib/player.js index 99753b584f..2d28d44ba1 100644 --- a/lib/player.js +++ b/lib/player.js @@ -2979,9 +2979,18 @@ shaka.Player = class extends shaka.util.FakeEventTarget { if (unloaded) { return; } + let enabledNativeTrack = false; for (const track of textTracks) { if (track.mode !== 'disabled') { - this.enableNativeTrack_(track); + if (!enabledNativeTrack) { + this.enableNativeTrack_(track); + enabledNativeTrack = true; + } else { + track.mode = 'disabled'; + shaka.log.warning( + 'Found more than one enabled text track, disabling it', + track); + } } } From 7929c9cc50b93447387a2f2653c28a377753fe88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Thu, 4 Jul 2024 15:03:15 +0200 Subject: [PATCH 03/14] alwaysWarn --- lib/player.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/player.js b/lib/player.js index 2d28d44ba1..8706804105 100644 --- a/lib/player.js +++ b/lib/player.js @@ -2987,7 +2987,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget { enabledNativeTrack = true; } else { track.mode = 'disabled'; - shaka.log.warning( + shaka.log.alwaysWarn( 'Found more than one enabled text track, disabling it', track); } From fbc7d7e7468e08a99603e37723f41b2e3dd4d6d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Thu, 4 Jul 2024 17:43:31 +0200 Subject: [PATCH 04/14] avoid memory grow & mitigate Infinity end time --- lib/player.js | 7 +++++-- lib/text/text_utils.js | 7 ++++--- test/text/text_utils_unit.js | 5 +++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/player.js b/lib/player.js index 8706804105..7268558b41 100644 --- a/lib/player.js +++ b/lib/player.js @@ -4891,9 +4891,12 @@ shaka.Player = class extends shaka.util.FakeEventTarget { */ enableNativeTrack_(track) { this.loadEventManager_.listen(track, 'cuechange', () => { - const cues = Array.from(track.activeCues || []).map( - shaka.text.Utils.mapNativeCue); + const defaultEnd = this.seekRange().end; + const cues = Array.from(track.activeCues || []).map((cue) => + shaka.text.Utils.mapNativeCue(cue, defaultEnd)); this.textDisplayer_.append(cues); + // Always remove cues from the past to avoid memory grow. + this.textDisplayer_.remove(0, this.video_.currentTime - 10); }); track.mode = 'hidden'; } diff --git a/lib/text/text_utils.js b/lib/text/text_utils.js index 458632d654..75b01770b6 100644 --- a/lib/text/text_utils.js +++ b/lib/text/text_utils.js @@ -182,11 +182,12 @@ shaka.text.Utils = class { /** * @param {!VTTCue} vttCue + * @param {number} defaultEnd * @return {!shaka.text.Cue} */ - static mapNativeCue(vttCue) { - const cue = new shaka.text.Cue(vttCue.startTime, vttCue.endTime, - vttCue.text); + static mapNativeCue(vttCue, defaultEnd) { + const endTime = vttCue.endTime !== Infinity ? vttCue.endTime : defaultEnd; + const cue = new shaka.text.Cue(vttCue.startTime, endTime, vttCue.text); cue.line = typeof vttCue.line === 'number' ? vttCue.line : null; cue.lineAlign = /** @type {shaka.text.Cue.lineAlign} */ (vttCue.lineAlign); cue.lineInterpretation = vttCue.snapToLines ? diff --git a/test/text/text_utils_unit.js b/test/text/text_utils_unit.js index cef266b845..afc5f7f7a3 100644 --- a/test/text/text_utils_unit.js +++ b/test/text/text_utils_unit.js @@ -23,7 +23,7 @@ describe('TextUtils', () => { vttCue.align = 'left'; vttCue.vertical = 'rl'; - const cue = shaka.text.Utils.mapNativeCue(vttCue); + const cue = shaka.text.Utils.mapNativeCue(vttCue, jasmine.any(Number)); expect(cue.startTime).toBe(10); expect(cue.endTime).toBe(20); expect(cue.payload).toBe('foo'); @@ -56,7 +56,8 @@ describe('TextUtils', () => { const vttCue = new VTTCue(10, 20, 'foo'); vttCue.region = vttRegion; - const region = shaka.text.Utils.mapNativeCue(vttCue).region; + const region = shaka.text.Utils.mapNativeCue(vttCue, jasmine.any(Number)) + .region; expect(region.id).toBe('bar'); expect(region.width).toBe(100); expect(region.widthUnits).toBe(shaka.text.CueRegion.units.PERCENTAGE); From 85872ec5f666e05f15e9e50475e0f9821038978f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Thu, 4 Jul 2024 17:47:14 +0200 Subject: [PATCH 05/14] fix test --- test/text/text_utils_unit.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/text/text_utils_unit.js b/test/text/text_utils_unit.js index afc5f7f7a3..b642b46529 100644 --- a/test/text/text_utils_unit.js +++ b/test/text/text_utils_unit.js @@ -23,7 +23,7 @@ describe('TextUtils', () => { vttCue.align = 'left'; vttCue.vertical = 'rl'; - const cue = shaka.text.Utils.mapNativeCue(vttCue, jasmine.any(Number)); + const cue = shaka.text.Utils.mapNativeCue(vttCue, 0); expect(cue.startTime).toBe(10); expect(cue.endTime).toBe(20); expect(cue.payload).toBe('foo'); @@ -56,8 +56,7 @@ describe('TextUtils', () => { const vttCue = new VTTCue(10, 20, 'foo'); vttCue.region = vttRegion; - const region = shaka.text.Utils.mapNativeCue(vttCue, jasmine.any(Number)) - .region; + const region = shaka.text.Utils.mapNativeCue(vttCue, 0).region; expect(region.id).toBe('bar'); expect(region.width).toBe(100); expect(region.widthUnits).toBe(shaka.text.CueRegion.units.PERCENTAGE); From 5cbfe6dbc8892553013975e0ac6d6f399068489c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Thu, 4 Jul 2024 18:12:05 +0200 Subject: [PATCH 06/14] experimental - fix anchoring --- lib/text/ui_text_displayer.js | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/lib/text/ui_text_displayer.js b/lib/text/ui_text_displayer.js index 4d2e3461d5..596186be3d 100644 --- a/lib/text/ui_text_displayer.js +++ b/lib/text/ui_text_displayer.js @@ -796,21 +796,35 @@ shaka.text.UITextDisplayer = class { // The position defines the indent of the text container in the // direction defined by the writing direction. - if (cue.position != null) { - if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) { - style.paddingLeft = cue.position; - } else { - style.paddingTop = cue.position; - } - } + // if (cue.position != null) { + // if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) { + // style.paddingLeft = cue.position; + // } else { + // style.paddingTop = cue.position; + // } + // } // The positionAlign attribute is an alignment for the text container in // the dimension of the writing direction. const computedPositionAlign = this.computeCuePositionAlignment_(cue); if (computedPositionAlign == Cue.positionAlign.LEFT) { style.cssFloat = 'left'; + if (cue.position !== null) { + if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) { + style.left = cue.position + '%'; + } else { + style.top = cue.position + '%'; + } + } } else if (computedPositionAlign == Cue.positionAlign.RIGHT) { style.cssFloat = 'right'; + if (cue.position !== null) { + if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) { + style.right = cue.position + '%'; + } else { + style.bottom = cue.position + '%'; + } + } } style.textAlign = cue.textAlign; From d8827113ba4b159c9dabd0ab2d378f76433b72fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Fri, 5 Jul 2024 11:00:26 +0200 Subject: [PATCH 07/14] nullify infinity cues --- lib/player.js | 14 +++++++++----- lib/text/text_utils.js | 12 +++++++----- test/text/text_utils_unit.js | 4 ++-- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/player.js b/lib/player.js index 7268558b41..dca5d3d546 100644 --- a/lib/player.js +++ b/lib/player.js @@ -51,7 +51,9 @@ 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.Iterables'); goog.require('shaka.util.LanguageUtils'); goog.require('shaka.util.ManifestParserUtils'); goog.require('shaka.util.MediaReadyState'); @@ -4891,12 +4893,14 @@ shaka.Player = class extends shaka.util.FakeEventTarget { */ enableNativeTrack_(track) { this.loadEventManager_.listen(track, 'cuechange', () => { - const defaultEnd = this.seekRange().end; - const cues = Array.from(track.activeCues || []).map((cue) => - shaka.text.Utils.mapNativeCue(cue, defaultEnd)); - this.textDisplayer_.append(cues); // Always remove cues from the past to avoid memory grow. - this.textDisplayer_.remove(0, this.video_.currentTime - 10); + const removeEnd = Math.max(0, + this.video_.currentTime - this.config_.streaming.bufferBehind); + this.textDisplayer_.remove(0, removeEnd); + const cues = shaka.util.Iterables.map(track.activeCues || [], + shaka.text.Utils.mapNativeCue) + .filter(shaka.util.Functional.isNotNull); + this.textDisplayer_.append(cues); }); track.mode = 'hidden'; } diff --git a/lib/text/text_utils.js b/lib/text/text_utils.js index 75b01770b6..4f3e605a71 100644 --- a/lib/text/text_utils.js +++ b/lib/text/text_utils.js @@ -182,12 +182,14 @@ shaka.text.Utils = class { /** * @param {!VTTCue} vttCue - * @param {number} defaultEnd - * @return {!shaka.text.Cue} + * @return {?shaka.text.Cue} */ - static mapNativeCue(vttCue, defaultEnd) { - const endTime = vttCue.endTime !== Infinity ? vttCue.endTime : defaultEnd; - const cue = new shaka.text.Cue(vttCue.startTime, endTime, vttCue.text); + static mapNativeCue(vttCue) { + if (vttCue.endTime === Infinity || vttCue.endTime < vttCue.startTime) { + return null; + } + const cue = new shaka.text.Cue(vttCue.startTime, vttCue.endTime, + vttCue.text); cue.line = typeof vttCue.line === 'number' ? vttCue.line : null; cue.lineAlign = /** @type {shaka.text.Cue.lineAlign} */ (vttCue.lineAlign); cue.lineInterpretation = vttCue.snapToLines ? diff --git a/test/text/text_utils_unit.js b/test/text/text_utils_unit.js index b642b46529..cef266b845 100644 --- a/test/text/text_utils_unit.js +++ b/test/text/text_utils_unit.js @@ -23,7 +23,7 @@ describe('TextUtils', () => { vttCue.align = 'left'; vttCue.vertical = 'rl'; - const cue = shaka.text.Utils.mapNativeCue(vttCue, 0); + const cue = shaka.text.Utils.mapNativeCue(vttCue); expect(cue.startTime).toBe(10); expect(cue.endTime).toBe(20); expect(cue.payload).toBe('foo'); @@ -56,7 +56,7 @@ describe('TextUtils', () => { const vttCue = new VTTCue(10, 20, 'foo'); vttCue.region = vttRegion; - const region = shaka.text.Utils.mapNativeCue(vttCue, 0).region; + const region = shaka.text.Utils.mapNativeCue(vttCue).region; expect(region.id).toBe('bar'); expect(region.width).toBe(100); expect(region.widthUnits).toBe(shaka.text.CueRegion.units.PERCENTAGE); From 101873961c4bc3674d8e2f690058eda1a2bbf75f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Fri, 5 Jul 2024 11:25:16 +0200 Subject: [PATCH 08/14] add another test --- test/text/text_utils_unit.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/text/text_utils_unit.js b/test/text/text_utils_unit.js index cef266b845..7598a47e3e 100644 --- a/test/text/text_utils_unit.js +++ b/test/text/text_utils_unit.js @@ -39,6 +39,14 @@ describe('TextUtils', () => { .toBe(shaka.text.Cue.writingMode.VERTICAL_RIGHT_TO_LEFT); }); + it('returns null if cue has invalid timing', () => { + let vttCue = new VTTCue(20, 10, ''); + expect(shaka.text.Utils.mapNativeCue(vttCue)).toBe(null); + + vttCue = new VTTCue(20, Infinity, ''); + expect(shaka.text.Utils.mapNativeCue(vttCue)).toBe(null); + }); + it('should map VTTRegion to shaka region', () => { if (!window.VTTRegion) { pending('VTTRegion not available'); From be75f27965a3445b293b8f18ba980be567d380e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Fri, 5 Jul 2024 12:58:38 +0200 Subject: [PATCH 09/14] dont use iterables --- lib/player.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/player.js b/lib/player.js index dca5d3d546..b62e52a24d 100644 --- a/lib/player.js +++ b/lib/player.js @@ -53,7 +53,6 @@ goog.require('shaka.util.FakeEvent'); goog.require('shaka.util.FakeEventTarget'); goog.require('shaka.util.Functional'); goog.require('shaka.util.IDestroyable'); -goog.require('shaka.util.Iterables'); goog.require('shaka.util.LanguageUtils'); goog.require('shaka.util.ManifestParserUtils'); goog.require('shaka.util.MediaReadyState'); @@ -4897,8 +4896,8 @@ shaka.Player = class extends shaka.util.FakeEventTarget { const removeEnd = Math.max(0, this.video_.currentTime - this.config_.streaming.bufferBehind); this.textDisplayer_.remove(0, removeEnd); - const cues = shaka.util.Iterables.map(track.activeCues || [], - shaka.text.Utils.mapNativeCue) + const cues = Array.from(track.activeCues || []) + .map(shaka.text.Utils.mapNativeCue) .filter(shaka.util.Functional.isNotNull); this.textDisplayer_.append(cues); }); From 214307d41337c6e7cc0e8eb728f91792190344a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Fri, 5 Jul 2024 14:37:34 +0200 Subject: [PATCH 10/14] fix test --- test/text/text_utils_unit.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/text/text_utils_unit.js b/test/text/text_utils_unit.js index 7598a47e3e..d253cfe8a4 100644 --- a/test/text/text_utils_unit.js +++ b/test/text/text_utils_unit.js @@ -43,7 +43,11 @@ describe('TextUtils', () => { let vttCue = new VTTCue(20, 10, ''); expect(shaka.text.Utils.mapNativeCue(vttCue)).toBe(null); - vttCue = new VTTCue(20, Infinity, ''); + // VTTCue constructor does not allow for creating a cue with + // non-finite time. However, Safari adds such cues to text tracks, + // so hack creation of object with such properties via cloning. + vttCue = shaka.util.ObjectUtils.shallowCloneObject(vttCue); + vttCue.endTime = Infinity; expect(shaka.text.Utils.mapNativeCue(vttCue)).toBe(null); }); From ebe752fcb0b6e2aeb50a7000bb4ee7a409f0a194 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Fri, 5 Jul 2024 16:28:04 +0200 Subject: [PATCH 11/14] parse payload --- lib/text/text_utils.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/text/text_utils.js b/lib/text/text_utils.js index 4f3e605a71..26a92be50b 100644 --- a/lib/text/text_utils.js +++ b/lib/text/text_utils.js @@ -218,6 +218,7 @@ shaka.text.Utils = class { cue.region.viewportAnchorY = vttCue.region.viewportAnchorY; cue.region.width = vttCue.region.width; } + shaka.text.Cue.parseCuePayload(cue); return cue; } From 5e4120dc61792692b8206069d02fc146a0cfbd93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Fri, 12 Jul 2024 08:57:11 +0200 Subject: [PATCH 12/14] Revert "experimental - fix anchoring" This reverts commit 5cbfe6dbc8892553013975e0ac6d6f399068489c. --- lib/text/ui_text_displayer.js | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/lib/text/ui_text_displayer.js b/lib/text/ui_text_displayer.js index 596186be3d..4d2e3461d5 100644 --- a/lib/text/ui_text_displayer.js +++ b/lib/text/ui_text_displayer.js @@ -796,35 +796,21 @@ shaka.text.UITextDisplayer = class { // The position defines the indent of the text container in the // direction defined by the writing direction. - // if (cue.position != null) { - // if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) { - // style.paddingLeft = cue.position; - // } else { - // style.paddingTop = cue.position; - // } - // } + if (cue.position != null) { + if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) { + style.paddingLeft = cue.position; + } else { + style.paddingTop = cue.position; + } + } // The positionAlign attribute is an alignment for the text container in // the dimension of the writing direction. const computedPositionAlign = this.computeCuePositionAlignment_(cue); if (computedPositionAlign == Cue.positionAlign.LEFT) { style.cssFloat = 'left'; - if (cue.position !== null) { - if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) { - style.left = cue.position + '%'; - } else { - style.top = cue.position + '%'; - } - } } else if (computedPositionAlign == Cue.positionAlign.RIGHT) { style.cssFloat = 'right'; - if (cue.position !== null) { - if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) { - style.right = cue.position + '%'; - } else { - style.bottom = cue.position + '%'; - } - } } style.textAlign = cue.textAlign; From 4bb0f6f1152d15964a9ad375f5b3e30876555be1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Mon, 15 Jul 2024 10:13:59 +0200 Subject: [PATCH 13/14] remove text utils --- lib/player.js | 2 +- lib/text/text_utils.js | 43 ------------------ test/text/text_utils_unit.js | 86 ------------------------------------ 3 files changed, 1 insertion(+), 130 deletions(-) delete mode 100644 test/text/text_utils_unit.js diff --git a/lib/player.js b/lib/player.js index 3f20fb3df1..7b1c6403a6 100644 --- a/lib/player.js +++ b/lib/player.js @@ -4908,7 +4908,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget { this.video_.currentTime - this.config_.streaming.bufferBehind); this.textDisplayer_.remove(0, removeEnd); const cues = Array.from(track.activeCues || []) - .map(shaka.text.Utils.mapNativeCue) + .map(shaka.text.Utils.mapNativeCueToShakaCue) .filter(shaka.util.Functional.isNotNull); this.textDisplayer_.append(cues); }); diff --git a/lib/text/text_utils.js b/lib/text/text_utils.js index efaf7c3773..a55b393e12 100644 --- a/lib/text/text_utils.js +++ b/lib/text/text_utils.js @@ -181,49 +181,6 @@ shaka.text.Utils = class { return uniqueCues; } - /** - * @param {!VTTCue} vttCue - * @return {?shaka.text.Cue} - */ - static mapNativeCue(vttCue) { - if (vttCue.endTime === Infinity || vttCue.endTime < vttCue.startTime) { - return null; - } - const cue = new shaka.text.Cue(vttCue.startTime, vttCue.endTime, - vttCue.text); - cue.line = typeof vttCue.line === 'number' ? vttCue.line : null; - cue.lineAlign = /** @type {shaka.text.Cue.lineAlign} */ (vttCue.lineAlign); - cue.lineInterpretation = vttCue.snapToLines ? - shaka.text.Cue.lineInterpretation.LINE_NUMBER : - shaka.text.Cue.lineInterpretation.PERCENTAGE; - cue.position = typeof vttCue.position === 'number' ? - vttCue.position : null; - cue.positionAlign = /** @type {shaka.text.Cue.positionAlign} */ - (vttCue.positionAlign); - cue.size = vttCue.size; - cue.textAlign = /** @type {shaka.text.Cue.textAlign} */ (vttCue.align); - if (vttCue.vertical === 'lr') { - cue.writingMode = shaka.text.Cue.writingMode.VERTICAL_LEFT_TO_RIGHT; - } else if (vttCue.vertical === 'rl') { - cue.writingMode = shaka.text.Cue.writingMode.VERTICAL_RIGHT_TO_LEFT; - } - if (vttCue.region) { - cue.region.id = vttCue.region.id; - cue.region.height = vttCue.region.lines; - cue.region.heightUnits = shaka.text.CueRegion.units.LINES; - cue.region.regionAnchorX = vttCue.region.regionAnchorX; - cue.region.regionAnchorY = vttCue.region.regionAnchorY; - cue.region.scroll = /** @type {shaka.text.CueRegion.scrollMode} */ - (vttCue.region.scroll); - cue.region.viewportAnchorX = vttCue.region.viewportAnchorX; - cue.region.viewportAnchorY = vttCue.region.viewportAnchorY; - cue.region.width = vttCue.region.width; - } - shaka.text.Cue.parseCuePayload(cue); - - return cue; - } - /** * @param {!shaka.text.Cue} shakaCue * @return {TextTrackCue} diff --git a/test/text/text_utils_unit.js b/test/text/text_utils_unit.js deleted file mode 100644 index d253cfe8a4..0000000000 --- a/test/text/text_utils_unit.js +++ /dev/null @@ -1,86 +0,0 @@ -/*! @license - * Shaka Player - * Copyright 2016 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -describe('TextUtils', () => { - describe('mapNativeCue', () => { - beforeEach(() => { - if (!window.VTTCue) { - pending('VTTCue not available'); - } - }); - - it('should map VTTCue to shaka cue', () => { - const vttCue = new VTTCue(10, 20, 'foo'); - vttCue.line = 8; - vttCue.lineAlign = 'center'; - vttCue.snapToLines = false; - vttCue.position = 'auto'; - vttCue.positionAlign = 'line-right'; - vttCue.size = 2; - vttCue.align = 'left'; - vttCue.vertical = 'rl'; - - const cue = shaka.text.Utils.mapNativeCue(vttCue); - expect(cue.startTime).toBe(10); - expect(cue.endTime).toBe(20); - expect(cue.payload).toBe('foo'); - expect(cue.line).toBe(8); - expect(cue.lineAlign).toBe(shaka.text.Cue.lineAlign.CENTER); - expect(cue.lineInterpretation) - .toBe(shaka.text.Cue.lineInterpretation.PERCENTAGE); - expect(cue.position).toBe(null); - expect(cue.positionAlign).toBe(shaka.text.Cue.positionAlign.RIGHT); - expect(cue.size).toBe(2); - expect(cue.textAlign).toBe('left'); - expect(cue.writingMode) - .toBe(shaka.text.Cue.writingMode.VERTICAL_RIGHT_TO_LEFT); - }); - - it('returns null if cue has invalid timing', () => { - let vttCue = new VTTCue(20, 10, ''); - expect(shaka.text.Utils.mapNativeCue(vttCue)).toBe(null); - - // VTTCue constructor does not allow for creating a cue with - // non-finite time. However, Safari adds such cues to text tracks, - // so hack creation of object with such properties via cloning. - vttCue = shaka.util.ObjectUtils.shallowCloneObject(vttCue); - vttCue.endTime = Infinity; - expect(shaka.text.Utils.mapNativeCue(vttCue)).toBe(null); - }); - - it('should map VTTRegion to shaka region', () => { - if (!window.VTTRegion) { - pending('VTTRegion not available'); - } - const vttRegion = new VTTRegion(); - vttRegion.id = 'bar'; - vttRegion.lines = 3; - vttRegion.regionAnchorX = 0; - vttRegion.regionAnchorY = 100; - vttRegion.scroll = 'up'; - vttRegion.viewportAnchorX = 50; - vttRegion.viewportAnchorY = 0; - vttRegion.width = 100; - - const vttCue = new VTTCue(10, 20, 'foo'); - vttCue.region = vttRegion; - - const region = shaka.text.Utils.mapNativeCue(vttCue).region; - expect(region.id).toBe('bar'); - expect(region.width).toBe(100); - expect(region.widthUnits).toBe(shaka.text.CueRegion.units.PERCENTAGE); - expect(region.height).toBe(3); - expect(region.heightUnits).toBe(shaka.text.CueRegion.units.LINES); - expect(region.scroll).toBe(shaka.text.CueRegion.scrollMode.UP); - expect(region.regionAnchorX).toBe(0); - expect(region.regionAnchorY).toBe(100); - expect(region.viewportAnchorX).toBe(50); - expect(region.viewportAnchorY).toBe(0); - expect(region.viewportAnchorUnits) - .toBe(shaka.text.CueRegion.units.PERCENTAGE); - }); - }); -}); From faa6ed6e4eef9a0f979632d68f24e388c745ad84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Mon, 15 Jul 2024 10:57:39 +0200 Subject: [PATCH 14/14] pip --- lib/player.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/player.js b/lib/player.js index 7b1c6403a6..a041c888eb 100644 --- a/lib/player.js +++ b/lib/player.js @@ -2903,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 @@ -4912,7 +4928,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget { .filter(shaka.util.Functional.isNotNull); this.textDisplayer_.append(cues); }); - track.mode = 'hidden'; + track.mode = document.pictureInPictureElement ? 'showing' : 'hidden'; } /**