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.

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

dynamically choose between ThroughputRule and BolaRule #2083

Merged
merged 8 commits into from
Aug 17, 2017

Conversation

spiterikevin
Copy link
Contributor

This PR dynamically chooses between ThroughputRule and BolaRule to get strengths of both.

  • ThroughputRule is desirable in transient conditions such as startup and seeking.
  • BolaRule is desirable in stable conditions with sufficient buffer.

A new stream starts on ThroughputRule and then switches to BolaRule when the buffer level reaches a threshold, switching back to ThroughputRule when the buffer level drops below a (lower-than-the-first) threshold and so on.

Note that while the automatic rule switching gets the best of both rules, both ThroughputRule and BolaRule can work well without the other.

After this change, the rich buffer rule is no longer needed. The rich buffer rule code in the AbrController is replaced by the new switching mechanism.

This PR builds on two previous ones:

#2003 introduced ThroughputHistory, reducing duplication (there was similar code in both ThroughputRule and BolaRule). ThroughputHistory also helps keep track of throughput even when the ThroughputRule is not active. Also, InsufficientBufferRule was updated to better handle low-buffer situations.

#2068 updated BolaRule to work alongside other rules, mainly by no longer assuming it is the only rule active. Particularly, if BolaRule is used after another rule has been used for a while, BolaRule will take the previous rule’s bitrate into account to avoid bitrate oscillations.

Copy link
Contributor

@robertbryer robertbryer left a comment

Choose a reason for hiding this comment

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

Looks very good. I'd like to pull it together with your other changes and do some A/B testing here to quantify the difference soon. Does the Work-in-progress flag mean there's still something more you want to do here?

@@ -2110,6 +2078,34 @@ function MediaPlayer() {

---------------------------------------------------------------------------
*/

/**
* @deprecated Since version 2.6.0.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's deprecated if it's no longer the preferred way to do it, but it still works. If it completely doesn't work, it's obsolete.

@spiterikevin
Copy link
Contributor Author

Thanks. I don't have any more changes planned here other than fixes for issues that come up during review and testing.

* @instance
*/
function setABRStrategy(value) {
if (value === Constants.ABR_STRATEGY_DYNAMIC || value === Constants.ABR_STRATEGY_BOLA || value === Constants.ABR_STRATEGY_THROUGHPUT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it time to create a public facing TYPES object accessible from outside Dash.js?

@spiterikevin
Copy link
Contributor Author

spiterikevin commented Aug 9, 2017

@robertbryer I was wondering if you have done any A/B testing on this. Thanks.

@dsparacio dsparacio merged commit 37ec323 into Dash-Industry-Forum:development Aug 17, 2017
@spiterikevin spiterikevin deleted the joinBolaTput branch August 17, 2017 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants