diff --git a/modules/congestion_controller/BUILD.gn b/modules/congestion_controller/BUILD.gn index 3e1e8c0e85..c0b064d9ed 100644 --- a/modules/congestion_controller/BUILD.gn +++ b/modules/congestion_controller/BUILD.gn @@ -22,12 +22,17 @@ rtc_library("congestion_controller") { sources = [ "include/receive_side_congestion_controller.h", "receive_side_congestion_controller.cc", + "remb_throttler.cc", + "remb_throttler.h", ] deps = [ "..:module_api", "../../api/transport:field_trial_based_config", "../../api/transport:network_control", + "../../api/units:data_rate", + "../../api/units:time_delta", + "../../api/units:timestamp", "../../rtc_base/synchronization:mutex", "../pacing", "../remote_bitrate_estimator", @@ -43,11 +48,17 @@ if (rtc_include_tests && !build_with_chromium) { rtc_library("congestion_controller_unittests") { testonly = true - sources = [ "receive_side_congestion_controller_unittest.cc" ] + sources = [ + "receive_side_congestion_controller_unittest.cc", + "remb_throttler_unittest.cc", + ] deps = [ ":congestion_controller", "../../api/test/network_emulation", "../../api/test/network_emulation:create_cross_traffic", + "../../api/units:data_rate", + "../../api/units:time_delta", + "../../api/units:timestamp", "../../system_wrappers", "../../test:test_support", "../../test/scenario", diff --git a/modules/congestion_controller/include/receive_side_congestion_controller.h b/modules/congestion_controller/include/receive_side_congestion_controller.h index 034f2e9517..b46cd8d7fc 100644 --- a/modules/congestion_controller/include/receive_side_congestion_controller.h +++ b/modules/congestion_controller/include/receive_side_congestion_controller.h @@ -16,7 +16,10 @@ #include "api/transport/field_trial_based_config.h" #include "api/transport/network_control.h" +#include "api/units/data_rate.h" +#include "modules/congestion_controller/remb_throttler.h" #include "modules/include/module.h" +#include "modules/pacing/packet_router.h" #include "modules/remote_bitrate_estimator/remote_estimator_proxy.h" #include "rtc_base/synchronization/mutex.h" @@ -32,12 +35,20 @@ class RemoteBitrateObserver; class ReceiveSideCongestionController : public CallStatsObserver, public Module { public: + // TODO(bugs.webrtc.org/12693): Deprecate ReceiveSideCongestionController(Clock* clock, PacketRouter* packet_router); + // TODO(bugs.webrtc.org/12693): Deprecate ReceiveSideCongestionController( Clock* clock, PacketRouter* packet_router, NetworkStateEstimator* network_state_estimator); + ReceiveSideCongestionController( + Clock* clock, + RemoteEstimatorProxy::TransportFeedbackSender feedback_sender, + RembThrottler::RembSender remb_sender, + NetworkStateEstimator* network_state_estimator); + ~ReceiveSideCongestionController() override {} virtual void OnReceivedPacket(int64_t arrival_time_ms, @@ -56,6 +67,10 @@ class ReceiveSideCongestionController : public CallStatsObserver, // This is send bitrate, used to control the rate of feedback messages. void OnBitrateChanged(int bitrate_bps); + // Ensures the remote party is notified of the receive bitrate no larger than + // |bitrate| using RTCP REMB. + void SetMaxDesiredReceiveBitrate(DataRate bitrate); + // Implements Module. int64_t TimeUntilNextProcess() override; void Process() override; @@ -103,6 +118,7 @@ class ReceiveSideCongestionController : public CallStatsObserver, }; const FieldTrialBasedConfig field_trial_config_; + RembThrottler remb_throttler_; WrappingBitrateEstimator remote_bitrate_estimator_; RemoteEstimatorProxy remote_estimator_proxy_; }; diff --git a/modules/congestion_controller/receive_side_congestion_controller.cc b/modules/congestion_controller/receive_side_congestion_controller.cc index 638cb2d295..e4e6cc9698 100644 --- a/modules/congestion_controller/receive_side_congestion_controller.cc +++ b/modules/congestion_controller/receive_side_congestion_controller.cc @@ -10,6 +10,7 @@ #include "modules/congestion_controller/include/receive_side_congestion_controller.h" +#include "api/units/data_rate.h" #include "modules/pacing/packet_router.h" #include "modules/remote_bitrate_estimator/include/bwe_defines.h" #include "modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h" @@ -127,9 +128,26 @@ ReceiveSideCongestionController::ReceiveSideCongestionController( Clock* clock, PacketRouter* packet_router, NetworkStateEstimator* network_state_estimator) - : remote_bitrate_estimator_(packet_router, clock), + : remb_throttler_([](auto...) {}, clock), + remote_bitrate_estimator_(packet_router, clock), + remote_estimator_proxy_( + clock, + [packet_router]( + std::vector> packets) { + packet_router->SendCombinedRtcpPacket(std::move(packets)); + }, + &field_trial_config_, + network_state_estimator) {} + +ReceiveSideCongestionController::ReceiveSideCongestionController( + Clock* clock, + RemoteEstimatorProxy::TransportFeedbackSender feedback_sender, + RembThrottler::RembSender remb_sender, + NetworkStateEstimator* network_state_estimator) + : remb_throttler_(std::move(remb_sender), clock), + remote_bitrate_estimator_(&remb_throttler_, clock), remote_estimator_proxy_(clock, - packet_router, + std::move(feedback_sender), &field_trial_config_, network_state_estimator) {} @@ -186,4 +204,9 @@ void ReceiveSideCongestionController::Process() { remote_bitrate_estimator_.Process(); } +void ReceiveSideCongestionController::SetMaxDesiredReceiveBitrate( + DataRate bitrate) { + remb_throttler_.SetMaxDesiredReceiveBitrate(bitrate); +} + } // namespace webrtc diff --git a/modules/congestion_controller/receive_side_congestion_controller_unittest.cc b/modules/congestion_controller/receive_side_congestion_controller_unittest.cc index 5622c84689..5e03179f42 100644 --- a/modules/congestion_controller/receive_side_congestion_controller_unittest.cc +++ b/modules/congestion_controller/receive_side_congestion_controller_unittest.cc @@ -20,10 +20,8 @@ using ::testing::_; using ::testing::AtLeast; -using ::testing::NiceMock; -using ::testing::Return; -using ::testing::SaveArg; -using ::testing::StrictMock; +using ::testing::ElementsAre; +using ::testing::MockFunction; namespace webrtc { @@ -37,34 +35,28 @@ uint32_t AbsSendTime(int64_t t, int64_t denom) { return (((t << 18) + (denom >> 1)) / denom) & 0x00fffffful; } -class MockPacketRouter : public PacketRouter { - public: - MOCK_METHOD(void, - OnReceiveBitrateChanged, - (const std::vector& ssrcs, uint32_t bitrate), - (override)); -}; - const uint32_t kInitialBitrateBps = 60000; } // namespace namespace test { -TEST(ReceiveSideCongestionControllerTest, OnReceivedPacketWithAbsSendTime) { - StrictMock packet_router; +TEST(ReceiveSideCongestionControllerTest, SendsRembWithAbsSendTime) { + MockFunction>)> + feedback_sender; + MockFunction)> remb_sender; SimulatedClock clock_(123456); - ReceiveSideCongestionController controller(&clock_, &packet_router); + ReceiveSideCongestionController controller( + &clock_, feedback_sender.AsStdFunction(), remb_sender.AsStdFunction(), + nullptr); size_t payload_size = 1000; RTPHeader header; header.ssrc = 0x11eb21c; header.extension.hasAbsoluteSendTime = true; - std::vector ssrcs; - EXPECT_CALL(packet_router, OnReceiveBitrateChanged(_, _)) - .WillRepeatedly(SaveArg<0>(&ssrcs)); + EXPECT_CALL(remb_sender, Call(_, ElementsAre(header.ssrc))).Times(AtLeast(1)); for (int i = 0; i < 10; ++i) { clock_.AdvanceTimeMilliseconds((1000 * payload_size) / kInitialBitrateBps); @@ -72,9 +64,20 @@ TEST(ReceiveSideCongestionControllerTest, OnReceivedPacketWithAbsSendTime) { 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(ReceiveSideCongestionControllerTest, + SendsRembAfterSetMaxDesiredReceiveBitrate) { + MockFunction>)> + feedback_sender; + MockFunction)> remb_sender; + SimulatedClock clock_(123456); + + ReceiveSideCongestionController controller( + &clock_, feedback_sender.AsStdFunction(), remb_sender.AsStdFunction(), + nullptr); + EXPECT_CALL(remb_sender, Call(123, _)); + controller.SetMaxDesiredReceiveBitrate(DataRate::BitsPerSec(123)); } TEST(ReceiveSideCongestionControllerTest, ConvergesToCapacity) { diff --git a/modules/congestion_controller/remb_throttler.cc b/modules/congestion_controller/remb_throttler.cc new file mode 100644 index 0000000000..fcc30af9a8 --- /dev/null +++ b/modules/congestion_controller/remb_throttler.cc @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2021 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 "modules/congestion_controller/remb_throttler.h" + +#include +#include + +namespace webrtc { + +namespace { +constexpr TimeDelta kRembSendInterval = TimeDelta::Millis(200); +} // namespace + +RembThrottler::RembThrottler(RembSender remb_sender, Clock* clock) + : remb_sender_(std::move(remb_sender)), + clock_(clock), + last_remb_time_(Timestamp::MinusInfinity()), + last_send_remb_bitrate_(DataRate::PlusInfinity()), + max_remb_bitrate_(DataRate::PlusInfinity()) {} + +void RembThrottler::OnReceiveBitrateChanged(const std::vector& ssrcs, + uint32_t bitrate_bps) { + DataRate receive_bitrate = DataRate::BitsPerSec(bitrate_bps); + Timestamp now = clock_->CurrentTime(); + { + MutexLock lock(&mutex_); + // % threshold for if we should send a new REMB asap. + const int64_t kSendThresholdPercent = 103; + if (receive_bitrate * kSendThresholdPercent / 100 > + last_send_remb_bitrate_ && + now < last_remb_time_ + kRembSendInterval) { + return; + } + last_remb_time_ = now; + last_send_remb_bitrate_ = receive_bitrate; + receive_bitrate = std::min(last_send_remb_bitrate_, max_remb_bitrate_); + } + remb_sender_(receive_bitrate.bps(), ssrcs); +} + +void RembThrottler::SetMaxDesiredReceiveBitrate(DataRate bitrate) { + Timestamp now = clock_->CurrentTime(); + { + MutexLock lock(&mutex_); + max_remb_bitrate_ = bitrate; + if (now - last_remb_time_ < kRembSendInterval && + !last_send_remb_bitrate_.IsZero() && + last_send_remb_bitrate_ <= max_remb_bitrate_) { + return; + } + } + remb_sender_(bitrate.bps(), /*ssrcs=*/{}); +} + +} // namespace webrtc diff --git a/modules/congestion_controller/remb_throttler.h b/modules/congestion_controller/remb_throttler.h new file mode 100644 index 0000000000..67c0280749 --- /dev/null +++ b/modules/congestion_controller/remb_throttler.h @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2021 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. + */ +#ifndef MODULES_CONGESTION_CONTROLLER_REMB_THROTTLER_H_ +#define MODULES_CONGESTION_CONTROLLER_REMB_THROTTLER_H_ + +#include +#include + +#include "api/units/data_rate.h" +#include "api/units/time_delta.h" +#include "api/units/timestamp.h" +#include "modules/remote_bitrate_estimator/remote_estimator_proxy.h" +#include "rtc_base/synchronization/mutex.h" + +namespace webrtc { + +// RembThrottler is a helper class used for throttling RTCP REMB messages. +// Throttles small changes to the received BWE within 200ms. +class RembThrottler : public RemoteBitrateObserver { + public: + using RembSender = + std::function ssrcs)>; + RembThrottler(RembSender remb_sender, Clock* clock); + + // Ensures the remote party is notified of the receive bitrate no larger than + // |bitrate| using RTCP REMB. + void SetMaxDesiredReceiveBitrate(DataRate bitrate); + + // Implements RemoteBitrateObserver; + // Called every time there is a new bitrate estimate for a receive channel + // group. This call will trigger a new RTCP REMB packet if the bitrate + // estimate has decreased or if no RTCP REMB packet has been sent for + // a certain time interval. + void OnReceiveBitrateChanged(const std::vector& ssrcs, + uint32_t bitrate_bps) override; + + private: + const RembSender remb_sender_; + Clock* const clock_; + mutable Mutex mutex_; + Timestamp last_remb_time_ RTC_GUARDED_BY(mutex_); + DataRate last_send_remb_bitrate_ RTC_GUARDED_BY(mutex_); + DataRate max_remb_bitrate_ RTC_GUARDED_BY(mutex_); +}; + +} // namespace webrtc +#endif // MODULES_CONGESTION_CONTROLLER_REMB_THROTTLER_H_ diff --git a/modules/congestion_controller/remb_throttler_unittest.cc b/modules/congestion_controller/remb_throttler_unittest.cc new file mode 100644 index 0000000000..3f8df8a7bb --- /dev/null +++ b/modules/congestion_controller/remb_throttler_unittest.cc @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2021 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 "modules/congestion_controller/remb_throttler.h" + +#include + +#include "api/units/data_rate.h" +#include "api/units/time_delta.h" +#include "system_wrappers/include/clock.h" +#include "test/gmock.h" +#include "test/gtest.h" + +namespace webrtc { + +using ::testing::_; +using ::testing::MockFunction; + +TEST(RembThrottlerTest, CallRembSenderOnFirstReceiveBitrateChange) { + SimulatedClock clock(Timestamp::Zero()); + MockFunction)> remb_sender; + RembThrottler remb_throttler(remb_sender.AsStdFunction(), &clock); + + EXPECT_CALL(remb_sender, Call(12345, std::vector({1, 2, 3}))); + remb_throttler.OnReceiveBitrateChanged({1, 2, 3}, /*bitrate_bps=*/12345); +} + +TEST(RembThrottlerTest, ThrottlesSmallReceiveBitrateDecrease) { + SimulatedClock clock(Timestamp::Zero()); + MockFunction)> remb_sender; + RembThrottler remb_throttler(remb_sender.AsStdFunction(), &clock); + + EXPECT_CALL(remb_sender, Call); + remb_throttler.OnReceiveBitrateChanged({1, 2, 3}, /*bitrate_bps=*/12346); + clock.AdvanceTime(TimeDelta::Millis(100)); + remb_throttler.OnReceiveBitrateChanged({1, 2, 3}, /*bitrate_bps=*/12345); + + EXPECT_CALL(remb_sender, Call(12345, _)); + clock.AdvanceTime(TimeDelta::Millis(101)); + remb_throttler.OnReceiveBitrateChanged({1, 2, 3}, /*bitrate_bps=*/12345); +} + +TEST(RembThrottlerTest, DoNotThrottleLargeReceiveBitrateDecrease) { + SimulatedClock clock(Timestamp::Zero()); + MockFunction)> remb_sender; + RembThrottler remb_throttler(remb_sender.AsStdFunction(), &clock); + + EXPECT_CALL(remb_sender, Call(2345, _)); + EXPECT_CALL(remb_sender, Call(1234, _)); + remb_throttler.OnReceiveBitrateChanged({1, 2, 3}, /*bitrate_bps=*/2345); + clock.AdvanceTime(TimeDelta::Millis(1)); + remb_throttler.OnReceiveBitrateChanged({1, 2, 3}, /*bitrate_bps=*/1234); +} + +TEST(RembThrottlerTest, ThrottlesReceiveBitrateIncrease) { + SimulatedClock clock(Timestamp::Zero()); + MockFunction)> remb_sender; + RembThrottler remb_throttler(remb_sender.AsStdFunction(), &clock); + + EXPECT_CALL(remb_sender, Call); + remb_throttler.OnReceiveBitrateChanged({1, 2, 3}, /*bitrate_bps=*/1234); + clock.AdvanceTime(TimeDelta::Millis(100)); + remb_throttler.OnReceiveBitrateChanged({1, 2, 3}, /*bitrate_bps=*/2345); + + // Updates 200ms after previous callback is not throttled. + EXPECT_CALL(remb_sender, Call(2345, _)); + clock.AdvanceTime(TimeDelta::Millis(101)); + remb_throttler.OnReceiveBitrateChanged({1, 2, 3}, /*bitrate_bps=*/2345); +} + +TEST(RembThrottlerTest, CallRembSenderOnSetMaxDesiredReceiveBitrate) { + SimulatedClock clock(Timestamp::Zero()); + MockFunction)> remb_sender; + RembThrottler remb_throttler(remb_sender.AsStdFunction(), &clock); + EXPECT_CALL(remb_sender, Call(1234, _)); + remb_throttler.SetMaxDesiredReceiveBitrate(DataRate::BitsPerSec(1234)); +} + +TEST(RembThrottlerTest, CallRembSenderWithMinOfMaxDesiredAndOnReceivedBitrate) { + SimulatedClock clock(Timestamp::Zero()); + MockFunction)> remb_sender; + RembThrottler remb_throttler(remb_sender.AsStdFunction(), &clock); + + EXPECT_CALL(remb_sender, Call(1234, _)); + remb_throttler.OnReceiveBitrateChanged({1, 2, 3}, /*bitrate_bps=*/1234); + clock.AdvanceTime(TimeDelta::Millis(1)); + remb_throttler.SetMaxDesiredReceiveBitrate(DataRate::BitsPerSec(4567)); + + clock.AdvanceTime(TimeDelta::Millis(200)); + EXPECT_CALL(remb_sender, Call(4567, _)); + remb_throttler.OnReceiveBitrateChanged({1, 2, 3}, /*bitrate_bps=*/5678); +} + +} // namespace webrtc diff --git a/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h b/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h index c60c030e8d..a9edfb3e1b 100644 --- a/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h +++ b/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h @@ -38,6 +38,7 @@ class RemoteBitrateObserver { virtual ~RemoteBitrateObserver() {} }; +// TODO(bugs.webrtc.org/12693): Deprecate class TransportFeedbackSenderInterface { public: virtual ~TransportFeedbackSenderInterface() = default; diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index a9cc170a35..a3361092bd 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -33,11 +33,11 @@ static constexpr int64_t kMaxTimeMs = RemoteEstimatorProxy::RemoteEstimatorProxy( Clock* clock, - TransportFeedbackSenderInterface* feedback_sender, + TransportFeedbackSender feedback_sender, const WebRtcKeyValueConfig* key_value_config, NetworkStateEstimator* network_state_estimator) : clock_(clock), - feedback_sender_(feedback_sender), + feedback_sender_(std::move(feedback_sender)), send_config_(key_value_config), last_process_time_ms_(-1), network_state_estimator_(network_state_estimator), @@ -217,7 +217,7 @@ void RemoteEstimatorProxy::SendPeriodicFeedbacks() { } packets.push_back(std::move(feedback_packet)); - feedback_sender_->SendCombinedRtcpPacket(std::move(packets)); + feedback_sender_(std::move(packets)); // Note: Don't erase items from packet_arrival_times_ after sending, in case // they need to be re-sent after a reordering. Removal will be handled // by OnPacketArrival once packets are too old. @@ -250,7 +250,7 @@ void RemoteEstimatorProxy::SendFeedbackOnRequest( RTC_DCHECK(feedback_sender_ != nullptr); std::vector> packets; packets.push_back(std::move(feedback_packet)); - feedback_sender_->SendCombinedRtcpPacket(std::move(packets)); + feedback_sender_(std::move(packets)); } int64_t RemoteEstimatorProxy::BuildFeedbackPacket( diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/modules/remote_bitrate_estimator/remote_estimator_proxy.h index a4adefc5ee..5aabfe1f0c 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.h @@ -11,7 +11,9 @@ #ifndef MODULES_REMOTE_BITRATE_ESTIMATOR_REMOTE_ESTIMATOR_PROXY_H_ #define MODULES_REMOTE_BITRATE_ESTIMATOR_REMOTE_ESTIMATOR_PROXY_H_ +#include #include +#include #include #include "api/transport/network_control.h" @@ -24,7 +26,6 @@ namespace webrtc { class Clock; -class PacketRouter; namespace rtcp { class TransportFeedback; } @@ -32,11 +33,14 @@ class TransportFeedback; // Class used when send-side BWE is enabled: This proxy is instantiated on the // receive side. It buffers a number of receive timestamps and then sends // transport feedback messages back too the send side. - class RemoteEstimatorProxy : public RemoteBitrateEstimator { public: + // Used for sending transport feedback messages when send side + // BWE is used. + using TransportFeedbackSender = std::function> packets)>; RemoteEstimatorProxy(Clock* clock, - TransportFeedbackSenderInterface* feedback_sender, + TransportFeedbackSender feedback_sender, const WebRtcKeyValueConfig* key_value_config, NetworkStateEstimator* network_state_estimator); ~RemoteEstimatorProxy() override; @@ -88,7 +92,7 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { rtcp::TransportFeedback* feedback_packet); Clock* const clock_; - TransportFeedbackSenderInterface* const feedback_sender_; + const TransportFeedbackSender feedback_sender_; const TransportWideFeedbackConfig send_config_; int64_t last_process_time_ms_; diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc index da995922d9..296724fa71 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc @@ -16,8 +16,8 @@ #include "api/transport/field_trial_based_config.h" #include "api/transport/network_types.h" #include "api/transport/test/mock_network_control.h" -#include "modules/pacing/packet_router.h" #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" +#include "modules/rtp_rtcp/source/rtp_header_extensions.h" #include "system_wrappers/include/clock.h" #include "test/gmock.h" #include "test/gtest.h" @@ -25,6 +25,7 @@ using ::testing::_; using ::testing::ElementsAre; using ::testing::Invoke; +using ::testing::MockFunction; using ::testing::Return; using ::testing::SizeIs; @@ -63,20 +64,12 @@ std::vector TimestampsMs( return timestamps; } -class MockTransportFeedbackSender : public TransportFeedbackSenderInterface { - public: - MOCK_METHOD(bool, - SendCombinedRtcpPacket, - (std::vector> feedback_packets), - (override)); -}; - class RemoteEstimatorProxyTest : public ::testing::Test { public: RemoteEstimatorProxyTest() : clock_(0), proxy_(&clock_, - &router_, + feedback_sender_.AsStdFunction(), &field_trial_config_, &network_state_estimator_) {} @@ -113,7 +106,8 @@ class RemoteEstimatorProxyTest : public ::testing::Test { FieldTrialBasedConfig field_trial_config_; SimulatedClock clock_; - ::testing::StrictMock router_; + MockFunction>)> + feedback_sender_; ::testing::NiceMock network_state_estimator_; RemoteEstimatorProxy proxy_; }; @@ -121,7 +115,7 @@ class RemoteEstimatorProxyTest : public ::testing::Test { TEST_F(RemoteEstimatorProxyTest, SendsSinglePacketFeedback) { IncomingPacket(kBaseSeq, kBaseTimeMs); - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -134,7 +128,6 @@ TEST_F(RemoteEstimatorProxyTest, SendsSinglePacketFeedback) { ElementsAre(kBaseSeq)); EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs)); - return true; })); Process(); @@ -144,7 +137,7 @@ TEST_F(RemoteEstimatorProxyTest, DuplicatedPackets) { IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kBaseSeq, kBaseTimeMs + 1000); - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -167,13 +160,13 @@ TEST_F(RemoteEstimatorProxyTest, FeedbackWithMissingStart) { // First feedback. IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1000); - EXPECT_CALL(router_, SendCombinedRtcpPacket).WillOnce(Return(true)); + EXPECT_CALL(feedback_sender_, Call); Process(); // Second feedback starts with a missing packet (DROP kBaseSeq + 2). IncomingPacket(kBaseSeq + 3, kBaseTimeMs + 3000); - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -186,7 +179,6 @@ TEST_F(RemoteEstimatorProxyTest, FeedbackWithMissingStart) { ElementsAre(kBaseSeq + 3)); EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs + 3000)); - return true; })); Process(); @@ -197,7 +189,7 @@ TEST_F(RemoteEstimatorProxyTest, SendsFeedbackWithVaryingDeltas) { IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kMaxSmallDeltaMs); IncomingPacket(kBaseSeq + 2, kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1); - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -211,7 +203,6 @@ TEST_F(RemoteEstimatorProxyTest, SendsFeedbackWithVaryingDeltas) { EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs, kBaseTimeMs + kMaxSmallDeltaMs, kBaseTimeMs + (2 * kMaxSmallDeltaMs) + 1)); - return true; })); Process(); @@ -224,7 +215,7 @@ TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) { IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kTooLargeDelta); - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -237,7 +228,6 @@ TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) { ElementsAre(kBaseSeq)); EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs)); - return true; })) .WillOnce(Invoke( [](std::vector> feedback_packets) { @@ -251,7 +241,6 @@ TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) { ElementsAre(kBaseSeq + 1)); EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs + kTooLargeDelta)); - return true; })); Process(); @@ -263,7 +252,7 @@ TEST_F(RemoteEstimatorProxyTest, HandlesReorderingAndWrap) { IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kLargeSeq, kBaseTimeMs + kDeltaMs); - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [&](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -274,7 +263,6 @@ TEST_F(RemoteEstimatorProxyTest, HandlesReorderingAndWrap) { EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs + kDeltaMs, kBaseTimeMs)); - return true; })); Process(); @@ -293,7 +281,7 @@ TEST_F(RemoteEstimatorProxyTest, HandlesMalformedSequenceNumbers) { } // Only expect feedback for the last two packets. - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [&](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -306,7 +294,6 @@ TEST_F(RemoteEstimatorProxyTest, HandlesMalformedSequenceNumbers) { EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs + 28 * kDeltaMs, kBaseTimeMs + 29 * kDeltaMs)); - return true; })); Process(); @@ -324,7 +311,7 @@ TEST_F(RemoteEstimatorProxyTest, HandlesBackwardsWrappingSequenceNumbers) { } // Only expect feedback for the first two packets. - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [&](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -336,7 +323,6 @@ TEST_F(RemoteEstimatorProxyTest, HandlesBackwardsWrappingSequenceNumbers) { ElementsAre(kBaseSeq + 40000, kBaseSeq)); EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs + kDeltaMs, kBaseTimeMs)); - return true; })); Process(); @@ -346,7 +332,7 @@ TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { IncomingPacket(kBaseSeq, kBaseTimeMs); IncomingPacket(kBaseSeq + 2, kBaseTimeMs + 2); - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -359,14 +345,13 @@ TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { ElementsAre(kBaseSeq, kBaseSeq + 2)); EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs, kBaseTimeMs + 2)); - return true; })); Process(); IncomingPacket(kBaseSeq + 1, kBaseTimeMs + 1); - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -379,7 +364,6 @@ TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { ElementsAre(kBaseSeq + 1, kBaseSeq + 2)); EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs + 1, kBaseTimeMs + 2)); - return true; })); Process(); @@ -390,7 +374,7 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { IncomingPacket(kBaseSeq + 2, kBaseTimeMs); - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -400,14 +384,13 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs)); - return true; })); Process(); IncomingPacket(kBaseSeq + 3, kTimeoutTimeMs); // kBaseSeq + 2 times out here. - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [&](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -417,7 +400,6 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kTimeoutTimeMs)); - return true; })); Process(); @@ -427,7 +409,7 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { IncomingPacket(kBaseSeq, kBaseTimeMs - 1); IncomingPacket(kBaseSeq + 1, kTimeoutTimeMs - 1); - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [&](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -440,7 +422,6 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs - 1, kTimeoutTimeMs - 1, kTimeoutTimeMs)); - return true; })); Process(); @@ -496,7 +477,7 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, TimeUntilNextProcessIsHigh) { TEST_F(RemoteEstimatorProxyOnRequestTest, ProcessDoesNotSendFeedback) { proxy_.SetSendPeriodicFeedback(false); IncomingPacket(kBaseSeq, kBaseTimeMs); - EXPECT_CALL(router_, SendCombinedRtcpPacket).Times(0); + EXPECT_CALL(feedback_sender_, Call).Times(0); Process(); } @@ -506,7 +487,7 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, RequestSinglePacketFeedback) { IncomingPacket(kBaseSeq + 1, kBaseTimeMs + kMaxSmallDeltaMs); IncomingPacket(kBaseSeq + 2, kBaseTimeMs + 2 * kMaxSmallDeltaMs); - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -519,7 +500,6 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, RequestSinglePacketFeedback) { ElementsAre(kBaseSeq + 3)); EXPECT_THAT(TimestampsMs(*feedback_packet), ElementsAre(kBaseTimeMs + 3 * kMaxSmallDeltaMs)); - return true; })); constexpr FeedbackRequest kSinglePacketFeedbackRequest = { @@ -535,7 +515,7 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, RequestLastFivePacketFeedback) { IncomingPacket(kBaseSeq + i, kBaseTimeMs + i * kMaxSmallDeltaMs); } - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -553,7 +533,6 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, RequestLastFivePacketFeedback) { kBaseTimeMs + 8 * kMaxSmallDeltaMs, kBaseTimeMs + 9 * kMaxSmallDeltaMs, kBaseTimeMs + 10 * kMaxSmallDeltaMs)); - return true; })); constexpr FeedbackRequest kFivePacketsFeedbackRequest = { @@ -571,7 +550,7 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, IncomingPacket(kBaseSeq + i, kBaseTimeMs + i * kMaxSmallDeltaMs); } - EXPECT_CALL(router_, SendCombinedRtcpPacket) + EXPECT_CALL(feedback_sender_, Call) .WillOnce(Invoke( [](std::vector> feedback_packets) { rtcp::TransportFeedback* feedback_packet = @@ -586,7 +565,6 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, ElementsAre(kBaseTimeMs + 6 * kMaxSmallDeltaMs, kBaseTimeMs + 8 * kMaxSmallDeltaMs, kBaseTimeMs + 10 * kMaxSmallDeltaMs)); - return true; })); constexpr FeedbackRequest kFivePacketsFeedbackRequest = { @@ -658,13 +636,7 @@ TEST_F(RemoteEstimatorProxyTest, SendTransportFeedbackAndNetworkStateUpdate) { AbsoluteSendTime::MsTo24Bits(kBaseTimeMs - 1))); EXPECT_CALL(network_state_estimator_, GetCurrentEstimate()) .WillOnce(Return(NetworkStateEstimate())); - EXPECT_CALL(router_, SendCombinedRtcpPacket) - .WillOnce( - [](std::vector> feedback_packets) { - EXPECT_THAT(feedback_packets, SizeIs(2)); - return true; - }); - + EXPECT_CALL(feedback_sender_, Call(SizeIs(2))); Process(); }