diff --git a/modules/rtp_rtcp/source/rtcp_packet/remb.cc b/modules/rtp_rtcp/source/rtcp_packet/remb.cc index 93c12d5672..39795fb79c 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/remb.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/remb.cc @@ -71,7 +71,8 @@ bool Remb::Parse(const CommonHeader& packet) { uint64_t mantissa = (static_cast(payload[13] & 0x03) << 16) | ByteReader::ReadBigEndian(&payload[14]); bitrate_bps_ = (mantissa << exponenta); - bool shift_overflow = (bitrate_bps_ >> exponenta) != mantissa; + bool shift_overflow = + (static_cast(bitrate_bps_) >> exponenta) != mantissa; if (shift_overflow) { RTC_LOG(LS_ERROR) << "Invalid remb bitrate value : " << mantissa << "*2^" << static_cast(exponenta); diff --git a/modules/rtp_rtcp/source/rtcp_packet/remb.h b/modules/rtp_rtcp/source/rtcp_packet/remb.h index 232b25b096..b7075c0f23 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/remb.h +++ b/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 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& 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 ssrcs_; }; } // namespace rtcp diff --git a/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc b/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc index ed5f48fec6..391a61de89 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/remb_unittest.cc +++ b/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, diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index c732a35bd0..754ad89327 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/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 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 kSsrcs = {kRemoteSsrc, kRemoteSsrc + 1}; rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize); rtcp_sender_->SetRemb(kBitrate, kSsrcs); diff --git a/modules/rtp_rtcp/source/rtcp_transceiver.h b/modules/rtp_rtcp/source/rtcp_transceiver.h index 8bdb0bf913..fe5c9695c3 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver.h +++ b/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 ssrcs) override; // Stops sending REMB in following compound packets. void UnsetRemb() override; diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_config.h b/modules/rtp_rtcp/source/rtcp_transceiver_config.h index 01330d0bc7..8a77e709d3 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_config.h +++ b/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 diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index 8a1a791eb9..5f2f2e02c3 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc @@ -156,12 +156,22 @@ void RtcpTransceiverImpl::SendCompoundPacket() { void RtcpTransceiverImpl::SetRemb(int64_t bitrate_bps, std::vector 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 remb; + remb.swap(remb_); + SendImmediateFeedback(*remb); + remb.swap(remb_); + } } void RtcpTransceiverImpl::UnsetRemb() { diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc index eff328329f..47ce4a825d 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc @@ -13,6 +13,7 @@ #include #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;