diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc index 25e08368c8..e8689e7fc6 100644 --- a/call/rtp_video_sender_unittest.cc +++ b/call/rtp_video_sender_unittest.cc @@ -911,7 +911,7 @@ TEST(RtpVideoSenderTest, OverheadIsSubtractedFromTargetBitrate) { constexpr uint32_t kTransportPacketOverheadBytes = 40; constexpr uint32_t kOverheadPerPacketBytes = kRtpHeaderSizeBytes + kTransportPacketOverheadBytes; - RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {}); + RtpVideoSenderTestFixture test({kSsrc1}, {}, kPayloadType, {}); test.router()->OnTransportOverheadChanged(kTransportPacketOverheadBytes); test.router()->SetActive(true); diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index ac20454a0f..13ec11195d 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -172,8 +172,6 @@ RTPSender::RTPSender(const RtpRtcpInterface::Configuration& config, max_packet_size_(IP_PACKET_SIZE - 28), // Default is IP-v4/UDP. last_payload_type_(-1), rtp_header_extension_map_(config.extmap_allow_mixed), - max_media_packet_header_(kRtpHeaderSize), - max_padding_fec_packet_header_(kRtpHeaderSize), // RTP variables sequence_number_forced_(false), always_send_mid_and_rid_(config.always_send_mid_and_rid), @@ -187,6 +185,7 @@ 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(); // Random start, 16 bits. Can't be 0. @@ -360,7 +359,11 @@ void RTPSender::OnReceivedAckOnSsrc(int64_t extended_highest_sequence_number) { void RTPSender::OnReceivedAckOnRtxSsrc( int64_t extended_highest_sequence_number) { MutexLock lock(&send_mutex_); + bool update_required = !rtx_ssrc_has_acked_; rtx_ssrc_has_acked_ = true; + if (update_required) { + UpdateHeaderSizes(); + } } void RTPSender::OnReceivedNack( @@ -874,10 +877,12 @@ void RTPSender::UpdateHeaderSizes() { rtp_header_extension_map_); // RtpStreamId and Mid are treated specially in that we check if they - // currently are being sent. RepairedRtpStreamId is still ignored since we - // assume RTX will not make up large enough bitrate to treat overhead - // differently. - const bool send_mid_rid = always_send_mid_and_rid_ || !ssrc_has_acked_; + // currently are being sent. RepairedRtpStreamId is ignored because it is sent + // instead of RtpStreamId on rtx packets and require the same size. + 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; std::vector non_volatile_extensions; for (auto& extension : audio_configured_ ? AudioExtensionSizes() : VideoExtensionSizes()) { @@ -901,5 +906,9 @@ void RTPSender::UpdateHeaderSizes() { max_media_packet_header_ = rtp_header_length + RtpHeaderExtensionSize(non_volatile_extensions, rtp_header_extension_map_); + // Reserve extra bytes if packet might be resent in an rtx packet. + if (rtx_ssrc_.has_value()) { + max_media_packet_header_ += kRtxHeaderSize; + } } } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 807d63dab7..69bdbb491e 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -34,6 +34,7 @@ #include "modules/rtp_rtcp/source/rtp_utility.h" #include "modules/rtp_rtcp/source/video_fec_generator.h" #include "rtc_base/arraysize.h" +#include "rtc_base/logging.h" #include "rtc_base/rate_limiter.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/task_utils/to_queued_task.h" @@ -415,7 +416,12 @@ class RtpSenderTest : public ::testing::TestWithParam { std::unique_ptr SendGenericPacket() { const int64_t kCaptureTimeMs = clock_->TimeInMilliseconds(); - return SendPacket(kCaptureTimeMs, sizeof(kPayloadData)); + // Use maximum allowed size to catch corner cases when packet is dropped + // because of lack of capacity for the media packet, or for an rtx packet + // containing the media packet. + return SendPacket(kCaptureTimeMs, + /*payload_length=*/rtp_sender()->MaxRtpPacketSize() - + rtp_sender()->ExpectedPerPacketOverhead()); } size_t GenerateAndSendPadding(size_t target_size_bytes) { @@ -596,6 +602,7 @@ TEST_P(RtpSenderTestWithoutPacer, AssignSequenceNumberSetPaddingTimestamps) { TEST_P(RtpSenderTestWithoutPacer, TransportFeedbackObserverGetsCorrectByteCount) { constexpr size_t kRtpOverheadBytesPerPacket = 12 + 8; + constexpr size_t kPayloadSize = 1400; RtpRtcpInterface::Configuration config; config.clock = clock_; @@ -612,10 +619,9 @@ TEST_P(RtpSenderTestWithoutPacer, kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); - const size_t expected_bytes = - GetParam().with_overhead - ? sizeof(kPayloadData) + kRtpOverheadBytesPerPacket - : sizeof(kPayloadData); + const size_t expected_bytes = GetParam().with_overhead + ? kPayloadSize + kRtpOverheadBytesPerPacket + : kPayloadSize; EXPECT_CALL(feedback_observer_, OnAddPacket(AllOf( @@ -629,7 +635,7 @@ TEST_P(RtpSenderTestWithoutPacer, .Times(1); EXPECT_EQ(rtp_sender()->ExpectedPerPacketOverhead(), kRtpOverheadBytesPerPacket); - SendGenericPacket(); + SendPacket(clock_->TimeInMilliseconds(), kPayloadSize); } TEST_P(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) { @@ -1968,12 +1974,12 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) { } TEST_P(RtpSenderTestWithoutPacer, BytesReportedCorrectly) { - // XXX const char* kPayloadName = "GENERIC"; const uint8_t kPayloadType = 127; + const size_t kPayloadSize = 1400; rtp_sender()->SetRtxPayloadType(kPayloadType - 1, kPayloadType); rtp_sender()->SetRtxStatus(kRtxRetransmitted | kRtxRedundantPayloads); - SendGenericPacket(); + SendPacket(clock_->TimeInMilliseconds(), kPayloadSize); // Will send 2 full-size padding packets. GenerateAndSendPadding(1); GenerateAndSendPadding(1); @@ -1984,7 +1990,7 @@ TEST_P(RtpSenderTestWithoutPacer, BytesReportedCorrectly) { // Payload EXPECT_GT(rtp_stats.first_packet_time_ms, -1); - EXPECT_EQ(rtp_stats.transmitted.payload_bytes, sizeof(kPayloadData)); + EXPECT_EQ(rtp_stats.transmitted.payload_bytes, kPayloadSize); EXPECT_EQ(rtp_stats.transmitted.header_bytes, 12u); EXPECT_EQ(rtp_stats.transmitted.padding_bytes, 0u); EXPECT_EQ(rtx_stats.transmitted.payload_bytes, 0u);