Do not remove the highest loss observation if we suddenly send a lot more.
Bug: webrtc:12707 Change-Id: Iccb05694867e681690e623e4d648efd60a2b1d5f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366643 Reviewed-by: Per Kjellander <perkj@webrtc.org> Commit-Queue: Diep Bui <diepbp@webrtc.org> Cr-Commit-Position: refs/heads/main@{#43307}
This commit is contained in:
parent
2b36b37d21
commit
6bf6cfb465
@ -469,6 +469,8 @@ std::optional<LossBasedBweV2::Config> LossBasedBweV2::CreateConfig(
|
||||
FieldTrialParameter<bool> bound_best_candidate("BoundBestCandidate", false);
|
||||
FieldTrialParameter<bool> pace_at_loss_based_estimate(
|
||||
"PaceAtLossBasedEstimate", false);
|
||||
FieldTrialParameter<double> median_sending_rate_factor(
|
||||
"MedianSendingRateFactor", 2.0);
|
||||
if (key_value_config) {
|
||||
ParseFieldTrial({&enabled,
|
||||
&bandwidth_rampup_upper_bound_factor,
|
||||
@ -509,7 +511,8 @@ std::optional<LossBasedBweV2::Config> LossBasedBweV2::CreateConfig(
|
||||
&use_byte_loss_rate,
|
||||
&padding_duration,
|
||||
&bound_best_candidate,
|
||||
&pace_at_loss_based_estimate},
|
||||
&pace_at_loss_based_estimate,
|
||||
&median_sending_rate_factor},
|
||||
key_value_config->Lookup("WebRTC-Bwe-LossBasedBweV2"));
|
||||
}
|
||||
|
||||
@ -575,6 +578,7 @@ std::optional<LossBasedBweV2::Config> LossBasedBweV2::CreateConfig(
|
||||
config.padding_duration = padding_duration.Get();
|
||||
config.bound_best_candidate = bound_best_candidate.Get();
|
||||
config.pace_at_loss_based_estimate = pace_at_loss_based_estimate.Get();
|
||||
config.median_sending_rate_factor = median_sending_rate_factor.Get();
|
||||
return config;
|
||||
}
|
||||
|
||||
@ -816,7 +820,7 @@ double LossBasedBweV2::CalculateAverageReportedByteLossRatio() const {
|
||||
DataSize max_lost_bytes = DataSize::Zero();
|
||||
DataSize min_bytes_received = DataSize::Zero();
|
||||
DataSize max_bytes_received = DataSize::Zero();
|
||||
|
||||
DataRate send_rate_of_max_loss_observation = DataRate::Zero();
|
||||
for (const Observation& observation : observations_) {
|
||||
if (!observation.IsInitialized()) {
|
||||
continue;
|
||||
@ -836,6 +840,7 @@ double LossBasedBweV2::CalculateAverageReportedByteLossRatio() const {
|
||||
max_loss_rate = loss_rate;
|
||||
max_lost_bytes = instant_temporal_weight * observation.lost_size;
|
||||
max_bytes_received = instant_temporal_weight * observation.size;
|
||||
send_rate_of_max_loss_observation = observation.sending_rate;
|
||||
}
|
||||
if (loss_rate < min_loss_rate) {
|
||||
min_loss_rate = loss_rate;
|
||||
@ -844,6 +849,13 @@ double LossBasedBweV2::CalculateAverageReportedByteLossRatio() const {
|
||||
}
|
||||
}
|
||||
}
|
||||
if (GetMedianSendingRate() * config_->median_sending_rate_factor <=
|
||||
send_rate_of_max_loss_observation) {
|
||||
// If the median sending rate is less than half of the sending rate of the
|
||||
// observation with max loss rate, i.e. we suddenly send a lot of data, then
|
||||
// the loss rate might not be due to a spike.
|
||||
return lost_bytes / total_bytes;
|
||||
}
|
||||
return (lost_bytes - min_lost_bytes - max_lost_bytes) /
|
||||
(total_bytes - max_bytes_received - min_bytes_received);
|
||||
}
|
||||
@ -1155,6 +1167,8 @@ bool LossBasedBweV2::PushBackObservation(
|
||||
const TimeDelta observation_duration =
|
||||
last_send_time - last_send_time_most_recent_observation_;
|
||||
// Too small to be meaningful.
|
||||
// To consider: what if it is too long?, i.e. we did not receive any packets
|
||||
// for a long time, then all the packets we received are too old.
|
||||
if (observation_duration <= TimeDelta::Zero() ||
|
||||
observation_duration < config_->observation_duration_lower_bound) {
|
||||
return false;
|
||||
@ -1205,4 +1219,25 @@ bool LossBasedBweV2::PaceAtLossBasedEstimate() const {
|
||||
loss_based_result_.state != LossBasedState::kDelayBasedEstimate;
|
||||
}
|
||||
|
||||
DataRate LossBasedBweV2::GetMedianSendingRate() const {
|
||||
std::vector<DataRate> sending_rates;
|
||||
for (const Observation& observation : observations_) {
|
||||
if (!observation.IsInitialized() || !IsValid(observation.sending_rate) ||
|
||||
observation.sending_rate.IsZero()) {
|
||||
continue;
|
||||
}
|
||||
sending_rates.push_back(observation.sending_rate);
|
||||
}
|
||||
if (sending_rates.empty()) {
|
||||
return DataRate::Zero();
|
||||
}
|
||||
absl::c_sort(sending_rates);
|
||||
if (sending_rates.size() % 2 == 0) {
|
||||
return (sending_rates[sending_rates.size() / 2 - 1] +
|
||||
sending_rates[sending_rates.size() / 2]) /
|
||||
2;
|
||||
}
|
||||
return sending_rates[sending_rates.size() / 2];
|
||||
}
|
||||
|
||||
} // namespace webrtc
|
||||
|
||||
@ -128,6 +128,7 @@ class LossBasedBweV2 {
|
||||
TimeDelta padding_duration = TimeDelta::Zero();
|
||||
bool bound_best_candidate = false;
|
||||
bool pace_at_loss_based_estimate = false;
|
||||
double median_sending_rate_factor = 0.0;
|
||||
};
|
||||
|
||||
struct Derivatives {
|
||||
@ -199,6 +200,7 @@ class LossBasedBweV2 {
|
||||
DataRate new_estimate);
|
||||
bool IsInLossLimitedState() const;
|
||||
bool CanKeepIncreasingState(DataRate estimate) const;
|
||||
DataRate GetMedianSendingRate() const;
|
||||
|
||||
std::optional<DataRate> acknowledged_bitrate_;
|
||||
std::optional<Config> config_;
|
||||
|
||||
@ -147,19 +147,23 @@ class LossBasedBweV2Test : public ::testing::TestWithParam<bool> {
|
||||
}
|
||||
|
||||
std::vector<PacketResult> CreatePacketResultsWith100pLossRate(
|
||||
Timestamp first_packet_timestamp) {
|
||||
std::vector<PacketResult> enough_feedback(2);
|
||||
enough_feedback[0].sent_packet.sequence_number =
|
||||
Timestamp first_packet_timestamp, unsigned num_packets = 2) {
|
||||
std::vector<PacketResult> enough_feedback(num_packets);
|
||||
for (unsigned i = 0; i < num_packets - 1; ++i) {
|
||||
enough_feedback[i].sent_packet.sequence_number =
|
||||
transport_sequence_number_++;
|
||||
enough_feedback[i].sent_packet.size = DataSize::Bytes(kPacketSize);
|
||||
enough_feedback[i].sent_packet.send_time =
|
||||
first_packet_timestamp + TimeDelta::Millis(i * 10);
|
||||
enough_feedback[i].receive_time = Timestamp::PlusInfinity();
|
||||
}
|
||||
enough_feedback[num_packets - 1].sent_packet.sequence_number =
|
||||
transport_sequence_number_++;
|
||||
enough_feedback[1].sent_packet.sequence_number =
|
||||
transport_sequence_number_++;
|
||||
enough_feedback[0].sent_packet.size = DataSize::Bytes(kPacketSize);
|
||||
enough_feedback[1].sent_packet.size = DataSize::Bytes(kPacketSize);
|
||||
enough_feedback[0].sent_packet.send_time = first_packet_timestamp;
|
||||
enough_feedback[1].sent_packet.send_time =
|
||||
enough_feedback[num_packets - 1].sent_packet.size =
|
||||
DataSize::Bytes(kPacketSize);
|
||||
enough_feedback[num_packets - 1].sent_packet.send_time =
|
||||
first_packet_timestamp + kObservationDurationLowerBound;
|
||||
enough_feedback[0].receive_time = Timestamp::PlusInfinity();
|
||||
enough_feedback[1].receive_time = Timestamp::PlusInfinity();
|
||||
enough_feedback[num_packets - 1].receive_time = Timestamp::PlusInfinity();
|
||||
return enough_feedback;
|
||||
}
|
||||
|
||||
@ -1842,6 +1846,40 @@ TEST_F(LossBasedBweV2Test, UseByteLossRateIgnoreLossSpike) {
|
||||
kDelayBasedEstimate);
|
||||
}
|
||||
|
||||
TEST_F(LossBasedBweV2Test, UseByteLossRateDoesNotIgnoreLossSpikeOnSendBurst) {
|
||||
ExplicitKeyValueConfig key_value_config(
|
||||
"WebRTC-Bwe-LossBasedBweV2/"
|
||||
"UseByteLossRate:true,ObservationWindowSize:5/");
|
||||
LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config);
|
||||
const DataRate kDelayBasedEstimate = DataRate::KilobitsPerSec(500);
|
||||
loss_based_bandwidth_estimator.SetBandwidthEstimate(kDelayBasedEstimate);
|
||||
|
||||
// Fill the observation window.
|
||||
for (int i = 0; i < 5; ++i) {
|
||||
loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
|
||||
CreatePacketResultsWithReceivedPackets(
|
||||
/*first_packet_timestamp=*/Timestamp::Zero() +
|
||||
i * kObservationDurationLowerBound),
|
||||
kDelayBasedEstimate,
|
||||
/*in_alr=*/false);
|
||||
}
|
||||
|
||||
// If the loss happens when increasing sending rate, then
|
||||
// the BWE should back down.
|
||||
loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
|
||||
CreatePacketResultsWith100pLossRate(
|
||||
/*first_packet_timestamp=*/Timestamp::Zero() +
|
||||
5 * kObservationDurationLowerBound,
|
||||
/*num_packets=*/5),
|
||||
kDelayBasedEstimate,
|
||||
/*in_alr=*/false);
|
||||
EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state,
|
||||
LossBasedState::kDecreasing);
|
||||
EXPECT_LE(
|
||||
loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate,
|
||||
kDelayBasedEstimate);
|
||||
}
|
||||
|
||||
TEST_F(LossBasedBweV2Test, PaceAtLossBasedEstimate) {
|
||||
ExplicitKeyValueConfig key_value_config(ShortObservationConfig(
|
||||
"PaceAtLossBasedEstimate:true,PaddingDuration:1000ms"));
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user