Skip to content

Commit

Permalink
Fix race in Player WRT external text tracks.
Browse files Browse the repository at this point in the history
When adding text tracks, the Player did not add the stream to the
manifest until after it is initialized.  However, the streaming
engine may schedule an update before the track is added causing it
to fail.  Now the Player will add the stream first.

Closes #418

Change-Id: I4758aabcddfb69bc54b805351c22df7516a57b2e
  • Loading branch information
TheModMaker committed Jun 29, 2016
1 parent 1d9626a commit 5390891
Showing 1 changed file with 16 additions and 4 deletions.
20 changes: 16 additions & 4 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ shaka.Player = function(video, opt_dependencyInjector) {
*/
this.nextExternalStreamId_ = 1e9;

/** @private {!Array.<number>} */
this.loadingTextStreamIds_ = [];

/** @private {boolean} */
this.buffering_ = false;

Expand Down Expand Up @@ -799,7 +802,11 @@ shaka.Player.prototype.getTracks = function() {

var activeStreams = this.streamingEngine_.getActiveStreams();
var period = this.streamingEngine_.getCurrentPeriod();
return shaka.util.StreamUtils.getTracks(period, activeStreams);
return shaka.util.StreamUtils.getTracks(period, activeStreams)
.filter(function(track) {
// Don't show any tracks that are being loaded still.
return this.loadingTextStreamIds_.indexOf(track.id) < 0;
}.bind(this));
};


Expand Down Expand Up @@ -984,12 +991,17 @@ shaka.Player.prototype.addTextTrack = function(
streams: [stream]
};

// Add the stream to the loading list to ensure it isn't switched to while it
// is initializing.
this.loadingTextStreamIds_.push(stream.id);
period.streamSets.push(streamSet);

return this.streamingEngine_.notifyNewStream('text', stream).then(function() {
if (this.destroyed_) return;

// Only add the stream once it has been initialized. This ensures that
// calls to getTracks do not return the uninitialized stream.
period.streamSets.push(streamSet);
// Remove the stream from the loading list.
this.loadingTextStreamIds_.splice(
this.loadingTextStreamIds_.indexOf(stream.id), 1);

// This will disable AbrManager but it will be enabled again below.
var chosen = this.chooseStreams_(period);
Expand Down

0 comments on commit 5390891

Please sign in to comment.