MinNumObservations is set to 3 per default as loss based BWE should not be ready if it has few feedbacks. We use a flag, rather than a const since we want to customize it for our unit tests, which often have 1-2 packet feedbacks only, and customize it later in prod if necessary.
Bug: webrtc:12707
Change-Id: Id1cd21aaf6137996de2e51cb5e33fc2a4bb07d8a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/323780
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40952}
Cleanup field trials for not probing when BWE limited due to high RTT,
loss.
Bug: webrtc:14754, webrtc:12707
Change-Id: Ib664784e321d9284d842ea42a0dd1d8361000f20
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/323640
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40949}
These features are not in use.
Bug: webrtc:12707
Change-Id: Ibe9fcae5e3fd7cb7ca289af80dad8480288c9ba3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/323601
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40938}
A follow up cl/ is to remove passing upper link capacity from goog_cc to loss_based_bwe_v2.
Bug: webrtc:12707
Change-Id: I45af8ca6e8ba185700d0b7eb57004d2b61edeb9e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/320780
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40780}
The purpose is to not allow an initial low link capacity estimate to reduce the current estimate.
Only delay overuse detection , low probe results or a loss event can
reduce the estimate.
Bug: webrtc:14392
Change-Id: Ib1618347f2c7681e3bd65d85ee687dec3cd67c97
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/320380
Reviewed-by: Diep Bui <diepbp@google.com>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40751}
The sequence number is generally not used for the estimation,
but may be used as a tie-breaker when ordering packet feedbacks.
Bug: b/299667054
Change-Id: I52a5145c889c8f6924838667cc267b1cd9565f7b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/320240
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40749}
Experiments has not showed significant metric changes. However, simulations has showed that RobustThroughputEstimator better follow the actually receive rate better. Especially during bursts of sent packets. Code is also simpler.
Bug: webrtc:13402 chromium:1411666
Change-Id: I38c309f74e8e1322602196354545b3a465866263
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/318040
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40653}
Start using RobustThoughputEstimator in DelayBasedBwe test in preparation for making it default.
Experiments has not showed significant metric changes. However, simulations has showed that RobustThroughputEstimator better follow the actually receive rate better. Especially during bursts of sent packets. Code is also simpler.
Bug: webrtc:13402 chromium:1411666
Change-Id: I83cfa1fc15486982b18cc22fbd0752ff59c1c1b4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/317600
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40644}
Ensure probes are not created until after the transport becomes writable even if the network route change.
DTLS negotiation must complete before there is a point in sending probes.
Bug: webrtc:14928
Change-Id: Ib3cb93aef9f38e306b094dd700ed595cf9eb3f32
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/301362
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39870}
Pass FieldTrialsView by const& to note it can't be null and doesn't need to outlive the constructor
In unittests use AimdRateControl object directly instead of through helpers
Use unit types (TimeDelta, DataRate) directly, reducing their conversion to plain numbers
Replace SimulatedClock with a single Timestamp now variable or constant
Bug: None
Change-Id: I147f85e629b4d8923aa19896ea211a6f9dca1e68
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299260
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39707}
Instead of use probe_bitrate as the bandwidth estimate, the change uses probe bitrate as the bandwidth limit.
The experiment is not started, so it does not affect production.
Bug: webrtc:12707
Change-Id: Iefd8cdfe87983057489e551816bf5d4cb38f7c9f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296040
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39603}
Packet pending time should be diffed between max_revc_time and receive time as it is done at line 436. The current implementation makes pending time to be negative.
Bug: webrtc:14850
Change-Id: Ie6590ef11caa67254750591abb6bf72679d76941
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/292461
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39311}
This will enable loss based bwe v2 by default. The default params were used in Chrome experiment and got positive result. Remove some tests in goog_cc, which are for loss based bwe v1.
Bug: webrtc:12707
Change-Id: Ice126a128f6e8cea8b861f879d09e390ee69e521
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/285740
Commit-Queue: Diep Bui <diepbp@google.com>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38851}
When best candidate estimate increases above the delay based estimate, the state should be DelayBasedEstimate because the final esimate is bounded by delay based bwe anyway.
Bug: webrtc:12707
Change-Id: I0bcae684b33e5f1e9a7c57cb32c431b4eb58fd35
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283802
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38677}
Add loss_limited_probe_scale as a scale factor which decides how much we should probe when bandwidth is loss limited.
Bug: webrtc:12707
Change-Id: I194b2b40c9a7861d82b61585bcaf484ab228eedb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/281360
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38636}
Motivation: loss based ramp-up can be incorrect when (1) bandwidth is loss limited, and (2) delay based estimate might be incorrect due to no delay signals. Therefore, bounding the loss based estimate by the delay based estimate is not much helpful in those cases.
Thus strengthening the bounding logic by using upper link capacity is one of solutions to avoid incorrect ramp-up.
Without the change: screen/qmLedxapJWvUTmn
With the change: screen/8sQcksWa6CptywK
Bug: webrtc:12707
Change-Id: I32ba82693b3ffa83cbb89c2cc9690fe16fb10c78
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283085
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38626}
Patchset 1 contrains the original cl.
Later patchsets contain fix.
Original description:
Continue probing if networkstat estimate increase
This fixes an issue where continues probing stops if networkstate estimate is low when a probe is sent, but increase as a consequence of the probe.
Bug: webrtc:14392
Change-Id: I8d4e1968020f9f8de18e12a4a0322a87f1a8fd2f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283082
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38612}
This reverts commit dd7dc25a30c5841e6620d195b83058a22ffff7cd.
Reason for revert: Bug in CL. Continuously probe if experiment for probing based on the link capacity is enabled.
Original change's description:
> Continue probing if networkstat estimate increase
>
> This fixes an issue where continues probing stops if networkstate estimate is low when a probe is sent, but increase as a consequence of the probe.
>
>
> Bug: webrtc:14392
> Change-Id: Id1d703f7efc824a6a6f8d899c367660291bd88c8
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282941
> Reviewed-by: Diep Bui <diepbp@webrtc.org>
> Commit-Queue: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38606}
Bug: webrtc:14392
Change-Id: Ib241b190951a78c436188c0b83d0247bf7d0dddd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283080
Auto-Submit: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#38609}
This fixes an issue where continues probing stops if networkstate estimate is low when a probe is sent, but increase as a consequence of the probe.
Bug: webrtc:14392
Change-Id: Id1d703f7efc824a6a6f8d899c367660291bd88c8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282941
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38606}
This is to avoid passing delay based estimate value twice from send side bwe.
Bug: webrtc:12707
Change-Id: Idc77cf7c2f4ecc60ae1dcfead325320532e7a7ca
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282864
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38600}
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 c1d5fda22c8ae456950c5549d22d099b478c67e2
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 c1d5fda22c8ae456950c5549d22d099b478c67e2.
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}
