Missing packet send time should not cause BWE backoff.
The removed coded causes problems if the same RTCP packet is forwarded to the congestion controller multiple times. Bug: webrtc:10125 Change-Id: I659d8f8f3ce3c643710156fa81176ceeaedd714a Reviewed-on: https://webrtc-review.googlesource.com/c/114165 Commit-Queue: Björn Terelius <terelius@webrtc.org> Reviewed-by: Sebastian Jansson <srte@webrtc.org> Cr-Commit-Position: refs/heads/master@{#26016}
This commit is contained in:
parent
e05d720f1c
commit
24779d8229
@ -46,8 +46,6 @@ constexpr size_t kDefaultTrendlineWindowSize = 20;
|
||||
constexpr double kDefaultTrendlineSmoothingCoeff = 0.9;
|
||||
constexpr double kDefaultTrendlineThresholdGain = 4.0;
|
||||
|
||||
constexpr int kMaxConsecutiveFailedLookups = 5;
|
||||
|
||||
const char kBweWindowSizeInPacketsExperiment[] =
|
||||
"WebRTC-BweWindowSizeInPackets";
|
||||
|
||||
@ -94,7 +92,6 @@ DelayBasedBwe::DelayBasedBwe(RtcEventLog* event_log)
|
||||
: kDefaultTrendlineWindowSize),
|
||||
trendline_smoothing_coeff_(kDefaultTrendlineSmoothingCoeff),
|
||||
trendline_threshold_gain_(kDefaultTrendlineThresholdGain),
|
||||
consecutive_delayed_feedbacks_(0),
|
||||
prev_bitrate_(DataRate::Zero()),
|
||||
prev_state_(BandwidthUsage::kBwNormal) {
|
||||
RTC_LOG(LS_INFO)
|
||||
@ -147,43 +144,12 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector(
|
||||
}
|
||||
|
||||
if (delayed_feedback) {
|
||||
Timestamp arrival_time = Timestamp::PlusInfinity();
|
||||
if (packet_feedback_vector.back().arrival_time_ms > 0)
|
||||
arrival_time =
|
||||
Timestamp::ms(packet_feedback_vector.back().arrival_time_ms);
|
||||
return OnDelayedFeedback(arrival_time);
|
||||
|
||||
} else {
|
||||
consecutive_delayed_feedbacks_ = 0;
|
||||
return MaybeUpdateEstimate(acked_bitrate, probe_bitrate,
|
||||
recovered_from_overuse, at_time);
|
||||
// TODO(bugs.webrtc.org/10125): Design a better mechanism to safe-guard
|
||||
// against building very large network queues.
|
||||
return Result();
|
||||
}
|
||||
return Result();
|
||||
}
|
||||
|
||||
DelayBasedBwe::Result DelayBasedBwe::OnDelayedFeedback(Timestamp receive_time) {
|
||||
++consecutive_delayed_feedbacks_;
|
||||
if (consecutive_delayed_feedbacks_ >= kMaxConsecutiveFailedLookups) {
|
||||
consecutive_delayed_feedbacks_ = 0;
|
||||
return OnLongFeedbackDelay(receive_time);
|
||||
}
|
||||
return Result();
|
||||
}
|
||||
|
||||
DelayBasedBwe::Result DelayBasedBwe::OnLongFeedbackDelay(
|
||||
Timestamp arrival_time) {
|
||||
// Estimate should always be valid since a start bitrate always is set in the
|
||||
// Call constructor. An alternative would be to return an empty Result here,
|
||||
// or to estimate the throughput based on the feedback we received.
|
||||
RTC_DCHECK(rate_control_.ValidEstimate());
|
||||
rate_control_.SetEstimate(rate_control_.LatestEstimate() / 2, arrival_time);
|
||||
Result result;
|
||||
result.updated = true;
|
||||
result.probe = false;
|
||||
result.target_bitrate = rate_control_.LatestEstimate();
|
||||
RTC_LOG(LS_WARNING) << "Long feedback delay detected, reducing BWE to "
|
||||
<< ToString(result.target_bitrate);
|
||||
return result;
|
||||
return MaybeUpdateEstimate(acked_bitrate, probe_bitrate,
|
||||
recovered_from_overuse, at_time);
|
||||
}
|
||||
|
||||
void DelayBasedBwe::IncomingPacketFeedback(
|
||||
|
||||
@ -49,7 +49,6 @@ class DelayBasedBwe {
|
||||
absl::optional<DataRate> acked_bitrate,
|
||||
absl::optional<DataRate> probe_bitrate,
|
||||
Timestamp at_time);
|
||||
Result OnDelayedFeedback(Timestamp receive_time);
|
||||
void OnRttUpdate(TimeDelta avg_rtt);
|
||||
bool LatestEstimate(std::vector<uint32_t>* ssrcs, DataRate* bitrate) const;
|
||||
void SetStartBitrate(DataRate start_bitrate);
|
||||
@ -60,7 +59,6 @@ class DelayBasedBwe {
|
||||
friend class GoogCcStatePrinter;
|
||||
void IncomingPacketFeedback(const PacketFeedback& packet_feedback,
|
||||
Timestamp at_time);
|
||||
Result OnLongFeedbackDelay(Timestamp arrival_time);
|
||||
Result MaybeUpdateEstimate(absl::optional<DataRate> acked_bitrate,
|
||||
absl::optional<DataRate> probe_bitrate,
|
||||
bool request_probe,
|
||||
@ -81,7 +79,6 @@ class DelayBasedBwe {
|
||||
size_t trendline_window_size_;
|
||||
double trendline_smoothing_coeff_;
|
||||
double trendline_threshold_gain_;
|
||||
int consecutive_delayed_feedbacks_;
|
||||
DataRate prev_bitrate_;
|
||||
BandwidthUsage prev_state_;
|
||||
|
||||
|
||||
@ -397,15 +397,9 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportLossReport(
|
||||
NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback(
|
||||
TransportPacketsFeedback report) {
|
||||
if (report.packet_feedbacks.empty()) {
|
||||
DelayBasedBwe::Result result = delay_based_bwe_->OnDelayedFeedback(
|
||||
report.sendless_arrival_times.back());
|
||||
NetworkControlUpdate update;
|
||||
if (result.updated) {
|
||||
bandwidth_estimation_->UpdateDelayBasedEstimate(report.feedback_time,
|
||||
result.target_bitrate);
|
||||
MaybeTriggerOnNetworkChanged(&update, report.feedback_time);
|
||||
}
|
||||
return update;
|
||||
// TODO(bugs.webrtc.org/10125): Design a better mechanism to safe-guard
|
||||
// against building very large network queues.
|
||||
return NetworkControlUpdate();
|
||||
}
|
||||
|
||||
if (congestion_window_pushback_controller_) {
|
||||
|
||||
@ -245,74 +245,6 @@ TEST_F(GoogCcNetworkControllerTest, ProbeOnRouteChange) {
|
||||
update = controller_->OnProcessInterval(DefaultInterval());
|
||||
}
|
||||
|
||||
// Estimated bitrate reduced when the feedbacks arrive with such a long delay,
|
||||
// that the send-time-history no longer holds the feedbacked packets.
|
||||
TEST_F(GoogCcNetworkControllerTest, LongFeedbackDelays) {
|
||||
TargetBitrateTrackingSetup();
|
||||
const webrtc::PacedPacketInfo kPacingInfo0(0, 5, 2000);
|
||||
const webrtc::PacedPacketInfo kPacingInfo1(1, 8, 4000);
|
||||
const int64_t kFeedbackTimeoutMs = 60001;
|
||||
const int kMaxConsecutiveFailedLookups = 5;
|
||||
for (int i = 0; i < kMaxConsecutiveFailedLookups; ++i) {
|
||||
std::vector<PacketResult> packets;
|
||||
packets.push_back(CreateResult(i * 100, 2 * i * 100, 1500, kPacingInfo0));
|
||||
packets.push_back(
|
||||
CreateResult(i * 100 + 10, 2 * i * 100 + 10, 1500, kPacingInfo0));
|
||||
packets.push_back(
|
||||
CreateResult(i * 100 + 20, 2 * i * 100 + 20, 1500, kPacingInfo0));
|
||||
packets.push_back(
|
||||
CreateResult(i * 100 + 30, 2 * i * 100 + 30, 1500, kPacingInfo1));
|
||||
packets.push_back(
|
||||
CreateResult(i * 100 + 40, 2 * i * 100 + 40, 1500, kPacingInfo1));
|
||||
|
||||
TransportPacketsFeedback feedback;
|
||||
for (PacketResult& packet : packets) {
|
||||
controller_->OnSentPacket(packet.sent_packet);
|
||||
feedback.sendless_arrival_times.push_back(packet.receive_time);
|
||||
}
|
||||
|
||||
feedback.feedback_time = packets[0].receive_time;
|
||||
AdvanceTimeMilliseconds(kFeedbackTimeoutMs);
|
||||
SentPacket later_packet;
|
||||
later_packet.send_time = Timestamp::ms(kFeedbackTimeoutMs + i * 200 + 40);
|
||||
later_packet.size = DataSize::bytes(1500);
|
||||
later_packet.pacing_info = kPacingInfo1;
|
||||
controller_->OnSentPacket(later_packet);
|
||||
|
||||
OnUpdate(controller_->OnTransportPacketsFeedback(feedback));
|
||||
}
|
||||
OnUpdate(controller_->OnProcessInterval(DefaultInterval()));
|
||||
|
||||
EXPECT_EQ(kInitialBitrateKbps / 2, target_bitrate_->kbps());
|
||||
|
||||
// Test with feedback that isn't late enough to time out.
|
||||
{
|
||||
std::vector<PacketResult> packets;
|
||||
packets.push_back(CreateResult(100, 200, 1500, kPacingInfo0));
|
||||
packets.push_back(CreateResult(110, 210, 1500, kPacingInfo0));
|
||||
packets.push_back(CreateResult(120, 220, 1500, kPacingInfo0));
|
||||
packets.push_back(CreateResult(130, 230, 1500, kPacingInfo1));
|
||||
packets.push_back(CreateResult(140, 240, 1500, kPacingInfo1));
|
||||
|
||||
for (const PacketResult& packet : packets)
|
||||
controller_->OnSentPacket(packet.sent_packet);
|
||||
|
||||
TransportPacketsFeedback feedback;
|
||||
feedback.feedback_time = packets[0].receive_time;
|
||||
feedback.packet_feedbacks = packets;
|
||||
|
||||
AdvanceTimeMilliseconds(kFeedbackTimeoutMs - 1);
|
||||
|
||||
SentPacket later_packet;
|
||||
later_packet.send_time = Timestamp::ms(kFeedbackTimeoutMs + 240);
|
||||
later_packet.size = DataSize::bytes(1500);
|
||||
later_packet.pacing_info = kPacingInfo1;
|
||||
controller_->OnSentPacket(later_packet);
|
||||
|
||||
OnUpdate(controller_->OnTransportPacketsFeedback(feedback));
|
||||
}
|
||||
}
|
||||
|
||||
// Bandwidth estimation is updated when feedbacks are received.
|
||||
// Feedbacks which show an increasing delay cause the estimation to be reduced.
|
||||
TEST_F(GoogCcNetworkControllerTest, UpdatesDelayBasedEstimate) {
|
||||
|
||||
@ -344,97 +344,6 @@ TEST_F(LegacySendSideCongestionControllerTest, ProbeOnRouteChange) {
|
||||
20 * kInitialBitrateBps);
|
||||
}
|
||||
|
||||
// Estimated bitrate reduced when the feedbacks arrive with such a long delay,
|
||||
// that the send-time-history no longer holds the feedbacked packets.
|
||||
TEST_F(LegacySendSideCongestionControllerTest, LongFeedbackDelays) {
|
||||
TargetBitrateTrackingSetup();
|
||||
|
||||
const int64_t kFeedbackTimeoutMs = 60001;
|
||||
const int kMaxConsecutiveFailedLookups = 5;
|
||||
for (int i = 0; i < kMaxConsecutiveFailedLookups; ++i) {
|
||||
std::vector<PacketFeedback> packets;
|
||||
packets.push_back(
|
||||
PacketFeedback(i * 100, 2 * i * 100, 0, 1500, kPacingInfo0));
|
||||
packets.push_back(
|
||||
PacketFeedback(i * 100 + 10, 2 * i * 100 + 10, 1, 1500, kPacingInfo0));
|
||||
packets.push_back(
|
||||
PacketFeedback(i * 100 + 20, 2 * i * 100 + 20, 2, 1500, kPacingInfo0));
|
||||
packets.push_back(
|
||||
PacketFeedback(i * 100 + 30, 2 * i * 100 + 30, 3, 1500, kPacingInfo1));
|
||||
packets.push_back(
|
||||
PacketFeedback(i * 100 + 40, 2 * i * 100 + 40, 4, 1500, kPacingInfo1));
|
||||
|
||||
for (const PacketFeedback& packet : packets)
|
||||
OnSentPacket(packet);
|
||||
|
||||
rtcp::TransportFeedback feedback;
|
||||
feedback.SetBase(packets[0].sequence_number,
|
||||
packets[0].arrival_time_ms * 1000);
|
||||
|
||||
for (const PacketFeedback& packet : packets) {
|
||||
EXPECT_TRUE(feedback.AddReceivedPacket(packet.sequence_number,
|
||||
packet.arrival_time_ms * 1000));
|
||||
}
|
||||
|
||||
feedback.Build();
|
||||
|
||||
clock_.AdvanceTimeMilliseconds(kFeedbackTimeoutMs);
|
||||
PacketFeedback later_packet(kFeedbackTimeoutMs + i * 100 + 40,
|
||||
kFeedbackTimeoutMs + i * 200 + 40, 5, 1500,
|
||||
kPacingInfo1);
|
||||
OnSentPacket(later_packet);
|
||||
|
||||
controller_->OnTransportFeedback(feedback);
|
||||
|
||||
// Check that packets have timed out.
|
||||
for (PacketFeedback& packet : packets) {
|
||||
packet.send_time_ms = PacketFeedback::kNoSendTime;
|
||||
packet.payload_size = 0;
|
||||
packet.pacing_info = PacedPacketInfo();
|
||||
}
|
||||
ComparePacketFeedbackVectors(packets,
|
||||
controller_->GetTransportFeedbackVector());
|
||||
}
|
||||
|
||||
controller_->Process();
|
||||
|
||||
EXPECT_EQ(kInitialBitrateBps / 2, target_bitrate_bps_);
|
||||
|
||||
// Test with feedback that isn't late enough to time out.
|
||||
{
|
||||
std::vector<PacketFeedback> packets;
|
||||
packets.push_back(PacketFeedback(100, 200, 0, 1500, kPacingInfo0));
|
||||
packets.push_back(PacketFeedback(110, 210, 1, 1500, kPacingInfo0));
|
||||
packets.push_back(PacketFeedback(120, 220, 2, 1500, kPacingInfo0));
|
||||
packets.push_back(PacketFeedback(130, 230, 3, 1500, kPacingInfo1));
|
||||
packets.push_back(PacketFeedback(140, 240, 4, 1500, kPacingInfo1));
|
||||
|
||||
for (const PacketFeedback& packet : packets)
|
||||
OnSentPacket(packet);
|
||||
|
||||
rtcp::TransportFeedback feedback;
|
||||
feedback.SetBase(packets[0].sequence_number,
|
||||
packets[0].arrival_time_ms * 1000);
|
||||
|
||||
for (const PacketFeedback& packet : packets) {
|
||||
EXPECT_TRUE(feedback.AddReceivedPacket(packet.sequence_number,
|
||||
packet.arrival_time_ms * 1000));
|
||||
}
|
||||
|
||||
feedback.Build();
|
||||
|
||||
clock_.AdvanceTimeMilliseconds(kFeedbackTimeoutMs - 1);
|
||||
PacketFeedback later_packet(kFeedbackTimeoutMs + 140,
|
||||
kFeedbackTimeoutMs + 240, 5, 1500,
|
||||
kPacingInfo1);
|
||||
OnSentPacket(later_packet);
|
||||
|
||||
controller_->OnTransportFeedback(feedback);
|
||||
ComparePacketFeedbackVectors(packets,
|
||||
controller_->GetTransportFeedbackVector());
|
||||
}
|
||||
}
|
||||
|
||||
// Bandwidth estimation is updated when feedbacks are received.
|
||||
// Feedbacks which show an increasing delay cause the estimation to be reduced.
|
||||
TEST_F(LegacySendSideCongestionControllerTest, UpdatesDelayBasedEstimate) {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user