Skip to content

Commit

Permalink
Fix text filtering and cleanup MIME computation
Browse files Browse the repository at this point in the history 
During recent refactoring, we introduced a bug in which we would
filter out any text stream with a non-empty codecs parameter.

Replaces PR #639

Change-Id: I9a45ef7e1bc250fd989e3972c0670f3e48336511
  • Loading branch information
@joeyparrish
joeyparrish committed Jan 4, 2017
1 parent 2a69c78 commit e093ab2
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 44 deletions.
12 changes: 6 additions & 6 deletions lib/util/stream_utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,10 @@ shaka.util.StreamUtils.filterPeriod = function(
// Filter text streams
for (var i = 0; i < period.textStreams.length; ++i) {
var stream = period.textStreams[i];
if (!shaka.media.TextEngine.isTypeSupported(stream.mimeType)) {
shaka.log.debug('Dropping text stream. Is not supported by the' +
var fullMimeType = StreamUtils.getFullMimeType(
stream.mimeType, stream.codecs);
if (!shaka.media.TextEngine.isTypeSupported(fullMimeType)) {
shaka.log.debug('Dropping text stream. Is not supported by the ' +
'platform.', stream);
period.textStreams.splice(i, 1);
--i;
Expand Down Expand Up @@ -134,10 +136,8 @@ shaka.util.StreamUtils.streamIsCompatible_ =
}

// Check if stream can be played by the platform
var fullMimeType = stream.mimeType;
if (stream.codecs) {
fullMimeType += '; codecs="' + stream.codecs + '"';
}
var fullMimeType = shaka.util.StreamUtils.getFullMimeType(
stream.mimeType, stream.codecs);

if (!shaka.media.MediaSourceEngine.isTypeSupported(fullMimeType))
return false;
Expand Down
106 changes: 68 additions & 38 deletions test/util/stream_utils_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,17 @@
*/

describe('StreamUtils', function() {
var config;
var manifest;

describe('chooses correct variants and text streams', function() {
var config;
var manifest;

beforeAll(function() {

config = /** @type {shakaExtern.PlayerConfiguration} */({
preferredAudioLanguage: 'en',
preferredTextLanguage: 'en'
});
beforeAll(function() {
config = /** @type {shakaExtern.PlayerConfiguration} */({
preferredAudioLanguage: 'en',
preferredTextLanguage: 'en'
});
});

describe('filterVariantsByConfig', function() {
it("chooses variants in user's preferred language", function() {
manifest = new shaka.test.ManifestGenerator()
.addPeriod(0)
Expand All @@ -47,24 +45,6 @@ describe('StreamUtils', function() {
expect(chosen[1]).toBe(manifest.periods[0].variants[2]);
});

it("chooses text streams in user's preferred language", function() {
manifest = new shaka.test.ManifestGenerator()
.addPeriod(0)
.addTextStream(1)
.language('en')
.addTextStream(2)
.language('es')
.addTextStream(3)
.language('en')
.build();

var chosen = shaka.util.StreamUtils.filterTextStreamsByConfig(
manifest.periods[0], config);
expect(chosen.length).toBe(2);
expect(chosen[0]).toBe(manifest.periods[0].textStreams[0]);
expect(chosen[1]).toBe(manifest.periods[0].textStreams[2]);
});

it('chooses primary variants', function() {
manifest = new shaka.test.ManifestGenerator()
.addPeriod(0)
Expand All @@ -83,6 +63,43 @@ describe('StreamUtils', function() {
expect(chosen[1]).toBe(manifest.periods[0].variants[3]);
});

it('filters out resctricted variants', function() {
manifest = new shaka.test.ManifestGenerator()
.addPeriod(0)
.addVariant(0)
.addVariant(1)
.addVariant(2)
.build();

manifest.periods[0].variants[0].allowedByKeySystem = false;
manifest.periods[0].variants[1].allowedByApplication = false;

var chosen = shaka.util.StreamUtils.filterVariantsByConfig(
manifest.periods[0], config);
expect(chosen.length).toBe(1);
expect(chosen[0]).toBe(manifest.periods[0].variants[2]);
});
});

describe('filterTextStreamsByConfig', function() {
it("chooses text streams in user's preferred language", function() {
manifest = new shaka.test.ManifestGenerator()
.addPeriod(0)
.addTextStream(1)
.language('en')
.addTextStream(2)
.language('es')
.addTextStream(3)
.language('en')
.build();

var chosen = shaka.util.StreamUtils.filterTextStreamsByConfig(
manifest.periods[0], config);
expect(chosen.length).toBe(2);
expect(chosen[0]).toBe(manifest.periods[0].textStreams[0]);
expect(chosen[1]).toBe(manifest.periods[0].textStreams[2]);
});

it('chooses primary text streams', function() {
manifest = new shaka.test.ManifestGenerator()
.addPeriod(0)
Expand All @@ -99,22 +116,35 @@ describe('StreamUtils', function() {
expect(chosen[0]).toBe(manifest.periods[0].textStreams[1]);
expect(chosen[1]).toBe(manifest.periods[0].textStreams[2]);
});
});

it('filters out resctricted variants', function() {
describe('filterPeriod', function() {
var fakeDrmEngine;

beforeAll(function() {
fakeDrmEngine = new shaka.test.FakeDrmEngine();
});

it('filters text streams with the full MIME type', function() {
manifest = new shaka.test.ManifestGenerator()
.addPeriod(0)
.addVariant(0)
.addVariant(1)
.addVariant(2)
.addTextStream(1).mime('text/vtt')
.addTextStream(2).mime('application/mp4', 'wvtt')
.addTextStream(3).mime('text/bogus')
.addTextStream(4).mime('application/mp4', 'bogus')
.build();

manifest.periods[0].variants[0].allowedByKeySystem = false;
manifest.periods[0].variants[1].allowedByApplication = false;
var activeStreams = {};
shaka.util.StreamUtils.filterPeriod(
fakeDrmEngine, activeStreams, manifest.periods[0]);

var chosen = shaka.util.StreamUtils.filterVariantsByConfig(
manifest.periods[0], config);
expect(chosen.length).toBe(1);
expect(chosen[0]).toBe(manifest.periods[0].variants[2]);
// Covers a regression in which we would remove streams with codecs.
// The last two streams should be removed because their full MIME types
// are bogus.
expect(manifest.periods[0].textStreams.length).toBe(2);
var textStreams = manifest.periods[0].textStreams;
expect(textStreams[0].id).toBe(1);
expect(textStreams[1].id).toBe(2);
});
});
});

0 comments on commit e093ab2

Please sign in to comment.