Attach Mid/Rid RTP header extension to pure padding packets

same as they attached to other packets.
Otherwise there is risk that ssrc will be acked after few initial pure padding packets are sent, before remote endpoint seen any mid or rid attached.

Bug: b/361257385
Change-Id: I695b379221debe2518ad33d13d65620877f0b2a7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/360660
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42851}
This commit is contained in:
Danil Chapovalov 2024-08-26 12:24:06 +02:00 committed by WebRTC LUCI CQ
parent ab009c27b4
commit c54c85fe8f
2 changed files with 60 additions and 9 deletions

View File

@ -435,6 +435,17 @@ std::vector<std::unique_ptr<RtpPacketToSend>> RTPSender::GeneratePadding(
break; break;
} }
padding_packet->SetSsrc(ssrc_); padding_packet->SetSsrc(ssrc_);
if (always_send_mid_and_rid_ || !ssrc_has_acked_) {
// These are no-ops if the corresponding header extension is not
// registered.
if (!mid_.empty()) {
padding_packet->SetExtension<RtpMid>(mid_);
}
if (!rid_.empty()) {
padding_packet->SetExtension<RtpStreamId>(rid_);
}
}
} else { } else {
// Without abs-send-time or transport sequence number a media packet // Without abs-send-time or transport sequence number a media packet
// must be sent before padding so that the timestamps used for // must be sent before padding so that the timestamps used for
@ -450,17 +461,20 @@ std::vector<std::unique_ptr<RtpPacketToSend>> RTPSender::GeneratePadding(
RTC_DCHECK(!rtx_payload_type_map_.empty()); RTC_DCHECK(!rtx_payload_type_map_.empty());
padding_packet->SetSsrc(*rtx_ssrc_); padding_packet->SetSsrc(*rtx_ssrc_);
padding_packet->SetPayloadType(rtx_payload_type_map_.begin()->second); padding_packet->SetPayloadType(rtx_payload_type_map_.begin()->second);
if (always_send_mid_and_rid_ || !rtx_ssrc_has_acked_) {
if (!mid_.empty()) {
padding_packet->SetExtension<RtpMid>(mid_);
}
if (!rid_.empty()) {
padding_packet->SetExtension<RepairedRtpStreamId>(rid_);
}
}
} }
if (rtp_header_extension_map_.IsRegistered(TransportSequenceNumber::kId)) {
padding_packet->ReserveExtension<TransportSequenceNumber>(); padding_packet->ReserveExtension<TransportSequenceNumber>();
}
if (rtp_header_extension_map_.IsRegistered(TransmissionOffset::kId)) {
padding_packet->ReserveExtension<TransmissionOffset>(); padding_packet->ReserveExtension<TransmissionOffset>();
}
if (rtp_header_extension_map_.IsRegistered(AbsoluteSendTime::kId)) {
padding_packet->ReserveExtension<AbsoluteSendTime>(); padding_packet->ReserveExtension<AbsoluteSendTime>();
}
padding_packet->SetPadding(padding_bytes_in_packet); padding_packet->SetPadding(padding_bytes_in_packet);
bytes_left -= std::min(bytes_left, padding_bytes_in_packet); bytes_left -= std::min(bytes_left, padding_bytes_in_packet);

View File

@ -1078,6 +1078,43 @@ TEST_F(RtpSenderTest, GeneratedPaddingHasBweExtensions) {
EXPECT_GT(payload_padding->payload_size(), 0u); EXPECT_GT(payload_padding->payload_size(), 0u);
} }
TEST_F(RtpSenderTest, GeneratedPaddingHasMidRidExtensions) {
EnableMidSending("mid");
EnableRidSending();
// Send a dummy video packet so it ends up in the packet history. Since we
// are not using RTX, it should never be used as padding.
packet_history_->SetStorePacketsStatus(
RtpPacketHistory::StorageMode::kStoreAndCull, 1);
std::unique_ptr<RtpPacketToSend> packet =
BuildRtpPacket(kPayload, true, 0, env_.clock().CurrentTime());
packet->set_allow_retransmission(true);
packet->SetPayloadSize(1234);
packet->set_packet_type(RtpPacketMediaType::kVideo);
sequencer_->Sequence(*packet);
packet_history_->PutRtpPacket(std::move(packet), env_.clock().CurrentTime());
std::vector<std::unique_ptr<RtpPacketToSend>> padding_packets =
GeneratePadding(/*target_size_bytes=*/1);
ASSERT_THAT(padding_packets, SizeIs(1));
EXPECT_TRUE(padding_packets[0]->HasExtension<RtpMid>());
EXPECT_TRUE(padding_packets[0]->HasExtension<RtpStreamId>());
}
TEST_F(RtpSenderTest, GeneratedPaddingOnRtxHasMidRidExtensions) {
EnableRtx();
EnableMidSending("mid");
EnableRidSending();
std::vector<std::unique_ptr<RtpPacketToSend>> padding_packets =
GeneratePadding(/*target_size_bytes=*/1);
ASSERT_THAT(padding_packets, SizeIs(1));
EXPECT_TRUE(padding_packets[0]->HasExtension<RtpMid>());
EXPECT_TRUE(padding_packets[0]->HasExtension<RepairedRtpStreamId>());
}
TEST_F(RtpSenderTest, GeneratePaddingResendsOldPacketsWithRtx) { TEST_F(RtpSenderTest, GeneratePaddingResendsOldPacketsWithRtx) {
// Min requested size in order to use RTX payload. // Min requested size in order to use RTX payload.
const size_t kMinPaddingSize = 50; const size_t kMinPaddingSize = 50;