specific rebufferingGoal override manifest one #1547 #1581

fadomire · 2018-09-12T15:35:59Z

EDIT: pull request updated to add manifest.dash.ignoreMinBufferTime

~~i'm not really satisfied of adding a defaultRebufferingGoal to config but at least changes in code are very light~~

googlebot · 2018-09-12T15:36:05Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

fadomire · 2018-09-12T15:37:40Z

I signed it!

googlebot · 2018-09-12T15:37:48Z

CLAs look good, thanks!

add the config field manifest.dash.ignoreMinBufferTime, which will default to false. If true, the DASH parser will ignore minBufferTime in the manifest, such that streaming.rebufferingGoal is the only factor in play.

joeyparrish · 2018-09-19T17:05:31Z

lib/dash/dash_parser.js

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


parseAttr() can return null if there's no such attribute, so please add || 0 to the end of this statement.

joeyparrish · 2018-09-19T17:07:44Z

lib/util/player_configuration.js

@@ -101,6 +101,7 @@ shaka.util.PlayerConfiguration = class {
        ignoreDrmInfo: false,
        xlinkFailGracefully: false,
        defaultPresentationDelay: 10,
+        ignoreMinBufferTime: false,


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!

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

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:

Fake manifest text. This can be very simple for your case, with only one Representation.

A call to fakeNetEngine.setResponseText() so the fake manifest text gets returned from the networking engine.

Any necessary configuration passed to parser.configure(). For you, this should set the new flag.

The (async) call to parser.start() to parse the manifest and get the results.

Expectations on the output. You would expect that minBufferTime is 0, in spite of the attribute in the manifest text.

Does that help?

definitly help, thanks

joeyparrish · 2018-09-19T17:10:11Z

Thank you for contributing! You should also add yourself to CONTRIBUTORS and AUTHORS files (alphabetically).

minBufferTime should have a default value of 0

joeyparrish · 2018-09-19T18:28:24Z

externs/shaka/player.js

@@ -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


Minor English nit: This sentence needs a period at the end, and the next line should begin with a capital letter.

fadomire · 2018-09-19T20:03:14Z

i added 2 tests for ignoreMinBufferTime

one to check that when ignoreMinBufferTime is true minBufferTime is 0
another to verify when it is false it should respect manifest minBufferTime

dont know if the second one is necessary or if it is already tested somehow

joeyparrish

Looks great! Thanks!

shaka-bot · 2018-09-21T19:25:31Z

All tests passed!

joeyparrish · 2018-11-09T22:43:31Z

Released in v2.5.0-beta2.

fadomire force-pushed the master branch from 0c52307 to 98b4e19 Compare September 12, 2018 16:41

add the config field manifest.dash.ignoreMinBufferTime

74c4060

add the config field manifest.dash.ignoreMinBufferTime, which will default to false. If true, the DASH parser will ignore minBufferTime in the manifest, such that streaming.rebufferingGoal is the only factor in play.

fadomire force-pushed the master branch from 98b4e19 to 74c4060 Compare September 19, 2018 08:24

fadomire mentioned this pull request Sep 19, 2018

minBufferTime config should not be overriden by dash manifest #1547

Closed

joeyparrish reviewed Sep 19, 2018

View reviewed changes

fadomire added 2 commits September 19, 2018 19:34

fix parseAttr() eventually returning null

0285c45

minBufferTime should have a default value of 0

add ignoreMinBufferTime to doc and fix tests

3be8be2

joeyparrish reviewed Sep 19, 2018

View reviewed changes

fadomire added 3 commits September 19, 2018 20:58

fix typo

f483d59

add name to authors and contributors

4f4436d

add tests for ignoreMinBufferTime

7091f3f

joeyparrish approved these changes Sep 21, 2018

View reviewed changes

joeyparrish merged commit 99ebc69 into shaka-project:master Sep 21, 2018

joeyparrish mentioned this pull request Nov 20, 2018

Playback Stalls For Few Seconds When Playback Initiated #1666

Closed

github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023

github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

specific rebufferingGoal override manifest one #1547 #1581

specific rebufferingGoal override manifest one #1547 #1581

fadomire commented Sep 12, 2018 •

edited

Loading

googlebot commented Sep 12, 2018

fadomire commented Sep 12, 2018

googlebot commented Sep 12, 2018

joeyparrish Sep 19, 2018

joeyparrish Sep 19, 2018

fadomire Sep 19, 2018

joeyparrish Sep 19, 2018

fadomire Sep 19, 2018

joeyparrish commented Sep 19, 2018

joeyparrish Sep 19, 2018

fadomire commented Sep 19, 2018 •

edited

Loading

joeyparrish left a comment

shaka-bot commented Sep 21, 2018

joeyparrish commented Nov 9, 2018

specific rebufferingGoal override manifest one #1547 #1581

specific rebufferingGoal override manifest one #1547 #1581

Conversation

fadomire commented Sep 12, 2018 • edited Loading

googlebot commented Sep 12, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

fadomire commented Sep 12, 2018

googlebot commented Sep 12, 2018

joeyparrish Sep 19, 2018

Choose a reason for hiding this comment

joeyparrish Sep 19, 2018

Choose a reason for hiding this comment

fadomire Sep 19, 2018

Choose a reason for hiding this comment

joeyparrish Sep 19, 2018

Choose a reason for hiding this comment

fadomire Sep 19, 2018

Choose a reason for hiding this comment

joeyparrish commented Sep 19, 2018

joeyparrish Sep 19, 2018

Choose a reason for hiding this comment

fadomire commented Sep 19, 2018 • edited Loading

joeyparrish left a comment

Choose a reason for hiding this comment

shaka-bot commented Sep 21, 2018

joeyparrish commented Nov 9, 2018

fadomire commented Sep 12, 2018 •

edited

Loading

fadomire commented Sep 19, 2018 •

edited

Loading