follow-up from https://webrtc-review.googlesource.com/c/src/+/262810
* replace Time::Millis(0) and TimeDelta::Millis(0) with ::Zero()
* drop unnecessary webrtc namespace from some TimeDeltas
* make TimeDelta do the unit conversion for stats
BUG=webrtc:13756
Change-Id: Ic60625ae0fc7959a47a6be9f5051851feaf76373
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265875
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37664}
Replace helper functions with the constant
Remove option to set min bitrate in RemoteBitrateEstimator as unused:
ReceivedSideCongestionController is the only user of the
RemoteBitrateEstimator interface, and it always sets the same value
right after construction that RemoteBitreateEstimators already use.
Bug: None
Change-Id: If179fdd72b1ded6ad1fd0a6dfffc97b302153322
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269383
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37613}
1. Add loss threshold for high bandwidth preference. If the average loss ratio is less than the threshold, then the model prefers higher bandwidth candidates. Otherwise, it prefers lower bandwidth candidates. Before, it always prefers higher bandwidth candidate. The default value is 0.99, means it always prefers high bandwidth candidates.
2. Only increase the estimate if the inherent loss (random loss) is equal to/greater than the average loss. If the inherent loss is less than the average loss, then it is oversending, thus should not increase the estimate.
Bug: webrtc:12707
Change-Id: I37eb536679ca29e017a4a47703b417efd4d103ca
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269101
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Jakob Ivarsson <jakobi@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37608}
Update an affected unit test by the change in goog_cc.
Bug: webrtc:14272
Change-Id: I83e97530c861b126bed876d57f6d4f91aa45de7e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269002
Reviewed-by: Jakob Ivarsson <jakobi@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37587}
This trial has been unused for some time, time to clean it up.
Bug: webrtc:10144
Change-Id: I2b1bd9ff0335efdc07f47a361878915f1be383a7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267410
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37421}
Return the bitrate estimate as DataRate type
Remove list of affected ssrcs as unused
Bug: None
Change-Id: Ie31dce591d861624736d834194f90eb6c93f70f6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267280
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37397}
This way call no longer needs dedicated process thread
Bug: webrtc:7219
Change-Id: I8ab677b1e6b909eeb726aefed5e6d10ce4bc43b7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265921
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37279}
The method can be used to ensure packets reported to NetworkStateEstimator include transport overhead.
Change-Id: I30f0271aac82633893660c61ea59e3b7c2cf9f31
Bug: webrtc:10742
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265405
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37179}
Instead of using field trials in BitrateProber for probe duration, use values provided in ProbeClusterConfig from GoogCC.
Field trials are instead read in ProbeController.
To avoid having to do a thread jump for every ProbeClusterConfig, RtpPacketPacer interface is changed to RtpPacketPacer::CreateProbeClusters(std::vector<ProbeClusterConfig>
Deprecates field trial "WebRTC-Bwe-ProbingConfiguration"
Change-Id: I3991e4b54770601855a3af2d6a16678f11d41c31
Bug: webrtc:14027
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261265
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36911}
The new TaskQueuePacedSender has been default-on in code since M97, and
there are no further usages of it that I can find. Let's clean this up!
The PacingController and associated tests will be cleaned up in a
follow-up cl.
Bug: webrtc:10809
Change-Id: I0cb888602939add953415977ee79ff0b3878fea5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/258025
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36890}
This is for cleaning loss based bwe v2 implementation according to some comments from https://webrtc-review.googlesource.com/c/src/+/261240.
Bug: webrtc:12707
Change-Id: I2cb278f136cddcd0eeb2c5e4c319a9cc6aab94a0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262251
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36875}
- Filter out very old packets (to ensure that the estimate doesn't drop to zero if sending is paused and later resumed).
- Discard packets older than previously discarded packets (to avoid the estimate dropping after deep reordering.)
- Add tests cases for high loss, deep reordering and paused/resumed streams to unittest.
- Remove some field trial settings that have very minor effect and rename some of the others.
- Change analyzer.cc to only draw data points if the estimators have valid estimates.
Bug: webrtc:13402
Change-Id: I47ead8aa4454cced5134d10895ca061d2c3e32f4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/236347
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36849}
The change bounds the estimate increment by MaxIncreaseFactor in DelayedIncreaseWindow after seeing loss. MaxIncreaseFactor is set to 1000 to disable the change by default.
Improve trendline integration: always allow to decrease the estimate, and only allow to increase the estimate if overusing and underusing are not in the state window.
Other improvement: bound candidates by delay based estimate, instance upper bound, and bandwidth limit in the current window.
Clean: remove the flag BackoffWhenOverusing since it has negative impacts when experimenting.
Bug: webrtc:12707
Change-Id: Ia4c1e58d692071967e8807a8b9d64b8ae4caf837
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261240
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36847}
Resolving https://bugs.chromium.org/p/webrtc/issues/detail?id=14023
At the moment, in DelayBasedBwe the time deltas are rounded to the
nearest millisecond. This change makes sure the numbers are passed as
doubles as expected by the TrendlineEstimator.
Change-Id: I68882547fb19af0e67e7b5d8de4159083a54d7eb
Bug: webrtc:14023
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261320
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36806}
It is hard to add an unit test for this change, because OnTransportPacketsFeedback is the way to change the loss based estimate, and it also changes the lower link capacity.
Thus we cannot track the two values without making changes in goog_cc, which seem to be unecessary.
Bug: none
Change-Id: I689ef52c4440c9bc2ff30f88b9cda96a90b0b64e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/256368
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36304}
The default backoff behavior of loss based 2.0 should remain the same.
Bug: webrtc:12707
Change-Id: Ica7b8da26c11317a6cb7cd9272c07d03d066f30b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/256005
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36258}
This cl/
1) move WebRtcKeyValueConfig from api/transport to api/ directory.
2) add a test/ScopedKeyValueConfig (compare ScopedFieldTrials).
3) removes usage of webrtc::field_trial:: from the pc/ directory.
4) removes a few unused includes of system_wrappers/field_trial.h.
Bug: webrtc:10335
Change-Id: If29c07900dbe791050b0a5ad05332bedfad035f2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/253903
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36160}
AimdRateControl
AimdRateControl can potentially clamp bitrate in SetEstimate.
DelayBasedBwe::MaybeUpdateEstimate should therefore check the result before using the probe bitrate.
Otherwise, BWE may drop on next update.
Bug: none
Change-Id: I8b1b3549a2bcd981e941b1cc802c984828d68261
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/252444
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36099}
This is a reland of 325789c457
Original change's description:
> Mark all bool conversion operators as explicit
>
> An explicit bool conversion operator will still be used implicitly
> when an expression appears in "bool context", e.g., as the condition
> in an if statement, or as argument to logical operators. The
> `explicit` annotation prevents conversion in other contexts, e.g.,
> converting both a and b to bool in an expression like `a == b`.
>
> Bug: None
> Change-Id: I79ef35b1ea831e6011ae472900375ae8a3e617ab
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250664
> Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Niels Moller <nisse@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#35927}
Bug: None
Change-Id: Ie057dfc8c0b5c498e2c8daff7620172c89f0e011
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251380
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35962}
This reverts commit 325789c457.
Reason for revert: Breaks downstream clients.
Original change's description:
> Mark all bool conversion operators as explicit
>
> An explicit bool conversion operator will still be used implicitly
> when an expression appears in "bool context", e.g., as the condition
> in an if statement, or as argument to logical operators. The
> `explicit` annotation prevents conversion in other contexts, e.g.,
> converting both a and b to bool in an expression like `a == b`.
>
> Bug: None
> Change-Id: I79ef35b1ea831e6011ae472900375ae8a3e617ab
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250664
> Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Niels Moller <nisse@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#35927}
TBR=mbonadei@webrtc.org,nisse@webrtc.org,tommi@webrtc.org,webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com
Change-Id: I392cd0c7bd96c90e0db20831864418adb7d58bc3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: None
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251080
Auto-Submit: Mirko Bonadei <mbonadei@webrtc.org>
Owners-Override: Mirko Bonadei <mbonadei@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35929}
An explicit bool conversion operator will still be used implicitly
when an expression appears in "bool context", e.g., as the condition
in an if statement, or as argument to logical operators. The
`explicit` annotation prevents conversion in other contexts, e.g.,
converting both a and b to bool in an expression like `a == b`.
Bug: None
Change-Id: I79ef35b1ea831e6011ae472900375ae8a3e617ab
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250664
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35927}
This is a reland of 9d230d54c7
Original change's description:
> (Un/)Subscribe RtpVideoSender for feedback on the transport queue.
>
> * RtpVideoSender now registers/unregisters for feedback callback
> inside of SetActive(), which runs on the transport queue.
> * Transport feedback is given on the transport queue
> * Registration/unregistration for feedback is done on the same
> * Removed the last mutex from TransportFeedbackDemuxer.
>
> Ultimately, this work is related to moving state from the Call
> class, that's related to network configuration, but due to the code
> is currently written is attached to the worker thread, over to the
> Transport, where it's used (e.g. suspended_video_send_ssrcs_).
>
> Bug: webrtc:13517, webrtc:11993
> Change-Id: I057d0e2597e6cb746b335e0308599cd547350e56
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/248165
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#35777}
Bug: webrtc:13517, webrtc:11993
Change-Id: I766e569abea8bae96d32267a951fcdc195ced8a7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249782
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35863}
This reverts commit 9d230d54c7.
Reason for revert: Speculative revert to see if it's the cause of a few perf changes (some bad, some not so bad).
Bug: webrtc:13613
Original change's description:
> (Un/)Subscribe RtpVideoSender for feedback on the transport queue.
>
> * RtpVideoSender now registers/unregisters for feedback callback
> inside of SetActive(), which runs on the transport queue.
> * Transport feedback is given on the transport queue
> * Registration/unregistration for feedback is done on the same
> * Removed the last mutex from TransportFeedbackDemuxer.
>
> Ultimately, this work is related to moving state from the Call
> class, that's related to network configuration, but due to the code
> is currently written is attached to the worker thread, over to the
> Transport, where it's used (e.g. suspended_video_send_ssrcs_).
>
> Bug: webrtc:13517, webrtc:11993
> Change-Id: I057d0e2597e6cb746b335e0308599cd547350e56
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/248165
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#35777}
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:13517, webrtc:11993
Change-Id: I824623b3b1c14f0ca7049a2a0890c6d97b7fb608
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249600
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35815}
