Instead of disabling probing when the total allocated bitrate has
changed in goog_cc, it can be done via a new field trial parameter,
"probe_max_allocation". Not that the currently used flag
RateControlSettings::TriggerProbeOnMaxAllocatedBitrateChange() is per
default enabled and will be cleaned up in a follow up cl.
The field trial flag "skip_if_est_larger_than_fraction_of_max" now also
skip probing if the current estimate is larger than the currently max
allocated bitrate. ie, alr probing is skippe if the current estimate >
max configured bitrate or current estimate > max send bitrate of all
streams.
Bug: webrtc:14392
Change-Id: I2a09be39f85a9122410edd5acb1158ece12fca60
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282860
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38597}
Instead of trying to guess the state from the loss based estimator by
looking at the estimate, use the state.
Bug: webrtc:14392
Change-Id: Ibf6e762f02bfbfff175f2aa2bc98f45b1c5beb1a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282823
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38589}
Previously, cleanup in GetCandidateBandwidthUpperBound in loss_based_bwe_v2.cc causes unbounded bandwidth estimate. It leads to many warning logs being printed out at loss_based_bwe_v2.cc:95.
However, the cleanup is still necessary because the param bandwidth_rampup_upper_bound_factor is not used in current launches.
To fix the infinite estimate, we set max_bitrate in loss based bwe, which is derived from goog_cc, and not allow the estimate to go above that value.
*** Original change description ***
* Revert "Probing integration in loss based bwe 2." (diepbp@webrtc.org)
* https://webrtc-review.googlesource.com/c/src/+/277400
***
Bug: webrtc:12707
Change-Id: If0cd16daba4a4941043a1610edca2a13c9564328
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/281280
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38574}
This is a reland of commit c1d5fda22c
Original change's description:
> Add documentation, tests and simplify webrtc::SimulatedNetwork.
>
> This CL increases the test coverage for webrtc::SimualtedNetwork, adds
> some more comments to the class and the interface it implements and
> simplify the logic around capacity and delay management in the
> simulated network.
>
> More CLs will follow to continue the refactoring but this is the
> ground work to make this more modular in the future.
>
> Bug: webrtc:14525, b/243202138
> Change-Id: Ib0408cf6e2c1cdceb71f8bec3202d2960c5b4d3c
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278042
> Reviewed-by: Artem Titov <titovartem@webrtc.org>
> Reviewed-by: Per Kjellander <perkj@webrtc.org>
> Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Reviewed-by: Björn Terelius <terelius@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38388}
Bug: webrtc:14525, b/243202138, b/256595485
Change-Id: Iaf8160eb8f8e29034b8f98e81ce07eb608663d30
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280963
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38557}
This reverts commit c1d5fda22c.
Reason for revert: This CL created thousands of metric alerts in the perf tests. It's possible that these are all expected, but since mbonadei@ is OOO right now, I think it's better to revert, and have him re-land when he is back.
Most alerts are here: https://bugs.chromium.org/p/webrtc/issues/detail?id=14549
Original change's description:
> Add documentation, tests and simplify webrtc::SimulatedNetwork.
>
> This CL increases the test coverage for webrtc::SimualtedNetwork, adds
> some more comments to the class and the interface it implements and
> simplify the logic around capacity and delay management in the
> simulated network.
>
> More CLs will follow to continue the refactoring but this is the
> ground work to make this more modular in the future.
>
> Bug: webrtc:14525, b/243202138
> Change-Id: Ib0408cf6e2c1cdceb71f8bec3202d2960c5b4d3c
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278042
> Reviewed-by: Artem Titov <titovartem@webrtc.org>
> Reviewed-by: Per Kjellander <perkj@webrtc.org>
> Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Reviewed-by: Björn Terelius <terelius@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38388}
Bug: webrtc:14525, b/243202138
Change-Id: I5bc56c954bb12e7c27cb859e838f0b7a89e006f8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279522
Owners-Override: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38415}
This reverts commit 332810ab5d.
Reason for revert: This commit chain seems to cause problems in LossBasedBwe.
Original change's description:
> Probing integration in loss based bwe 2.
>
> - Loss based bwe has 3 states: increasing (increasing when loss limited), decreasing (decreasing when loss limited), or delay based bwe (the same as delay based estimate).
> - When bandwidth is loss limited and decreasing, and probe result is available, GetLossBasedResult = min(estimate, probe result).
> - When bandwidth is loss limited and increasing, and the estimate is bounded by acked bitrate * a factor.
> - When bandwidth is loss limited and probe result is available, use probe bitrate as the current estimate, and reset probe bitrate.
>
> Bug: webrtc:12707
> Change-Id: I53cb82aa16397941c0cfaf1035116f775bdce72b
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277400
> Commit-Queue: Diep Bui <diepbp@webrtc.org>
> Reviewed-by: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38382}
Bug: webrtc:12707
Change-Id: Ied86323b0ce94b87ac503a2ee34753cebef5f53d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279500
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38412}
This reverts commit aa71259b06.
Reason for revert: This commit chain seems to cause problems in LossBasedBwe.
Original change's description:
> Probe when network is loss limited.
>
> Trigger probes next process intervals if the loss based current state is either increasing or decreasing. 0/ first probe at the loss based estimate. 1/ if increasing: allow further probing. 2/ if decreasing: not allow further probing.
>
>
> Bug: webrtc:12707
> Change-Id: I4e99edcbe4e2c315e8498ffb7fb2e589cdb4e666
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279041
> Commit-Queue: Diep Bui <diepbp@webrtc.org>
> Reviewed-by: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38395}
Bug: webrtc:12707
Change-Id: I1fb61337148faf6faaa0056dc25f14536a19a462
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279480
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38410}
Trigger probes next process intervals if the loss based current state is either increasing or decreasing. 0/ first probe at the loss based estimate. 1/ if increasing: allow further probing. 2/ if decreasing: not allow further probing.
Bug: webrtc:12707
Change-Id: I4e99edcbe4e2c315e8498ffb7fb2e589cdb4e666
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279041
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38395}
This CL increases the test coverage for webrtc::SimualtedNetwork, adds
some more comments to the class and the interface it implements and
simplify the logic around capacity and delay management in the
simulated network.
More CLs will follow to continue the refactoring but this is the
ground work to make this more modular in the future.
Bug: webrtc:14525, b/243202138
Change-Id: Ib0408cf6e2c1cdceb71f8bec3202d2960c5b4d3c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278042
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38388}
- Loss based bwe has 3 states: increasing (increasing when loss limited), decreasing (decreasing when loss limited), or delay based bwe (the same as delay based estimate).
- When bandwidth is loss limited and decreasing, and probe result is available, GetLossBasedResult = min(estimate, probe result).
- When bandwidth is loss limited and increasing, and the estimate is bounded by acked bitrate * a factor.
- When bandwidth is loss limited and probe result is available, use probe bitrate as the current estimate, and reset probe bitrate.
Bug: webrtc:12707
Change-Id: I53cb82aa16397941c0cfaf1035116f775bdce72b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277400
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38382}
When loss rate is above a certain threshold, set instant_limit = 500 - 1000 * average_loss_rate, which returns 200kbps at 30% loss rate, or 100kbps at 40% loss rate. When the loss rate is above 50%, use the min_bitrate from send_side_bandwidth_estimation.
The high_loss_rate_threshold is set to 1.0, so the change is not activated by default.
Tested the change with hamrit, when average loss rate is above 50%, bandwidth backed to 10kbps, and it took ~10s to ramp up to 1.5Mbps.
https://screenshot.googleplex.com/7dvPoWa2b5SgMSL
Bug: webrtc:12707
Change-Id: I5eea04ef709a183bdf696246094dbd4a204e48f6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272061
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38243}
This add field trial string "skip_if_est_larger_than_fraction_of_max"
Dont send a probe if min(estimate, network state estimate) is larger than this
fraction of the set max bitrate.
Bug: webrtc:14392
Change-Id: I7333f6ef45ab0c019f21b9e4c604352219e1d025
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275940
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38123}
The value is today set to 200 which is too low for an audio packet to trigger sending probes.
For the initial probing, it would be good if audio packets, that may arrive before the first video frame can trigger sending a probe.
Also fix field trial parsing of required number of probes.
Bug: webrc:14392
Change-Id: I1f3cebcda38b71446e3602eef9cfa76de61a1ccf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275620
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38089}
Loss based BWE v2 rate is updated immediately when transport feedback is received.
This ensure that when GoogCcNetworkController::MaybeTriggerOnNetworkChanged is invoked, the loss based estimate is updated.
Bug: webrtc:14392
Change-Id: If404576c5793a29096cea52884862807cde8b615
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275306
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38070}
Change ProbeController field trial to also probe when loss limited but probe at the current estimate.
Bug: webrtc:14392
Change-Id: I8b30e316b935a0f2c375e2204a8e33e6671eb956
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273901
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38004}
Ensure initial second probe can be disabled.
Can configure separate probe duration if the network state estimate is known.
Can probe immediately if network state estimate increase more than a factor
Bug: webrtc:14392
Change-Id: Iefb980f0b10c7c51db62793c3bd3f187fc67593d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273349
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37966}
Replace most instances. SetAlrStartTime is set as is should be cleaned up together with the callsite.
Bug: webrtc:14404
Change-Id: I8ec532828ef665afbf08f0943465a429ab40baa1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273300
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37932}
Add field trial to not probe if loss based limited
If both Alr probing and periodic probing of networkstate estimate is enabled, probes are limited by the network state estimate * factor controlled by field trial.
Bug: webrtc:14392
Change-Id: I46e1dbdd8b14f63a7c223b4c03c114717b802023
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272805
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37915}
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}
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}
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}
