diff --git a/webrtc/modules/pacing/packet_router.cc b/webrtc/modules/pacing/packet_router.cc index 64e1aeacc1..b5cd444495 100644 --- a/webrtc/modules/pacing/packet_router.cc +++ b/webrtc/modules/pacing/packet_router.cc @@ -10,6 +10,9 @@ #include "webrtc/modules/pacing/packet_router.h" +#include +#include + #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" @@ -18,10 +21,17 @@ #include "webrtc/rtc_base/timeutils.h" namespace webrtc { +namespace { + +constexpr int kRembSendIntervalMs = 200; + +} // namespace PacketRouter::PacketRouter() : last_remb_time_ms_(rtc::TimeMillis()), last_send_bitrate_bps_(0), + bitrate_bps_(0), + max_bitrate_bps_(std::numeric_limits::max()), active_remb_module_(nullptr), transport_seq_(0) {} @@ -142,8 +152,6 @@ uint16_t PacketRouter::AllocateSequenceNumber() { void PacketRouter::OnReceiveBitrateChanged(const std::vector& ssrcs, uint32_t bitrate_bps) { - const int kRembSendIntervalMs = 200; - // % threshold for if we should send a new REMB asap. const uint32_t kSendThresholdPercent = 97; @@ -173,10 +181,26 @@ void PacketRouter::OnReceiveBitrateChanged(const std::vector& ssrcs, // a module to actually send it. last_remb_time_ms_ = now_ms; last_send_bitrate_bps_ = bitrate_bps; + // Cap the value to send in remb with configured value. + bitrate_bps = std::min(bitrate_bps, max_bitrate_bps_); } SendRemb(bitrate_bps, ssrcs); } +void PacketRouter::SetMaxDesiredReceiveBitrate(uint32_t bitrate_bps) { + { + rtc::CritScope lock(&remb_crit_); + max_bitrate_bps_ = bitrate_bps; + if (rtc::TimeMillis() - last_remb_time_ms_ < kRembSendIntervalMs && + last_send_bitrate_bps_ > 0 && + last_send_bitrate_bps_ <= max_bitrate_bps_) { + // Recent measured bitrate is already below the cap. + return; + } + } + SendRemb(bitrate_bps, /*ssrcs=*/{}); +} + bool PacketRouter::SendRemb(uint32_t bitrate_bps, const std::vector& ssrcs) { rtc::CritScope lock(&modules_crit_); diff --git a/webrtc/modules/pacing/packet_router.h b/webrtc/modules/pacing/packet_router.h index e5abdeb2ab..945b1c2caa 100644 --- a/webrtc/modules/pacing/packet_router.h +++ b/webrtc/modules/pacing/packet_router.h @@ -83,6 +83,10 @@ class PacketRouter : public PacedSender::PacketSender, void OnReceiveBitrateChanged(const std::vector& ssrcs, uint32_t bitrate_bps) override; + // Ensures remote party notified of the receive bitrate limit no larger than + // |bitrate_bps|. + void SetMaxDesiredReceiveBitrate(uint32_t bitrate_bps); + // Send REMB feedback. virtual bool SendRemb(uint32_t bitrate_bps, const std::vector& ssrcs); @@ -111,6 +115,7 @@ class PacketRouter : public PacedSender::PacketSender, uint32_t last_send_bitrate_bps_ GUARDED_BY(remb_crit_); // The last bitrate update. uint32_t bitrate_bps_ GUARDED_BY(remb_crit_); + uint32_t max_bitrate_bps_ GUARDED_BY(remb_crit_); // Candidates for the REMB module can be RTP sender/receiver modules, with // the sender modules taking precedence. diff --git a/webrtc/modules/pacing/packet_router_unittest.cc b/webrtc/modules/pacing/packet_router_unittest.cc index de3206688a..c33f25ce93 100644 --- a/webrtc/modules/pacing/packet_router_unittest.cc +++ b/webrtc/modules/pacing/packet_router_unittest.cc @@ -20,14 +20,6 @@ #include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" -using ::testing::_; -using ::testing::AnyNumber; -using ::testing::Field; -using ::testing::NiceMock; -using ::testing::Return; -using ::testing::ReturnPointee; -using ::testing::SaveArg; - namespace webrtc { // TODO(eladalon): Restructure and/or replace the existing monolithic tests @@ -36,6 +28,18 @@ namespace webrtc { // (I'm not removing any tests during CL, so as to demonstrate no regressions.) namespace { + +using ::testing::_; +using ::testing::AnyNumber; +using ::testing::AtLeast; +using ::testing::Field; +using ::testing::Gt; +using ::testing::Le; +using ::testing::NiceMock; +using ::testing::Return; +using ::testing::ReturnPointee; +using ::testing::SaveArg; + constexpr int kProbeMinProbes = 5; constexpr int kProbeMinBytes = 1000; @@ -562,6 +566,146 @@ TEST(PacketRouterRembTest, OnlyOneRembForRepeatedOnReceiveBitrateChanged) { packet_router.RemoveSendRtpModule(&rtp); } +TEST(PacketRouterRembTest, SetMaxDesiredReceiveBitrateLimitsSetREMBData) { + rtc::ScopedFakeClock clock; + PacketRouter packet_router; + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + NiceMock remb_sender; + constexpr bool remb_candidate = true; + packet_router.AddSendRtpModule(&remb_sender, remb_candidate); + ASSERT_TRUE(remb_sender.REMB()); + + const uint32_t cap_bitrate = 100000; + EXPECT_CALL(remb_sender, SetREMBData(Le(cap_bitrate), _)).Times(AtLeast(1)); + EXPECT_CALL(remb_sender, SetREMBData(Gt(cap_bitrate), _)).Times(0); + + const std::vector ssrcs = {1234}; + packet_router.SetMaxDesiredReceiveBitrate(cap_bitrate); + packet_router.OnReceiveBitrateChanged(ssrcs, cap_bitrate + 5000); + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + packet_router.OnReceiveBitrateChanged(ssrcs, cap_bitrate - 5000); + + // Test tear-down. + packet_router.RemoveSendRtpModule(&remb_sender); +} + +TEST(PacketRouterRembTest, + SetMaxDesiredReceiveBitrateTriggersRembWhenMoreRestrictive) { + rtc::ScopedFakeClock clock; + PacketRouter packet_router; + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + NiceMock remb_sender; + constexpr bool remb_candidate = true; + packet_router.AddSendRtpModule(&remb_sender, remb_candidate); + ASSERT_TRUE(remb_sender.REMB()); + + const uint32_t measured_bitrate_bps = 150000; + const uint32_t cap_bitrate_bps = measured_bitrate_bps - 5000; + const std::vector ssrcs = {1234}; + EXPECT_CALL(remb_sender, SetREMBData(measured_bitrate_bps, _)); + packet_router.OnReceiveBitrateChanged(ssrcs, measured_bitrate_bps); + + EXPECT_CALL(remb_sender, SetREMBData(cap_bitrate_bps, _)); + packet_router.SetMaxDesiredReceiveBitrate(cap_bitrate_bps); + + // Test tear-down. + packet_router.RemoveSendRtpModule(&remb_sender); +} + +TEST(PacketRouterRembTest, + SetMaxDesiredReceiveBitrateDoesNotTriggerRembWhenAsRestrictive) { + rtc::ScopedFakeClock clock; + PacketRouter packet_router; + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + NiceMock remb_sender; + constexpr bool remb_candidate = true; + packet_router.AddSendRtpModule(&remb_sender, remb_candidate); + ASSERT_TRUE(remb_sender.REMB()); + + const uint32_t measured_bitrate_bps = 150000; + const uint32_t cap_bitrate_bps = measured_bitrate_bps; + const std::vector ssrcs = {1234}; + EXPECT_CALL(remb_sender, SetREMBData(measured_bitrate_bps, _)); + packet_router.OnReceiveBitrateChanged(ssrcs, measured_bitrate_bps); + + EXPECT_CALL(remb_sender, SetREMBData(_, _)).Times(0); + packet_router.SetMaxDesiredReceiveBitrate(cap_bitrate_bps); + + // Test tear-down. + packet_router.RemoveSendRtpModule(&remb_sender); +} + +TEST(PacketRouterRembTest, + SetMaxDesiredReceiveBitrateDoesNotTriggerRembWhenLessRestrictive) { + rtc::ScopedFakeClock clock; + PacketRouter packet_router; + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + NiceMock remb_sender; + constexpr bool remb_candidate = true; + packet_router.AddSendRtpModule(&remb_sender, remb_candidate); + ASSERT_TRUE(remb_sender.REMB()); + + const uint32_t measured_bitrate_bps = 150000; + const uint32_t cap_bitrate_bps = measured_bitrate_bps + 500; + const std::vector ssrcs = {1234}; + EXPECT_CALL(remb_sender, SetREMBData(measured_bitrate_bps, _)); + packet_router.OnReceiveBitrateChanged(ssrcs, measured_bitrate_bps); + + EXPECT_CALL(remb_sender, SetREMBData(_, _)).Times(0); + packet_router.SetMaxDesiredReceiveBitrate(cap_bitrate_bps); + + // Test tear-down. + packet_router.RemoveSendRtpModule(&remb_sender); +} + +TEST(PacketRouterRembTest, + SetMaxDesiredReceiveBitrateTriggersRembWhenNoRecentMeasure) { + rtc::ScopedFakeClock clock; + PacketRouter packet_router; + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + NiceMock remb_sender; + constexpr bool remb_candidate = true; + packet_router.AddSendRtpModule(&remb_sender, remb_candidate); + ASSERT_TRUE(remb_sender.REMB()); + + const uint32_t measured_bitrate_bps = 150000; + const uint32_t cap_bitrate_bps = measured_bitrate_bps + 5000; + const std::vector ssrcs = {1234}; + EXPECT_CALL(remb_sender, SetREMBData(measured_bitrate_bps, _)); + packet_router.OnReceiveBitrateChanged(ssrcs, measured_bitrate_bps); + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + + EXPECT_CALL(remb_sender, SetREMBData(cap_bitrate_bps, _)); + packet_router.SetMaxDesiredReceiveBitrate(cap_bitrate_bps); + + // Test tear-down. + packet_router.RemoveSendRtpModule(&remb_sender); +} + +TEST(PacketRouterRembTest, + SetMaxDesiredReceiveBitrateTriggersRembWhenNoMeasures) { + rtc::ScopedFakeClock clock; + PacketRouter packet_router; + clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); + NiceMock remb_sender; + constexpr bool remb_candidate = true; + packet_router.AddSendRtpModule(&remb_sender, remb_candidate); + ASSERT_TRUE(remb_sender.REMB()); + + // Set cap. + EXPECT_CALL(remb_sender, SetREMBData(100000, _)).Times(1); + packet_router.SetMaxDesiredReceiveBitrate(100000); + // Increase cap. + EXPECT_CALL(remb_sender, SetREMBData(200000, _)).Times(1); + packet_router.SetMaxDesiredReceiveBitrate(200000); + // Decrease cap. + EXPECT_CALL(remb_sender, SetREMBData(150000, _)).Times(1); + packet_router.SetMaxDesiredReceiveBitrate(150000); + + // Test tear-down. + packet_router.RemoveSendRtpModule(&remb_sender); +} + // Only register receiving modules and make sure we fallback to trigger a REMB // packet on this one. TEST(PacketRouterRembTest, NoSendingRtpModule) {