Skip to content

Commit

Permalink
Always create media source for external text.
Browse files Browse the repository at this point in the history
We used to only create media source for external text
when captions are turned on. It created problems with combining
external text with native controls. Native captions button changes
the state of a text track (unlike our button, which also triggers
media source creation if needed), but because there is no media source for
text, we had nothing to display.

Fixes #1596.

Change-Id: Ifeca9bf987f78061d77ccbc89bf94066061ebd74
  • Loading branch information
ismena committed Sep 25, 2018
1 parent 1155ad3 commit 0c231d0
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 12 deletions.
8 changes: 3 additions & 5 deletions lib/media/streaming_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -520,11 +520,10 @@ shaka.media.StreamingEngine.prototype.getStream_ = function(type) {
* the stream has been set up, and a media state has been created.
*
* @param {shaka.extern.Stream} stream
* @param {boolean} createMediaState
* @return {!Promise}
*/
shaka.media.StreamingEngine.prototype.loadNewTextStream = async function(
stream, createMediaState) {
stream) {
const ContentType = shaka.util.ManifestParserUtils.ContentType;

// Clear MediaSource's buffered text, so that the new text stream will
Expand Down Expand Up @@ -553,8 +552,7 @@ shaka.media.StreamingEngine.prototype.loadNewTextStream = async function(
await this.setupStreams_(streamSet);
if (this.destroyed_) { return; }

if (createMediaState &&
(this.textStreamSequenceId_ == currentSequenceId) &&
if ((this.textStreamSequenceId_ == currentSequenceId) &&
!this.mediaStates_.has(ContentType.TEXT) &&
!this.unloadingTextStream_) {
let playheadTime = this.playerInterface_.playhead.getTime();
Expand Down Expand Up @@ -660,7 +658,7 @@ shaka.media.StreamingEngine.prototype.switchInternal_ = function(

if (!mediaState && stream.type == ContentType.TEXT &&
this.config_.ignoreTextStreamFailures) {
this.loadNewTextStream(stream, /* createMediaState */ true);
this.loadNewTextStream(stream);
return;
}
goog.asserts.assert(mediaState, 'switch: expected mediaState to exist');
Expand Down
9 changes: 4 additions & 5 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -1965,8 +1965,7 @@ shaka.Player.prototype.setTextTrackVisibility = function(on) {
this.currentTextRole_);
let stream = textStreams[0];
if (stream) {
this.streamingEngine_.loadNewTextStream(
stream, /* createMediaState */ true);
this.streamingEngine_.loadNewTextStream(stream);
}
} else {
this.streamingEngine_.unloadTextStream();
Expand Down Expand Up @@ -2171,9 +2170,9 @@ shaka.Player.prototype.addTextTrack = function(
// is initializing.
this.loadingTextStreamIds_.push(stream.id);
period.textStreams.push(stream);
// Don't create the media state until subtitles are actually enabled
return this.streamingEngine_.loadNewTextStream(
stream, this.textVisibility_).then(function() {

return this.streamingEngine_.loadNewTextStream(stream)
.then(function() {
if (this.destroyer_.destroyed()) {
return;
}
Expand Down
3 changes: 1 addition & 2 deletions test/media/streaming_engine_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -631,8 +631,7 @@ describe('StreamingEngine', function() {
if (playheadTime == 20) {
mediaSourceEngine.clear.calls.reset();
mediaSourceEngine.init.calls.reset();
streamingEngine.loadNewTextStream(textStream1,
/* createMediaState */ true);
streamingEngine.loadNewTextStream(textStream1);
expect(mediaSourceEngine.clear).toHaveBeenCalledWith('text');

const expectedObject = new Map();
Expand Down

0 comments on commit 0c231d0

Please sign in to comment.