diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index f1f7544b92..b057d6db0b 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -113,6 +113,7 @@ bool IsNonVolatile(RTPExtensionType type) { case kRtpExtensionTransportSequenceNumber: case kRtpExtensionTransportSequenceNumber02: case kRtpExtensionRtpStreamId: + case kRtpExtensionRepairedRtpStreamId: case kRtpExtensionMid: case kRtpExtensionGenericFrameDescriptor00: case kRtpExtensionGenericFrameDescriptor02: @@ -124,7 +125,6 @@ bool IsNonVolatile(RTPExtensionType type) { case kRtpExtensionVideoContentType: case kRtpExtensionVideoLayersAllocation: case kRtpExtensionVideoTiming: - case kRtpExtensionRepairedRtpStreamId: case kRtpExtensionColorSpace: case kRtpExtensionVideoFrameTrackingId: return false; @@ -747,20 +747,23 @@ void RTPSender::UpdateHeaderSizes() { rtp_header_length + RtpHeaderExtensionSize(kFecOrPaddingExtensionSizes, rtp_header_extension_map_); - // RtpStreamId and Mid are treated specially in that we check if they - // currently are being sent. RepairedRtpStreamId is ignored because it is sent - // instead of RtpStreamId on rtx packets and require the same size. + // RtpStreamId, Mid and RepairedRtpStreamId are treated specially in that + // we check if they currently are being sent. RepairedRtpStreamId can be + // sent instead of RtpStreamID on RTX packets and may share the same space. + // When the primary SSRC has already been acked but the RTX SSRC has not + // yet been acked, RepairedRtpStreamId needs to be taken into account + // separately. const bool send_mid_rid_on_rtx = - rtx_ssrc_.has_value() && !rtx_ssrc_has_acked_; - const bool send_mid_rid = - always_send_mid_and_rid_ || !ssrc_has_acked_ || send_mid_rid_on_rtx; + rtx_ssrc_.has_value() && + (always_send_mid_and_rid_ || !rtx_ssrc_has_acked_); + const bool send_mid_rid = always_send_mid_and_rid_ || !ssrc_has_acked_; std::vector non_volatile_extensions; for (auto& extension : audio_configured_ ? AudioExtensionSizes() : VideoExtensionSizes()) { if (IsNonVolatile(extension.type)) { switch (extension.type) { case RTPExtensionType::kRtpExtensionMid: - if (send_mid_rid && !mid_.empty()) { + if ((send_mid_rid || send_mid_rid_on_rtx) && !mid_.empty()) { non_volatile_extensions.push_back(extension); } break; @@ -769,6 +772,11 @@ void RTPSender::UpdateHeaderSizes() { non_volatile_extensions.push_back(extension); } break; + case RTPExtensionType::kRtpExtensionRepairedRtpStreamId: + if (send_mid_rid_on_rtx && !send_mid_rid && !rid_.empty()) { + non_volatile_extensions.push_back(extension); + } + break; default: non_volatile_extensions.push_back(extension); } diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index c9e98ff3c3..432c5e72d3 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -921,6 +921,39 @@ TEST_F(RtpSenderTest, CountMidOnlyUntilAcked) { EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 12u); } +TEST_F(RtpSenderTest, CountMidRidRridUntilAcked) { + RtpRtcpInterface::Configuration config = GetDefaultConfig(); + CreateSender(config); + + // Base RTP overhead is 12B and we use RTX which has an additional 2 bytes + // overhead. + EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 14u); + + rtp_sender_->RegisterRtpHeaderExtension(RtpMid::Uri(), kMidExtensionId); + + // Counted only if set. + EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 14u); + rtp_sender_->SetMid("foo"); + EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 38u); + + rtp_sender_->RegisterRtpHeaderExtension(RtpStreamId::Uri(), kRidExtensionId); + EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 54u); + + // mid/rrid may be shared with mid/rid when both are active. + rtp_sender_->RegisterRtpHeaderExtension(RepairedRtpStreamId::Uri(), + kRepairedRidExtensionId); + EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 54u); + + // Ack received, mid/rid no longer sent but we still need space for + // mid/rrid which can no longer be shared with mid/rid. + rtp_sender_->OnReceivedAckOnSsrc(0); + EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 54u); + + // Ack received for RTX, no need to send RRID anymore. + rtp_sender_->OnReceivedAckOnRtxSsrc(0); + EXPECT_EQ(rtp_sender_->ExpectedPerPacketOverhead(), 14u); +} + TEST_F(RtpSenderTest, DontCountVolatileExtensionsIntoOverhead) { RtpRtcpInterface::Configuration config = GetDefaultConfig(); config.rtx_send_ssrc = {};