Skip to content

Commit

Permalink
Track manually added text tracks in active tracks.
Browse files Browse the repository at this point in the history
There was a bug where manually added text tracks weren't added to the
active track list in Player.  This caused assertion failures and
problems with selecting those tracks.  Now those tracks will be tracked
correctly.  Note that Player will correctly select text tracks based on
language and will allow switching.

Closes #821

Change-Id: Ie9465a2d3bc757f43281ef40658daa488f58bc12
  • Loading branch information
TheModMaker committed Jun 6, 2017
1 parent b3f29a3 commit 70ac498
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 10 deletions.
3 changes: 2 additions & 1 deletion lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,8 @@ shaka.media.StreamingEngine.prototype.getCurrentPeriod = function() {

/**
* Gets a map of all the active streams.
* @return {!Object.<string, shakaExtern.Stream>}
* @return {!Object.<shaka.util.ManifestParserUtils.ContentType,
* shakaExtern.Stream>}
*/
shaka.media.StreamingEngine.prototype.getActiveStreams = function() {
goog.asserts.assert(this.mediaStates_, 'Must be initialized');
Expand Down
14 changes: 13 additions & 1 deletion lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -1578,6 +1578,15 @@ shaka.Player.prototype.addTextTrack = function(
return this.streamingEngine_.notifyNewTextStream(stream).then(function() {
if (this.destroyed_) return;

// If this was the first text stream, StreamingEngine will start streaming
// it in notifyNewTextStream. So update the active stream.
var curPeriodIdx = this.manifest_.periods.indexOf(period);
var activeStreams = this.streamingEngine_.getActiveStreams();
if (activeStreams[ContentType.TEXT]) {
this.activeStreamsByPeriod_[curPeriodIdx][ContentType.TEXT] =
activeStreams[ContentType.TEXT].id;
}

// Remove the stream from the loading list.
this.loadingTextStreamIds_.splice(
this.loadingTextStreamIds_.indexOf(stream.id), 1);
Expand Down Expand Up @@ -1988,7 +1997,10 @@ shaka.Player.prototype.assertCorrectActiveStreams_ = function() {

var playerActive = this.activeStreamsByPeriod_[currentPeriodIndex] || {};
for (var type in streamingActive) {
goog.asserts.assert(streamingActive[type].id == playerActive[type],
var activeId = streamingActive[type].id;
if (this.deferredSwitches_[type])
activeId = this.deferredSwitches_[type].stream.id;
goog.asserts.assert(activeId == playerActive[type],
'Inconsistent active stream');
}
};
Expand Down
17 changes: 17 additions & 0 deletions test/player_integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,23 @@ describe('Player', function() {
});

describe('plays', function() {
it('while external text tracks', function(done) {
player.load('test:sintel_no_text_compiled').then(function() {
player.addTextTrack('/base/test/test/assets/text-clip.vtt', 'en',
'subtitles', 'text/vtt');

video.play();
return Util.delay(5);
}).then(function() {
var textTracks = player.getTextTracks();
expect(textTracks).toBeTruthy();
expect(textTracks.length).toBe(1);

expect(textTracks[0].active).toBe(true);
expect(textTracks[0].language).toEqual('en');
}).catch(fail).then(done);
});

window.shakaAssets.testAssets.forEach(function(asset) {
if (asset.disabled) return;

Expand Down
41 changes: 33 additions & 8 deletions test/test/util/test_scheme.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,29 @@ shaka.test.TestScheme.DATA = {
},
duration: 30
},
'sintel_no_text': {
video: {
initSegmentUri: '/base/test/test/assets/sintel-video-init.mp4',
mvhdOffset: 0x24,
segmentUri: '/base/test/test/assets/sintel-video-segment.mp4',
tfdtOffset: 0x38,
segmentDuration: 10,
presentationTimeOffset: 0,
mimeType: 'video/mp4',
codecs: 'avc1.42c01e'
},
audio: {
initSegmentUri: '/base/test/test/assets/sintel-audio-init.mp4',
mvhdOffset: 0x20,
segmentUri: '/base/test/test/assets/sintel-audio-segment.mp4',
tfdtOffset: 0x3c,
segmentDuration: 10.005,
presentationTimeOffset: 0,
mimeType: 'audio/mp4',
codecs: 'mp4a.40.2'
},
duration: 30
},
'sintel-enc': {
video: {
initSegmentUri: '/base/test/test/assets/encrypted-sintel-video-init.mp4',
Expand Down Expand Up @@ -293,15 +316,17 @@ shaka.test.TestScheme.createManifests = function(shaka, suffix) {
gen.addAudio(2);
addStreamInfo(gen, data, ContentType.AUDIO, name);

// This seems to be necessary. Otherwise, we end up with a URL like
// "http:/base/..." which then fails to load on Safari for some reason.
var locationUri = new goog.Uri(location.href);
var partialUri = new goog.Uri(data.text.uri);
var absoluteUri = locationUri.resolve(partialUri);
if (data.text) {
// This seems to be necessary. Otherwise, we end up with a URL like
// "http:/base/..." which then fails to load on Safari for some reason.
var locationUri = new goog.Uri(location.href);
var partialUri = new goog.Uri(data.text.uri);
var absoluteUri = locationUri.resolve(partialUri);

gen.addTextStream(3)
.mime(data.text.mimeType, data.text.codecs)
.textStream(absoluteUri.toString());
gen.addTextStream(3)
.mime(data.text.mimeType, data.text.codecs)
.textStream(absoluteUri.toString());
}

MANIFESTS[name + suffix] = gen.build();
}
Expand Down

0 comments on commit 70ac498

Please sign in to comment.