Revert "Revert "Revert "Reland "Moved congestion controller to task queue.""""
This reverts commit 65792c5a5c542201f7b9feefded505842692e6ed.
Reason for revert: <INSERT REASONING HERE>
Original change's description:
> Revert "Revert "Reland "Moved congestion controller to task queue."""
>
> This reverts commit 4e849f6925b2ac44b0957a228d7131fc391fca54.
>
> Reason for revert: <INSERT REASONING HERE>
>
> Original change's description:
> > Revert "Reland "Moved congestion controller to task queue.""
> >
> > This reverts commit 57daeb7ac7f3d80992905b53fea500953fcfd793.
> >
> > Reason for revert: Cause increased congestion and deadlocks in downstream project
> >
> > Original change's description:
> > > Reland "Moved congestion controller to task queue."
> > >
> > > This is a reland of 0cbcba7ea0dced1a7f353c64d6cf91d46ccb29f9.
> > >
> > > Original change's description:
> > > > Moved congestion controller to task queue.
> > > >
> > > > The goal of this work is to make it easier to experiment with the
> > > > bandwidth estimation implementation. For this reason network control
> > > > functionality is moved from SendSideCongestionController(SSCC),
> > > > PacedSender and BitrateController to the newly created
> > > > GoogCcNetworkController which implements the newly created
> > > > NetworkControllerInterface. This allows the implementation to be
> > > > replaced at runtime in the future.
> > > >
> > > > This is the first part of a split of a larger CL, see:
> > > > https://webrtc-review.googlesource.com/c/src/+/39788/8
> > > > For further explanations.
> > > >
> > > > Bug: webrtc:8415
> > > > Change-Id: I770189c04cc31b313bd4e57821acff55fbcb1ad3
> > > > Reviewed-on: https://webrtc-review.googlesource.com/43840
> > > > Commit-Queue: Sebastian Jansson <srte@webrtc.org>
> > > > Reviewed-by: Björn Terelius <terelius@webrtc.org>
> > > > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > > > Cr-Commit-Position: refs/heads/master@{#21868}
> > >
> > > Bug: webrtc:8415
> > > Change-Id: I1d1756a30deed5b421b1c91c1918a13b6bb455da
> > > Reviewed-on: https://webrtc-review.googlesource.com/48000
> > > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > > Commit-Queue: Sebastian Jansson <srte@webrtc.org>
> > > Cr-Commit-Position: refs/heads/master@{#21899}
> >
> > TBR=terelius@webrtc.org,stefan@webrtc.org,srte@webrtc.org
> >
> > # Not skipping CQ checks because original CL landed > 1 day ago.
> >
> > Bug: webrtc:8415
> > Change-Id: Ida8074dcac2cc28b3629228eb22846d8a8e81b83
> > Reviewed-on: https://webrtc-review.googlesource.com/52980
> > Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> > Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#22017}
>
> TBR=danilchap@webrtc.org,terelius@webrtc.org,stefan@webrtc.org,srte@webrtc.org
>
> Change-Id: I3393b74370c4f4d0955f50728005b2b925be169b
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:8415
> Reviewed-on: https://webrtc-review.googlesource.com/53262
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Commit-Queue: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#22023}
TBR=danilchap@webrtc.org,terelius@webrtc.org,stefan@webrtc.org,srte@webrtc.org
Change-Id: Id68ad986ee51142b7be3381d0793709b4392fe2c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:8415
Reviewed-on: https://webrtc-review.googlesource.com/53360
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22024}diff --git a/modules/bitrate_controller/bitrate_controller_unittest.cc b/modules/bitrate_controller/bitrate_controller_unittest.cc
index 8bd7800..66ca5b9 100644
--- a/modules/bitrate_controller/bitrate_controller_unittest.cc
+++ b/modules/bitrate_controller/bitrate_controller_unittest.cc
@@ -340,11 +340,11 @@
report_blocks.clear();
// All packets lost on stream with few packets, no back-off.
- report_blocks.push_back(CreateReportBlock(1, 2, 0, sequence_number[0]));
+ report_blocks.push_back(CreateReportBlock(1, 2, 1, sequence_number[0]));
report_blocks.push_back(CreateReportBlock(1, 3, 255, sequence_number[1]));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms);
EXPECT_EQ(bitrate_observer_.last_bitrate_, last_bitrate);
- EXPECT_EQ(WeightedLoss(20, 0, 1, 255), bitrate_observer_.last_fraction_loss_);
+ EXPECT_EQ(WeightedLoss(20, 1, 1, 255), bitrate_observer_.last_fraction_loss_);
EXPECT_EQ(50, bitrate_observer_.last_rtt_);
last_bitrate = bitrate_observer_.last_bitrate_;
sequence_number[0] += 20;
diff --git a/modules/bitrate_controller/send_side_bandwidth_estimation.cc b/modules/bitrate_controller/send_side_bandwidth_estimation.cc
index 323c210..d3bce59 100644
--- a/modules/bitrate_controller/send_side_bandwidth_estimation.cc
+++ b/modules/bitrate_controller/send_side_bandwidth_estimation.cc
@@ -105,7 +105,7 @@
} // namespace
SendSideBandwidthEstimation::SendSideBandwidthEstimation(RtcEventLog* event_log)
- : lost_packets_since_last_loss_update_(0),
+ : lost_packets_since_last_loss_update_Q8_(0),
expected_packets_since_last_loss_update_(0),
current_bitrate_bps_(0),
min_bitrate_configured_(congestion_controller::GetMinBitrateBps()),
@@ -125,7 +125,6 @@
initially_lost_packets_(0),
bitrate_at_2_seconds_kbps_(0),
uma_update_state_(kNoUpdate),
- uma_rtt_state_(kNoUpdate),
rampup_uma_stats_updated_(kNumUmaRampupMetrics, false),
event_log_(event_log),
last_rtc_event_log_ms_(-1),
@@ -207,28 +206,24 @@
}
void SendSideBandwidthEstimation::UpdateReceiverBlock(uint8_t fraction_loss,
- int64_t rtt_ms,
+ int64_t rtt,
int number_of_packets,
int64_t now_ms) {
- const int kRoundingConstant = 128;
- int packets_lost = (static_cast<int>(fraction_loss) * number_of_packets +
- kRoundingConstant) >>
- 8;
- UpdatePacketsLost(packets_lost, number_of_packets, now_ms);
- UpdateRtt(rtt_ms, now_ms);
-}
-
-void SendSideBandwidthEstimation::UpdatePacketsLost(int packets_lost,
- int number_of_packets,
- int64_t now_ms) {
last_feedback_ms_ = now_ms;
if (first_report_time_ms_ == -1)
first_report_time_ms_ = now_ms;
+ // Update RTT if we were able to compute an RTT based on this RTCP.
+ // FlexFEC doesn't send RTCP SR, which means we won't be able to compute RTT.
+ if (rtt > 0)
+ last_round_trip_time_ms_ = rtt;
+
// Check sequence number diff and weight loss report
if (number_of_packets > 0) {
+ // Calculate number of lost packets.
+ const int num_lost_packets_Q8 = fraction_loss * number_of_packets;
// Accumulate reports.
- lost_packets_since_last_loss_update_ += packets_lost;
+ lost_packets_since_last_loss_update_Q8_ += num_lost_packets_Q8;
expected_packets_since_last_loss_update_ += number_of_packets;
// Don't generate a loss rate until it can be based on enough packets.
@@ -236,22 +231,21 @@
return;
has_decreased_since_last_fraction_loss_ = false;
- int64_t lost_q8 = lost_packets_since_last_loss_update_ << 8;
- int64_t expected = expected_packets_since_last_loss_update_;
- last_fraction_loss_ = std::min<int>(lost_q8 / expected, 255);
+ last_fraction_loss_ = lost_packets_since_last_loss_update_Q8_ /
+ expected_packets_since_last_loss_update_;
// Reset accumulators.
-
- lost_packets_since_last_loss_update_ = 0;
+ lost_packets_since_last_loss_update_Q8_ = 0;
expected_packets_since_last_loss_update_ = 0;
last_packet_report_ms_ = now_ms;
UpdateEstimate(now_ms);
}
- UpdateUmaStatsPacketsLost(now_ms, packets_lost);
+ UpdateUmaStats(now_ms, rtt, (fraction_loss * number_of_packets) >> 8);
}
-void SendSideBandwidthEstimation::UpdateUmaStatsPacketsLost(int64_t now_ms,
- int packets_lost) {
+void SendSideBandwidthEstimation::UpdateUmaStats(int64_t now_ms,
+ int64_t rtt,
+ int lost_packets) {
int bitrate_kbps = static_cast<int>((current_bitrate_bps_ + 500) / 1000);
for (size_t i = 0; i < kNumUmaRampupMetrics; ++i) {
if (!rampup_uma_stats_updated_[i] &&
@@ -262,12 +256,14 @@
}
}
if (IsInStartPhase(now_ms)) {
- initially_lost_packets_ += packets_lost;
+ initially_lost_packets_ += lost_packets;
} else if (uma_update_state_ == kNoUpdate) {
uma_update_state_ = kFirstDone;
bitrate_at_2_seconds_kbps_ = bitrate_kbps;
RTC_HISTOGRAM_COUNTS("WebRTC.BWE.InitiallyLostPackets",
initially_lost_packets_, 0, 100, 50);
+ RTC_HISTOGRAM_COUNTS("WebRTC.BWE.InitialRtt", static_cast<int>(rtt), 0,
+ 2000, 50);
RTC_HISTOGRAM_COUNTS("WebRTC.BWE.InitialBandwidthEstimate",
bitrate_at_2_seconds_kbps_, 0, 2000, 50);
} else if (uma_update_state_ == kFirstDone &&
@@ -280,19 +276,6 @@
}
}
-void SendSideBandwidthEstimation::UpdateRtt(int64_t rtt_ms, int64_t now_ms) {
- // Update RTT if we were able to compute an RTT based on this RTCP.
- // FlexFEC doesn't send RTCP SR, which means we won't be able to compute RTT.
- if (rtt_ms > 0)
- last_round_trip_time_ms_ = rtt_ms;
-
- if (!IsInStartPhase(now_ms) && uma_rtt_state_ == kNoUpdate) {
- uma_rtt_state_ = kDone;
- RTC_HISTOGRAM_COUNTS("WebRTC.BWE.InitialRtt", static_cast<int>(rtt_ms), 0,
- 2000, 50);
- }
-}
-
void SendSideBandwidthEstimation::UpdateEstimate(int64_t now_ms) {
uint32_t new_bitrate = current_bitrate_bps_;
// We trust the REMB and/or delay-based estimate during the first 2 seconds if
@@ -374,7 +357,7 @@
new_bitrate *= 0.8;
// Reset accumulators since we've already acted on missing feedback and
// shouldn't to act again on these old lost packets.
- lost_packets_since_last_loss_update_ = 0;
+ lost_packets_since_last_loss_update_Q8_ = 0;
expected_packets_since_last_loss_update_ = 0;
last_timeout_ms_ = now_ms;
}
diff --git a/modules/bitrate_controller/send_side_bandwidth_estimation.h b/modules/bitrate_controller/send_side_bandwidth_estimation.h
index d09184c..59d1c32 100644
--- a/modules/bitrate_controller/send_side_bandwidth_estimation.h
+++ b/modules/bitrate_controller/send_side_bandwidth_estimation.h
@@ -42,18 +42,10 @@
// Call when we receive a RTCP message with a ReceiveBlock.
void UpdateReceiverBlock(uint8_t fraction_loss,
- int64_t rtt_ms,
+ int64_t rtt,
int number_of_packets,
int64_t now_ms);
- // Call when we receive a RTCP message with a ReceiveBlock.
- void UpdatePacketsLost(int packets_lost,
- int number_of_packets,
- int64_t now_ms);
-
- // Call when we receive a RTCP message with a ReceiveBlock.
- void UpdateRtt(int64_t rtt, int64_t now_ms);
-
void SetBitrates(int send_bitrate,
int min_bitrate,
int max_bitrate);
@@ -66,7 +58,7 @@
bool IsInStartPhase(int64_t now_ms) const;
- void UpdateUmaStatsPacketsLost(int64_t now_ms, int packets_lost);
+ void UpdateUmaStats(int64_t now_ms, int64_t rtt, int lost_packets);
// Updates history of min bitrates.
// After this method returns min_bitrate_history_.front().second contains the
@@ -80,7 +72,7 @@
std::deque<std::pair<int64_t, uint32_t> > min_bitrate_history_;
// incoming filters
- int lost_packets_since_last_loss_update_;
+ int lost_packets_since_last_loss_update_Q8_;
int expected_packets_since_last_loss_update_;
uint32_t current_bitrate_bps_;
@@ -103,7 +95,6 @@
int initially_lost_packets_;
int bitrate_at_2_seconds_kbps_;
UmaState uma_update_state_;
- UmaState uma_rtt_state_;
std::vector<bool> rampup_uma_stats_updated_;
RtcEventLog* event_log_;
int64_t last_rtc_event_log_ms_;