Take into account the playbackRate in bandwidth calculations #2329
Conversation
…ybackRate in the stats
|
@joeyparrish Can you review it again? |
| @@ -2791,6 +2791,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget { | |||
| this.playRateController_.set(rate); | |||
|
|
|||
| if (this.loadMode_ == shaka.Player.LoadMode.MEDIA_SOURCE) { | |||
| this.abrManager_.playbackRateChanged(rate); | |||
There was a problem hiding this comment.
Since you added a method to a plugin interface, you'll need to account for backward compatibility. I see two good options:
- Check for playbackRateChange each time before calling it.
- When this.abrManager_ is set, check for playbackRateChange at that time and attach an empty function if it's missing.
I would strongly prefer #2, because you can then use shaka.Deprecate.deprecateFeature at the same time to tell the app developer that they need to upgrade their plugin by v2.7.
There was a problem hiding this comment.
I add a check for playbackRateChanged and I followed your second option. Thanks!
|
@joeyparrish Can you review it again? |
|
Up! |
|
@avelad Let me nag Joey in person on your behalf (: |
|
All tests passed! |
|
I fixed the comment. |
|
@joeyparrish , Is the last change enough? |
|
Looks great, thanks! We very much appreciate the time you take to send PRs. I know they take a long time to review because of time zones and missed emails, so we're also grateful for your patience in the review process. |
github-actions
bot
added
the
status: archived
Resolves #812