Skip to content

Commit

Permalink
New Error Code for Content unsupported by Browser
Browse files Browse the repository at this point in the history
Previously "UNPLAYABLE_PERIOD" exception is thrown when a browser
doesn't support the container or codecs in a piece of content, which is
confusing to developers and customers.
Changing it to "CONTENT_NOT_SUPPORTED_BY_BROWSER" exception.

Test manifest:
https://media-ci.foxford.ru/dist/hls-issue/issue.master.m3u8

Closes #868.

Change-Id: Ied135b687190919abbeb1561c2bff36a7203136e
  • Loading branch information
michellezhuogg committed Aug 4, 2017
1 parent 7e0f469 commit d67764b
Show file tree
Hide file tree
Showing 17 changed files with 312 additions and 46 deletions.
9 changes: 6 additions & 3 deletions externs/shaka/manifest_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ shakaExtern.ManifestParser = function() {};
/**
* @typedef {{
* networkingEngine: !shaka.net.NetworkingEngine,
* filterPeriod: function(shakaExtern.Period),
* filterNewPeriod: function(shakaExtern.Period),
* filterAllPeriods: function(!Array.<!shakaExtern.Period>),
* onTimelineRegionAdded: function(shakaExtern.TimelineRegionInfo),
* onEvent: function(!Event),
* onError: function(!shaka.util.Error)
Expand All @@ -52,8 +53,10 @@ shakaExtern.ManifestParser = function() {};
*
* @property {!shaka.net.NetworkingEngine} networkingEngine
* The networking engine to use for network requests.
* @property {function(shakaExtern.Period)} filterPeriod
* Should be called on all new Periods so that they can be filtered.
* @property {function(shakaExtern.Period)} filterNewPeriod
* Should be called on a new Period so that it can be filtered.
* @property {function(!Array.<!shakaExtern.Period>)} filterAllPeriods
* Should be called on all Periods so that they can be filtered.
* @property {function(shakaExtern.TimelineRegionInfo)} onTimelineRegionAdded
* Should be called when a new timeline region is added.
* @property {function(!Event)} onEvent
Expand Down
15 changes: 11 additions & 4 deletions lib/dash/dash_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -629,12 +629,14 @@ shaka.dash.DashParser.prototype.parsePeriods_ = function(

// If there are any new periods, call the callback and add them to the
// manifest. If this is the first parse, it will see all of them as new.
// Call filterAllPeriods for the first parse, and filterNewPeriod for any
// new period that comes up.
var periodId = context.period.id;
if (this.periodIds_.every(Functional.isNotEqualFunc(periodId))) {
this.playerInterface_.filterPeriod(period);
if (this.manifest_ &&
this.periodIds_.every(Functional.isNotEqualFunc(periodId))) {
this.playerInterface_.filterNewPeriod(period);
this.periodIds_.push(periodId);
if (this.manifest_)
this.manifest_.periods.push(period);
this.manifest_.periods.push(period);
}


Expand All @@ -653,6 +655,11 @@ shaka.dash.DashParser.prototype.parsePeriods_ = function(
}

prevEnd = start + periodDuration;
} // end of period parsing loop

// Call filterAllPeriods if this is the initial parse.
if (this.manifest_ == null) {
this.playerInterface_.filterAllPeriods(periods);
}

if (presentationDuration != null) {
Expand Down
2 changes: 1 addition & 1 deletion lib/hls/hls_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ shaka.hls.HlsParser.prototype.parseManifest_ = function(data, uri) {
return this.createPeriod_(playlist).then(function(period) {
// HLS has no notion of periods. We're treating the whole presentation as
// one period.
this.playerInterface_.filterPeriod(period);
this.playerInterface_.filterAllPeriods([period]);
return {
presentationTimeline: this.presentationTimeline_,
periods: [period],
Expand Down
4 changes: 2 additions & 2 deletions lib/offline/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ shaka.offline.Storage.prototype.loadInternal = function(

var playerInterface = {
networkingEngine: netEngine,
filterPeriod: this.filterPeriod_.bind(this),
filterNewPeriod: this.filterPeriod_.bind(this),
onTimelineRegionAdded: function() {},
onEvent: function() {},
onError: onError
Expand Down Expand Up @@ -624,7 +624,7 @@ shaka.offline.Storage.prototype.filterPeriod_ = function(period) {
if (variant.audio) activeStreams[ContentType.AUDIO] = variant.audio;
}
}
StreamUtils.filterPeriod(this.drmEngine_, activeStreams, period);
StreamUtils.filterNewPeriod(this.drmEngine_, activeStreams, period);
StreamUtils.applyRestrictions(
period, this.player_.getConfiguration().restrictions,
/* maxHwRes */ { width: Infinity, height: Infinity });
Expand Down
78 changes: 69 additions & 9 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,8 @@ shaka.Player.prototype.load = function(manifestUri, opt_startTime,
goog.asserts.assert(this.networkingEngine_, 'Must not be destroyed');
var playerInterface = {
networkingEngine: this.networkingEngine_,
filterPeriod: this.filterPeriod_.bind(this),
filterNewPeriod: this.filterNewPeriod_.bind(this),
filterAllPeriods: this.filterAllPeriods_.bind(this),
onTimelineRegionAdded: this.onTimelineRegionAdded_.bind(this),
onEvent: this.onEvent_.bind(this),
onError: this.onError_.bind(this)
Expand All @@ -534,7 +535,7 @@ shaka.Player.prototype.load = function(manifestUri, opt_startTime,
// Use a string index here so the compiler doesn't complain about the
// incorrect arguments.
return this.parser_['start'](
manifestUri, this.networkingEngine_, playerInterface.filterPeriod,
manifestUri, this.networkingEngine_, playerInterface.filterNewPeriod,
playerInterface.onError, playerInterface.onEvent);
}

Expand Down Expand Up @@ -574,7 +575,7 @@ shaka.Player.prototype.load = function(manifestUri, opt_startTime,
return this.drmEngine_.init(manifest, false /* isOffline */);
}.bind(this)).then(function() {
// Re-filter the manifest after DRM has been initialized.
this.manifest_.periods.forEach(this.filterPeriod_.bind(this));
this.filterAllPeriods_(this.manifest_.periods);

this.lastTimeStatsUpdateTimestamp_ = Date.now() / 1000;

Expand Down Expand Up @@ -644,7 +645,7 @@ shaka.Player.prototype.load = function(manifestUri, opt_startTime,
}

// Re-filter the manifest after streams have been chosen.
this.manifest_.periods.forEach(this.filterPeriod_.bind(this));
this.manifest_.periods.forEach(this.filterNewPeriod_.bind(this));
// Dispatch a 'trackschanged' event now that all initial filtering is done.
this.onTracksChanged_();
// Since the first streams just became active, send an adaptation event.
Expand Down Expand Up @@ -863,7 +864,9 @@ shaka.Player.prototype.createStreamingEngine = function() {
onError: this.onError_.bind(this),
onEvent: this.onEvent_.bind(this),
onManifestUpdate: this.onManifestUpdate_.bind(this),
onSegmentAppended: this.onSegmentAppended_.bind(this)
onSegmentAppended: this.onSegmentAppended_.bind(this),
filterNewPeriod: this.filterNewPeriod_.bind(this),
filterAllPeriods: this.filterAllPeriods_.bind(this)
};
return new shaka.media.StreamingEngine(this.manifest_, playerInterface);
};
Expand Down Expand Up @@ -923,8 +926,8 @@ shaka.Player.prototype.applyConfig_ = function() {

// Need to apply the restrictions to every period.
try {
// this.filterPeriod_() may throw.
this.manifest_.periods.forEach(this.filterPeriod_.bind(this));
// this.filterNewPeriod_() may throw.
this.manifest_.periods.forEach(this.filterNewPeriod_.bind(this));
} catch (error) {
this.onError_(error);
}
Expand Down Expand Up @@ -1986,16 +1989,73 @@ shaka.Player.prototype.getCleanStats_ = function() {


/**
* Filters a list of periods.
* @param {!Array.<!shakaExtern.Period>} periods
* @private
*/
shaka.Player.prototype.filterAllPeriods_ = function(periods) {
goog.asserts.assert(this.video_, 'Must not be destroyed');
var StreamUtils = shaka.util.StreamUtils;

var activeStreams =
this.streamingEngine_ ? this.streamingEngine_.getActiveStreams() : {};
periods.forEach(function(period) {
StreamUtils.filterNewPeriod(this.drmEngine_, activeStreams, period);
}.bind(this));

var validPeriodsCount = 0;
periods.forEach(function(period) {
if (StreamUtils.getPlayableVariants(period.variants).length > 0)
validPeriodsCount++;
}.bind(this));

// If no periods is playable, throw CONTENT_UNSUPPORTED_BY_BROWSER.
// If only some of the periods are playable, throw UNPLAYABLE_PERIOD.
if (validPeriodsCount == 0) {
throw new shaka.util.Error(
shaka.util.Error.Severity.CRITICAL,
shaka.util.Error.Category.MANIFEST,
shaka.util.Error.Code.CONTENT_UNSUPPORTED_BY_BROWSER);
} else if (validPeriodsCount < periods.length) {
throw new shaka.util.Error(
shaka.util.Error.Severity.CRITICAL,
shaka.util.Error.Category.MANIFEST,
shaka.util.Error.Code.UNPLAYABLE_PERIOD);
}

periods.forEach(function(period) {
var tracksChanged = shaka.util.StreamUtils.applyRestrictions(
period, this.config_.restrictions, this.maxHwRes_);
if (tracksChanged && this.streamingEngine_ &&
this.streamingEngine_.getCurrentPeriod() == period) {
this.onTracksChanged_();
}

var allVariantsRestricted =
StreamUtils.getPlayableVariants(period.variants).length < 1;

if (allVariantsRestricted) {
throw new shaka.util.Error(
shaka.util.Error.Severity.CRITICAL,
shaka.util.Error.Category.MANIFEST,
shaka.util.Error.Code.RESTRICTIONS_CANNOT_BE_MET);
}
}.bind(this));
};


/**
* Filters a new period.
* @param {shakaExtern.Period} period
* @private
*/
shaka.Player.prototype.filterPeriod_ = function(period) {
shaka.Player.prototype.filterNewPeriod_ = function(period) {
goog.asserts.assert(this.video_, 'Must not be destroyed');
var StreamUtils = shaka.util.StreamUtils;

var activeStreams =
this.streamingEngine_ ? this.streamingEngine_.getActiveStreams() : {};
StreamUtils.filterPeriod(this.drmEngine_, activeStreams, period);
StreamUtils.filterNewPeriod(this.drmEngine_, activeStreams, period);

// Check for playable variants before restrictions to give a different error
// if we have restricted all the tracks rather than there being none.
Expand Down
8 changes: 7 additions & 1 deletion lib/util/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -767,5 +767,11 @@ shaka.util.Error.Code = {
* local player instance. To fix this, use Player directly with Storage
* instead of the results of CastProxy.prototype.getPlayer().
*/
'LOCAL_PLAYER_INSTANCE_REQUIRED': 9008
'LOCAL_PLAYER_INSTANCE_REQUIRED': 9008,

/**
* When the manifest contains no period playable streams, it means the
* manifest is unsupported by the browser.
*/
'CONTENT_UNSUPPORTED_BY_BROWSER': 9009
};
2 changes: 1 addition & 1 deletion lib/util/stream_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ shaka.util.StreamUtils.applyRestrictions =
* @param {!Object.<string, shakaExtern.Stream>} activeStreams
* @param {shakaExtern.Period} period
*/
shaka.util.StreamUtils.filterPeriod = function(
shaka.util.StreamUtils.filterNewPeriod = function(
drmEngine, activeStreams, period) {
var StreamUtils = shaka.util.StreamUtils;
var ContentType = shaka.util.ManifestParserUtils.ContentType;
Expand Down
3 changes: 2 additions & 1 deletion test/dash/dash_parser_content_protection_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ describe('DashParser ContentProtection', function() {
});
var playerEvents = {
networkingEngine: netEngine,
filterPeriod: function() {},
filterNewPeriod: function() {},
filterAllPeriods: function() {},
onTimelineRegionAdded: fail, // Should not have any EventStream elements.
onEvent: fail,
onError: fail
Expand Down
18 changes: 13 additions & 5 deletions test/dash/dash_parser_live_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ describe('DashParser Live', function() {
});
playerInterface = {
networkingEngine: fakeNetEngine,
filterPeriod: function() {},
filterNewPeriod: function() {},
filterAllPeriods: function() {},
onTimelineRegionAdded: fail, // Should not have any EventStream elements.
onEvent: fail,
onError: fail
Expand Down Expand Up @@ -362,21 +363,28 @@ describe('DashParser Live', function() {
sprintf(template, {updateTime: updateTime, contents: lines.join('\n')});
var firstManifest = makeSimpleLiveManifestText(lines, updateTime);

var filterPeriod = jasmine.createSpy('filterPeriod');
playerInterface.filterPeriod = Util.spyFunc(filterPeriod);
var filterNewPeriod = jasmine.createSpy('filterNewPeriod');
playerInterface.filterNewPeriod = Util.spyFunc(filterNewPeriod);

var filterAllPeriods = jasmine.createSpy('filterAllPeriods');
playerInterface.filterAllPeriods = Util.spyFunc(filterAllPeriods);

fakeNetEngine.setResponseMapAsText({'dummy://foo': firstManifest});
parser.start('dummy://foo', playerInterface)
.then(function(manifest) {
expect(manifest.periods.length).toBe(1);
expect(filterPeriod.calls.count()).toBe(1);
// Should call filterAllPeriods for parsing the first manifest
expect(filterNewPeriod.calls.count()).toBe(0);
expect(filterAllPeriods.calls.count()).toBe(1);

fakeNetEngine.setResponseMapAsText({'dummy://foo': secondManifest});
delayForUpdatePeriod();

// Should update the same manifest object.
expect(manifest.periods.length).toBe(2);
expect(filterPeriod.calls.count()).toBe(2);
// Should call filterNewPeriod for parsing the new manifest
expect(filterAllPeriods.calls.count()).toBe(1);
expect(filterNewPeriod.calls.count()).toBe(1);
}).catch(fail).then(done);
shaka.polyfill.Promise.flush();
});
Expand Down
3 changes: 2 additions & 1 deletion test/dash/dash_parser_manifest_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ describe('DashParser Manifest', function() {
onEventSpy = jasmine.createSpy('onEvent');
playerInterface = {
networkingEngine: fakeNetEngine,
filterPeriod: function() {},
filterNewPeriod: function() {},
filterAllPeriods: function() {},
onTimelineRegionAdded: fail, // Should not have any EventStream elements.
onEvent: shaka.test.Util.spyFunc(onEventSpy),
onError: fail
Expand Down
3 changes: 2 additions & 1 deletion test/dash/dash_parser_segment_base_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ describe('DashParser SegmentBase', function() {

playerInterface = {
networkingEngine: fakeNetEngine,
filterPeriod: function() {},
filterNewPeriod: function() {},
filterAllPeriods: function() {},
onTimelineRegionAdded: fail, // Should not have any EventStream elements.
onEvent: fail,
onError: fail
Expand Down
3 changes: 2 additions & 1 deletion test/dash/dash_parser_segment_template_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ describe('DashParser SegmentTemplate', function() {

playerInterface = {
networkingEngine: fakeNetEngine,
filterPeriod: function() {},
filterNewPeriod: function() {},
filterAllPeriods: function() {},
onTimelineRegionAdded: fail, // Should not have any EventStream elements.
onEvent: fail,
onError: fail
Expand Down
41 changes: 38 additions & 3 deletions test/hls/hls_parser_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@


describe('HlsParser', function() {
/** @const */
var Util = shaka.test.Util;
/** @type {!shaka.test.FakeNetworkingEngine} */
var fakeNetEngine;
/** @type {!shaka.hls.HlsParser} */
Expand All @@ -42,11 +44,12 @@ describe('HlsParser', function() {
}
};
playerInterface = {
filterPeriod: function() {},
filterNewPeriod: function() {},
filterAllPeriods: function() {},
networkingEngine: fakeNetEngine,
onError: fail,
onEvent: function() {},
onTimelineRegionAdded: function() {}
onEvent: fail,
onTimelineRegionAdded: fail
};
parser = new shaka.hls.HlsParser();
parser.configure(config);
Expand Down Expand Up @@ -447,6 +450,38 @@ describe('HlsParser', function() {
testHlsParser(master, media, manifest, done);
});

it('should call filterAllPeriods for parsing', function(done) {
var master = [
'#EXTM3U\n',
'#EXT-X-STREAM-INF:BANDWIDTH=200,CODECS="avc1",',
'RESOLUTION=960x540,FRAME-RATE=60\n',
'test://video'
].join('');

var media = [
'#EXTM3U\n',
'#EXT-X-MAP:URI="test://main.mp4",BYTERANGE="616@0"\n',
'#EXTINF:5,\n',
'#EXT-X-BYTERANGE:121090@616\n',
'test://main.mp4'
].join('');

fakeNetEngine.setResponseMapAsText({
'test://master': master,
'test://audio': media,
'test://video': media,
'test://text': media
});

var filterAllPeriods = jasmine.createSpy('filterAllPeriods');
playerInterface.filterAllPeriods = Util.spyFunc(filterAllPeriods);

parser.start('test://master', playerInterface)
.then(function(manifest) {
expect(filterAllPeriods.calls.count()).toBe(1);
}).catch(fail).then(done);
});

it('gets mime type from header request', function(done) {
var master = [
'#EXTM3U\n',
Expand Down
Loading

0 comments on commit d67764b

Please sign in to comment.