Simplify RtpRtcp interface for REMB

Remove REMB accessor as used for debug checks only.
Merge SetRembData and SetRembStatus(true) eliminating 
state 'remb can be send, but no data available yet'

Bug: None
Change-Id: I4c1c19435657e5cde02a17de90ec6de9f00b7daf
Reviewed-on: https://webrtc-review.googlesource.com/7983
Reviewed-by: Elad Alon <eladalon@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20240}

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
+  // The Add* and Remove* methods above ensure that REMB is disabled on all
-  // has REMB enabled. REMB should be disabled on all other modules, because
+  // other modules, because otherwise, they will send REMB with stale info.
-  // otherwise, they will send REMB with stale info.
+  active_remb_module_->SetRemb(bitrate_bps, ssrcs);
-  RTC_DCHECK(active_remb_module_->REMB());
-  active_remb_module_->SetREMBData(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_->UnsetRemb();
-  active_remb_module_->SetREMBStatus(false);
   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 (new_active_remb_module != active_remb_module_ && active_remb_module_) {
-    if (active_remb_module_) {
+    UnsetActiveRembModule();
-      UnsetActiveRembModule();
-    }
-    if (new_active_remb_module_) {
-      RTC_DCHECK(!new_active_remb_module_->REMB());
-      new_active_remb_module_->SetREMBStatus(true);
-    }
   }
 
-  active_remb_module_ = new_active_remb_module_;
+  active_remb_module_ = new_active_remb_module;
 }
 
 }  // namespace webrtc

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<MockRtpRtcpWithRembTracking> rtp_recv;
-  NiceMock<MockRtpRtcpWithRembTracking> 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<uint32_t> 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<MockRtpRtcpWithRembTracking> rtp;
+  NiceMock<MockRtpRtcp> rtp;
   PacketRouter packet_router;
 
   packet_router.AddSendRtpModule(&rtp, true);
-  EXPECT_TRUE(rtp.REMB());
 
   uint32_t bitrate_estimate = 456;
   const std::vector<uint32_t> 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<uint32_t> 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<uint32_t> 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<uint32_t> 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<uint32_t> 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<MockRtpRtcpWithRembTracking> remb_sender;
+  NiceMock<MockRtpRtcp> 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, SetRemb(Le(cap_bitrate), _)).Times(AtLeast(1));
-  EXPECT_CALL(remb_sender, SetREMBData(Gt(cap_bitrate), _)).Times(0);
+  EXPECT_CALL(remb_sender, SetRemb(Gt(cap_bitrate), _)).Times(0);
 
   const std::vector<uint32_t> 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<MockRtpRtcpWithRembTracking> remb_sender;
+  NiceMock<MockRtpRtcp> 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<uint32_t> 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<MockRtpRtcpWithRembTracking> remb_sender;
+  NiceMock<MockRtpRtcp> 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<uint32_t> 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<MockRtpRtcpWithRembTracking> remb_sender;
+  NiceMock<MockRtpRtcp> 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<uint32_t> 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<MockRtpRtcpWithRembTracking> remb_sender;
+  NiceMock<MockRtpRtcp> 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<uint32_t> 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<MockRtpRtcpWithRembTracking> remb_sender;
+  NiceMock<MockRtpRtcp> 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<MockRtpRtcp> rtp;
   PacketRouter packet_router;
 
-  EXPECT_CALL(rtp, SetREMBStatus(true)).Times(1);
   packet_router.AddReceiveRtpModule(&rtp, true);
 
   uint32_t bitrate_estimate = 456;
   const std::vector<uint32_t> 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<MockRtpRtcpWithRembTracking> module;
+  NiceMock<MockRtpRtcp> 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<uint32_t> 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<MockRtpRtcpWithRembTracking> module;
+  NiceMock<MockRtpRtcp> 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<uint32_t> 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<MockRtpRtcpWithRembTracking> module;
+  NiceMock<MockRtpRtcp> 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<uint32_t> 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<MockRtpRtcpWithRembTracking> module;
+  NiceMock<MockRtpRtcp> 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<uint32_t> 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<MockRtpRtcpWithRembTracking> send_module;
+  NiceMock<MockRtpRtcp> send_module;
-  NiceMock<MockRtpRtcpWithRembTracking> receive_module;
+  NiceMock<MockRtpRtcp> 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<uint32_t> ssrcs = {1234};
-  EXPECT_CALL(send_module, SetREMBData(bitrate_estimate, ssrcs)).Times(1);
+  EXPECT_CALL(send_module, SetRemb(bitrate_estimate, ssrcs)).Times(1);
-  EXPECT_CALL(receive_module, SetREMBData(_, _)).Times(0);
+  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<MockRtpRtcpWithRembTracking> send_module;
+  NiceMock<MockRtpRtcp> send_module;
-  NiceMock<MockRtpRtcpWithRembTracking> receive_module;
+  NiceMock<MockRtpRtcp> 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<uint32_t> ssrcs = {1234};
-  EXPECT_CALL(send_module, SetREMBData(bitrate_estimate, ssrcs)).Times(1);
+  EXPECT_CALL(send_module, SetRemb(bitrate_estimate, ssrcs)).Times(1);
-  EXPECT_CALL(receive_module, SetREMBData(_, _)).Times(0);
+  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<MockRtpRtcpWithRembTracking> send_module;
+  NiceMock<MockRtpRtcp> send_module;
-  NiceMock<MockRtpRtcpWithRembTracking> receive_module;
+  NiceMock<MockRtpRtcp> 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<uint32_t> ssrcs = {1234};
-  EXPECT_CALL(send_module, SetREMBData(_, _)).Times(0);
+  EXPECT_CALL(send_module, SetRemb(_, _)).Times(0);
-  EXPECT_CALL(receive_module, SetREMBData(bitrate_estimate, ssrcs)).Times(1);
+  EXPECT_CALL(receive_module, SetRemb(bitrate_estimate, ssrcs)).Times(1);
 
   clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1000));
   packet_router.OnReceiveBitrateChanged(ssrcs, bitrate_estimate);

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<uint32_t>& ssrcs) = 0;
+  // Stops sending REMB on next and following sender/receiver reports.
+  virtual void UnsetRemb() = 0;
 
-  virtual void SetREMBStatus(bool enable) = 0;
+  RTC_DEPRECATED void SetREMBStatus(bool enable) {
-
+    if (!enable)
-  virtual void SetREMBData(uint32_t bitrate,
+      UnsetRemb();
-                           const std::vector<uint32_t>& ssrcs) = 0;
+  }
+  RTC_DEPRECATED void SetREMBData(uint32_t bitrate,
+                                  const std::vector<uint32_t>& ssrcs) {
+    SetRemb(bitrate, ssrcs);
+  }
 
   // (TMMBR) Temporary Max Media Bit Rate
   virtual bool TMMBR() const = 0;

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_METHOD2(SetRemb,
-  MOCK_METHOD1(SetREMBStatus, void(bool enable));
-  MOCK_METHOD2(SetREMBData,
                void(uint32_t bitrate, const std::vector<uint32_t>& ssrcs));
+  MOCK_METHOD0(UnsetRemb, void());
   MOCK_CONST_METHOD0(TMMBR, bool());
   MOCK_METHOD1(SetTMMBRStatus, void(bool enable));
   MOCK_METHOD1(OnBandwidthEstimateUpdate, void(uint16_t bandwidth_kbit));

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 {
+void ModuleRtpRtcpImpl::SetRemb(uint32_t bitrate_bps,
-  return rtcp_sender_.REMB();
+                                const std::vector<uint32_t>& ssrcs) {
+  rtcp_sender_.SetREMBStatus(true);
+  rtcp_sender_.SetREMBData(bitrate_bps, ssrcs);
 }
 
-void ModuleRtpRtcpImpl::SetREMBStatus(const bool enable) {
+void ModuleRtpRtcpImpl::UnsetRemb() {
-  rtcp_sender_.SetREMBStatus(enable);
+  rtcp_sender_.SetREMBStatus(false);
-}
-
-void ModuleRtpRtcpImpl::SetREMBData(const uint32_t bitrate,
-                                    const std::vector<uint32_t>& ssrcs) {
-  rtcp_sender_.SetREMBData(bitrate, ssrcs);
 }
 
 int32_t ModuleRtpRtcpImpl::RegisterSendRtpHeaderExtension(

modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -192,12 +192,9 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp {
       std::vector<RTCPReportBlock>* receive_blocks) const override;
 
   // (REMB) Receiver Estimated Max Bitrate.
-  bool REMB() const override;
+  void SetRemb(uint32_t bitrate_bps,
-
+               const std::vector<uint32_t>& ssrcs) override;
-  void SetREMBStatus(bool enable) override;
+  void UnsetRemb() override;
-
-  void SetREMBData(uint32_t bitrate,
-                   const std::vector<uint32_t>& ssrcs) override;
 
   // (TMMBR) Temporary Max Media Bit Rate.
   bool TMMBR() const override;

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<uint32_t>(&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<uint32_t>(&sender_ssrc_, &sender_ssrc_ + 1));
                 rtp_rtcp_->SendRTCP(kRtcpRr);

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,
+          rtp_rtcp_->SetRemb(kRembBitrateBps,
-                                 std::vector<uint32_t>(1, header.ssrc));
+                             std::vector<uint32_t>(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);
     }