diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc index 7f76c6626a..8b3ffc19a6 100644 --- a/modules/pacing/packet_router.cc +++ b/modules/pacing/packet_router.cc @@ -209,11 +209,9 @@ bool PacketRouter::SendRemb(uint32_t bitrate_bps, return false; } - // The Add* and Remove* methods above ensure that this (and only this) module - // has REMB enabled. REMB should be disabled on all other modules, because - // otherwise, they will send REMB with stale info. - RTC_DCHECK(active_remb_module_->REMB()); - active_remb_module_->SetREMBData(bitrate_bps, ssrcs); + // The Add* and Remove* methods above ensure that REMB is disabled on all + // other modules, because otherwise, they will send REMB with stale info. + active_remb_module_->SetRemb(bitrate_bps, ssrcs); return true; } @@ -266,8 +264,7 @@ void PacketRouter::MaybeRemoveRembModuleCandidate(RtpRtcp* candidate_module, void PacketRouter::UnsetActiveRembModule() { RTC_CHECK(active_remb_module_); - RTC_DCHECK(active_remb_module_->REMB()); - active_remb_module_->SetREMBStatus(false); + active_remb_module_->UnsetRemb(); active_remb_module_ = nullptr; } @@ -277,27 +274,21 @@ void PacketRouter::DetermineActiveRembModule() { // When adding the first sender module, we should change the active REMB // module to be that. Otherwise, we remain with the current active module. - RtpRtcp* new_active_remb_module_; + RtpRtcp* new_active_remb_module; if (!sender_remb_candidates_.empty()) { - new_active_remb_module_ = sender_remb_candidates_.front(); + new_active_remb_module = sender_remb_candidates_.front(); } else if (!receiver_remb_candidates_.empty()) { - new_active_remb_module_ = receiver_remb_candidates_.front(); + new_active_remb_module = receiver_remb_candidates_.front(); } else { - new_active_remb_module_ = nullptr; + new_active_remb_module = nullptr; } - if (new_active_remb_module_ != active_remb_module_) { - if (active_remb_module_) { - UnsetActiveRembModule(); - } - if (new_active_remb_module_) { - RTC_DCHECK(!new_active_remb_module_->REMB()); - new_active_remb_module_->SetREMBStatus(true); - } + if (new_active_remb_module != active_remb_module_ && active_remb_module_) { + UnsetActiveRembModule(); } - active_remb_module_ = new_active_remb_module_; + active_remb_module_ = new_active_remb_module; } } // namespace webrtc diff --git a/modules/pacing/packet_router_unittest.cc b/modules/pacing/packet_router_unittest.cc index 7a3847c494..54c8f12c05 100644 --- a/modules/pacing/packet_router_unittest.cc +++ b/modules/pacing/packet_router_unittest.cc @@ -43,16 +43,6 @@ using ::testing::SaveArg; constexpr int kProbeMinProbes = 5; constexpr int kProbeMinBytes = 1000; -class MockRtpRtcpWithRembTracking : public MockRtpRtcp { - public: - MockRtpRtcpWithRembTracking() { - ON_CALL(*this, SetREMBStatus(_)).WillByDefault(SaveArg<0>(&remb_)); - ON_CALL(*this, REMB()).WillByDefault(ReturnPointee(&remb_)); - } - - private: - bool remb_ = false; -}; } // namespace TEST(PacketRouterTest, Sanity_NoModuleRegistered_TimeToSendPacket) { @@ -376,53 +366,12 @@ TEST(PacketRouterTest, RemovalOfNeverAddedReceiveModuleDisallowed) { } #endif // RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) -// TODO(eladalon): Remove this test; it should be covered by: -// 1. SendCandidatePreferredOverReceiveCandidate_SendModuleAddedFirst -// 2. SendCandidatePreferredOverReceiveCandidate_ReceiveModuleAddedFirst -// 3. LowerEstimateToSendRemb -// (Not removing in this CL to prove it doesn't break this test.) -TEST(PacketRouterRembTest, PreferSendModuleOverReceiveModule) { - rtc::ScopedFakeClock clock; - NiceMock rtp_recv; - NiceMock rtp_send; - PacketRouter packet_router; - - packet_router.AddReceiveRtpModule(&rtp_recv, true); - ASSERT_TRUE(rtp_recv.REMB()); - - const uint32_t bitrate_estimate = 456; - const std::vector ssrcs = {1234}; - - packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - // Call OnReceiveBitrateChanged twice to get a first estimate. - clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - EXPECT_CALL(rtp_recv, SetREMBData(bitrate_estimate, ssrcs)).Times(1); - packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - - // Add a send module, which should be preferred over the receive module. - packet_router.AddSendRtpModule(&rtp_send, true); - EXPECT_FALSE(rtp_recv.REMB()); - EXPECT_TRUE(rtp_send.REMB()); - - // Lower bitrate to send another REMB packet. - EXPECT_CALL(rtp_send, SetREMBData(bitrate_estimate - 100, ssrcs)).Times(1); - packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate - 100); - - packet_router.RemoveSendRtpModule(&rtp_send); - EXPECT_TRUE(rtp_recv.REMB()); - EXPECT_FALSE(rtp_send.REMB()); - - packet_router.RemoveReceiveRtpModule(&rtp_recv); -} - TEST(PacketRouterRembTest, LowerEstimateToSendRemb) { rtc::ScopedFakeClock clock; - NiceMock rtp; + NiceMock rtp; PacketRouter packet_router; packet_router.AddSendRtpModule(&rtp, true); - EXPECT_TRUE(rtp.REMB()); uint32_t bitrate_estimate = 456; const std::vector ssrcs = {1234}; @@ -431,17 +380,16 @@ TEST(PacketRouterRembTest, LowerEstimateToSendRemb) { // Call OnReceiveBitrateChanged twice to get a first estimate. clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(rtp, SetRemb(bitrate_estimate, ssrcs)).Times(1); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - // Lower the estimate with more than 3% to trigger a call to SetREMBData right + // Lower the estimate with more than 3% to trigger a call to SetRemb right // away. bitrate_estimate = bitrate_estimate - 100; - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(rtp, SetRemb(bitrate_estimate, ssrcs)).Times(1); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); packet_router.RemoveSendRtpModule(&rtp); - EXPECT_FALSE(rtp.REMB()); } TEST(PacketRouterRembTest, VerifyIncreasingAndDecreasing) { @@ -453,18 +401,17 @@ TEST(PacketRouterRembTest, VerifyIncreasingAndDecreasing) { uint32_t bitrate_estimate[] = {456, 789}; std::vector ssrcs = {1234, 5678}; - ON_CALL(rtp, REMB()).WillByDefault(Return(true)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate[0]); // Call OnReceiveBitrateChanged twice to get a first estimate. - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate[0], ssrcs)).Times(1); + EXPECT_CALL(rtp, SetRemb(bitrate_estimate[0], ssrcs)).Times(1); clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate[0]); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate[1] + 100); // Lower the estimate to trigger a callback. - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate[1], ssrcs)).Times(1); + EXPECT_CALL(rtp, SetRemb(bitrate_estimate[1], ssrcs)).Times(1); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate[1]); packet_router.RemoveSendRtpModule(&rtp); @@ -479,20 +426,19 @@ TEST(PacketRouterRembTest, NoRembForIncreasedBitrate) { uint32_t bitrate_estimate = 456; std::vector ssrcs = {1234, 5678}; - ON_CALL(rtp, REMB()).WillByDefault(Return(true)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); // Call OnReceiveBitrateChanged twice to get a first estimate. - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(rtp, SetRemb(bitrate_estimate, ssrcs)).Times(1); clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); // Increased estimate shouldn't trigger a callback right away. - EXPECT_CALL(rtp, SetREMBData(_, _)).Times(0); + EXPECT_CALL(rtp, SetRemb(_, _)).Times(0); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate + 1); // Decreasing the estimate less than 3% shouldn't trigger a new callback. - EXPECT_CALL(rtp, SetREMBData(_, _)).Times(0); + EXPECT_CALL(rtp, SetRemb(_, _)).Times(0); int lower_estimate = bitrate_estimate * 98 / 100; packet_router.OnReceiveBitrateChanged(ssrcs, lower_estimate); @@ -510,29 +456,25 @@ TEST(PacketRouterRembTest, ChangeSendRtpModule) { uint32_t bitrate_estimate = 456; std::vector ssrcs = {1234, 5678}; - ON_CALL(rtp_send, REMB()).WillByDefault(Return(true)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); // Call OnReceiveBitrateChanged twice to get a first estimate. clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - EXPECT_CALL(rtp_send, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(rtp_send, SetRemb(bitrate_estimate, ssrcs)).Times(1); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); // Decrease estimate to trigger a REMB. bitrate_estimate = bitrate_estimate - 100; - EXPECT_CALL(rtp_send, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(rtp_send, SetRemb(bitrate_estimate, ssrcs)).Times(1); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); // Remove the sending module -> should get remb on the second module. packet_router.RemoveSendRtpModule(&rtp_send); - ON_CALL(rtp_send, REMB()).WillByDefault(Return(false)); - ON_CALL(rtp_recv, REMB()).WillByDefault(Return(true)); - packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); bitrate_estimate = bitrate_estimate - 100; - EXPECT_CALL(rtp_recv, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(rtp_recv, SetRemb(bitrate_estimate, ssrcs)).Times(1); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); packet_router.RemoveReceiveRtpModule(&rtp_recv); @@ -547,37 +489,35 @@ TEST(PacketRouterRembTest, OnlyOneRembForRepeatedOnReceiveBitrateChanged) { uint32_t bitrate_estimate = 456; const std::vector ssrcs = {1234}; - ON_CALL(rtp, REMB()).WillByDefault(Return(true)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); // Call OnReceiveBitrateChanged twice to get a first estimate. clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - EXPECT_CALL(rtp, SetREMBData(_, _)).Times(1); + EXPECT_CALL(rtp, SetRemb(_, _)).Times(1); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); - // Lower the estimate, should trigger a call to SetREMBData right away. + // Lower the estimate, should trigger a call to SetRemb right away. bitrate_estimate = bitrate_estimate - 100; - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(rtp, SetRemb(bitrate_estimate, ssrcs)).Times(1); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); // Call OnReceiveBitrateChanged again, this should not trigger a new callback. - EXPECT_CALL(rtp, SetREMBData(_, _)).Times(0); + EXPECT_CALL(rtp, SetRemb(_, _)).Times(0); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); packet_router.RemoveSendRtpModule(&rtp); } -TEST(PacketRouterRembTest, SetMaxDesiredReceiveBitrateLimitsSetREMBData) { +TEST(PacketRouterRembTest, SetMaxDesiredReceiveBitrateLimitsSetRemb) { rtc::ScopedFakeClock clock; PacketRouter packet_router; clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - NiceMock remb_sender; + 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); + EXPECT_CALL(remb_sender, SetRemb(Le(cap_bitrate), _)).Times(AtLeast(1)); + EXPECT_CALL(remb_sender, SetRemb(Gt(cap_bitrate), _)).Times(0); const std::vector ssrcs = {1234}; packet_router.SetMaxDesiredReceiveBitrate(cap_bitrate); @@ -594,18 +534,17 @@ TEST(PacketRouterRembTest, rtc::ScopedFakeClock clock; PacketRouter packet_router; clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - NiceMock remb_sender; + 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, _)); + EXPECT_CALL(remb_sender, SetRemb(measured_bitrate_bps, _)); packet_router.OnReceiveBitrateChanged(ssrcs, measured_bitrate_bps); - EXPECT_CALL(remb_sender, SetREMBData(cap_bitrate_bps, _)); + EXPECT_CALL(remb_sender, SetRemb(cap_bitrate_bps, _)); packet_router.SetMaxDesiredReceiveBitrate(cap_bitrate_bps); // Test tear-down. @@ -617,18 +556,17 @@ TEST(PacketRouterRembTest, rtc::ScopedFakeClock clock; PacketRouter packet_router; clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - NiceMock remb_sender; + 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, _)); + EXPECT_CALL(remb_sender, SetRemb(measured_bitrate_bps, _)); packet_router.OnReceiveBitrateChanged(ssrcs, measured_bitrate_bps); - EXPECT_CALL(remb_sender, SetREMBData(_, _)).Times(0); + EXPECT_CALL(remb_sender, SetRemb(_, _)).Times(0); packet_router.SetMaxDesiredReceiveBitrate(cap_bitrate_bps); // Test tear-down. @@ -640,18 +578,17 @@ TEST(PacketRouterRembTest, rtc::ScopedFakeClock clock; PacketRouter packet_router; clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - NiceMock remb_sender; + 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, _)); + EXPECT_CALL(remb_sender, SetRemb(measured_bitrate_bps, _)); packet_router.OnReceiveBitrateChanged(ssrcs, measured_bitrate_bps); - EXPECT_CALL(remb_sender, SetREMBData(_, _)).Times(0); + EXPECT_CALL(remb_sender, SetRemb(_, _)).Times(0); packet_router.SetMaxDesiredReceiveBitrate(cap_bitrate_bps); // Test tear-down. @@ -663,19 +600,18 @@ TEST(PacketRouterRembTest, rtc::ScopedFakeClock clock; PacketRouter packet_router; clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - NiceMock remb_sender; + 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, _)); + EXPECT_CALL(remb_sender, SetRemb(measured_bitrate_bps, _)); packet_router.OnReceiveBitrateChanged(ssrcs, measured_bitrate_bps); clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - EXPECT_CALL(remb_sender, SetREMBData(cap_bitrate_bps, _)); + EXPECT_CALL(remb_sender, SetRemb(cap_bitrate_bps, _)); packet_router.SetMaxDesiredReceiveBitrate(cap_bitrate_bps); // Test tear-down. @@ -687,19 +623,18 @@ TEST(PacketRouterRembTest, rtc::ScopedFakeClock clock; PacketRouter packet_router; clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - NiceMock remb_sender; + 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); + EXPECT_CALL(remb_sender, SetRemb(100000, _)).Times(1); packet_router.SetMaxDesiredReceiveBitrate(100000); // Increase cap. - EXPECT_CALL(remb_sender, SetREMBData(200000, _)).Times(1); + EXPECT_CALL(remb_sender, SetRemb(200000, _)).Times(1); packet_router.SetMaxDesiredReceiveBitrate(200000); // Decrease cap. - EXPECT_CALL(remb_sender, SetREMBData(150000, _)).Times(1); + EXPECT_CALL(remb_sender, SetRemb(150000, _)).Times(1); packet_router.SetMaxDesiredReceiveBitrate(150000); // Test tear-down. @@ -713,41 +648,38 @@ TEST(PacketRouterRembTest, NoSendingRtpModule) { NiceMock rtp; PacketRouter packet_router; - EXPECT_CALL(rtp, SetREMBStatus(true)).Times(1); packet_router.AddReceiveRtpModule(&rtp, true); uint32_t bitrate_estimate = 456; const std::vector ssrcs = {1234}; - ON_CALL(rtp, REMB()).WillByDefault(Return(true)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); // Call OnReceiveBitrateChanged twice to get a first estimate. clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(rtp, SetRemb(bitrate_estimate, ssrcs)).Times(1); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); // Lower the estimate to trigger a new packet REMB packet. - EXPECT_CALL(rtp, SetREMBData(bitrate_estimate - 100, ssrcs)).Times(1); + EXPECT_CALL(rtp, SetRemb(bitrate_estimate - 100, ssrcs)).Times(1); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate - 100); - EXPECT_CALL(rtp, SetREMBStatus(false)).Times(1); + EXPECT_CALL(rtp, UnsetRemb()).Times(1); packet_router.RemoveReceiveRtpModule(&rtp); } TEST(PacketRouterRembTest, NonCandidateSendRtpModuleNotUsedForRemb) { rtc::ScopedFakeClock clock; PacketRouter packet_router; - NiceMock module; + NiceMock module; constexpr bool remb_candidate = false; packet_router.AddSendRtpModule(&module, remb_candidate); - EXPECT_FALSE(module.REMB()); constexpr uint32_t bitrate_estimate = 456; const std::vector ssrcs = {1234}; - EXPECT_CALL(module, SetREMBData(_, _)).Times(0); + EXPECT_CALL(module, SetRemb(_, _)).Times(0); clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); @@ -758,16 +690,15 @@ TEST(PacketRouterRembTest, NonCandidateSendRtpModuleNotUsedForRemb) { TEST(PacketRouterRembTest, CandidateSendRtpModuleUsedForRemb) { rtc::ScopedFakeClock clock; PacketRouter packet_router; - NiceMock module; + NiceMock module; constexpr bool remb_candidate = true; packet_router.AddSendRtpModule(&module, remb_candidate); - EXPECT_TRUE(module.REMB()); constexpr uint32_t bitrate_estimate = 456; const std::vector ssrcs = {1234}; - EXPECT_CALL(module, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(module, SetRemb(bitrate_estimate, ssrcs)).Times(1); clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); @@ -778,16 +709,15 @@ TEST(PacketRouterRembTest, CandidateSendRtpModuleUsedForRemb) { TEST(PacketRouterRembTest, NonCandidateReceiveRtpModuleNotUsedForRemb) { rtc::ScopedFakeClock clock; PacketRouter packet_router; - NiceMock module; + NiceMock module; constexpr bool remb_candidate = false; packet_router.AddReceiveRtpModule(&module, remb_candidate); - ASSERT_FALSE(module.REMB()); constexpr uint32_t bitrate_estimate = 456; const std::vector ssrcs = {1234}; - EXPECT_CALL(module, SetREMBData(_, _)).Times(0); + EXPECT_CALL(module, SetRemb(_, _)).Times(0); clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); @@ -798,16 +728,15 @@ TEST(PacketRouterRembTest, NonCandidateReceiveRtpModuleNotUsedForRemb) { TEST(PacketRouterRembTest, CandidateReceiveRtpModuleUsedForRemb) { rtc::ScopedFakeClock clock; PacketRouter packet_router; - NiceMock module; + NiceMock module; constexpr bool remb_candidate = true; packet_router.AddReceiveRtpModule(&module, remb_candidate); - EXPECT_TRUE(module.REMB()); constexpr uint32_t bitrate_estimate = 456; const std::vector ssrcs = {1234}; - EXPECT_CALL(module, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(module, SetRemb(bitrate_estimate, ssrcs)).Times(1); clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); @@ -819,24 +748,21 @@ TEST(PacketRouterRembTest, SendCandidatePreferredOverReceiveCandidate_SendModuleAddedFirst) { rtc::ScopedFakeClock clock; PacketRouter packet_router; - NiceMock send_module; - NiceMock receive_module; + NiceMock send_module; + NiceMock receive_module; constexpr bool remb_candidate = true; // Send module added - activated. packet_router.AddSendRtpModule(&send_module, remb_candidate); - ASSERT_TRUE(send_module.REMB()); // Receive module added - the send module remains the active one. packet_router.AddReceiveRtpModule(&receive_module, remb_candidate); - EXPECT_TRUE(send_module.REMB()); - EXPECT_FALSE(receive_module.REMB()); constexpr uint32_t bitrate_estimate = 456; const std::vector ssrcs = {1234}; - EXPECT_CALL(send_module, SetREMBData(bitrate_estimate, ssrcs)).Times(1); - EXPECT_CALL(receive_module, SetREMBData(_, _)).Times(0); + EXPECT_CALL(send_module, SetRemb(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(receive_module, SetRemb(_, _)).Times(0); clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); @@ -850,24 +776,21 @@ TEST(PacketRouterRembTest, SendCandidatePreferredOverReceiveCandidate_ReceiveModuleAddedFirst) { rtc::ScopedFakeClock clock; PacketRouter packet_router; - NiceMock send_module; - NiceMock receive_module; + NiceMock send_module; + NiceMock receive_module; constexpr bool remb_candidate = true; // Receive module added - activated. packet_router.AddReceiveRtpModule(&receive_module, remb_candidate); - ASSERT_TRUE(receive_module.REMB()); // Send module added - replaces receive module as active. packet_router.AddSendRtpModule(&send_module, remb_candidate); - EXPECT_FALSE(receive_module.REMB()); - EXPECT_TRUE(send_module.REMB()); constexpr uint32_t bitrate_estimate = 456; const std::vector ssrcs = {1234}; - EXPECT_CALL(send_module, SetREMBData(bitrate_estimate, ssrcs)).Times(1); - EXPECT_CALL(receive_module, SetREMBData(_, _)).Times(0); + EXPECT_CALL(send_module, SetRemb(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(receive_module, SetRemb(_, _)).Times(0); clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); @@ -880,25 +803,21 @@ TEST(PacketRouterRembTest, TEST(PacketRouterRembTest, ReceiveModuleTakesOverWhenLastSendModuleRemoved) { rtc::ScopedFakeClock clock; PacketRouter packet_router; - NiceMock send_module; - NiceMock receive_module; + NiceMock send_module; + NiceMock receive_module; constexpr bool remb_candidate = true; // Send module active, receive module inactive. packet_router.AddSendRtpModule(&send_module, remb_candidate); packet_router.AddReceiveRtpModule(&receive_module, remb_candidate); - ASSERT_TRUE(send_module.REMB()); - ASSERT_FALSE(receive_module.REMB()); // Send module removed - receive module becomes active. packet_router.RemoveSendRtpModule(&send_module); - EXPECT_FALSE(send_module.REMB()); - EXPECT_TRUE(receive_module.REMB()); constexpr uint32_t bitrate_estimate = 456; const std::vector ssrcs = {1234}; - EXPECT_CALL(send_module, SetREMBData(_, _)).Times(0); - EXPECT_CALL(receive_module, SetREMBData(bitrate_estimate, ssrcs)).Times(1); + EXPECT_CALL(send_module, SetRemb(_, _)).Times(0); + EXPECT_CALL(receive_module, SetRemb(bitrate_estimate, ssrcs)).Times(1); clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000)); packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate); diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index 8d77a87e24..5142bdf6c7 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -340,12 +340,20 @@ class RtpRtcp : public Module { virtual bool RtcpXrRrtrStatus() const = 0; // (REMB) Receiver Estimated Max Bitrate. - virtual bool REMB() const = 0; + // Schedules sending REMB on next and following sender/receiver reports. + virtual void SetRemb(uint32_t bitrate_bps, + const std::vector& ssrcs) = 0; + // Stops sending REMB on next and following sender/receiver reports. + virtual void UnsetRemb() = 0; - virtual void SetREMBStatus(bool enable) = 0; - - virtual void SetREMBData(uint32_t bitrate, - const std::vector& ssrcs) = 0; + RTC_DEPRECATED void SetREMBStatus(bool enable) { + if (!enable) + UnsetRemb(); + } + RTC_DEPRECATED void SetREMBData(uint32_t bitrate, + const std::vector& ssrcs) { + SetRemb(bitrate, ssrcs); + } // (TMMBR) Temporary Max Media Bit Rate virtual bool TMMBR() const = 0; diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 1a89446dfb..10461ebdd6 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -157,10 +157,9 @@ class MockRtpRtcp : public RtpRtcp { MOCK_METHOD1(SetRTCPVoIPMetrics, int32_t(const RTCPVoIPMetric* voip_metric)); MOCK_METHOD1(SetRtcpXrRrtrStatus, void(bool enable)); MOCK_CONST_METHOD0(RtcpXrRrtrStatus, bool()); - MOCK_CONST_METHOD0(REMB, bool()); - MOCK_METHOD1(SetREMBStatus, void(bool enable)); - MOCK_METHOD2(SetREMBData, + MOCK_METHOD2(SetRemb, void(uint32_t bitrate, const std::vector& ssrcs)); + MOCK_METHOD0(UnsetRemb, void()); MOCK_CONST_METHOD0(TMMBR, bool()); MOCK_METHOD1(SetTMMBRStatus, void(bool enable)); MOCK_METHOD1(OnBandwidthEstimateUpdate, void(uint16_t bandwidth_kbit)); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index b9e9c0af12..b1c91095ff 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -629,17 +629,14 @@ int32_t ModuleRtpRtcpImpl::RemoteRTCPStat( } // (REMB) Receiver Estimated Max Bitrate. -bool ModuleRtpRtcpImpl::REMB() const { - return rtcp_sender_.REMB(); +void ModuleRtpRtcpImpl::SetRemb(uint32_t bitrate_bps, + const std::vector& ssrcs) { + rtcp_sender_.SetREMBStatus(true); + rtcp_sender_.SetREMBData(bitrate_bps, ssrcs); } -void ModuleRtpRtcpImpl::SetREMBStatus(const bool enable) { - rtcp_sender_.SetREMBStatus(enable); -} - -void ModuleRtpRtcpImpl::SetREMBData(const uint32_t bitrate, - const std::vector& ssrcs) { - rtcp_sender_.SetREMBData(bitrate, ssrcs); +void ModuleRtpRtcpImpl::UnsetRemb() { + rtcp_sender_.SetREMBStatus(false); } int32_t ModuleRtpRtcpImpl::RegisterSendRtpHeaderExtension( diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index b18e2dc7c4..ebe942b916 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -192,12 +192,9 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { std::vector* receive_blocks) const override; // (REMB) Receiver Estimated Max Bitrate. - bool REMB() const override; - - void SetREMBStatus(bool enable) override; - - void SetREMBData(uint32_t bitrate, - const std::vector& ssrcs) override; + void SetRemb(uint32_t bitrate_bps, + const std::vector& ssrcs) override; + void UnsetRemb() override; // (TMMBR) Temporary Max Media Bit Rate. bool TMMBR() const override; diff --git a/video/end_to_end_tests.cc b/video/end_to_end_tests.cc index 2ae3ceefbc..9c9a170bac 100644 --- a/video/end_to_end_tests.cc +++ b/video/end_to_end_tests.cc @@ -2317,7 +2317,6 @@ TEST_F(EndToEndTest, RembWithSendSideBwe) { rtp_rtcp_.reset(RtpRtcp::CreateRtpRtcp(config)); rtp_rtcp_->SetRemoteSSRC((*receive_configs)[0].rtp.remote_ssrc); rtp_rtcp_->SetSSRC((*receive_configs)[0].rtp.local_ssrc); - rtp_rtcp_->SetREMBStatus(true); rtp_rtcp_->SetRTCPStatus(RtcpMode::kReducedSize); } @@ -2338,7 +2337,7 @@ TEST_F(EndToEndTest, RembWithSendSideBwe) { if (stats.send_bandwidth_bps >= remb_bitrate_bps_) { state_ = kWaitForRemb; remb_bitrate_bps_ /= 2; - rtp_rtcp_->SetREMBData( + rtp_rtcp_->SetRemb( remb_bitrate_bps_, std::vector(&sender_ssrc_, &sender_ssrc_ + 1)); rtp_rtcp_->SendRTCP(kRtcpRr); @@ -2349,7 +2348,7 @@ TEST_F(EndToEndTest, RembWithSendSideBwe) { if (stats.send_bandwidth_bps == remb_bitrate_bps_) { state_ = kWaitForSecondRampUp; remb_bitrate_bps_ *= 2; - rtp_rtcp_->SetREMBData( + rtp_rtcp_->SetRemb( remb_bitrate_bps_, std::vector(&sender_ssrc_, &sender_ssrc_ + 1)); rtp_rtcp_->SendRTCP(kRtcpRr); diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index faf47c7ffa..e21bc068cd 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -1444,8 +1444,8 @@ TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) { "bps", false); if (total_bitrate_bps > kHighBitrateBps) { - rtp_rtcp_->SetREMBData(kRembBitrateBps, - std::vector(1, header.ssrc)); + rtp_rtcp_->SetRemb(kRembBitrateBps, + std::vector(1, header.ssrc)); rtp_rtcp_->Process(); bitrate_capped_ = true; } else if (bitrate_capped_ && @@ -1465,7 +1465,6 @@ TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) { config.outgoing_transport = feedback_transport_.get(); config.retransmission_rate_limiter = &retranmission_rate_limiter_; rtp_rtcp_.reset(RtpRtcp::CreateRtpRtcp(config)); - rtp_rtcp_->SetREMBStatus(true); rtp_rtcp_->SetRTCPStatus(RtcpMode::kReducedSize); }