From b27ea82e7f1ccbf45fdf07bcd4c2e52796268692 Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Mon, 5 Mar 2018 14:03:17 -0800 Subject: [PATCH] Allow Player to attach/detach from video This adds attach/detach methods to replace the media element in the Player constructor. Now applications can take back control of the media element or provide a reference later in the Player's life cycle. This also allows applications to decide whether or not to set up MediaSource in advance, through an optional argument on attach and unload. The default will be to set up MediaSource in advance, which maintains current behavior. This advanced setup of MediaSource can improve load latency later. This change also introduces async/await for the first time in the project, which required changes to eslint config, Closure build arguments, Babel & Babel-polyfill setup, and the esprima parser used by our extern generator. The use of async/await will improve readability in many places, and these infrastructure changes to enable async/await should also unblock issues #1336 and #1337. Closes #1087 Change-Id: I0d6b4e0e2af27a6520a3d070fa92b7139b2cb8b0 --- .eslintrc.js | 3 + build/build.py | 1 - karma.conf.js | 17 +- lib/cast/cast_proxy.js | 24 ++- lib/cast/cast_utils.js | 2 + lib/media/media_source_engine.js | 9 + lib/player.js | 225 +++++++++++++-------- lib/util/error.js | 7 + package.json | 3 +- test/player_integration.js | 81 ++++++++ test/player_unit.js | 2 +- test/test/util/fake_media_source_engine.js | 3 + 12 files changed, 280 insertions(+), 97 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 2d2dda87f..cdd3dcf35 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -23,6 +23,9 @@ module.exports = { "browser": true, "es6": true }, + "parserOptions": { + "ecmaVersion": 2017 + }, "extends": ["eslint:recommended", "google"], "rules": { // Things the compiler already takes care of, with more precision: {{{ diff --git a/build/build.py b/build/build.py index b91b20d89..2e42f04e4 100755 --- a/build/build.py +++ b/build/build.py @@ -49,7 +49,6 @@ import shakaBuildHelpers common_closure_opts = [ - '--language_in', 'ECMASCRIPT6', '--language_out', 'ECMASCRIPT3', '--jscomp_error=*', diff --git a/karma.conf.js b/karma.conf.js index cb7f770c2..1decce0f8 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -75,19 +75,20 @@ module.exports = function(config) { // list of files / patterns to load in the browser files: [ - // closure base first - 'third_party/closure/goog/base.js', - - // deps next - 'dist/deps.js', - 'shaka-player.uncompiled.js', - - // Promise polyfill for IE11 and some older TVs + // Polyfills first, primarily for IE 11 and older TVs: + // Promise polyfill 'node_modules/es6-promise-polyfill/promise.js', + // Babel polyfill, required for async/await + 'node_modules/babel-polyfill/dist/polyfill.js', // muxjs module next 'node_modules/mux.js/dist/mux.js', + // load closure base, the deps tree, and the uncompiled library + 'third_party/closure/goog/base.js', + 'dist/deps.js', + 'shaka-player.uncompiled.js', + // cajon module (an AMD variant of requirejs) next 'node_modules/cajon/cajon.js', diff --git a/lib/cast/cast_proxy.js b/lib/cast/cast_proxy.js index ddb018f44..af4362c31 100644 --- a/lib/cast/cast_proxy.js +++ b/lib/cast/cast_proxy.js @@ -521,15 +521,29 @@ shaka.cast.CastProxy.prototype.playerProxyGet_ = function(name) { return this.localPlayer_.getNetworkingEngine.bind(this.localPlayer_); } - if (name == 'getManifest') { - if (this.sender_.isCasting()) { + if (this.sender_.isCasting()) { + // These methods are unavailable or otherwise stubbed during casting. + if (name == 'getManifest') { return function() { - shaka.log.error('getManifest() does not work while casting!'); + shaka.log.alwaysWarn('getManifest() does not work while casting!'); return null; }; } - return this.localPlayer_.getManifest.bind(this.localPlayer_); - } + + if (name == 'attach') { + return function() { + shaka.log.alwaysWarn('attach() does not work while casting!'); + return Promise.resolve(); + }; + } + + if (name == 'detach') { + return function() { + shaka.log.alwaysWarn('detach() does not work while casting!'); + return Promise.resolve(); + }; + } + } // if (this.sender_.isCasting()) // If we are casting, but the first update has not come in yet, use local // getters, but not local methods. diff --git a/lib/cast/cast_utils.js b/lib/cast/cast_utils.js index 979e042f1..577f46ec2 100644 --- a/lib/cast/cast_utils.js +++ b/lib/cast/cast_utils.js @@ -195,6 +195,8 @@ shaka.cast.CastUtils.PlayerVoidMethods = [ * @const {!Array.} */ shaka.cast.CastUtils.PlayerPromiseMethods = [ + 'attach', + 'detach', // The opt_manifestFactory method is not supported. 'load', 'unload' diff --git a/lib/media/media_source_engine.js b/lib/media/media_source_engine.js index db6470be1..262de5fe1 100644 --- a/lib/media/media_source_engine.js +++ b/lib/media/media_source_engine.js @@ -265,6 +265,15 @@ shaka.media.MediaSourceEngine.prototype.destroy = function() { }; +/** + * @return {!Promise} Resolved when MediaSource is open and attached to the + * media element. This process is actually initiated by the constructor. + */ +shaka.media.MediaSourceEngine.prototype.open = function() { + return this.mediaSourceOpen_; +}; + + /** * Initialize MediaSourceEngine. * diff --git a/lib/player.js b/lib/player.js index 53f4cdb1c..a15582c80 100644 --- a/lib/player.js +++ b/lib/player.js @@ -47,9 +47,8 @@ goog.require('shaka.util.StreamUtils'); /** * Construct a Player. * - * @param {!HTMLMediaElement} video Any existing TextTracks attached to this - * element that were not created by Shaka will be disabled. A new TextTrack - * may be created to display captions or subtitles. + * @param {HTMLMediaElement=} video If provided, this is equivalent to calling + * attach(video, true) immediately after construction. * @param {function(shaka.Player)=} opt_dependencyInjector Optional callback * which is called to inject mocks into the Player. Used for testing. * @@ -66,7 +65,7 @@ shaka.Player = function(video, opt_dependencyInjector) { this.destroyed_ = false; /** @private {HTMLMediaElement} */ - this.video_ = video; + this.video_ = null; /** * Only holds the visibility setting until a textDisplayer_ is created. @@ -179,7 +178,10 @@ shaka.Player = function(video, opt_dependencyInjector) { } this.networkingEngine_ = this.createNetworkingEngine(); - this.initialize_(); + + if (video) { + this.attach(video, true /* initializeMediaSource */); + } }; goog.inherits(shaka.Player, shaka.util.FakeEventTarget); @@ -217,28 +219,25 @@ shaka.Player.prototype.cancelLoadChain_ = function() { * @override * @export */ -shaka.Player.prototype.destroy = function() { +shaka.Player.prototype.destroy = async function() { + // First, detach from the media element. This implies unloading content + // and canceling pending loads. + await this.detach(); + // Then, destroy other components and clear fields. + this.destroyed_ = true; - return this.cancelLoadChain_().then(function() { - let p = Promise.all([ - // We need to destroy the current fields as well as waiting for an - // existing unload to complete. It is fine to call destroyStreaming_ if - // there is an unload since it resets the fields immediately. - this.unloadChain_, - this.destroyStreaming_(), - this.eventManager_ ? this.eventManager_.destroy() : null, - this.networkingEngine_ ? this.networkingEngine_.destroy() : null - ]); + let p = Promise.all([ + this.eventManager_ ? this.eventManager_.destroy() : null, + this.networkingEngine_ ? this.networkingEngine_.destroy() : null + ]); - this.video_ = null; - this.textVisibility_ = false; - this.eventManager_ = null; - this.abrManager_ = null; - this.networkingEngine_ = null; - this.config_ = null; - return p; - }.bind(this)); + this.textVisibility_ = false; + this.eventManager_ = null; + this.abrManager_ = null; + this.networkingEngine_ = null; + this.config_ = null; + await p; }; @@ -486,6 +485,76 @@ shaka.Player.probeSupport = function() { }; +/** + * Attach the Player to a media element (audio or video tag). + * + * If the Player is already attached to a media element, the previous element + * will first be detached. + * + * After calling attach, the media element is owned by the Player and should not + * be used for other purposes until detach or destroy() are called. + * + * @param {!HTMLMediaElement} video + * @param {boolean=} initializeMediaSource If true, start initializing + * MediaSource right away. This can improve load() latency for + * MediaSource-based playbacks. Defaults to true. + * + * @return {!Promise} If initializeMediaSource is false, the Promise is resolved + * as soon as the Player has released any previous media element and taken + * ownership of the new one. If initializeMediaSource is true, the Promise + * resolves after MediaSource has been subsequently initialized on the new + * media element. + * @export + */ +shaka.Player.prototype.attach = async function(video, initializeMediaSource) { + if (initializeMediaSource === undefined) { + initializeMediaSource = true; + } + + if (this.video_) { + await this.detach(); + } + + this.video_ = video; + goog.asserts.assert(video, 'Cannot attach to a null media element!'); + + // Listen for video errors. + this.eventManager_.listen(this.video_, 'error', + this.onVideoError_.bind(this)); + + if (initializeMediaSource) { + // Start the (potentially slow) process of opening MediaSource now. + this.mediaSourceEngine_ = this.createMediaSourceEngine(); + await this.mediaSourceEngine_.open(); + } +}; + + +/** + * Detaches the Player from the media element (audio or video tag). + * + * After calling detach and waiting for the Promise to be resolved, the media + * element is no longer owned by the Player and may be used for other purposes. + * + * @return {!Promise} Resolved when the Player has released any previous media + * element. + * @export + */ +shaka.Player.prototype.detach = async function() { + if (!this.video_) { + return; + } + + // Unload any loaded content. + await this.unload(false); + + // Stop listening for video errors. + this.eventManager_.unlisten(this.video_, 'error'); + + this.video_ = null; +}; + + /** * Load a manifest. * @@ -502,6 +571,13 @@ shaka.Player.probeSupport = function() { */ shaka.Player.prototype.load = function(manifestUri, opt_startTime, opt_manifestParserFactory) { + if (!this.video_) { + return Promise.reject(new shaka.util.Error( + shaka.util.Error.Severity.CRITICAL, + shaka.util.Error.Category.PLAYER, + shaka.util.Error.Code.NO_VIDEO_ELEMENT)); + } + let unloadPromise = this.unload(); let loadChain = new shaka.util.CancelableChain(); this.loadChain_ = loadChain; @@ -978,7 +1054,7 @@ shaka.Player.prototype.resetConfiguration = function() { /** * @return {HTMLMediaElement} A reference to the HTML Media Element passed - * in during initialization. + * to the constructor or to attach(). * @export */ shaka.Player.prototype.getMediaElement = function() { @@ -1117,23 +1193,43 @@ shaka.Player.prototype.isBuffering = function() { /** * Unload the current manifest and make the Player available for re-use. * - * @return {!Promise} Resolved when streaming has stopped and the previous - * content, if any, has been unloaded. + * @param {boolean=} reinitializeMediaSource If true, start reinitializing + * MediaSource right away. This can improve load() latency for + * MediaSource-based playbacks. Defaults to true. + * @return {!Promise} If reinitializeMediaSource is false, the Promise is + * resolved as soon as streaming has stopped and the previous content, if any, + * has been unloaded. If reinitializeMediaSource is true, the Promise + * resolves after MediaSource has been subsequently reinitialized. * @export */ -shaka.Player.prototype.unload = function() { - if (this.destroyed_) return Promise.resolve(); +shaka.Player.prototype.unload = async function(reinitializeMediaSource) { + if (this.destroyed_) { + return; + } + + if (reinitializeMediaSource === undefined) { + reinitializeMediaSource = true; + } + this.dispatchEvent(new shaka.util.FakeEvent('unloading')); - return this.cancelLoadChain_().then(function() { - // If there is an existing unload operation, use that. - if (!this.unloadChain_) { - this.unloadChain_ = this.resetStreaming_().then(function() { - this.unloadChain_ = null; - }.bind(this)); - } - return this.unloadChain_; - }.bind(this)); + await this.cancelLoadChain_(); + + // If there is an existing unload operation, use that. + if (!this.unloadChain_) { + this.unloadChain_ = this.destroyStreaming_().then(() => { + // Force an exit from the buffering state. + this.onBuffering_(false); + this.unloadChain_ = null; + }); + } + await this.unloadChain_; + + if (reinitializeMediaSource) { + // Start the (potentially slow) process of opening MediaSource now. + this.mediaSourceEngine_ = this.createMediaSourceEngine(); + await this.mediaSourceEngine_.open(); + } }; @@ -1834,21 +1930,6 @@ shaka.Player.prototype.getManifest = function() { }; -/** - * Initialize the Player. - * @private - */ -shaka.Player.prototype.initialize_ = function() { - // Start the (potentially slow) process of opening MediaSource now. - this.mediaSourceEngine_ = this.createMediaSourceEngine(); - this.mediaSourceEngine_.init({}); - - // Listen for video errors. - this.eventManager_.listen(this.video_, 'error', - this.onVideoError_.bind(this)); -}; - - /** * @param {shakaExtern.Variant} variant * @param {boolean} fromAdaptation @@ -1964,31 +2045,6 @@ shaka.Player.prototype.destroyStreaming_ = function() { }; -/** - * Reset the streaming system. - * @return {!Promise} - * @private - */ -shaka.Player.prototype.resetStreaming_ = function() { - if (!this.parser_) { - // Nothing is playing, so this is effectively a no-op. - return Promise.resolve(); - } - - // Destroy the streaming system before we recreate everything. - return this.destroyStreaming_().then(function() { - if (this.destroyed_) return; - - // Force an exit from the buffering state. - this.onBuffering_(false); - - // Start the (potentially slow) process of opening MediaSource now. - this.mediaSourceEngine_ = this.createMediaSourceEngine(); - this.mediaSourceEngine_.init({}); - }.bind(this)); -}; - - /** * @return {!Object} * @private @@ -2032,6 +2088,15 @@ shaka.Player.prototype.defaultConfig_ = function() { // observed and absorbed. } + // Because this.video_ may not be set when the config is built, the default + // TextDisplay factory must capture a reference to "this" as "self" to use at + // the time we call the factory. Bind can't be used here because we call the + // factory with "new", effectively removing any binding to "this". + const self = this; + const defaultTextDisplayFactory = function() { + return new shaka.text.SimpleTextDisplayer(self.video_); + }; + return { drm: { retryParameters: shaka.net.NetworkingEngine.defaultRetryParameters(), @@ -2074,9 +2139,7 @@ shaka.Player.prototype.defaultConfig_ = function() { durationBackoff: 1 }, abrFactory: shaka.abr.SimpleAbrManager, - textDisplayFactory: function(videoElement) { - return new shaka.text.SimpleTextDisplayer(videoElement); - }.bind(null, this.video_), + textDisplayFactory: defaultTextDisplayFactory, abr: { enabled: true, defaultBandwidthEstimate: bandwidthEstimate, diff --git a/lib/util/error.js b/lib/util/error.js index c92016889..18a5292cb 100644 --- a/lib/util/error.js +++ b/lib/util/error.js @@ -696,6 +696,13 @@ shaka.util.Error.Code = { */ 'OPERATION_ABORTED': 7001, + /** + * The call to Player.load() failed because the Player does not have a video + * element. The video element must either be provided to the constructor or + * to Player.attach() before Player.load() is called. + */ + 'NO_VIDEO_ELEMENT': 7002, + /** * The Cast API is unavailable. This may be because of one of the following: diff --git a/package.json b/package.json index e1c398c3d..f6e50cd44 100644 --- a/package.json +++ b/package.json @@ -13,13 +13,14 @@ "devDependencies": { "array-includes": "~3.0.3", "babel-core": "^6.26.0", + "babel-polyfill": "^6.26.0", "babel-preset-env": "^1.6.1", "cajon": "^0.4.4", "es6-promise-polyfill": "^1.2.0", "es6-shim": "~0.35.3", "eslint": "^4.14.0", "eslint-config-google": "^0.9.1", - "esprima": "~3.1.3", + "esprima": "^4.0.0", "htmlhint": "yaniswang/HTMLHint#152a114f", "in-publish": "~2.0.0", "jasmine-ajax": "~3.3.1", diff --git a/test/player_integration.js b/test/player_integration.js index baa35e07e..616d4356d 100644 --- a/test/player_integration.js +++ b/test/player_integration.js @@ -97,6 +97,87 @@ describe('Player', function() { document.body.removeChild(video); }); + describe('constructor', function() { + beforeEach(async function() { + // To test the constructor, destroy the player that was constructed + // in the outermost beforeEach(). Then we can control the details in + // each constructor test. + await player.destroy(); + }); + + it('sets video.src when video is provided', async function() { + expect(video.src).toBeFalsy(); + player = new compiledShaka.Player(video); + + // This should always be enough time to set up MediaSource. + await Util.delay(2); + expect(video.src).toBeTruthy(); + }); + + it('does not set video.src when no video is provided', async function() { + expect(video.src).toBeFalsy(); + player = new compiledShaka.Player(); + + // This should always be enough time to set up MediaSource. + await Util.delay(2); + expect(video.src).toBeFalsy(); + }); + }); + + describe('attach', function() { + beforeEach(async function() { + // To test attach, we want to construct a player without a video element + // attached in advance. To do that, we destroy the player that was + // constructed in the outermost beforeEach(), then construct a new one + // without a video element. + await player.destroy(); + player = new compiledShaka.Player(); + }); + + it('sets video.src when initializeMediaSource is true', async function() { + expect(video.src).toBeFalsy(); + await player.attach(video, true); + expect(video.src).toBeTruthy(); + }); + + it('does not set video.src when initializeMediaSource is false', + async function() { + expect(video.src).toBeFalsy(); + await player.attach(video, false); + expect(video.src).toBeFalsy(); + }); + + it('can be used before load()', async function() { + await player.attach(video); + await player.load('test:sintel_compiled'); + }); + }); + + describe('unload', function() { + it('unsets video.src when reinitializeMediaSource is false', + async function() { + await player.load('test:sintel_compiled'); + expect(video.src).toBeTruthy(); + + await player.unload(false); + expect(video.src).toBeFalsy(); + + await Util.delay(0.4); + // After a long delay, we have not implicitly set MediaSource up + // again. video.src stays unset. + expect(video.src).toBeFalsy(); + }); + + it('resets video.src when reinitializeMediaSource is true', + async function() { + await player.load('test:sintel_compiled'); + expect(video.src).toBeTruthy(); + + await player.unload(true); + expect(video.src).toBeTruthy(); + }); + }); + describe('getStats', function() { it('gives stats about current stream', function(done) { // This is tested more in player_unit.js. This is here to test the public diff --git a/test/player_unit.js b/test/player_unit.js index 33e50adde..fc2b9c2bc 100644 --- a/test/player_unit.js +++ b/test/player_unit.js @@ -114,6 +114,7 @@ describe('Player', function() { onChooseStreams, onCanSwitch); mediaSourceEngine = { init: jasmine.createSpy('init').and.returnValue(Promise.resolve()), + open: jasmine.createSpy('open').and.returnValue(Promise.resolve()), destroy: jasmine.createSpy('destroy').and. returnValue(Promise.resolve()), setUseEmbeddedText: jasmine.createSpy('setUseEmbeddedText'), @@ -125,7 +126,6 @@ describe('Player', function() { player.createNetworkingEngine = function() { return networkingEngine; }; player.createPlayhead = function() { return playhead; }; player.createPlayheadObserver = function() { return playheadObserver; }; - player.createMediaSource = function() { return Promise.resolve(); }; player.createMediaSourceEngine = function() { return mediaSourceEngine; }; player.createStreamingEngine = function() { // This captures the variable |manifest| so this should only be used diff --git a/test/test/util/fake_media_source_engine.js b/test/test/util/fake_media_source_engine.js index 95552fbc7..9bf54d2b8 100644 --- a/test/test/util/fake_media_source_engine.js +++ b/test/test/util/fake_media_source_engine.js @@ -72,6 +72,9 @@ shaka.test.FakeMediaSourceEngine = function(segmentData, opt_drift) { /** @type {!jasmine.Spy} */ this.init = jasmine.createSpy('init').and.returnValue(Promise.resolve()); + /** @type {!jasmine.Spy} */ + this.open = jasmine.createSpy('open').and.returnValue(Promise.resolve()); + /** @type {!jasmine.Spy} */ this.reinitText = jasmine.createSpy('reinitText').and.stub();