Add property RtpPacketToSend::transport_sequence_number()
And move writing of the header extension from PacketRouter to
RtpSenderEgress::SendPacket.
Bug: webrtc:15368
Change-Id: Ieb18af4bc20115bf02d37e1f9a815a5c120975a9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343786
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Auto-Submit: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41949}
diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc
index 0ad64f2..382ffcd 100644
--- a/modules/pacing/packet_router.cc
+++ b/modules/pacing/packet_router.cc
@@ -27,7 +27,7 @@
namespace webrtc {
-PacketRouter::PacketRouter() : PacketRouter(0) {}
+PacketRouter::PacketRouter() : PacketRouter(1) {}
PacketRouter::PacketRouter(uint16_t start_transport_seq)
: last_send_module_(nullptr),
@@ -161,8 +161,7 @@
bool assign_transport_sequence_number =
packet->HasExtension<TransportSequenceNumber>();
if (assign_transport_sequence_number) {
- packet->SetExtension<TransportSequenceNumber>((transport_seq_ + 1) &
- 0xFFFF);
+ packet->set_transport_sequence_number(transport_seq_);
}
uint32_t ssrc = packet->Ssrc();
diff --git a/modules/pacing/packet_router_unittest.cc b/modules/pacing/packet_router_unittest.cc
index b91c309..434cdcc 100644
--- a/modules/pacing/packet_router_unittest.cc
+++ b/modules/pacing/packet_router_unittest.cc
@@ -390,31 +390,28 @@
packet_router_.AddSendRtpModule(&rtp_1, false);
packet_router_.AddSendRtpModule(&rtp_2, false);
- // Transport sequence numbers start at 1, for historical reasons.
- uint16_t transport_sequence_number = 1;
-
auto packet = BuildRtpPacket(kSsrc1);
EXPECT_TRUE(packet->ReserveExtension<TransportSequenceNumber>());
- EXPECT_CALL(
- rtp_1,
- TrySendPacket(Pointee(Property(
- &RtpPacketToSend::GetExtension<TransportSequenceNumber>,
- transport_sequence_number)),
- _))
- .WillOnce(Return(true));
+ EXPECT_CALL(rtp_1, TrySendPacket)
+ .WillOnce([&](std::unique_ptr<RtpPacketToSend> packet,
+ const PacedPacketInfo& pacing_info) {
+ // Transport sequence numbers start at 1 per default, for historical
+ // reasons.
+ EXPECT_EQ(packet->transport_sequence_number(), 1);
+ return true;
+ });
packet_router_.SendPacket(std::move(packet), PacedPacketInfo());
- ++transport_sequence_number;
packet = BuildRtpPacket(kSsrc2);
EXPECT_TRUE(packet->ReserveExtension<TransportSequenceNumber>());
- EXPECT_CALL(
- rtp_2,
- TrySendPacket(Pointee(Property(
- &RtpPacketToSend::GetExtension<TransportSequenceNumber>,
- transport_sequence_number)),
- _))
- .WillOnce(Return(true));
+ EXPECT_CALL(rtp_2, TrySendPacket)
+
+ .WillOnce([&](std::unique_ptr<RtpPacketToSend> packet,
+ const PacedPacketInfo& pacing_info) {
+ EXPECT_EQ(packet->transport_sequence_number(), 2);
+ return true;
+ });
packet_router_.SendPacket(std::move(packet), PacedPacketInfo());
packet_router_.OnBatchComplete();
@@ -435,13 +432,14 @@
// 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(Pointee(Property(
- &RtpPacketToSend::GetExtension<TransportSequenceNumber>,
- kStartTransportSequenceNumber)),
- _))
- .WillOnce(Return(false));
+ EXPECT_CALL(rtp, TrySendPacket)
+
+ .WillOnce([&](std::unique_ptr<RtpPacketToSend> packet,
+ const PacedPacketInfo& pacing_info) {
+ EXPECT_EQ(packet->transport_sequence_number(),
+ kStartTransportSequenceNumber);
+ return false;
+ });
packet_router_.SendPacket(std::move(packet), PacedPacketInfo());
// Send another packet, verify transport sequence number is still at the
@@ -449,13 +447,13 @@
packet = BuildRtpPacket(kSsrc);
EXPECT_TRUE(packet->ReserveExtension<TransportSequenceNumber>());
- EXPECT_CALL(
- rtp,
- TrySendPacket(Pointee(Property(
- &RtpPacketToSend::GetExtension<TransportSequenceNumber>,
- kStartTransportSequenceNumber)),
- _))
- .WillOnce(Return(true));
+ EXPECT_CALL(rtp, TrySendPacket)
+ .WillOnce([&](std::unique_ptr<RtpPacketToSend> packet,
+ const PacedPacketInfo& pacing_info) {
+ EXPECT_EQ(packet->transport_sequence_number(),
+ kStartTransportSequenceNumber);
+ return false;
+ });
packet_router_.SendPacket(std::move(packet), PacedPacketInfo());
packet_router_.OnBatchComplete();
diff --git a/modules/rtp_rtcp/source/rtp_packet_to_send.h b/modules/rtp_rtcp/source/rtp_packet_to_send.h
index 64f9ee1..515fdc2 100644
--- a/modules/rtp_rtcp/source/rtp_packet_to_send.h
+++ b/modules/rtp_rtcp/source/rtp_packet_to_send.h
@@ -136,11 +136,20 @@
absl::optional<TimeDelta> time_in_send_queue() const {
return time_in_send_queue_;
}
+ // A sequence number guaranteed to be monotically increasing by one for all
+ // packets where transport feedback is expected.
+ absl::optional<int64_t> transport_sequence_number() const {
+ return transport_sequence_number_;
+ }
+ void set_transport_sequence_number(int64_t transport_sequence_number) {
+ transport_sequence_number_ = transport_sequence_number;
+ }
private:
webrtc::Timestamp capture_time_ = webrtc::Timestamp::Zero();
absl::optional<RtpPacketMediaType> packet_type_;
absl::optional<OriginalType> original_packet_type_;
+ absl::optional<int64_t> transport_sequence_number_;
bool allow_retransmission_ = false;
absl::optional<uint16_t> retransmitted_sequence_number_;
rtc::scoped_refptr<rtc::RefCountedBase> additional_data_;
diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc
index 7d7a30d..46b6610 100644
--- a/modules/rtp_rtcp/source/rtp_sender_egress.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc
@@ -209,6 +209,11 @@
if (packet->HasExtension<AbsoluteSendTime>()) {
packet->SetExtension<AbsoluteSendTime>(AbsoluteSendTime::To24Bits(now));
}
+ if (packet->HasExtension<TransportSequenceNumber>() &&
+ packet->transport_sequence_number()) {
+ packet->SetExtension<TransportSequenceNumber>(
+ *packet->transport_sequence_number() & 0xFFFF);
+ }
if (packet->HasExtension<VideoTimingExtension>()) {
if (populate_network2_timestamp_) {
diff --git a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc
index b278ea2..f648cb4 100644
--- a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc
@@ -370,6 +370,23 @@
EXPECT_EQ(video_timing.pacer_exit_delta_ms, kPacerExitMs);
}
+TEST_F(RtpSenderEgressTest, WritesTransportSequenceNumberExtensionIfAllocated) {
+ RtpSenderEgress sender(DefaultConfig(), &packet_history_);
+ header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId,
+ TransportSequenceNumber::Uri());
+ std::unique_ptr<RtpPacketToSend> packet = BuildRtpPacket();
+ ASSERT_TRUE(packet->HasExtension<TransportSequenceNumber>());
+ const int64_t kTransportSequenceNumber = 0xFFFF000F;
+ packet->set_transport_sequence_number(kTransportSequenceNumber);
+
+ sender.SendPacket(std::move(packet), PacedPacketInfo());
+
+ ASSERT_TRUE(transport_.last_packet().has_value());
+ EXPECT_EQ(
+ transport_.last_packet()->packet.GetExtension<TransportSequenceNumber>(),
+ kTransportSequenceNumber & 0xFFFF);
+}
+
TEST_F(RtpSenderEgressTest, OnSendPacketUpdated) {
std::unique_ptr<RtpSenderEgress> sender = CreateRtpSenderEgress();
header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId,
