zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Don't increment transport sequence number on send failures.

Browse Source 
Bug: webrtc:14130
Change-Id: Idee794445872f3db8ffae7c3e2cef5e72843ef25
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265640
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37190}
This commit is contained in:
Erik Språng
2022-06-11 12:18:47 +02:00
committed by WebRTC LUCI CQ
parent 72b5dbc44b
commit c62e1b8d10
2 changed files with 48 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
modules/pacing/packet_router.cc
View File

@ -143,8 +143,11 @@ void PacketRouter::SendPacket(std::unique_ptr<RtpPacketToSend> packet,
MutexLock lock(&modules_mutex_); MutexLock lock(&modules_mutex_);
// With the new pacer code path, transport sequence numbers are only set here, // With the new pacer code path, transport sequence numbers are only set here,
// on the pacer thread. Therefore we don't need atomics/synchronization. // on the pacer thread. Therefore we don't need atomics/synchronization.
if (packet->HasExtension<TransportSequenceNumber>()) { bool assign_transport_sequence_number =
packet->SetExtension<TransportSequenceNumber>((++transport_seq_) & 0xFFFF); packet->HasExtension<TransportSequenceNumber>();
if (assign_transport_sequence_number) {
packet->SetExtension<TransportSequenceNumber>((transport_seq_ + 1) &
0xFFFF);
} }
uint32_t ssrc = packet->Ssrc(); uint32_t ssrc = packet->Ssrc();
@ -163,6 +166,12 @@ void PacketRouter::SendPacket(std::unique_ptr<RtpPacketToSend> packet,
return; return;
} }
// Sending succeeded.
if (assign_transport_sequence_number) {
++transport_seq_;
}
if (rtp_module->SupportsRtxPayloadPadding()) { if (rtp_module->SupportsRtxPayloadPadding()) {
// This is now the last module to send media, and has the desired // This is now the last module to send media, and has the desired
// properties needed for payload based padding. Cache it for later use. // properties needed for payload based padding. Cache it for later use.

37
modules/pacing/packet_router_unittest.cc
View File

@ -399,6 +399,43 @@ TEST_F(PacketRouterTest, SendPacketAssignsTransportSequenceNumbers) {
packet_router_.RemoveSendRtpModule(&rtp_2); packet_router_.RemoveSendRtpModule(&rtp_2);
} }
TEST_F(PacketRouterTest, DoesNotIncrementTransportSequenceNumberOnSendFailure) {
NiceMock<MockRtpRtcpInterface> rtp;
constexpr uint32_t kSsrc = 1234;
ON_CALL(rtp, SSRC).WillByDefault(Return(kSsrc));
packet_router_.AddSendRtpModule(&rtp, false);
// Transport sequence numbers start at 1, for historical reasons.
const uint16_t kStartTransportSequenceNumber = 1;
// Build and send a packet - it should be assigned the start sequence number.
// Return failure status code to make sure sequence number is not incremented.
auto packet = BuildRtpPacket(kSsrc);
EXPECT_TRUE(packet->ReserveExtension<TransportSequenceNumber>());
EXPECT_CALL(
rtp, TrySendPacket(
Property(&RtpPacketToSend::GetExtension<TransportSequenceNumber>,
kStartTransportSequenceNumber),
_))
.WillOnce(Return(false));
packet_router_.SendPacket(std::move(packet), PacedPacketInfo());
// Send another packet, verify transport sequence number is still at the
// start state.
packet = BuildRtpPacket(kSsrc);
EXPECT_TRUE(packet->ReserveExtension<TransportSequenceNumber>());
EXPECT_CALL(
rtp, TrySendPacket(
Property(&RtpPacketToSend::GetExtension<TransportSequenceNumber>,
kStartTransportSequenceNumber),
_))
.WillOnce(Return(true));
packet_router_.SendPacket(std::move(packet), PacedPacketInfo());
packet_router_.RemoveSendRtpModule(&rtp);
}
#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) #if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
using PacketRouterDeathTest = PacketRouterTest; using PacketRouterDeathTest = PacketRouterTest;
TEST_F(PacketRouterDeathTest, DoubleRegistrationOfSendModuleDisallowed) { TEST_F(PacketRouterDeathTest, DoubleRegistrationOfSendModuleDisallowed) {