Skip to content

Commit

Permalink
Fix text tracks on IE/Edge.
Browse files Browse the repository at this point in the history
IE/Edge don't accept cues that are out of time order.  So when we
reverse the order of cues for #848, the cues would be rejected.  Now
we sort the cues and only reverse the order of cues with equal times.

Change-Id: I860e1ea9694eb95ff2e74d9545c92373eb371686
  • Loading branch information
TheModMaker committed Aug 10, 2017
1 parent 2af0de2 commit 404d42c
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 6 deletions.
23 changes: 18 additions & 5 deletions lib/text/simple_text_displayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,24 @@ shaka.text.SimpleTextDisplayer.prototype.append = function(cues) {
textTrackCues.push(cue);
}

// Push these to the browser in reverse order. The browser will display cues
// with identical time ranges from the bottom up. Feeding them in backwards
// means the first one parsed will be at the top, as you would expect.
// See https://github.com/google/shaka-player/issues/848 for more information.
textTrackCues.reverse().forEach(function(cue) {
// Sort the cues based on start/end times. Make a copy of the array so
// we can get the index in the original ordering. Out of order cues are
// rejected by IE/Edge. See https://goo.gl/BirBy9
var sortedCues = textTrackCues.slice().sort(function(a, b) {
if (a.startTime != b.startTime)
return a.startTime - b.startTime;
else if (a.endTime != b.endTime)
return a.endTime - b.startTime;
else
// The browser will display cues with identical time ranges from the
// bottom up. Reversing the order of equal cues means the first one
// parsed will be at the top, as you would expect.
// See https://github.com/google/shaka-player/issues/848 for more
// information.
return textTrackCues.indexOf(b) - textTrackCues.indexOf(a);
});

sortedCues.forEach(function(cue) {
this.textTrack_.addCue(cue);
}.bind(this));
};
Expand Down
17 changes: 16 additions & 1 deletion test/text/simple_text_displayer_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,22 @@ describe('SimpleTextDisplayer', function() {
});

describe('append', function() {
it('appends cues in reverse order', function() {
it('sorts cues before inserting', function() {
// See: https://goo.gl/BirBy9
verifyHelper(
[
{start: 10, end: 20, text: 'Test1'},
{start: 20, end: 30, text: 'Test2'},
{start: 30, end: 40, text: 'Test3'}
],
[
new shaka.text.Cue(20, 30, 'Test2'),
new shaka.text.Cue(30, 40, 'Test3'),
new shaka.text.Cue(10, 20, 'Test1')
]);
});

it('appends equal time cues in reverse order', function() {
// Regression test for https://github.com/google/shaka-player/issues/848
verifyHelper(
[
Expand Down

0 comments on commit 404d42c

Please sign in to comment.