Refactor csrcs managment in RtpSender
contributing sources are usually decided per packet, and thus having persistent member for csrcs makes them less natural to use.
Bug: None
Change-Id: I804d58ace574368f8cdd4356a15471110e530744
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291334
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40547}
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index 967d38a..a94a7c4 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -504,12 +504,25 @@
return max_media_packet_header_;
}
-std::unique_ptr<RtpPacketToSend> RTPSender::AllocatePacket() const {
+std::unique_ptr<RtpPacketToSend> RTPSender::AllocatePacket(
+ rtc::ArrayView<const uint32_t> csrcs) {
MutexLock lock(&send_mutex_);
+ // TODO(danilchap): Remove this fallback together with SetCsrcs.
+ // New code shouldn't set csrcs_, keeping it empty,
+ // Old code would pass default value for csrcs, which is empty.
+ RTC_DCHECK(csrcs.empty() || csrcs_.empty());
+ if (csrcs.empty()) {
+ csrcs = csrcs_;
+ }
+ RTC_DCHECK_LE(csrcs.size(), kRtpCsrcSize);
+ if (csrcs.size() > max_num_csrcs_) {
+ max_num_csrcs_ = csrcs.size();
+ UpdateHeaderSizes();
+ }
auto packet = std::make_unique<RtpPacketToSend>(&rtp_header_extension_map_,
max_packet_size_);
packet->SetSsrc(ssrc_);
- packet->SetCsrcs(csrcs_);
+ packet->SetCsrcs(csrcs);
// Reserve extensions, if registered, RtpSender set in SendToNetwork.
packet->ReserveExtension<AbsoluteSendTime>();
@@ -605,16 +618,10 @@
UpdateHeaderSizes();
}
-std::vector<uint32_t> RTPSender::Csrcs() const {
- MutexLock lock(&send_mutex_);
- return csrcs_;
-}
-
void RTPSender::SetCsrcs(const std::vector<uint32_t>& csrcs) {
RTC_DCHECK_LE(csrcs.size(), kRtpCsrcSize);
MutexLock lock(&send_mutex_);
csrcs_ = csrcs;
- UpdateHeaderSizes();
}
static void CopyHeaderAndExtensionsToRtxPacket(const RtpPacketToSend& packet,
@@ -768,7 +775,7 @@
void RTPSender::UpdateHeaderSizes() {
const size_t rtp_header_length =
- kRtpHeaderLength + sizeof(uint32_t) * csrcs_.size();
+ kRtpHeaderLength + sizeof(uint32_t) * max_num_csrcs_;
max_padding_fec_packet_header_ =
rtp_header_length + RtpHeaderExtensionSize(kFecOrPaddingExtensionSizes,
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index 633a69b..082d150 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -65,9 +65,9 @@
uint16_t SequenceNumber() const RTC_LOCKS_EXCLUDED(send_mutex_);
void SetSequenceNumber(uint16_t seq) RTC_LOCKS_EXCLUDED(send_mutex_);
- std::vector<uint32_t> Csrcs() const;
- void SetCsrcs(const std::vector<uint32_t>& csrcs)
- RTC_LOCKS_EXCLUDED(send_mutex_);
+ [[deprecated("Pass csrcs in the AllocatePacket")]] //
+ void
+ SetCsrcs(const std::vector<uint32_t>& csrcs) RTC_LOCKS_EXCLUDED(send_mutex_);
void SetMaxRtpPacketSize(size_t max_packet_size)
RTC_LOCKS_EXCLUDED(send_mutex_);
@@ -130,8 +130,10 @@
// Create empty packet, fills ssrc, csrcs and reserve place for header
// extensions RtpSender updates before sending.
- std::unique_ptr<RtpPacketToSend> AllocatePacket() const
+ std::unique_ptr<RtpPacketToSend> AllocatePacket(
+ rtc::ArrayView<const uint32_t> csrcs = {})
RTC_LOCKS_EXCLUDED(send_mutex_);
+
// Maximum header overhead per fec/padding packet.
size_t FecOrPaddingPacketMaxRtpHeaderLength() const
RTC_LOCKS_EXCLUDED(send_mutex_);
@@ -207,6 +209,8 @@
bool ssrc_has_acked_ RTC_GUARDED_BY(send_mutex_);
bool rtx_ssrc_has_acked_ RTC_GUARDED_BY(send_mutex_);
std::vector<uint32_t> csrcs_ RTC_GUARDED_BY(send_mutex_);
+ // Maximum number of csrcs this sender is used with.
+ size_t max_num_csrcs_ RTC_GUARDED_BY(send_mutex_) = 0;
int rtx_ RTC_GUARDED_BY(send_mutex_);
// Mapping rtx_payload_type_map_[associated] = rtx.
std::map<int8_t, int8_t> rtx_payload_type_map_ RTC_GUARDED_BY(send_mutex_);
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 6b4c756..65c49f7 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -81,6 +81,7 @@
using ::testing::Contains;
using ::testing::Each;
using ::testing::ElementsAre;
+using ::testing::ElementsAreArray;
using ::testing::Eq;
using ::testing::Field;
using ::testing::Gt;
@@ -269,17 +270,15 @@
}
};
-TEST_F(RtpSenderTest, AllocatePacketSetCsrc) {
+TEST_F(RtpSenderTest, AllocatePacketSetCsrcs) {
// Configure rtp_sender with csrc.
- std::vector<uint32_t> csrcs;
- csrcs.push_back(0x23456789);
- rtp_sender_->SetCsrcs(csrcs);
+ uint32_t csrcs[] = {0x23456789};
- auto packet = rtp_sender_->AllocatePacket();
+ auto packet = rtp_sender_->AllocatePacket(csrcs);
ASSERT_TRUE(packet);
EXPECT_EQ(rtp_sender_->SSRC(), packet->Ssrc());
- EXPECT_EQ(csrcs, packet->Csrcs());
+ EXPECT_THAT(packet->Csrcs(), ElementsAreArray(csrcs));
}
TEST_F(RtpSenderTest, AllocatePacketReserveExtensions) {
@@ -876,8 +875,9 @@
// Base RTP overhead is 12B.
EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 12u);
- // Adding two csrcs adds 2*4 bytes to the header.
- rtp_sender_->SetCsrcs({1, 2});
+ // Using packet with two csrcs adds 2*4 bytes to the header.
+ uint32_t csrcs[] = {1, 2};
+ rtp_sender_->AllocatePacket(csrcs);
EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 20u);
}
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc
index f4d30eb..23af5e2 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -526,10 +526,8 @@
packet_capacity -= rtp_sender_->RtxPacketOverhead();
}
- rtp_sender_->SetCsrcs(std::move(csrcs));
-
std::unique_ptr<RtpPacketToSend> single_packet =
- rtp_sender_->AllocatePacket();
+ rtp_sender_->AllocatePacket(csrcs);
RTC_DCHECK_LE(packet_capacity, single_packet->capacity());
single_packet->SetPayloadType(payload_type);
single_packet->SetTimestamp(rtp_timestamp);
@@ -765,7 +763,7 @@
return SendVideo(payload_type, codec_type, rtp_timestamp,
encoded_image.CaptureTime(), encoded_image,
encoded_image.size(), video_header,
- expected_retransmission_time, rtp_sender_->Csrcs());
+ expected_retransmission_time, /*csrcs=*/{});
}
DataRate RTPSenderVideo::PostEncodeOverhead() const {
