Move delay_based_bwe_ into CongestionController

webrtc/modules/congestion_controller/congestion_controller_unittest.cc

 /*
  *  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 "webrtc/logging/rtc_event_log/mock/mock_rtc_event_log.h"
-#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
+#include "webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h"
 #include "webrtc/modules/congestion_controller/include/congestion_controller.h"
 #include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h"
 #include "webrtc/modules/pacing/mock/mock_paced_sender.h"
 #include "webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h"
 #include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_observer.h"
+#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 #include "webrtc/system_wrappers/include/clock.h"
 #include "webrtc/test/gmock.h"
 #include "webrtc/test/gtest.h"
 
 using testing::_;
 using testing::AtLeast;
 using testing::NiceMock;
 using testing::Return;
 using testing::SaveArg;
 using testing::StrictMock;
 
 namespace {
+const webrtc::PacedPacketInfo kPacingInfo0(0, 5, 2000);
+const webrtc::PacedPacketInfo kPacingInfo1(1, 8, 4000);
 
 // Helper to convert some time format to resolution used in absolute send time
 // header extension, rounded upwards. |t| is the time to convert, in some
 // resolution. |denom| is the value to divide |t| by to get whole seconds,
 // e.g. |denom| = 1000 if |t| is in milliseconds.
 uint32_t AbsSendTime(int64_t t, int64_t denom) {
   return (((t << 18) + (denom >> 1)) / denom) & 0x00fffffful;
 }
 
+// TODO(elad.alon): This is not meant to be landed like this. Ask reviewers

[elad.alon_webrtc.org, 2017/03/02 11:51:47]

Soliciting feedback. :-)

+// where would be a good place to put ComparePacketVectors(), so that it would
+// be visible to both unit-test suites.
+void ComparePacketVectors(const std::vector<webrtc::PacketFeedback>& truth,
+                          const std::vector<webrtc::PacketFeedback>& input) {
+  ASSERT_EQ(truth.size(), input.size());
+  size_t len = truth.size();
+  // truth contains the input data for the test, and input is what will be
+  // sent to the bandwidth estimator. truth.arrival_tims_ms is used to
+  // populate the transport feedback messages. As these times may be changed
+  // (because of resolution limits in the packets, and because of the time
+  // base adjustment performed by the TransportFeedbackAdapter at the first
+  // packet, the truth[x].arrival_time and input[x].arrival_time may not be
+  // equal. However, the difference must be the same for all x.
+  int64_t arrival_time_delta =
+      truth[0].arrival_time_ms - input[0].arrival_time_ms;
+  for (size_t i = 0; i < len; ++i) {
+    RTC_CHECK(truth[i].arrival_time_ms != webrtc::PacketFeedback::kNotReceived);
+    if (input[i].arrival_time_ms != webrtc::PacketFeedback::kNotReceived) {
+      EXPECT_EQ(truth[i].arrival_time_ms,
+                input[i].arrival_time_ms + arrival_time_delta);
+    }
+    EXPECT_EQ(truth[i].send_time_ms, input[i].send_time_ms);
+    EXPECT_EQ(truth[i].sequence_number, input[i].sequence_number);
+    EXPECT_EQ(truth[i].payload_size, input[i].payload_size);
+    EXPECT_EQ(truth[i].pacing_info, input[i].pacing_info);
+  }
+}
 }  // namespace
 
 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.
     controller_.reset(new CongestionController(
         &clock_, &observer_, &remote_bitrate_observer_, &event_log_,
-        &packet_router_, std::move(pacer)));
+        &packet_router_, std::move(pacer),
+        std::unique_ptr<BitrateController>(
+            BitrateController::CreateBitrateController(&clock_, &event_log_))));
     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);
   }
 
+  void OnSentPacket(const PacketFeedback& packet_feedback) {
+    controller_->AddPacket(packet_feedback.sequence_number,
+                           packet_feedback.payload_size,
+                           packet_feedback.pacing_info);
+    controller_->OnSentPacket(rtc::SentPacket(packet_feedback.sequence_number,
+                                              packet_feedback.send_time_ms));
+  }
+
+  class MockBitrateControllerAdapter : public MockBitrateController {

[stefan-webrtc, 2017/03/02 15:58:07]

I would prefer not to have this mock, and to not expose the bitrate controller
in the constructor at all.

It should be possible to write the test so that we can see that the delay-based
bwe does what it should without this. For instance, insert packets at a fixed
rate that is as now and verify that the target bitrate is high enough. Then
insert packets where the rate is lower, and where the delay is growing, and
verify that the target bitrate has decreased.

+   public:
+    explicit MockBitrateControllerAdapter(CongestionControllerTest* owner)
+        : MockBitrateController(), owner_(owner) {}
+
+    ~MockBitrateControllerAdapter() override {}
+
+    void OnDelayBasedBweResult(const DelayBasedBwe::Result& result) override {
+      owner_->target_bitrate_bps_ =
+          rtc::Optional<uint32_t>(result.target_bitrate_bps);
+    }
+
+    CongestionControllerTest* const owner_;
+  };
+
   SimulatedClock clock_;
   StrictMock<MockCongestionObserver> observer_;
   NiceMock<MockPacedSender>* pacer_;
   NiceMock<MockRemoteBitrateObserver> remote_bitrate_observer_;
   NiceMock<MockRtcEventLog> event_log_;
   std::unique_ptr<RtcpBandwidthObserver> bandwidth_observer_;
   PacketRouter packet_router_;
   std::unique_ptr<CongestionController> controller_;
   const uint32_t kInitialBitrateBps = 60000;
+  rtc::Optional<uint32_t> target_bitrate_bps_;
 };
 
 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);
@@ skipping 126 matching lines @@
   EXPECT_CALL(*pacer_, SetEstimatedBitrate(_));
   bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps * 2);
   clock_.AdvanceTimeMilliseconds(25);
   controller_->Process();
 }
 
 TEST_F(CongestionControllerTest, OnReceivedPacketWithAbsSendTime) {
   NiceMock<MockCongestionObserver> observer;
   StrictMock<MockRemoteBitrateObserver> remote_bitrate_observer;
   std::unique_ptr<PacedSender> pacer(new NiceMock<MockPacedSender>());
-  controller_.reset(
-      new CongestionController(&clock_, &observer, &remote_bitrate_observer,
-                               &event_log_, &packet_router_, std::move(pacer)));
+  controller_.reset(new CongestionController(
+      &clock_, &observer, &remote_bitrate_observer, &event_log_,
+      &packet_router_, std::move(pacer),
+      std::unique_ptr<BitrateController>(
+          BitrateController::CreateBitrateController(&clock_, &event_log_))));
 
   size_t payload_size = 1000;
   RTPHeader header;
   header.ssrc = 0x11eb21c;
   header.extension.hasAbsoluteSendTime = true;
 
   std::vector<unsigned int> ssrcs;
   EXPECT_CALL(remote_bitrate_observer, OnReceiveBitrateChanged(_, _))
       .WillRepeatedly(SaveArg<0>(&ssrcs));
 
   for (int i = 0; i < 10; ++i) {
     clock_.AdvanceTimeMilliseconds((1000 * payload_size) / kInitialBitrateBps);
     int64_t now_ms = clock_.TimeInMilliseconds();
     header.extension.absoluteSendTime = AbsSendTime(now_ms, 1000);
     controller_->OnReceivedPacket(now_ms, payload_size, header);
   }
 
   ASSERT_EQ(1u, ssrcs.size());
   EXPECT_EQ(header.ssrc, ssrcs[0]);
 }
+
+TEST_F(CongestionControllerTest, UpdatesDelayBasedEstimate) {
+  NiceMock<MockCongestionObserver> observer;
+  StrictMock<MockRemoteBitrateObserver> remote_bitrate_observer;
+  std::unique_ptr<PacedSender> pacer(new NiceMock<MockPacedSender>());
+  controller_.reset(
+      new CongestionController(&clock_, &observer, &remote_bitrate_observer,
+                               &event_log_, &packet_router_, std::move(pacer),
+                               std::unique_ptr<BitrateController>(
+                                   new MockBitrateControllerAdapter(this))));
+
+  uint16_t seq_num = 0;
+  size_t kPayloadSize = 1000;
+  // The test must run and insert packets/feedback long enough that the
+  // BWE computes a valid estimate.
+  const int64_t kRunTimeMs = 6000;
+  int64_t start_time_ms = clock_.TimeInMilliseconds();
+  while (clock_.TimeInMilliseconds() - start_time_ms < kRunTimeMs) {
+    PacketFeedback packet(clock_.TimeInMilliseconds(),
+                          clock_.TimeInMilliseconds(), seq_num, kPayloadSize,
+                          PacedPacketInfo());
+    OnSentPacket(packet);
+    // Create expected feedback and send into adapter.
+    std::unique_ptr<rtcp::TransportFeedback> feedback(
+        new rtcp::TransportFeedback());
+    feedback->SetBase(packet.sequence_number, packet.arrival_time_ms * 1000);
+    EXPECT_TRUE(feedback->AddReceivedPacket(packet.sequence_number,
+                                            packet.arrival_time_ms * 1000));
+    rtc::Buffer raw_packet = feedback->Build();
+    feedback = rtcp::TransportFeedback::ParseFrom(raw_packet.data(),
+                                                  raw_packet.size());
+    EXPECT_TRUE(feedback.get() != nullptr);
+    controller_->OnTransportFeedback(*feedback.get());
+    clock_.AdvanceTimeMilliseconds(50);
+    ++seq_num;
+  }

[stefan-webrtc, 2017/03/02 15:58:07]

Keep this loop, and also add a similar after:

while (clock_.TimeInMilliseconds() - start_time_ms < kRunTimeMs) {
  // insert late packets by increasing the arrival time more than the send time.
}

Then verified decraesed bitrate.

+
+  EXPECT_TRUE(target_bitrate_bps_);
+}
+
+TEST_F(CongestionControllerTest, LongFeedbackDelays) {
+  // Custom setup
+  std::unique_ptr<PacedSender> pacer(new NiceMock<MockPacedSender>());
+  auto mock_bitrate_controller = std::unique_ptr<MockBitrateControllerAdapter>(
+      new MockBitrateControllerAdapter(this));
+  EXPECT_CALL(*mock_bitrate_controller, SetBitrates(_, _, _));
+  EXPECT_CALL(*mock_bitrate_controller, GetNetworkParameters(_, _, _));
+  EXPECT_CALL(observer_, OnNetworkChanged(_, _, _, _));
+  controller_.reset(new CongestionController(
+      &clock_, &observer_, &remote_bitrate_observer_, &event_log_,
+      &packet_router_, std::move(pacer),
+      std::unique_ptr<BitrateController>(std::move(mock_bitrate_controller))));
+  controller_->SetBweBitrates(0, kInitialBitrateBps, 5 * kInitialBitrateBps);
+
+  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 = -1;
+      packet.payload_size = 0;
+      packet.pacing_info = PacedPacketInfo();
+    }
+    ComparePacketVectors(packets, controller_->GetTransportFeedbackVector());
+  }
+
+  // Target bitrate should have halved due to feedback delays.
+  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);
+    ComparePacketVectors(packets, controller_->GetTransportFeedbackVector());
+  }
+}
 }  // namespace test
 }  // namespace webrtc