Skip to content

Commit

Permalink
feat: Use source tags instead of src attribute (#7406)
Browse files Browse the repository at this point in the history
Needed for #5022 

This PR does not enable AirPlay on MSE yet, but moves shaka from using
`src` attribute to `source` tags. With this change we will be able to
enable it more easily, as `src` and `source` should not be used
together.
  • Loading branch information
tykus160 authored Oct 15, 2024
1 parent ce38dd9 commit 445b0ce
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 47 deletions.
4 changes: 3 additions & 1 deletion lib/media/drm_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,9 @@ shaka.media.DrmEngine = class {
// Webkit EME implementation requires the src to be defined to clear
// the MediaKeys.
if (!shaka.util.Platform.isMediaKeysPolyfilled('webkit')) {
goog.asserts.assert(!this.video_.src,
goog.asserts.assert(
!this.video_.src &&
!this.video_.getElementsByTagName('source').length,
'video src must be removed first!');
}

Expand Down
18 changes: 15 additions & 3 deletions lib/media/media_source_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ goog.require('shaka.text.TextEngine');
goog.require('shaka.transmuxer.TransmuxerEngine');
goog.require('shaka.util.BufferUtils');
goog.require('shaka.util.Destroyer');
goog.require('shaka.util.Dom');
goog.require('shaka.util.Error');
goog.require('shaka.util.EventManager');
goog.require('shaka.util.FakeEvent');
Expand Down Expand Up @@ -117,6 +118,9 @@ shaka.media.MediaSourceEngine = class {
/** @private {boolean} */
this.playbackHasBegun_ = false;

/** @private {HTMLSourceElement} */
this.source_ = null;

/** @private {MediaSource} */
this.mediaSource_ = this.createMediaSource(this.mediaSourceOpen_);

Expand Down Expand Up @@ -212,7 +216,13 @@ shaka.media.MediaSourceEngine = class {
// Store the object URL for releasing it later.
this.url_ = shaka.media.MediaSourceEngine.createObjectURL(mediaSource);

this.video_.src = this.url_;
this.video_.removeAttribute('src');
if (this.source_) {
this.video_.removeChild(this.source_);
}
this.source_ = shaka.util.Dom.createSourceElement(this.url_);
this.video_.appendChild(this.source_);
this.video_.load();

return mediaSource;
}
Expand Down Expand Up @@ -409,11 +419,13 @@ shaka.media.MediaSourceEngine = class {
this.eventManager_ = null;
}

if (this.video_) {
if (this.video_ && this.source_) {
// "unload" the video element.
this.video_.removeAttribute('src');
this.video_.removeChild(this.source_);
this.video_.load();
this.video_.disableRemotePlayback = false;
this.video_ = null;
this.source_ = null;
}

this.config_ = null;
Expand Down
39 changes: 23 additions & 16 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -1458,16 +1458,17 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
if (this.video_) {
// Remove all track nodes
shaka.util.Dom.removeAllChildren(this.video_);
}

// 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_ && this.video_.src) {
this.video_.removeAttribute('src');
this.video_.load();
// 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');
this.video_.load();
}
}

if (this.drmEngine_) {
Expand Down Expand Up @@ -2406,6 +2407,10 @@ shaka.Player = class extends shaka.util.FakeEventTarget {

this.makeStateChangeEvent_('media-source');

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

this.createTextDisplayer_();
goog.asserts.assert(this.textDisplayer_,
'Text displayer should be created already');
Expand Down Expand Up @@ -2991,12 +2996,14 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
playbackUri += ',' + this.config_.playRangeEnd;
}
}
mediaElement.src = playbackUri;
// If ManagedMediaSource exists, we've disabled remote playback on our own.
// Reenable it so AirPlay can be used via RemotePlayback API.
if (this.mediaSourceEngine_ && window.ManagedMediaSource) {
mediaElement.disableRemotePlayback = false;

if (this.mediaSourceEngine_ ) {
await this.mediaSourceEngine_.destroy();
this.mediaSourceEngine_ = null;
}
shaka.util.Dom.removeAllChildren(mediaElement);

mediaElement.src = playbackUri;

// Tizen 3 / WebOS won't load anything unless you call load() explicitly,
// no matter the value of the preload attribute. This is harmful on some
Expand Down Expand Up @@ -5360,7 +5367,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
* @export
*/
getChaptersTracks() {
if (this.video_ && this.video_.src && this.video_.textTracks) {
if (this.video_ && this.video_.currentSrc && this.video_.textTracks) {
const textTracks = this.getChaptersTracks_();
const StreamUtils = shaka.util.StreamUtils;
return textTracks.map((text) => StreamUtils.html5TextTrackToTrack(text));
Expand All @@ -5377,7 +5384,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
* @export
*/
getChapters(language) {
if (!this.video_ || !this.video_.src || !this.video_.textTracks) {
if (!this.video_ || !this.video_.currentSrc || !this.video_.textTracks) {
return [];
}
const LanguageUtils = shaka.util.LanguageUtils;
Expand Down
14 changes: 14 additions & 0 deletions lib/util/dom_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,20 @@ shaka.util.Dom = class {
}


/**
* @param {string} url
* @param {string=} mimeType
* @return {!HTMLSourceElement}
*/
static createSourceElement(url, mimeType = '') {
const source =
/** @type {HTMLSourceElement} */ (document.createElement('source'));
source.src = url;
source.type = mimeType;
return source;
}


/**
* Cast a Node/Element to an HTMLElement
*
Expand Down
2 changes: 1 addition & 1 deletion test/media/media_source_engine_integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ describe('MediaSourceEngine', () => {
mediaSourceEngine.configure(config);

mediaSource = /** @type {?} */(mediaSourceEngine)['mediaSource_'];
expect(video.src).toBeTruthy();
expect(video.getElementsByTagName('source').length).toBe(1);
await mediaSourceEngine.init(new Map(), false);
});

Expand Down
20 changes: 9 additions & 11 deletions test/media/media_source_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,24 +218,22 @@ describe('MediaSourceEngine', () => {
// - seek to flush the pipeline on some platforms
// - check buffered.length to assert that flushing the pipeline is okay
mockVideo = {
src: '',
firstElementChild: undefined,
error: null,
currentTime: 0,
buffered: {
length: 0,
},
removeAttribute: /** @this {HTMLVideoElement} */ (attr) => {
// Only called with attr == 'src'.
// This assertion alerts us if the requirements for this mock change.
goog.asserts.assert(attr == 'src', 'Unexpected removeAttribute() call');
mockVideo.src = '';
appendChild: (element) => {
mockVideo.firstElementChild = element;
},
removeChild: () => {
mockVideo.firstElementChild = undefined;
},
removeAttribute: jasmine.createSpy('removeAttribute'),
addEventListener: jasmine.createSpy('addVideoEventListener'),
removeEventListener: jasmine.createSpy('removeVideoEventListener'),
load: /** @this {HTMLVideoElement} */ () => {
// This assertion alerts us if the requirements for this mock change.
goog.asserts.assert(mockVideo.src == '', 'Unexpected load() call');
},
load: jasmine.createSpy('load'),
play: jasmine.createSpy('play'),
paused: true,
autoplay: false,
Expand Down Expand Up @@ -329,7 +327,7 @@ describe('MediaSourceEngine', () => {

expect(createMediaSourceSpy).toHaveBeenCalled();
expect(createObjectURLSpy).toHaveBeenCalled();
expect(mockVideo.src).toBe('blob:foo');
expect(mockVideo.firstElementChild.src).toBe('blob:foo');
});

it('revokes object URL after MediaSource opens', () => {
Expand Down
12 changes: 6 additions & 6 deletions test/player_load_graph_integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,18 @@ describe('Player Load Graph', () => {
async () => {
createPlayer();

expect(video.src).toBeFalsy();
expect(video.getElementsByTagName('source').length).toBeFalsy();
await player.attach(video, /* initializeMediaSource= */ true);
expect(video.src).toBeTruthy();
expect(video.getElementsByTagName('source').length).toBeTruthy();
});

it('attach + initializeMediaSource=false will not intialize media source',
async () => {
createPlayer();

expect(video.src).toBeFalsy();
expect(video.getElementsByTagName('source').length).toBeFalsy();
await player.attach(video, /* initializeMediaSource= */ false);
expect(video.src).toBeFalsy();
expect(video.getElementsByTagName('source').length).toBeFalsy();
});

it('unload + initializeMediaSource=false does not initialize media source',
Expand All @@ -80,7 +80,7 @@ describe('Player Load Graph', () => {
await player.load('test:sintel');

await player.unload(/* initializeMediaSource= */ false);
expect(video.src).toBeFalsy();
expect(video.getElementsByTagName('source').length).toBeFalsy();
});

it('unload + initializeMediaSource=true initializes media source',
Expand All @@ -91,7 +91,7 @@ describe('Player Load Graph', () => {
await player.load('test:sintel');

await player.unload(/* initializeMediaSource= */ true);
expect(video.src).toBeTruthy();
expect(video.getElementsByTagName('source').length).toBeTruthy();
});

it('load and unload can be called multiple times', async () => {
Expand Down
6 changes: 3 additions & 3 deletions test/player_src_equals_integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,19 +255,19 @@ describe('Player Src Equals', () => {
it('configures play and seek range for VOD with start', async () => {
player.configure({playRangeStart: 3});
await loadWithSrcEquals(SMALL_MP4_CONTENT_URI, /* startTime= */ null);
expect(video.src.includes('#t=3')).toBeTruthy();
expect(video.src).toContain('#t=3');
});

it('configures play and seek range for VOD with end', async () => {
player.configure({playRangeEnd: 8});
await loadWithSrcEquals(SMALL_MP4_CONTENT_URI, /* startTime= */ null);
expect(video.src.includes('#t=,8')).toBeTruthy();
expect(video.src).toContain('#t=,8');
});

it('configures play and seek range for VOD with start and end', async () => {
player.configure({playRangeStart: 3, playRangeEnd: 8});
await loadWithSrcEquals(SMALL_MP4_CONTENT_URI, /* startTime= */ null);
expect(video.src.includes('#t=3,8')).toBeTruthy();
expect(video.src).toContain('#t=3,8');
});

// TODO: test HLS on platforms with native HLS
Expand Down
16 changes: 15 additions & 1 deletion test/test/util/simple_fakes.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,10 @@ shaka.test.FakeVideo = class {
this.autoplay = false;
this.paused = false;
this.buffered = createFakeBuffered([]);
this.src = '';
this.offsetWidth = 1000;
this.offsetHeight = 1000;
/** @private {!Array<!Element>} */
this.children_ = [];

/** @type {!jasmine.Spy} */
this.addTextTrack =
Expand Down Expand Up @@ -234,6 +235,19 @@ shaka.test.FakeVideo = class {

/** @type {!jasmine.Spy} */
this.canPlayType = jasmine.createSpy('canPlayType');

/** @type {!jasmine.Spy} */
this.appendChild =
jasmine.createSpy('appendChild').and.callFake((element) => {
this.children_.push(element);
});

/** @type {!jasmine.Spy} */
this.getElementsByTagName =
jasmine.createSpy('getElementsByTagName').and.callFake((tagName) => {
tagName = tagName.toUpperCase();
return this.children_.filter((tag) => tag.tagName === tagName);
});
}
};

Expand Down
14 changes: 9 additions & 5 deletions ui/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -485,17 +485,21 @@ shaka.ui.Overlay = class {
// <source src='foo.m2u8'/>
// </video>
// It should not be specified on both.
const urls = [];
const src = video.getAttribute('src');
if (src) {
const sourceElem = document.createElement('source');
sourceElem.setAttribute('src', src);
video.appendChild(sourceElem);
urls.push(src);
video.removeAttribute('src');
}

for (const elem of video.querySelectorAll('source')) {
for (const source of video.getElementsByTagName('source')) {
urls.push(/** @type {!HTMLSourceElement} */ (source).src);
video.removeChild(source);
}

for (const url of urls) {
try { // eslint-disable-next-line no-await-in-loop
await ui.getControls().getPlayer().load(elem.getAttribute('src'));
await ui.getControls().getPlayer().load(url);
break;
} catch (e) {
shaka.log.error('Error auto-loading asset', e);
Expand Down

0 comments on commit 445b0ce

Please sign in to comment.