If pacing rate, (current loss based bwe * pacing factor) is larger than the current upper link capacity estimate, reduce pacing factor to max of current bwe and upper link capacity.
Bug: webrtc:42220543
Change-Id: I5246da1f38530f8d411e7314adaa8651fc848f48
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349601
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42210}
This is to avoid the case where the initial probe fail and the BWE is not updated, which can lead to a long period of low bandwidth.
Bug: webrtc:14928
Change-Id: Ie8f84270507b59995d57e4ab6e2a984570191529
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349580
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42208}
This reverts commit 501c4f37bf.
Patch set 1 contains pure reland.
The reason why we want to do this is because audio can allocate a needed bitrate before video when starting a call, which may lead to a race between the first probe result and updating the allocated bitrate.
That is the, initial probe will try to probe up to the max configured bitrate.
Bug: webrtc:14928
Change-Id: I6a8660da20ac54237f04a29461e03b31bd988bb0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/347643
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Erik Språng <sprang@google.com>
Owners-Override: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42086}
This reverts commit 33cc83595a.
Reason for revert: Perf bots showed that this cl cause a change in metrics. It looks like it is for the better, but we want this to be behind a field trial.
Original change's description:
> Ignore allocated bitrate during initial exponential BWE.
>
> The reason why we want to do this is because audio can allocate a needed bitrate before video when starting a call, which may lead to a race between the first probe result and updating the allocated bitrate.
> That is the, initial probe will try to probe up to the max configured bitrate.
>
> ProbeController::SetFirstProbeToMaxBitrate will allow the first probe to
> continue up to the max configured bitrate, regardless of of the max
> allocated bitrate.
>
> Bug: webrtc:14928
> Change-Id: I6e0ae90e21a78466527f3464951e6033dc846470
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/346760
> Reviewed-by: Diep Bui <diepbp@webrtc.org>
> Commit-Queue: Per Kjellander <perkj@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#42049}
Bug: webrtc:14928
Change-Id: I56ba58560b6857b6069552c02df822691f7af64d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/347622
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Owners-Override: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42081}
The reason why we want to do this is because audio can allocate a needed bitrate before video when starting a call, which may lead to a race between the first probe result and updating the allocated bitrate.
That is the, initial probe will try to probe up to the max configured bitrate.
ProbeController::SetFirstProbeToMaxBitrate will allow the first probe to
continue up to the max configured bitrate, regardless of of the max
allocated bitrate.
Bug: webrtc:14928
Change-Id: I6e0ae90e21a78466527f3464951e6033dc846470
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/346760
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42049}
This reverts commit 802dd5bdbd.
First patch set is pure reland.
New patch set adds field trial flag.
Original change's description:
Stop exponential probing if 2xmax allocated bitrate lower than BWE.
Without this, if max allocated bitrate is lowered while exponential probing is ongoing, a new probe can be sent at a rate of the new low max allocated bitrate which may cause BWE to decrease.
Bug: webrtc:14928
Change-Id: I35c341bbaced800d9931657d62c73a17a3279b7c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/344440
Auto-Submit: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41965}
This reverts commit 5a4ce03101.
Reason for revert: Breaks tests in downstream project.
Original change's description:
> Stop exponential probing if 2xmax allocated bitrate lower than BWE.
>
> Without this, if max allocated bitrate is lowered while exponential probing is ongoing, a new probe can be sent at a rate of the new low max allocated bitrate which may cause BWE to decrease.
>
> Bug: webrtc:14928
> Change-Id: Id8e314740c2403d3b801d28f634dbc718f77c16e
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343384
> Reviewed-by: Diep Bui <diepbp@webrtc.org>
> Commit-Queue: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#41929}
Bug: webrtc:14928
Change-Id: I0d48b37bfb8684fd490f5685e510b438a83254b9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343900
Commit-Queue: Ying Wang <yinwa@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Ying Wang <yinwa@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41958}
Without this, if max allocated bitrate is lowered while exponential probing is ongoing, a new probe can be sent at a rate of the new low max allocated bitrate which may cause BWE to decrease.
Bug: webrtc:14928
Change-Id: Id8e314740c2403d3b801d28f634dbc718f77c16e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343384
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41929}
The new field trial can be used to ensure probes are limited by the current BWE and does not automatically send a probe at the new max rate.
Also removes unused
FieldTrialFlag allocation_allow_further_probing;
FieldTrialParameter<DataRate> allocation_probe_max;
Bug: webrtc:14928
Change-Id: I0d5c350c0231ca0600033ad8211dca0574104201
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339840
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41744}
Add a small clean up in LossBasedBandwidthEstimatorV2ReadyForUse since IsReady() includes IsEnabled().
Bug: webrtc:12707
Change-Id: I20dfeb2ab31e7724041f89af9f312211a3ae3d23
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339521
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41727}
This CL addresses a crash we started seeing in M121 where a
function is being called on loss_based_bandwidth_estimator_v2_
without checking whether it is enabled (it's not) which leads
to absl::optional<> throwing since config_ is not valid.
Bug: chromium:1518852
Change-Id: Iffef1051fe7988046e33a709ce281aebefd2bcd7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/334103
Commit-Queue: Joe Downing <joedow@google.com>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41538}
After https://webrtc-review.googlesource.com/c/src/+/329141, best candidate can still be less than acked rate if not_increase_if_inherent_loss_less_than_average_loss, or the selected candidate is 95% of current estimate. This cl/ is ensure the previous cl works as intended. And add unit test.
Bug: webrtc:12707
Change-Id: Ie5683ca8ea51f6d80c4c59cbf08c22e8b24c0cb9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/329441
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41298}
These flags were never experimented or launched.
Bug: webrtc:12707
Change-Id: Iefedeade52fdcf7f978894c4bf837261810f41bd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/329080
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41265}
To ensure padding, we increase 1 bit instead of 1kbps to avoid that 1kbps adds up over time.
Not have unit test for this, but did manual/hamrit tests many times.
Bug: webrtc:12707
Change-Id: I9b3160ab1808cb3a21ff0609446359a4ec3a4949
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/325520
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41056}
The change is under field trial use_in_start_phase.
Bug: webrtc:12707
Change-Id: I2ba8245c5d126b3c8a2e54b826853d98aad6e4f9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/325184
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41047}
Increasing BWE by 1kbps should be safe/no-op in practice, and it ensures that padding in kIncreasing state will be triggered.
Bug: webrtc:12707
Change-Id: I82493d07a80abd60c93d9cff74baf0a55e77f2b7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/325286
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41046}
If we have been sending padding for 1s and estimate still is unchanged, then stop padding by transitioning to decrease state.
Bug: webrtc:12707
Change-Id: I0dca2e5cd98263fc7fae9882c23c21634413c7a0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324740
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41018}
2 main reasons:
1. Packet sizes are much different thus a lost audio packet should not be treated similar to a lost video packet. In low bandwidth/traffic policing scenario, the number of send packet is few, thus the computed loss can be imprecise.
2. Given a candidate bandwidth estimate, the objective function (how good the candidate is) is computed by recomputing loss rate = send rate/estimate bandwith + inherent loss. It means the objective function is byte based rather than packet based.
Potential risk: the current algorithm params are tuned based on packet count, thus it might not work with byte count, which is much higher than packet count.
The change is under field trial and disabled by default.
Bug: webrtc:12707
Change-Id: I8b832e7920d2b4cadcd4a072b3a4d4f26a213a20
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/325065
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41013}
The initial hold duration is 300ms.
Whenever it enters kDecreasing state, it will double the current hold duration. The hold duration will be reset as soon as the delay based estimate works, e.g. the state is kDelayBased to avoid getting stuck at low bitrate.
Bug: webrtc:12707
Change-Id: I3906ff80b071ba3eb6274b012fb31922f4cbc7b6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324304
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40991}
Addes field trial UpperBoundCandidateInAlr to LossBasedBweV2. If an
instant upper bound exist in ALR that are lower than current estimate,
use it as a candidate.
Bug: webrtc:12707
Change-Id: I55595c7225c4289e1bc4edde9d9576e0443d3dce
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324220
Auto-Submit: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40986}
UsePadding - signals to GoogCC that padding should be used to fill up to
BWE while BWE is ramping up.
Bug: webrtc:12707
Change-Id: I7b4922dff3a83da370c50c567050bfa748190b40
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324160
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40979}
Ensure acked bitrate is not used for lower loss based estimate if
estimate improve.
Ensure LossBasedBweV2 is in state DelayBased if reached max rate.
Bug: webrtc:12707
Change-Id: I20230b99e0c2b530570e2f2de8ea88179f795c50
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324140
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40977}
Expect OnNetworkAvailabability to be invoked when the transport becomes writable.
Before this change, ProbeController in GoogCC was expected to be created when the transport is writable or explicitly notifed after creation that network is not writable.
Bug: None
Change-Id: I623b1c34e40a82e912f85b92fea49629e7e72d4e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/323463
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40975}
The state should be computed from the actual estimate rather than the best estimate candidate. The fix is NOT under field trial.
And some other cleanup:
1. Loss based result will be computed in UpdateBandwidthEstimate method. Currently it is re-computed in GetLossBasedResult.
2. Rename current_estimate to current_best_estimate to avoid misunderstanding that current_estimate is the `final estimate`. The final estimate is computed by applying lower and upper bound on current_best_estimate
3. Remove current_state_. The state is stored directly in loss_based_result_.
Bug: webrtc:12707
Change-Id: Ie612845f907b9e6333fbd8249ddc9b93ad9f8042
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324022
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40964}
This will be used in an experiment to ramp up BWE when BWE is reduced
due to loss.
Bug: webrtc:12707
Change-Id: I3b78f9dd3fe8ef9f94a9616640ffb8b2225e161e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324042
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40961}
This cl/ makes sure that the estimate cannot go lower than a factor of acked bitrate. The current flag LowerBoundByAckedRateFactor is set to 0, means we dont use it.
Bug: webrtc:12707
Change-Id: I75d5881f0b85a374af3f7039b82c71aee97fb7b0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/323881
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40958}
This ensure probe results can not be lower than 85% percentage of the
acked bitrate.
Bug: webrtc:11498
Change-Id: I501eeb84f7a049140c45c89e7de7e8080c13f94d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324040
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Auto-Submit: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40957}
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}
