Skip to content

Commit

Permalink
Prefer no role text streams when no preferred role
Browse files Browse the repository at this point in the history
Beforehand, if there were some text streams with a given language
which had a role, and some which did not, the no-role text streams
would never be chosen.
Now, they are chosen if there is no preferred role.

Closes #1212

Change-Id: I28b426304b1a828f500012425b8d89c94ccc3178
  • Loading branch information
theodab authored and joeyparrish committed Jan 19, 2018
1 parent 8c501ed commit 07b2d90
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
7 changes: 7 additions & 0 deletions lib/util/stream_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,13 @@ shaka.util.StreamUtils.filterStreamsByLanguageAndRole = function(
} else {
shaka.log.warning('No exact match for the text role could be found.');
}
} else {
// Prefer text streams with no roles, if they exist.
var noRoleMatches = chosen.filter(function(stream) {
return stream.roles.length == 0;
});
if (noRoleMatches.length)
return noRoleMatches;
}

// Either there was no role preference, or it could not be satisfied.
Expand Down
42 changes: 42 additions & 0 deletions test/util/stream_utils_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,48 @@ describe('StreamUtils', function() {
expect(chosen[0]).toBe(manifest.periods[0].textStreams[0]);
});

it('prefers no-role streams if there is no preferred role', function() {
manifest = new shaka.test.ManifestGenerator()
.addPeriod(0)
.addTextStream(0)
.language('en')
.roles(['commentary'])
.addTextStream(1)
.language('en')
.addTextStream(2)
.language('en')
.roles(['secondary'])
.build();

var chosen = filterStreamsByLanguageAndRole(
manifest.periods[0].textStreams,
'en',
'');
expect(chosen.length).toBe(1);
expect(chosen[0].roles.length).toBe(0); // Pick a stream with no role.
});

it('ignores no-role streams if there is a preferred role', function() {
manifest = new shaka.test.ManifestGenerator()
.addPeriod(0)
.addTextStream(0)
.language('en')
.roles(['commentary'])
.addTextStream(1)
.language('en')
.addTextStream(2)
.language('en')
.roles(['secondary'])
.build();

var chosen = filterStreamsByLanguageAndRole(
manifest.periods[0].textStreams,
'en',
'main'); // A role that is not present.
expect(chosen.length).toBe(1);
expect(chosen[0].roles.length).toBe(1); // Pick a stream with a role.
});

it('chooses only one role, even if none is preferred', function() {
// Regression test for https://github.com/google/shaka-player/issues/949
manifest = new shaka.test.ManifestGenerator()
Expand Down

0 comments on commit 07b2d90

Please sign in to comment.