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}
This commit is contained in:
Danil Chapovalov 2023-08-14 14:19:47 +02:00 committed by WebRTC LUCI CQ
parent 6b51e728e6
commit 4c17e2dbce
4 changed files with 34 additions and 25 deletions

View File

@ -504,12 +504,25 @@ size_t RTPSender::ExpectedPerPacketOverhead() const {
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 @@ void RTPSender::SetMid(absl::string_view mid) {
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 @@ RtpState RTPSender::GetRtxRtpState() const {
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,

View File

@ -65,9 +65,9 @@ class RTPSender {
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 @@ class RTPSender {
// 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 @@ class RTPSender {
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_);

View File

@ -81,6 +81,7 @@ using ::testing::AtLeast;
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 @@ class RtpSenderTest : public ::testing::Test {
}
};
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 @@ TEST_F(RtpSenderTest, UpdatingCsrcsUpdatedOverhead) {
// 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);
}

View File

@ -526,10 +526,8 @@ bool RTPSenderVideo::SendVideo(int payload_type,
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 @@ bool RTPSenderVideo::SendEncodedImage(int payload_type,
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 {