Allow REMB messages to be sent immediately in RtcpTransceiver

This cl add a configuration flag to allow REMB messages to be sent immediately when the bitrate value have changed.
The remb message is still included in all following compound packets.

Bug: None
Change-Id: I9f71d30cddbccd095e1d2971247c731bd1727d32
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/169221
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30627}

modules/rtp_rtcp/source/rtcp_packet/remb.cc

@@ -71,7 +71,8 @@ bool Remb::Parse(const CommonHeader& packet) {
   uint64_t mantissa = (static_cast<uint32_t>(payload[13] & 0x03) << 16) |
                       ByteReader<uint16_t>::ReadBigEndian(&payload[14]);
   bitrate_bps_ = (mantissa << exponenta);
-  bool shift_overflow = (bitrate_bps_ >> exponenta) != mantissa;
+  bool shift_overflow =
+      (static_cast<uint64_t>(bitrate_bps_) >> exponenta) != mantissa;
   if (shift_overflow) {
     RTC_LOG(LS_ERROR) << "Invalid remb bitrate value : " << mantissa << "*2^"
                       << static_cast<int>(exponenta);

modules/rtp_rtcp/source/rtcp_packet/remb.h

@@ -32,9 +32,9 @@ class Remb : public Psfb {
   bool Parse(const CommonHeader& packet);
 
   bool SetSsrcs(std::vector<uint32_t> ssrcs);
-  void SetBitrateBps(uint64_t bitrate_bps) { bitrate_bps_ = bitrate_bps; }
+  void SetBitrateBps(int64_t bitrate_bps) { bitrate_bps_ = bitrate_bps; }
 
-  uint64_t bitrate_bps() const { return bitrate_bps_; }
+  int64_t bitrate_bps() const { return bitrate_bps_; }
   const std::vector<uint32_t>& ssrcs() const { return ssrcs_; }
 
   size_t BlockLength() const override;
@@ -51,7 +51,7 @@ class Remb : public Psfb {
   void SetMediaSsrc(uint32_t);
   uint32_t media_ssrc() const;
 
-  uint64_t bitrate_bps_;
+  int64_t bitrate_bps_;
   std::vector<uint32_t> ssrcs_;
 };
 }  // namespace rtcp

modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc

@@ -24,7 +24,7 @@ namespace {
 const uint32_t kSenderSsrc = 0x12345678;
 const uint32_t kRemoteSsrcs[] = {0x23456789, 0x2345678a, 0x2345678b};
 const uint32_t kBitrateBps = 0x3fb93 * 2;  // 522022;
-const uint64_t kBitrateBps64bit = 0x3fb93ULL << 30;
+const int64_t kBitrateBps64bit = int64_t{0x3fb93} << 30;
 const uint8_t kPacket[] = {0x8f, 206,  0x00, 0x07, 0x12, 0x34, 0x56, 0x78,
                            0x00, 0x00, 0x00, 0x00, 'R',  'E',  'M',  'B',
                            0x03, 0x07, 0xfb, 0x93, 0x23, 0x45, 0x67, 0x89,

modules/rtp_rtcp/source/rtcp_sender_unittest.cc

@@ -445,7 +445,7 @@ TEST_F(RtcpSenderTest, RembNotIncludedBeforeSet) {
 }
 
 TEST_F(RtcpSenderTest, RembNotIncludedAfterUnset) {
-  const uint64_t kBitrate = 261011;
+  const int64_t kBitrate = 261011;
   const std::vector<uint32_t> kSsrcs = {kRemoteSsrc, kRemoteSsrc + 1};
   rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
   rtcp_sender_->SetRemb(kBitrate, kSsrcs);
@@ -461,7 +461,7 @@ TEST_F(RtcpSenderTest, RembNotIncludedAfterUnset) {
 }
 
 TEST_F(RtcpSenderTest, SendRemb) {
-  const uint64_t kBitrate = 261011;
+  const int64_t kBitrate = 261011;
   const std::vector<uint32_t> kSsrcs = {kRemoteSsrc, kRemoteSsrc + 1};
   rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
   rtcp_sender_->SetRemb(kBitrate, kSsrcs);

modules/rtp_rtcp/source/rtcp_transceiver.h

@@ -67,7 +67,8 @@ class RtcpTransceiver : public RtcpFeedbackSenderInterface {
   void SendCompoundPacket();
 
   // (REMB) Receiver Estimated Max Bitrate.
-  // Includes REMB in following compound packets.
+  // Includes REMB in following compound packets and sends a REMB message
+  // immediately if 'RtcpTransceiverConfig::send_remb_on_change' is set.
   void SetRemb(int64_t bitrate_bps, std::vector<uint32_t> ssrcs) override;
   // Stops sending REMB in following compound packets.
   void UnsetRemb() override;

modules/rtp_rtcp/source/rtcp_transceiver_config.h

@@ -97,6 +97,10 @@ struct RtcpTransceiverConfig {
   // Estimate RTT as non-sender as described in
   // https://tools.ietf.org/html/rfc3611#section-4.4 and #section-4.5
   bool non_sender_rtt_measurement = false;
+
+  // Allows a REMB message to be sent immediately when SetRemb is called without
+  // having to wait for the next compount message to be sent.
+  bool send_remb_on_change = false;
 };
 
 }  // namespace webrtc

modules/rtp_rtcp/source/rtcp_transceiver_impl.cc

@@ -156,12 +156,22 @@ void RtcpTransceiverImpl::SendCompoundPacket() {
 void RtcpTransceiverImpl::SetRemb(int64_t bitrate_bps,
                                   std::vector<uint32_t> ssrcs) {
   RTC_DCHECK_GE(bitrate_bps, 0);
+
+  bool send_now = config_.send_remb_on_change &&
+                  (!remb_.has_value() || bitrate_bps != remb_->bitrate_bps());
   remb_.emplace();
   remb_->SetSsrcs(std::move(ssrcs));
   remb_->SetBitrateBps(bitrate_bps);
+  remb_->SetSenderSsrc(config_.feedback_ssrc);
   // TODO(bugs.webrtc.org/8239): Move logic from PacketRouter for sending remb
   // immideately on large bitrate change when there is one RtcpTransceiver per
   // rtp transport.
+  if (send_now) {
+    absl::optional<rtcp::Remb> remb;
+    remb.swap(remb_);
+    SendImmediateFeedback(*remb);
+    remb.swap(remb_);
+  }
 }
 
 void RtcpTransceiverImpl::UnsetRemb() {

modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc

@@ -13,6 +13,7 @@
 #include <vector>
 
 #include "absl/memory/memory.h"
+#include "api/rtp_headers.h"
 #include "api/video/video_bitrate_allocation.h"
 #include "modules/rtp_rtcp/include/receive_statistics.h"
 #include "modules/rtp_rtcp/mocks/mock_rtcp_rtt_stats.h"
@@ -395,7 +396,7 @@ TEST(RtcpTransceiverImplTest, SetRembIncludesRembInNextCompoundPacket) {
 
   EXPECT_EQ(rtcp_parser.remb()->num_packets(), 1);
   EXPECT_EQ(rtcp_parser.remb()->sender_ssrc(), kSenderSsrc);
-  EXPECT_EQ(rtcp_parser.remb()->bitrate_bps(), 10000u);
+  EXPECT_EQ(rtcp_parser.remb()->bitrate_bps(), 10000);
   EXPECT_THAT(rtcp_parser.remb()->ssrcs(), ElementsAre(54321, 64321));
 }
 
@@ -413,17 +414,61 @@ TEST(RtcpTransceiverImplTest, SetRembUpdatesValuesToSend) {
   rtcp_transceiver.SendCompoundPacket();
 
   EXPECT_EQ(rtcp_parser.remb()->num_packets(), 1);
-  EXPECT_EQ(rtcp_parser.remb()->bitrate_bps(), 10000u);
+  EXPECT_EQ(rtcp_parser.remb()->bitrate_bps(), 10000);
   EXPECT_THAT(rtcp_parser.remb()->ssrcs(), ElementsAre(54321, 64321));
 
   rtcp_transceiver.SetRemb(/*bitrate_bps=*/70000, /*ssrcs=*/{67321});
   rtcp_transceiver.SendCompoundPacket();
 
   EXPECT_EQ(rtcp_parser.remb()->num_packets(), 2);
-  EXPECT_EQ(rtcp_parser.remb()->bitrate_bps(), 70000u);
+  EXPECT_EQ(rtcp_parser.remb()->bitrate_bps(), 70000);
   EXPECT_THAT(rtcp_parser.remb()->ssrcs(), ElementsAre(67321));
 }
 
+TEST(RtcpTransceiverImplTest, SetRembSendsImmediatelyIfSendRembOnChange) {
+  const uint32_t kSenderSsrc = 12345;
+  RtcpTransceiverConfig config;
+  config.send_remb_on_change = true;
+  config.feedback_ssrc = kSenderSsrc;
+  RtcpPacketParser rtcp_parser;
+  RtcpParserTransport transport(&rtcp_parser);
+  config.outgoing_transport = &transport;
+  config.schedule_periodic_compound_packets = false;
+  RtcpTransceiverImpl rtcp_transceiver(config);
+
+  rtcp_transceiver.SetRemb(/*bitrate_bps=*/10000, /*ssrcs=*/{});
+  EXPECT_EQ(rtcp_parser.remb()->num_packets(), 1);
+  EXPECT_EQ(rtcp_parser.remb()->sender_ssrc(), kSenderSsrc);
+  EXPECT_EQ(rtcp_parser.remb()->bitrate_bps(), 10000);
+
+  // If there is no change, the packet is not sent immediately.
+  rtcp_transceiver.SetRemb(/*bitrate_bps=*/10000, /*ssrcs=*/{});
+  EXPECT_EQ(rtcp_parser.remb()->num_packets(), 1);
+
+  rtcp_transceiver.SetRemb(/*bitrate_bps=*/20000, /*ssrcs=*/{});
+  EXPECT_EQ(rtcp_parser.remb()->num_packets(), 2);
+  EXPECT_EQ(rtcp_parser.remb()->bitrate_bps(), 20000);
+}
+
+TEST(RtcpTransceiverImplTest,
+     SetRembSendsImmediatelyIfSendRembOnChangeReducedSize) {
+  const uint32_t kSenderSsrc = 12345;
+  RtcpTransceiverConfig config;
+  config.send_remb_on_change = true;
+  config.rtcp_mode = webrtc::RtcpMode::kReducedSize;
+  config.feedback_ssrc = kSenderSsrc;
+  RtcpPacketParser rtcp_parser;
+  RtcpParserTransport transport(&rtcp_parser);
+  config.outgoing_transport = &transport;
+  config.schedule_periodic_compound_packets = false;
+  RtcpTransceiverImpl rtcp_transceiver(config);
+
+  rtcp_transceiver.SetRemb(/*bitrate_bps=*/10000, /*ssrcs=*/{});
+  EXPECT_EQ(rtcp_parser.remb()->num_packets(), 1);
+  EXPECT_EQ(rtcp_parser.remb()->sender_ssrc(), kSenderSsrc);
+  EXPECT_EQ(rtcp_parser.remb()->bitrate_bps(), 10000);
+}
+
 TEST(RtcpTransceiverImplTest, SetRembIncludesRembInAllCompoundPackets) {
   const uint32_t kSenderSsrc = 12345;
   RtcpTransceiverConfig config;