minBufferTime config should not be overriden by dash manifest

[fadomire]

Hello, i understand that currently the code behave as follow :
A DASH manifest's minBufferTime, if greater, overrides rebufferingGoal.

The problem i see in that behavior is that client knows better about network conditions and may want to adapt the rebufferingGoal value based on that. Currently it is not possible.

Why not make the following rule :
config takes precedence over dash manifest that takes precedence over default value

[ismena]

Hi, thanks for reaching out.

Having config be the source of truth might mean we're no longer compliant with the dash spec, but let me see if our TL is open to a discussion on this @joeyparrish

[joeyparrish]

That seems reasonable. I'll put this on the backlog, but we would be happy to accept a PR for this if you have time to work on it. Thanks!

[fadomire]

yes sure i’ll prepare a PR in the following weeks

[fadomire]

this seems less easy than i first thought because of the following flow:
specific config overrides default value and is then passed to manifest parsers

so when the parser get the dash manifest minBufferTime it does not know if the config value is the default one or a specific config one

i can think of some hack around it, but i don't like that

i'll need more time to come with a proper solution if any, so i'll let you know

ideas welcome

[vaage]

@fadomire I'll want @joeyparrish to chime in on this suggestion, but what we could do is replace minBufferTime: number with resolveMinBufferTime:function(?number). With this, the app could replace the logic that determines what min buffer time to use (using the manifest provided value as a guiding principle).

From the manifest parser they would then have a callback usable as:

// When the manifest does not have their own value.
const minBufferTime = this.config_.resolveMinBufferTime(null);

and

// When the manifest has their own value.
const minBufferTime = this.config_.resolveMinBufferTime(this.mySuggestedMinBufferTime);

The library could provide a default like:

(manifestMinBufferTime) => {
  const myDefault = /* some number */;
  return manifestMinBufferTime ? Math.max(manifestMinBufferTime, myDefault) : myDefault;
};

Then apps could replace it further with whatever logic they need/want.

[fadomire]

@vaage thanks for the suggestion, it seems a good path

i still made a pull request with minor changes, but i feel it does not fit well with the current code flow :
default config merged with specific user config

tho i will use this fork for demo purpose and see later how shaka team wants to implement it

[joeyparrish]

After some more discussion internally, I think we should 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]

i updated the PR #1581 to implement manifest.dash.ignoreMinBufferTime logic

let me know if ok for you or if it needs some tweak

thanks !

[joeyparrish]

Released in v2.5.0-beta2.