Revert 5775 "Modify bitrate controller to update bitrate based o..."
This triggered an occasional TSAN failure in
CallTest.ReceivesPliAndRecoversWithNack e.g.:
http://build.chromium.org/p/client.webrtc/builders/Linux%20Tsan/builds/1444/steps/memory%20test%3A%20video_engine_tests/logs/stdio
I managed to reproduce this locally and verified that reverting this CL
corrected it.
> Modify bitrate controller to update bitrate based on process call and not
> only whenever a RTCP receiver block is received.
>
> Additionally:
> Add condition to only start rampup after a receiver block is received. This was same as old behaviour but now an explicit check is needed to verify process does not ramps up before the first block.
>
> Fix logic around capping max bitrate increase at 8% per second. Before it was only increasing once every 1 second and each increase would be as high as 8%. If receiver blocks had a different interval before it would lose an update or waste an update slot and not ramp up as much as a 8% (e.g. if RTCP received < 1 second).
>
> Did not touch decrease logic, however since it can be triggered more often it
> may decrease much faster and closer to the original written cap of once every
> 300ms + rtt.
>
> Note:
> rampup_tests.cc don't seem to be affected by this since there is no packet loss or REMB that go higher than expected cap.
> bitrate_controller_unittests.cc are don't really simulate a clock and the process thread, but trigger update by inserting an rtcp block.
>
> BUG=3065
> R=stefan@webrtc.org, mflodman@webrtc.org
>
> Review URL: https://webrtc-codereview.appspot.com/10529004
TBR=andresp@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/10079005
git-svn-id: http://webrtc.googlecode.com/svn/trunk@5785 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
index edfe343..c53928b 100644
--- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
+++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc
@@ -57,12 +57,12 @@
class BitrateControllerTest : public ::testing::Test {
protected:
- BitrateControllerTest() : clock_(0), enforce_min_bitrate_(true) {}
+ BitrateControllerTest() : enforce_min_bitrate_(true) {}
~BitrateControllerTest() {}
virtual void SetUp() {
- controller_ = BitrateController::CreateBitrateController(
- &clock_, enforce_min_bitrate_);
+ controller_ =
+ BitrateController::CreateBitrateController(enforce_min_bitrate_);
bandwidth_observer_ = controller_->CreateRtcpBandwidthObserver();
}
@@ -70,8 +70,6 @@
delete bandwidth_observer_;
delete controller_;
}
-
- webrtc::SimulatedClock clock_;
bool enforce_min_bitrate_;
BitrateController* controller_;
RtcpBandwidthObserver* bandwidth_observer_;
@@ -89,50 +87,52 @@
// Receive a high remb, test bitrate inc.
bandwidth_observer_->OnReceivedEstimatedBitrate(400000);
- EXPECT_EQ(200000u, bitrate_observer.last_bitrate_);
- EXPECT_EQ(0, bitrate_observer.last_fraction_loss_);
- EXPECT_EQ(0u, bitrate_observer.last_rtt_);
- // Test bitrate increase 8% per second.
+ // Test start bitrate.
webrtc::ReportBlockList report_blocks;
report_blocks.push_back(CreateReportBlock(1, 2, 0, 1));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1);
- EXPECT_EQ(217000u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(200000u, bitrate_observer.last_bitrate_);
EXPECT_EQ(0, bitrate_observer.last_fraction_loss_);
EXPECT_EQ(50u, bitrate_observer.last_rtt_);
+ // Test bitrate increase 8% per second.
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 21));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1001);
- EXPECT_EQ(235360u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(217000u, bitrate_observer.last_bitrate_);
EXPECT_EQ(0, bitrate_observer.last_fraction_loss_);
EXPECT_EQ(50u, bitrate_observer.last_rtt_);
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 41));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 2001);
- EXPECT_EQ(255189u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(235360u, bitrate_observer.last_bitrate_);
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 61));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 3001);
- EXPECT_EQ(276604u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(255189u, bitrate_observer.last_bitrate_);
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 801));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 4001);
- EXPECT_EQ(299732u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(276604u, bitrate_observer.last_bitrate_);
- // Reach max cap.
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 101));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 5001);
- EXPECT_EQ(300000u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(299732u, bitrate_observer.last_bitrate_);
+
+ report_blocks.clear();
+ report_blocks.push_back(CreateReportBlock(1, 2, 0, 121));
+ bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 6001);
+ EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); // Max cap.
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 141));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 7001);
- EXPECT_EQ(300000u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); // Max cap.
// Test that a low REMB trigger immediately.
bandwidth_observer_->OnReceivedEstimatedBitrate(250000);
@@ -154,9 +154,6 @@
// Receive a high remb, test bitrate inc.
bandwidth_observer_->OnReceivedEstimatedBitrate(400000);
- EXPECT_EQ(200000u, bitrate_observer.last_bitrate_);
- EXPECT_EQ(0, bitrate_observer.last_fraction_loss_);
- EXPECT_EQ(0u, bitrate_observer.last_rtt_);
// Test start bitrate.
webrtc::ReportBlockList report_blocks;
@@ -164,7 +161,7 @@
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1);
second_bandwidth_observer->OnReceivedRtcpReceiverReport(
report_blocks, 100, 1);
- EXPECT_EQ(217000u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(200000u, bitrate_observer.last_bitrate_);
EXPECT_EQ(0, bitrate_observer.last_fraction_loss_);
EXPECT_EQ(100u, bitrate_observer.last_rtt_);
@@ -174,7 +171,7 @@
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 501);
second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 100,
1001);
- EXPECT_EQ(235360u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(217000u, bitrate_observer.last_bitrate_);
EXPECT_EQ(0, bitrate_observer.last_fraction_loss_);
EXPECT_EQ(100u, bitrate_observer.last_rtt_);
@@ -183,45 +180,50 @@
report_blocks.push_back(CreateReportBlock(1, 2, 0, 31));
second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 100,
1501);
- EXPECT_EQ(235360u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(217000u, bitrate_observer.last_bitrate_);
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 41));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 2001);
- EXPECT_EQ(255189u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(235360u, bitrate_observer.last_bitrate_);
// Second report should not change estimate.
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 41));
second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 100,
2001);
- EXPECT_EQ(255189u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(235360u, bitrate_observer.last_bitrate_);
// Reports from only one bandwidth observer is ok.
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 61));
second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 50,
3001);
- EXPECT_EQ(276604u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(255189u, bitrate_observer.last_bitrate_);
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 81));
second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 50,
4001);
+ EXPECT_EQ(276604u, bitrate_observer.last_bitrate_);
+
+ report_blocks.clear();
+ report_blocks.push_back(CreateReportBlock(1, 2, 0, 101));
+ second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 50,
+ 5001);
EXPECT_EQ(299732u, bitrate_observer.last_bitrate_);
- // Reach max cap.
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 121));
- second_bandwidth_observer->OnReceivedRtcpReceiverReport(
- report_blocks, 50, 5001);
- EXPECT_EQ(300000u, bitrate_observer.last_bitrate_);
+ second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 50,
+ 6001);
+ EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); // Max cap.
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 141));
- second_bandwidth_observer->OnReceivedRtcpReceiverReport(
- report_blocks, 50, 6001);
- EXPECT_EQ(300000u, bitrate_observer.last_bitrate_);
+ second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 50,
+ 7001);
+ EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); // Max cap.
// Test that a low REMB trigger immediately.
// We don't care which bandwidth observer that delivers the REMB.
@@ -230,9 +232,8 @@
EXPECT_EQ(0, bitrate_observer.last_fraction_loss_);
EXPECT_EQ(50u, bitrate_observer.last_rtt_);
- // Min cap.
bandwidth_observer_->OnReceivedEstimatedBitrate(1000);
- EXPECT_EQ(100000u, bitrate_observer.last_bitrate_);
+ EXPECT_EQ(100000u, bitrate_observer.last_bitrate_); // Min cap.
controller_->RemoveBitrateObserver(&bitrate_observer);
delete second_bandwidth_observer;
}
@@ -316,24 +317,27 @@
controller_->SetBitrateObserver(&bitrate_observer_1, 200000, 100000, 300000);
// Receive a high remb, test bitrate inc.
- // Test too low start bitrate, hence lower than sum of min.
bandwidth_observer_->OnReceivedEstimatedBitrate(400000);
- EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_);
- EXPECT_EQ(0, bitrate_observer_1.last_fraction_loss_);
- EXPECT_EQ(0u, bitrate_observer_1.last_rtt_);
- // Test bitrate increase 8% per second, distributed equally.
+ // Test too low start bitrate, hence lower than sum of min.
webrtc::ReportBlockList report_blocks;
report_blocks.push_back(CreateReportBlock(1, 2, 0, 1));
- bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1001);
- EXPECT_EQ(112500u, bitrate_observer_1.last_bitrate_);
+ bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1);
+ EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_);
EXPECT_EQ(0, bitrate_observer_1.last_fraction_loss_);
EXPECT_EQ(50u, bitrate_observer_1.last_rtt_);
- EXPECT_EQ(212500u, bitrate_observer_2.last_bitrate_);
+ EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_);
EXPECT_EQ(0, bitrate_observer_2.last_fraction_loss_);
EXPECT_EQ(50u, bitrate_observer_2.last_rtt_);
+ // Test bitrate increase 8% per second, distributed equally.
+ report_blocks.clear();
+ report_blocks.push_back(CreateReportBlock(1, 2, 0, 21));
+ bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1001);
+ EXPECT_EQ(112500u, bitrate_observer_1.last_bitrate_);
+ EXPECT_EQ(212500u, bitrate_observer_2.last_bitrate_);
+
report_blocks.clear();
report_blocks.push_back(CreateReportBlock(1, 2, 0, 41));
bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 2001);