Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix: Wait for chapters track to be loaded (#4228)
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 committed May 17, 2022
1 parent d6b47bb commit dec3d9c
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
Expand Up @@ -1953,6 +1953,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 @@ -2251,6 +2280,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 @@ -2263,19 +2293,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 @@ -2532,12 +2566,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 @@ -2550,10 +2584,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 @@ -4537,25 +4568,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 @@ -4609,6 +4656,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 @@ -4624,12 +4672,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 = this.cmcdManager_.appendTextTrackData(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 @@ -4639,7 +4689,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
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
Expand Up @@ -1069,16 +1069,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 @@ -1098,8 +1098,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 @@ -1128,15 +1126,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 @@ -1152,5 +1148,5 @@ describe('Player', () => {
expect(chapter3.startTime).toBe(30);
expect(chapter3.endTime).toBe(61.349);
});
}); // describe('chapters')
}); // describe('addChaptersTrack')
});

0 comments on commit dec3d9c

Please sign in to comment.