Deprecate setter RtpRtcpInterface::SetRid

This setter method is replaced by a construction-time config setting.

Bug: None
Change-Id: Iddffaeeb719a56328bccde3c4a1a0a852d2131b1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264501
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37060}

call/rtp_video_sender.cc

@@ -238,6 +238,12 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
 
   RTC_DCHECK(rtp_config.rtx.ssrcs.empty() ||
              rtp_config.rtx.ssrcs.size() == rtp_config.ssrcs.size());
+
+  // Some streams could have been disabled, but the rids are still there.
+  // This will occur when simulcast has been disabled for a codec (e.g. VP9)
+  RTC_DCHECK(rtp_config.rids.empty() ||
+             rtp_config.rids.size() >= rtp_config.ssrcs.size());
+
   for (size_t i = 0; i < rtp_config.ssrcs.size(); ++i) {
     RTPSenderVideo::Config video_config;
     configuration.local_media_ssrc = rtp_config.ssrcs[i];
@@ -251,6 +257,8 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
     RTC_DCHECK_EQ(configuration.rtx_send_ssrc.has_value(),
                   !rtp_config.rtx.ssrcs.empty());
 
+    configuration.rid = (i < rtp_config.rids.size()) ? rtp_config.rids[i] : "";
+
     configuration.need_rtp_packet_infos = rtp_config.lntf.enabled;
 
     std::unique_ptr<ModuleRtpRtcpImpl2> rtp_rtcp(
@@ -425,7 +433,6 @@ RtpVideoSender::RtpVideoSender(
   }
 
   ConfigureSsrcs(suspended_ssrcs);
-  ConfigureRids();
 
   if (!rtp_config_.mid.empty()) {
     for (const RtpStreamSender& stream : rtp_streams_) {
@@ -760,18 +767,6 @@ void RtpVideoSender::ConfigureSsrcs(
   }
 }
 
-void RtpVideoSender::ConfigureRids() {
-  if (rtp_config_.rids.empty())
-    return;
-
-  // Some streams could have been disabled, but the rids are still there.
-  // This will occur when simulcast has been disabled for a codec (e.g. VP9)
-  RTC_DCHECK(rtp_config_.rids.size() >= rtp_streams_.size());
-  for (size_t i = 0; i < rtp_streams_.size(); ++i) {
-    rtp_streams_[i].rtp_rtcp->SetRid(rtp_config_.rids[i]);
-  }
-}
-
 void RtpVideoSender::OnNetworkAvailability(bool network_available) {
   for (const RtpStreamSender& stream : rtp_streams_) {
     stream.rtp_rtcp->SetRTCPStatus(network_available ? rtp_config_.rtcp_mode

call/rtp_video_sender.h

@@ -159,7 +159,6 @@ class RtpVideoSender : public RtpVideoSenderInterface,
   void UpdateModuleSendingState() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
   void ConfigureProtection();
   void ConfigureSsrcs(const std::map<uint32_t, RtpState>& suspended_ssrcs);
-  void ConfigureRids();
   bool NackEnabled() const;
   uint32_t GetPacketizationOverheadRate() const;
   DataRate CalculateOverheadRate(DataRate data_rate,

modules/rtp_rtcp/source/rtp_rtcp_interface.h

@@ -148,6 +148,12 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface {
     // 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;
+
+    // If non-empty, sets the value for sending in the RID (and Repaired) RTP
+    // header extension. RIDs are used to identify an RTP stream if SSRCs are
+    // not negotiated. If the RID and Repaired RID extensions are not
+    // registered, the RID will not be sent.
+    std::string rid;
   };
 
   // Stats for RTCP sender reports (SR) for a specific SSRC.
@@ -256,7 +262,8 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface {
   // RIDs are used to identify an RTP stream if SSRCs are not negotiated.
   // If the RID and Repaired RID extensions are not registered, the RID will
   // not be sent.
-  virtual void SetRid(absl::string_view rid) = 0;
+  [[deprecated("Use the rid member of config struct instead'")]] virtual void
+  SetRid(absl::string_view rid) = 0;
 
   // Sets the value for sending in the MID RTP header extension.
   // The MID RTP header extension should be registered for this to do anything.

modules/rtp_rtcp/source/rtp_sender.cc

@@ -173,6 +173,7 @@ RTPSender::RTPSender(const RtpRtcpInterface::Configuration& config,
       max_packet_size_(IP_PACKET_SIZE - 28),  // Default is IP-v4/UDP.
       rtp_header_extension_map_(config.extmap_allow_mixed),
       // RTP variables
+      rid_(config.rid),
       always_send_mid_and_rid_(config.always_send_mid_and_rid),
       ssrc_has_acked_(false),
       rtx_ssrc_has_acked_(false),
@@ -180,12 +181,14 @@ RTPSender::RTPSender(const RtpRtcpInterface::Configuration& config,
       rtx_(kRtxOff),
       supports_bwe_extension_(false),
       retransmission_rate_limiter_(config.retransmission_rate_limiter) {
-  UpdateHeaderSizes();
   // This random initialization is not intended to be cryptographic strong.
   timestamp_offset_ = random_.Rand<uint32_t>();
 
   RTC_DCHECK(paced_sender_);
   RTC_DCHECK(packet_history_);
+  RTC_DCHECK_LE(rid_.size(), RtpStreamId::kMaxValueSizeBytes);
+
+  UpdateHeaderSizes();
 }
 
 RTPSender::~RTPSender() {

modules/rtp_rtcp/source/rtp_sender_unittest.cc

@@ -72,6 +72,8 @@ const uint8_t kPayloadData[] = {47, 11, 32, 93, 89};
 const int64_t kDefaultExpectedRetransmissionTimeMs = 125;
 const size_t kMaxPaddingLength = 224;      // Value taken from rtp_sender.cc.
 const uint32_t kTimestampTicksPerMs = 90;  // 90kHz clock.
+constexpr absl::string_view kMid = "mid";
+constexpr absl::string_view kRid = "f";
 
 using ::testing::_;
 using ::testing::AllOf;
@@ -161,6 +163,9 @@ class RtpSenderTest : public ::testing::Test {
     config.retransmission_rate_limiter = &retransmission_rate_limiter_;
     config.paced_sender = &mock_paced_sender_;
     config.field_trials = &field_trials_;
+    // Configure rid unconditionally, it has effect only if
+    // corresponding header extension is enabled.
+    config.rid = std::string(kRid);
     return config;
   }
 
@@ -274,12 +279,11 @@ class RtpSenderTest : public ::testing::Test {
 
   // Enable sending of the RSID header extension for the primary SSRC and the
   // RRSID header extension for the RTX SSRC.
-  void EnableRidSending(absl::string_view rid) {
+  void EnableRidSending() {
     rtp_sender_->RegisterRtpHeaderExtension(RtpStreamId::Uri(),
                                             kRidExtensionId);
     rtp_sender_->RegisterRtpHeaderExtension(RepairedRtpStreamId::Uri(),
                                             kRepairedRidExtensionId);
-    rtp_sender_->SetRid(rid);
   }
 };
 
@@ -593,7 +597,6 @@ TEST_F(RtpSenderTest, KeepsTimestampsOnPayloadPadding) {
 // Test that the MID header extension is included on sent packets when
 // configured.
 TEST_F(RtpSenderTest, MidIncludedOnSentPackets) {
-  const char kMid[] = "mid";
   EnableMidSending(kMid);
 
   // Send a couple packets, expect both packets to have the MID set.
@@ -606,8 +609,7 @@ TEST_F(RtpSenderTest, MidIncludedOnSentPackets) {
 }
 
 TEST_F(RtpSenderTest, RidIncludedOnSentPackets) {
-  const char kRid[] = "f";
-  EnableRidSending(kRid);
+  EnableRidSending();
 
   EXPECT_CALL(mock_paced_sender_,
               EnqueuePackets(ElementsAre(Pointee(Property(
@@ -616,9 +618,8 @@ TEST_F(RtpSenderTest, RidIncludedOnSentPackets) {
 }
 
 TEST_F(RtpSenderTest, RidIncludedOnRtxSentPackets) {
-  const char kRid[] = "f";
   EnableRtx();
-  EnableRidSending(kRid);
+  EnableRidSending();
 
   EXPECT_CALL(mock_paced_sender_,
               EnqueuePackets(ElementsAre(Pointee(AllOf(
@@ -641,11 +642,8 @@ TEST_F(RtpSenderTest, RidIncludedOnRtxSentPackets) {
 }
 
 TEST_F(RtpSenderTest, MidAndRidNotIncludedOnSentPacketsAfterAck) {
-  const char kMid[] = "mid";
-  const char kRid[] = "f";
-
   EnableMidSending(kMid);
-  EnableRidSending(kRid);
+  EnableRidSending();
 
   // This first packet should include both MID and RID.
   EXPECT_CALL(
@@ -667,10 +665,8 @@ TEST_F(RtpSenderTest, MidAndRidNotIncludedOnSentPacketsAfterAck) {
 
 TEST_F(RtpSenderTest, MidAndRidAlwaysIncludedOnSentPacketsWhenConfigured) {
   SetUpRtpSender(false, /*always_send_mid_and_rid=*/true, nullptr);
-  const char kMid[] = "mid";
-  const char kRid[] = "f";
   EnableMidSending(kMid);
-  EnableRidSending(kRid);
+  EnableRidSending();
 
   // Send two media packets: one before and one after the ack.
   // Due to the configuration, both sent packets should contain MID and RID.
@@ -690,12 +686,9 @@ TEST_F(RtpSenderTest, MidAndRidAlwaysIncludedOnSentPacketsWhenConfigured) {
 // the first packets for a given SSRC, and RTX packets are sent on a separate
 // SSRC.
 TEST_F(RtpSenderTest, MidAndRidIncludedOnFirstRtxPacket) {
-  const char kMid[] = "mid";
-  const char kRid[] = "f";
-
   EnableRtx();
   EnableMidSending(kMid);
-  EnableRidSending(kRid);
+  EnableRidSending();
 
   // This first packet will include both MID and RID.
   EXPECT_CALL(mock_paced_sender_, EnqueuePackets);
@@ -724,12 +717,9 @@ TEST_F(RtpSenderTest, MidAndRidIncludedOnFirstRtxPacket) {
 // not include either MID or RRID even if the packet being retransmitted did
 // had a MID or RID.
 TEST_F(RtpSenderTest, MidAndRidNotIncludedOnRtxPacketsAfterAck) {
-  const char kMid[] = "mid";
-  const char kRid[] = "f";
-
   EnableRtx();
   EnableMidSending(kMid);
-  EnableRidSending(kRid);
+  EnableRidSending();
 
   // This first packet will include both MID and RID.
   auto first_built_packet = SendGenericPacket();
@@ -768,11 +758,9 @@ TEST_F(RtpSenderTest, MidAndRidNotIncludedOnRtxPacketsAfterAck) {
 
 TEST_F(RtpSenderTest, MidAndRidAlwaysIncludedOnRtxPacketsWhenConfigured) {
   SetUpRtpSender(false, /*always_send_mid_and_rid=*/true, nullptr);
-  const char kMid[] = "mid";
-  const char kRid[] = "f";
   EnableRtx();
   EnableMidSending(kMid);
-  EnableRidSending(kRid);
+  EnableRidSending();
 
   // Send two media packets: one before and one after the ack.
   EXPECT_CALL(
@@ -814,11 +802,8 @@ TEST_F(RtpSenderTest, MidAndRidAlwaysIncludedOnRtxPacketsWhenConfigured) {
 // Test that if the RtpState indicates an ACK has been received on that SSRC
 // then neither the MID nor RID header extensions will be sent.
 TEST_F(RtpSenderTest, MidAndRidNotIncludedOnSentPacketsAfterRtpStateRestored) {
-  const char kMid[] = "mid";
-  const char kRid[] = "f";
-
   EnableMidSending(kMid);
-  EnableRidSending(kRid);
+  EnableRidSending();
 
   RtpState state = rtp_sender_->GetRtpState();
   EXPECT_FALSE(state.ssrc_has_acked);
@@ -837,12 +822,9 @@ TEST_F(RtpSenderTest, MidAndRidNotIncludedOnSentPacketsAfterRtpStateRestored) {
 // RTX SSRC then neither the MID nor RRID header extensions will be sent on
 // RTX packets.
 TEST_F(RtpSenderTest, MidAndRridNotIncludedOnRtxPacketsAfterRtpStateRestored) {
-  const char kMid[] = "mid";
-  const char kRid[] = "f";
-
   EnableRtx();
   EnableMidSending(kMid);
-  EnableRidSending(kRid);
+  EnableRidSending();
 
   RtpState rtx_state = rtp_sender_->GetRtxRtpState();
   EXPECT_FALSE(rtx_state.ssrc_has_acked);
@@ -947,13 +929,12 @@ TEST_F(RtpSenderTest, CountMidOnlyUntilAcked) {
   EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 12u);
 
   rtp_sender_->RegisterRtpHeaderExtension(RtpMid::Uri(), kMidExtensionId);
-  rtp_sender_->RegisterRtpHeaderExtension(RtpStreamId::Uri(), kRidExtensionId);
 
   // Counted only if set.
   EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 12u);
   rtp_sender_->SetMid("foo");
   EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 36u);
-  rtp_sender_->SetRid("bar");
+  rtp_sender_->RegisterRtpHeaderExtension(RtpStreamId::Uri(), kRidExtensionId);
   EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 52u);
 
   // Ack received, mid/rid no longer sent.