Skip to content

Commit

Permalink
fix: Wait for chapters track to be loaded (#4228)
Browse files Browse the repository at this point in the history 
addChaptersTrack() was already an async method, but it did not wait
for the chapter data to be loaded by the browser.  The solution is to
wait for the `load` event on the `<track>` element we create.

To accomplish this, some cleanup and refactoring was done in how
tracks are managed.  Summary of changes:

 - The `addtrack` event, which triggered management of tracks in src=
   playback, is now used for all types of playback.  In src= mode, it
   manages all tracks, and in MSE mode, it only manages chapters
   tracks (which are added to the video element as in src= mode).
 - `processChaptersTrack_()` has been renamed to
   `activateChaptersTrack_()`, since its only job is to set the
   track's mode field to make the browser load it.
 - `activateChaptersTrack_()` is now only ever called via the
   `addtrack` event.
 - `activateChaptersTrack_()` no longer loops over all chapter tracks
   on a timer, and instead only touches the single track it was
   called for.
 - `addSrcTrackElement_()` now returns the HTML `<track>` element it
   creates.
 - `addChaptersTrack()` now awaits a `load` or `error` event to
   complete (or fail) the operation.
 - Existing tests for addChaptersTrack had long delays to work around
   this issue; these delays have simply been removed.

Fixes #4186
  • Loading branch information
@joeyparrish
joeyparrish committed May 17, 2022
1 parent b3d4c1c commit 0245971
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 106 deletions.
78 changes: 65 additions & 13 deletions lib/player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1869,6 +1869,35 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
// "streaming" so that they can access internal information.
this.loadMode_ = shaka.Player.LoadMode.MEDIA_SOURCE;

// This flag is used below in track management to check if this load was
// canceled before the necessary events fired.
let unloaded = false;
this.cleanupOnUnload_.push(() => {
unloaded = true;
});

if (this.video_.textTracks) {
this.eventManager_.listen(this.video_.textTracks, 'addtrack', (e) => {
// If we have moved on to another piece of content while waiting for
// the above event, we should not process tracks here.
if (unloaded) {
return;
}

const trackEvent = /** @type {!TrackEvent} */(e);
if (trackEvent.track) {
const track = trackEvent.track;
goog.asserts.assert(track instanceof TextTrack, 'Wrong track type!');

switch (track.kind) {
case 'chapters':
this.activateChaptersTrack_(track);
break;
}
}
});
}

// The event must be fired after we filter by restrictions but before the
// active stream is picked to allow those listening for the "streaming"
// event to make changes before streaming starts.
Expand Down Expand Up @@ -2167,6 +2196,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
this.eventManager_.listen(
this.video_.audioTracks, 'change', () => this.onTracksChanged_());
}

if (this.video_.textTracks) {
this.eventManager_.listen(this.video_.textTracks, 'addtrack', (e) => {
// If we have moved on to another piece of content while waiting for
Expand All @@ -2179,19 +2209,23 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
if (trackEvent.track) {
const track = trackEvent.track;
goog.asserts.assert(track instanceof TextTrack, 'Wrong track type!');

switch (track.kind) {
case 'metadata':
this.processTimedMetadataSrcEqls_(track);
break;

case 'chapters':
this.processChaptersTrack_(track);
this.activateChaptersTrack_(track);
break;

default:
this.onTracksChanged_();
break;
}
}
});

this.eventManager_.listen(
this.video_.textTracks, 'removetrack', () => this.onTracksChanged_());
this.eventManager_.listen(
Expand Down Expand Up @@ -2444,12 +2478,12 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
}

/**
* We're looking for chapters tracks to process the chapters.
* Set the mode on a chapters track so that it loads.
*
* @param {?TextTrack} track
* @private
*/
processChaptersTrack_(track) {
activateChaptersTrack_(track) {
if (!track || track.kind != 'chapters') {
return;
}
Expand All @@ -2462,10 +2496,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
// this process to be repeated several times to ensure that it has been put
// in the correct mode.
const timer = new shaka.util.Timer(() => {
const chaptersTracks = this.getChaptersTracks_();
for (const chaptersTrack of chaptersTracks) {
chaptersTrack.mode = 'hidden';
}
track.mode = 'hidden';
}).tickNow().tickAfter(0.5);

this.cleanupOnUnload_.push(() => {
Expand Down Expand Up @@ -4557,25 +4588,41 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
if (!mimeType) {
mimeType = await this.getTextMimetype_(uri);
}

let adCuePoints = [];
if (this.adManager_) {
try {
adCuePoints = this.adManager_.getServerSideCuePoints();
} catch (error) {}
}
await this.addSrcTrackElement_(uri, language, /* kind= */ 'chapters',
mimeType, /* label= */ '', adCuePoints);

/** @type {!HTMLTrackElement} */
const trackElement = await this.addSrcTrackElement_(
uri, language, /* kind= */ 'chapters', mimeType, /* label= */ '',
adCuePoints);

const chaptersTracks = this.getChaptersTracks();
const chaptersTrack = chaptersTracks.find((t) => {
return t.language == language;
});

if (chaptersTrack) {
const html5ChaptersTracks = this.getChaptersTracks_();
for (const html5ChaptersTrack of html5ChaptersTracks) {
this.processChaptersTrack_(html5ChaptersTrack);
}
await new Promise((resolve, reject) => {
// The chapter data isn't available until the 'load' event fires, and
// that won't happen until the chapters track is activated by the
// process method.
this.eventManager_.listenOnce(trackElement, 'load', resolve);
this.eventManager_.listenOnce(trackElement, 'error', (event) => {
reject(new shaka.util.Error(
shaka.util.Error.Severity.RECOVERABLE,
shaka.util.Error.Category.TEXT,
shaka.util.Error.Code.CHAPTERS_TRACK_FAILED));
});
});

return chaptersTrack;
}

// This should not happen, but there are browser implementations that may
// not support the Track element.
shaka.log.error('Cannot add this text when loaded with src=');
Expand Down Expand Up @@ -4629,6 +4676,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
* @param {string} mimeType
* @param {string} label
* @param {!Array.<!shaka.extern.AdCuePoint>} adCuePoints
* @return {!Promise.<!HTMLTrackElement>}
* @private
*/
async addSrcTrackElement_(uri, language, kind, mimeType, label,
Expand All @@ -4644,12 +4692,14 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
uri = shaka.media.MediaSourceEngine.createObjectURL(blob);
mimeType = 'text/vtt';
}

const trackElement =
/** @type {!HTMLTrackElement} */(document.createElement('track'));
trackElement.src = uri;
trackElement.label = label;
trackElement.kind = kind;
trackElement.srclang = language;

// Because we're pulling in the text track file via Javascript, the
// same-origin policy applies. If you'd like to have a player served
// from one domain, but the text track served from another, you'll
Expand All @@ -4659,7 +4709,9 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
if (!this.video_.getAttribute('crossorigin')) {
this.video_.setAttribute('crossorigin', 'anonymous');
}

this.video_.appendChild(trackElement);
return trackElement;
}

/**
Expand Down
7 changes: 7 additions & 0 deletions lib/util/error.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,13 @@ shaka.util.Error.Code = {
*/
'MISSING_TEXT_PLUGIN': 2014,

/**
* The chapters track failed to load. The browser does not provide any
* information in this case to identify why it failed, but there may be
* details in the JavaScript console.
*/
'CHAPTERS_TRACK_FAILED': 2015,

/**
* Some component tried to read past the end of a buffer. The segment index,
* init segment, or PSSH may be malformed.
Expand Down
16 changes: 6 additions & 10 deletions test/player_integration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -953,16 +953,16 @@ describe('Player', () => {
});
}); // describe('unloading')

describe('chapters', () => {
it('add external chapters in vtt format', async () => {
describe('addChaptersTrack', () => {
it('adds external chapters in vtt format', async () => {
await player.load('test:sintel_no_text_compiled');
const locationUri = new goog.Uri(location.href);
const partialUri1 = new goog.Uri('/base/test/test/assets/chapters.vtt');
const absoluteUri1 = locationUri.resolve(partialUri1);
await player.addChaptersTrack(absoluteUri1.toString(), 'en');

await shaka.test.Util.delay(1.5);

// Data should be available as soon as addChaptersTrack resolves.
// See https://github.com/shaka-project/shaka-player/issues/4186
const chapters = player.getChapters('en');
expect(chapters.length).toBe(3);
const chapter1 = chapters[0];
Expand All @@ -982,8 +982,6 @@ describe('Player', () => {
const absoluteUri2 = locationUri.resolve(partialUri2);
await player.addChaptersTrack(absoluteUri2.toString(), 'en');

await shaka.test.Util.delay(1.5);

const chaptersUpdated = player.getChapters('en');
expect(chaptersUpdated.length).toBe(6);
const chapterUpdated1 = chaptersUpdated[0];
Expand Down Expand Up @@ -1012,15 +1010,13 @@ describe('Player', () => {
expect(chapterUpdated6.endTime).toBe(61.349);
});

it('add external chapters in srt format', async () => {
it('adds external chapters in srt format', async () => {
await player.load('test:sintel_no_text_compiled');
const locationUri = new goog.Uri(location.href);
const partialUri = new goog.Uri('/base/test/test/assets/chapters.srt');
const absoluteUri = locationUri.resolve(partialUri);
await player.addChaptersTrack(absoluteUri.toString(), 'es');

await shaka.test.Util.delay(1.5);

const chapters = player.getChapters('es');
expect(chapters.length).toBe(3);
const chapter1 = chapters[0];
Expand All @@ -1036,5 +1032,5 @@ describe('Player', () => {
expect(chapter3.startTime).toBe(30);
expect(chapter3.endTime).toBe(61.349);
});
}); // describe('chapters')
}); // describe('addChaptersTrack')
});
Loading

0 comments on commit 0245971

Please sign in to comment.