Remove SendPacer from ViEEncoder and make sure SendPacer starts at a valid bitrate
This reverts commit e30c27205148b34ba421184efe65f6a0780b436d (https://codereview.webrtc.org/1958053002/)
Original reverted cl is in patch set #1.
Changes in following patch sets.
The cl now also make sure SendPacer starts with the configured bitrate provided in a call to CongestionController::SetBweBitrates)()
It turns out that the failing tests in 609816 is due to a bug in the current code that runs the proper at 300kbit regardless of configured start bitrate.
Original cl description:
Remove SendPacer from ViEEncoder
This CL moves the logic where the ViEEncoder pause if the pacer is full to the BitrateController. If the queue is full, the controller reports a bitrate of zero to Call (and BitrateAllocator)
BUG=chromium:609816, webrtc:5687
TBR=mflodman@webrtc.org
NOTRY=True // Due to bug in android_x86 cq builder....
Review-Url: https://codereview.webrtc.org/1958113003
Cr-Commit-Position: refs/heads/master@{#12688}
diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc
new file mode 100644
index 0000000..20bf1a8
--- /dev/null
+++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc
@@ -0,0 +1,117 @@
+/*
+ * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/modules/pacing/mock/mock_paced_sender.h"
+#include "webrtc/modules/congestion_controller/include/congestion_controller.h"
+#include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h"
+#include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_observer.h"
+#include "webrtc/system_wrappers/include/clock.h"
+
+using testing::_;
+using testing::NiceMock;
+using testing::Return;
+using testing::SaveArg;
+using testing::StrictMock;
+
+namespace webrtc {
+namespace test {
+
+class CongestionControllerTest : public ::testing::Test {
+ protected:
+ CongestionControllerTest() : clock_(123456) {}
+ ~CongestionControllerTest() override {}
+
+ void SetUp() override {
+ pacer_ = new NiceMock<MockPacedSender>();
+ std::unique_ptr<PacedSender> pacer(pacer_); // Passes ownership.
+ std::unique_ptr<PacketRouter> packet_router(new PacketRouter());
+ controller_.reset(
+ new CongestionController(&clock_, &observer_, &remote_bitrate_observer_,
+ std::move(packet_router), std::move(pacer)));
+ bandwidth_observer_.reset(
+ controller_->GetBitrateController()->CreateRtcpBandwidthObserver());
+
+ // Set the initial bitrate estimate and expect the |observer| and |pacer_|
+ // to be updated.
+ EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _));
+ EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps));
+ controller_->SetBweBitrates(0, kInitialBitrateBps, 5 * kInitialBitrateBps);
+ }
+
+ SimulatedClock clock_;
+ StrictMock<MockCongestionObserver> observer_;
+ NiceMock<MockPacedSender>* pacer_;
+ NiceMock<MockRemoteBitrateObserver> remote_bitrate_observer_;
+ std::unique_ptr<RtcpBandwidthObserver> bandwidth_observer_;
+ std::unique_ptr<CongestionController> controller_;
+ const uint32_t kInitialBitrateBps = 60000;
+};
+
+TEST_F(CongestionControllerTest, OnNetworkChanged) {
+ // Test no change.
+ clock_.AdvanceTimeMilliseconds(25);
+ controller_->Process();
+
+ EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _));
+ EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps * 2));
+ bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps * 2);
+ clock_.AdvanceTimeMilliseconds(25);
+ controller_->Process();
+
+ EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _));
+ EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps));
+ bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps);
+ clock_.AdvanceTimeMilliseconds(25);
+ controller_->Process();
+}
+
+TEST_F(CongestionControllerTest, OnSendQueueFull) {
+ EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
+ .WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1));
+
+ EXPECT_CALL(observer_, OnNetworkChanged(0, _, _));
+ controller_->Process();
+
+ // Let the pacer not be full next time the controller checks.
+ EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
+ .WillOnce(Return(PacedSender::kMaxQueueLengthMs - 1));
+
+ EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _));
+ controller_->Process();
+}
+
+TEST_F(CongestionControllerTest, OnSendQueueFullAndEstimateChange) {
+ EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
+ .WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1));
+ EXPECT_CALL(observer_, OnNetworkChanged(0, _, _));
+ controller_->Process();
+
+ // Receive new estimate but let the queue still be full.
+ bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps * 2);
+ EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
+ .WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1));
+ // The send pacer should get the new estimate though.
+ EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps * 2));
+ clock_.AdvanceTimeMilliseconds(25);
+ controller_->Process();
+
+ // Let the pacer not be full next time the controller checks.
+ // |OnNetworkChanged| should be called with the new estimate.
+ EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
+ .WillOnce(Return(PacedSender::kMaxQueueLengthMs - 1));
+ EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _));
+ clock_.AdvanceTimeMilliseconds(25);
+ controller_->Process();
+}
+
+} // namespace test
+} // namespace webrtc
