Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

specific rebufferingGoal override manifest one #1547 #1581

Merged
merged 6 commits into from
Sep 21, 2018
Merged

specific rebufferingGoal override manifest one #1547 #1581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
74c4060
add the config field manifest.dash.ignoreMinBufferTime
fadomire Sep 19, 2018
0285c45
fix parseAttr() eventually returning null
fadomire Sep 19, 2018
3be8be2
add ignoreMinBufferTime to doc and fix tests
fadomire Sep 19, 2018
f483d59
fix typo
fadomire Sep 19, 2018
4f4436d
add name to authors and contributors
fadomire Sep 19, 2018
7091f3f
add tests for ignoreMinBufferTime
fadomire Sep 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Alugha GmbH <*@alugha.com>
Bonnier Broadcasting <*@bonnierbroadcasting.com>
Bryan Huh <[email protected]>
Esteban Dosztal <[email protected]>
Fadomire <[email protected]>
Google Inc. <*@google.com>
Edgeware AB <*@edgeware.tv>
Giuseppe Samela <[email protected]>
Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Costel Madalin Grecu <[email protected]>
Donato Borrello <[email protected]>
Duc Pham <[email protected]>
Esteban Dosztal <[email protected]>
Fadomire <[email protected]>
François Beaufort <[email protected]>
Giuseppe Samela <[email protected]>
Itay Kinnrot <[email protected]>
Expand Down
7 changes: 6 additions & 1 deletion externs/shaka/player.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,8 @@ shaka.extern.DrmConfiguration;
* clockSyncUri: string,
* ignoreDrmInfo: boolean,
* xlinkFailGracefully: boolean,
* defaultPresentationDelay: number
* defaultPresentationDelay: number,
* ignoreMinBufferTime: boolean
* }}
*
* @property {shaka.extern.DashContentProtectionCallback} customScheme
Expand All @@ -536,6 +537,10 @@ shaka.extern.DrmConfiguration;
* @property {number} defaultPresentationDelay
* A default presentationDelay if suggestedPresentationDelay is missing
* in the MPEG DASH manifest. This has to be bigger than minBufferTime * 1.5.
* @property {boolean} ignoreMinBufferTime
* If true will cause DASH parser to ignore minBufferTime from manifest.
* It allows player config to take precedence over manifest for
* rebufferingGoal. Defaults to false if not provided.
*
* @exportDoc
*/
Expand Down
7 changes: 6 additions & 1 deletion lib/dash/dash_parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,13 @@ shaka.dash.DashParser.prototype.processManifest_ =
let baseUris = shaka.util.ManifestParserUtils.resolveUris(
manifestBaseUris, uris);

let minBufferTime =
let ignoreMinBufferTime = this.config_.dash.ignoreMinBufferTime;
let minBufferTime;
if (!ignoreMinBufferTime) {
minBufferTime =
XmlUtils.parseAttr(mpd, 'minBufferTime', XmlUtils.parseDuration);
}

this.updatePeriod_ = /** @type {number} */ (XmlUtils.parseAttr(
mpd, 'minimumUpdatePeriod', XmlUtils.parseDuration, -1));

Expand Down
1 change: 1 addition & 0 deletions lib/util/player_configuration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ shaka.util.PlayerConfiguration = class {
ignoreDrmInfo: false,
xlinkFailGracefully: false,
defaultPresentationDelay: 10,
ignoreMinBufferTime: false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be added to externs/shaka/player.js, in the shaka.extern.DashManifestConfiguration type. Please add @property to describe the property, and also add it to the @typedef block.

This will probably produce a few build errors, as the DASH config object in several of our tests will need to be updated to match the updated type. Be sure to run python build/all.py and fix any errors that come up.

After that, please add a new test in test/dash/dash_parser_manifest_unit.js that shows the effect of ignoreMinBufferTime. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not yet sure on how to write a test properly but will check it later on

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest looking at the very last test in that file ('exposes Representation IDs') as a template.

The important parts of the test are:

  1. Fake manifest text. This can be very simple for your case, with only one Representation.
  2. A call to fakeNetEngine.setResponseText() so the fake manifest text gets returned from the networking engine.
  3. Any necessary configuration passed to parser.configure(). For you, this should set the new flag.
  4. The (async) call to parser.start() to parse the manifest and get the results.
  5. Expectations on the output. You would expect that minBufferTime is 0, in spite of the attribute in the manifest text.

Does that help?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitly help, thanks

},
};

Expand Down
1 change: 1 addition & 0 deletions test/dash/dash_parser_content_protection_unit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe('DashParser ContentProtection', function() {
ignoreDrmInfo: ignoreDrmInfo,
xlinkFailGracefully: false,
defaultPresentationDelay: 10,
ignoreMinBufferTime: false,
},
});
let playerEvents = {
Expand Down
2 changes: 2 additions & 0 deletions test/dash/dash_parser_live_unit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ describe('DashParser Live', function() {
ignoreDrmInfo: false,
xlinkFailGracefully: false,
defaultPresentationDelay: 10,
ignoreMinBufferTime: false,
},
});
playerInterface = {
Expand Down Expand Up @@ -713,6 +714,7 @@ describe('DashParser Live', function() {
ignoreDrmInfo: false,
xlinkFailGracefully: false,
defaultPresentationDelay: 10,
ignoreMinBufferTime: false,
},
});

Expand Down
64 changes: 64 additions & 0 deletions test/dash/dash_parser_manifest_unit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1126,4 +1126,68 @@ describe('DashParser Manifest', function() {
expect(variant.video.originalId).toEqual('video-sd');
expect(textStream.originalId).toEqual('text-en');
});

it('override manifest value if ignoreMinBufferTime is true', async () => {
let manifestText = [
'<MPD minBufferTime="PT75S">',
' <Period id="1" duration="PT30S">',
' <AdaptationSet id="1" mimeType="video/mp4">',
' <Representation id="video-sd" width="640" height="480">',
' <BaseURL>v-sd.mp4</BaseURL>',
' <SegmentBase indexRange="100-200" />',
' </Representation>',
' </AdaptationSet>',
' </Period>',
'</MPD>',
].join('\n');

fakeNetEngine.setResponseText('dummy://foo', manifestText);
parser.configure({
retryParameters: shaka.net.NetworkingEngine.defaultRetryParameters(),
availabilityWindowOverride: NaN,
dash: {
clockSyncUri: '',
customScheme: function(node) { return null; },
ignoreDrmInfo: false,
xlinkFailGracefully: false,
defaultPresentationDelay: 10,
ignoreMinBufferTime: true,
},
});
const manifest = await parser.start('dummy://foo', playerInterface);
const minBufferTime = manifest.minBufferTime;
expect(minBufferTime).toEqual(0);
});

it('get manifest value if ignoreMinBufferTime is false', async () => {
let manifestText = [
'<MPD minBufferTime="PT75S">',
' <Period id="1" duration="PT30S">',
' <AdaptationSet id="1" mimeType="video/mp4">',
' <Representation id="video-sd" width="640" height="480">',
' <BaseURL>v-sd.mp4</BaseURL>',
' <SegmentBase indexRange="100-200" />',
' </Representation>',
' </AdaptationSet>',
' </Period>',
'</MPD>',
].join('\n');

fakeNetEngine.setResponseText('dummy://foo', manifestText);
parser.configure({
retryParameters: shaka.net.NetworkingEngine.defaultRetryParameters(),
availabilityWindowOverride: NaN,
dash: {
clockSyncUri: '',
customScheme: function(node) { return null; },
ignoreDrmInfo: false,
xlinkFailGracefully: false,
defaultPresentationDelay: 10,
ignoreMinBufferTime: false,
},
});
const manifest = await parser.start('dummy://foo', playerInterface);
const minBufferTime = manifest.minBufferTime;
expect(minBufferTime).toEqual(75);
});
});
2 changes: 2 additions & 0 deletions test/hls/hls_live_unit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ describe('HlsParser live', function() {
ignoreDrmInfo: false,
xlinkFailGracefully: false,
defaultPresentationDelay: 10,
ignoreMinBufferTime: false,
},
};

Expand Down Expand Up @@ -498,6 +499,7 @@ describe('HlsParser live', function() {
ignoreDrmInfo: false,
xlinkFailGracefully: false,
defaultPresentationDelay: 10,
ignoreMinBufferTime: false,
},
});

Expand Down
1 change: 1 addition & 0 deletions test/hls/hls_parser_unit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ describe('HlsParser', function() {
ignoreDrmInfo: false,
xlinkFailGracefully: false,
defaultPresentationDelay: 10,
ignoreMinBufferTime: false,
},
};

Expand Down
1 change: 1 addition & 0 deletions test/test/util/dash_parser_util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ shaka.test.Dash.makeDashParser = function() {
ignoreDrmInfo: false,
xlinkFailGracefully: false,
defaultPresentationDelay: 10,
ignoreMinBufferTime: false,
},
});
return parser;
Expand Down