From 831dfa616804e07a098327a130f91666169a848f Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Mon, 29 Apr 2019 17:08:33 -0700 Subject: [PATCH] Consider src= fully loaded only after first frame This fixes the definition of load() to wait for a frame before resolving the load() Promise for src= playbacks. Now methods like isAudioOnly can be trusted as soon as load() resolves. This also allows load() to fail for src= playbacks if an error event fires from the media element. Issue #816 Issue #997 Change-Id: I0f6120d1334bbebcb78efdbbca65c7981f3ef265 --- lib/player.js | 60 +++++++++++++++++++++++++++----- test/player_integration.js | 70 ++++++++++++++++++++------------------ 2 files changed, 87 insertions(+), 43 deletions(-) diff --git a/lib/player.js b/lib/player.js index 50b4d4eae9..fada633f27 100644 --- a/lib/player.js +++ b/lib/player.js @@ -1916,7 +1916,31 @@ shaka.Player.prototype.onSrcEquals_ = function(has, wants) { // Dispatch a 'trackschanged' event, for the same reasons as 'streaming'. this.onTracksChanged_(); - return shaka.util.AbortableOperation.completed(undefined); + // This is fully loaded when we have loaded the first frame. + const fullyLoaded = new shaka.util.PublicPromise(); + if (this.video_.readyState >= HTMLMediaElement.HAVE_CURRENT_DATA) { + // Already done! + fullyLoaded.resolve(); + } else if (this.video_.error) { + // Already failed! + fullyLoaded.reject(this.videoErrorToShakaError_()); + } else { + // Wait for success or failure. + this.eventManager_.listenOnce(this.video_, 'loadeddata', () => { + fullyLoaded.resolve(); + }); + this.eventManager_.listenOnce(this.video_, 'error', () => { + fullyLoaded.reject(this.videoErrorToShakaError_()); + }); + } + return new shaka.util.AbortableOperation(fullyLoaded, /* onAbort= */ () => { + const abortedError = new shaka.util.Error( + shaka.util.Error.Severity.CRITICAL, + shaka.util.Error.Category.PLAYER, + shaka.util.Error.Code.OPERATION_ABORTED); + fullyLoaded.reject(abortedError); + return Promise.resolve(); // Abort complete. + }); }; /** @@ -4421,17 +4445,22 @@ shaka.Player.prototype.onRegionEvent_ = function(eventName, region) { /** - * @param {!Event} event + * Turn the media element's error object into a Shaka Player error object. + * + * @return {shaka.util.Error} * @private */ -shaka.Player.prototype.onVideoError_ = function(event) { - if (!this.video_.error) return; +shaka.Player.prototype.videoErrorToShakaError_ = function() { + goog.asserts.assert(this.video_.error, 'Video error expected, but missing!'); + if (!this.video_.error) { + return null; + } - let code = this.video_.error.code; + const code = this.video_.error.code; if (code == 1 /* MEDIA_ERR_ABORTED */) { // Ignore this error code, which should only occur when navigating away or // deliberately stopping playback of HTTP content. - return; + return null; } // Extra error information from MS Edge and IE11: @@ -4446,13 +4475,26 @@ shaka.Player.prototype.onVideoError_ = function(event) { } // Extra error information from Chrome: - let message = this.video_.error.message; + const message = this.video_.error.message; - this.onError_(new shaka.util.Error( + return new shaka.util.Error( shaka.util.Error.Severity.CRITICAL, shaka.util.Error.Category.MEDIA, shaka.util.Error.Code.VIDEO_ERROR, - code, extended, message)); + code, extended, message); +}; + + +/** + * @param {!Event} event + * @private + */ +shaka.Player.prototype.onVideoError_ = function(event) { + const error = this.videoErrorToShakaError_(); + if (!error) { + return; + } + this.onError_(error); }; diff --git a/test/player_integration.js b/test/player_integration.js index 7fa4771b16..745beb916f 100644 --- a/test/player_integration.js +++ b/test/player_integration.js @@ -570,6 +570,8 @@ describe('Player Manifest Retries', function() { // This test suite focuses on how the player moves through the different load // states. describe('Player Load Path', () => { + const SMALL_MP4_CONTENT_URI = '/base/test/test/assets/small.mp4'; + /** @type {!HTMLVideoElement} */ let video; /** @type {shaka.Player} */ @@ -618,17 +620,17 @@ describe('Player Load Path', () => { it('attach and initialize media source when constructed with media element', async () => { - expect(video.src).toBeFalsy(); + expect(video.src).toBeFalsy(); - createPlayer(/* attachedTo= */ video); + createPlayer(/* attachedTo= */ video); - // Wait until we enter the media source state. - await new Promise((resolve) => { - whenEnteringState('media-source', resolve); - }); + // Wait until we enter the media source state. + await new Promise((resolve) => { + whenEnteringState('media-source', resolve); + }); - expect(video.src).toBeTruthy(); - }); + expect(video.src).toBeTruthy(); + }); it('does not set video.src when no video is provided', async function() { expect(video.src).toBeFalsy(); @@ -644,43 +646,43 @@ describe('Player Load Path', () => { it('attach + initializeMediaSource=true will initialize media source', async () => { - createPlayer(/* attachedTo= */ null); + createPlayer(/* attachedTo= */ null); - expect(video.src).toBeFalsy(); - await player.attach(video, /* initializeMediaSource= */ true); - expect(video.src).toBeTruthy(); - }); + expect(video.src).toBeFalsy(); + await player.attach(video, /* initializeMediaSource= */ true); + expect(video.src).toBeTruthy(); + }); it('attach + initializeMediaSource=false will not intialize media source', async () => { - createPlayer(/* attachedTo= */ null); + createPlayer(/* attachedTo= */ null); - expect(video.src).toBeFalsy(); - await player.attach(video, /* initializeMediaSource= */ false); - expect(video.src).toBeFalsy(); - }); + expect(video.src).toBeFalsy(); + await player.attach(video, /* initializeMediaSource= */ false); + expect(video.src).toBeFalsy(); + }); it('unload + initializeMediaSource=false does not initialize media source', async () => { - createPlayer(/* attachedTo= */ null); + createPlayer(/* attachedTo= */ null); - await player.attach(video); - await player.load('test:sintel'); + await player.attach(video); + await player.load('test:sintel'); - await player.unload(/* initializeMediaSource= */ false); - expect(video.src).toBeFalsy(); - }); + await player.unload(/* initializeMediaSource= */ false); + expect(video.src).toBeFalsy(); + }); it('unload + initializeMediaSource=true initializes media source', async () => { - createPlayer(/* attachedTo= */ null); + createPlayer(/* attachedTo= */ null); - await player.attach(video); - await player.load('test:sintel'); + await player.attach(video); + await player.load('test:sintel'); - await player.unload(/* initializeMediaSource= */ true); - expect(video.src).toBeTruthy(); - }); + await player.unload(/* initializeMediaSource= */ true); + expect(video.src).toBeTruthy(); + }); // There was a bug when calling unload before calling load would cause // the load to continue before the (first) unload was complete. @@ -1083,7 +1085,7 @@ describe('Player Load Path', () => { // Normally the player would load content like this with the media source // path, but since we don't have media source support, it should use the // src= path. - player.load('test:sintel'); + player.load(SMALL_MP4_CONTENT_URI); const event = await spyIsCalled(stateIdleSpy); expect(event.state).toBe('src-equals'); @@ -1091,7 +1093,7 @@ describe('Player Load Path', () => { it('unloading ignores init media source flag', async () => { await player.attach(video, /* initMediaSource= */ false); - await player.load('test:sintel'); + await player.load(SMALL_MP4_CONTENT_URI); // Normally the player would try to go to the media source state because // we are saying to initialize media source after unloading, but since we @@ -1378,7 +1380,7 @@ describe('Player Load Path', () => { }) .set('src-equals', async () => { await player.attach(video, /* initMediaSource= */ false); - await player.load('test:sintel', 0, 'video/mp4'); + await player.load(SMALL_MP4_CONTENT_URI, 0, 'video/mp4'); }); const action = actions.get(state); @@ -1472,7 +1474,7 @@ describe('Player Load Path', () => { return player.load('test:sintel'); }) .set('src-equals', () => { - return player.load('test:sintel', 0, 'video/mp4'); + return player.load(SMALL_MP4_CONTENT_URI, 0, 'video/mp4'); }); const action = actions.get(state);