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}

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 @@ void PacketRouter::SendPacket(std::unique_ptr<RtpPacketToSend> packet,
   bool assign_transport_sequence_number =
       packet->HasExtension<TransportSequenceNumber>();
   if (assign_transport_sequence_number) {
-    packet->SetExtension<TransportSequenceNumber>((transport_seq_ + 1) &
+    packet->set_transport_sequence_number(transport_seq_);
-                                                  0xFFFF);
   }
 
   uint32_t ssrc = packet->Ssrc();

modules/pacing/packet_router_unittest.cc

@@ -390,31 +390,28 @@ TEST_F(PacketRouterTest, SendPacketAssignsTransportSequenceNumbers) {
   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(
+  EXPECT_CALL(rtp_1, TrySendPacket)
-      rtp_1,
+      .WillOnce([&](std::unique_ptr<RtpPacketToSend> packet,
-      TrySendPacket(Pointee(Property(
+                    const PacedPacketInfo& pacing_info) {
-                        &RtpPacketToSend::GetExtension<TransportSequenceNumber>,
+        // Transport sequence numbers start at 1 per default, for historical
-                        transport_sequence_number)),
+        // reasons.
-                    _))
+        EXPECT_EQ(packet->transport_sequence_number(), 1);
-      .WillOnce(Return(true));
+        return true;
+      });
   packet_router_.SendPacket(std::move(packet), PacedPacketInfo());
 
-  ++transport_sequence_number;
   packet = BuildRtpPacket(kSsrc2);
   EXPECT_TRUE(packet->ReserveExtension<TransportSequenceNumber>());
 
-  EXPECT_CALL(
+  EXPECT_CALL(rtp_2, TrySendPacket)
-      rtp_2,
+
-      TrySendPacket(Pointee(Property(
+      .WillOnce([&](std::unique_ptr<RtpPacketToSend> packet,
-                        &RtpPacketToSend::GetExtension<TransportSequenceNumber>,
+                    const PacedPacketInfo& pacing_info) {
-                        transport_sequence_number)),
+        EXPECT_EQ(packet->transport_sequence_number(), 2);
-                    _))
+        return true;
-      .WillOnce(Return(true));
+      });
   packet_router_.SendPacket(std::move(packet), PacedPacketInfo());
 
   packet_router_.OnBatchComplete();
@@ -435,13 +432,14 @@ TEST_F(PacketRouterTest, DoesNotIncrementTransportSequenceNumberOnSendFailure) {
   // Return failure status code to make sure sequence number is not incremented.
   auto packet = BuildRtpPacket(kSsrc);
   EXPECT_TRUE(packet->ReserveExtension<TransportSequenceNumber>());
-  EXPECT_CALL(
+  EXPECT_CALL(rtp, TrySendPacket)
-      rtp,
+
-      TrySendPacket(Pointee(Property(
+      .WillOnce([&](std::unique_ptr<RtpPacketToSend> packet,
-                        &RtpPacketToSend::GetExtension<TransportSequenceNumber>,
+                    const PacedPacketInfo& pacing_info) {
-                        kStartTransportSequenceNumber)),
+        EXPECT_EQ(packet->transport_sequence_number(),
-                    _))
+                  kStartTransportSequenceNumber);
-      .WillOnce(Return(false));
+        return false;
+      });
   packet_router_.SendPacket(std::move(packet), PacedPacketInfo());
 
   // Send another packet, verify transport sequence number is still at the
@@ -449,13 +447,13 @@ TEST_F(PacketRouterTest, DoesNotIncrementTransportSequenceNumberOnSendFailure) {
   packet = BuildRtpPacket(kSsrc);
   EXPECT_TRUE(packet->ReserveExtension<TransportSequenceNumber>());
 
-  EXPECT_CALL(
+  EXPECT_CALL(rtp, TrySendPacket)
-      rtp,
+      .WillOnce([&](std::unique_ptr<RtpPacketToSend> packet,
-      TrySendPacket(Pointee(Property(
+                    const PacedPacketInfo& pacing_info) {
-                        &RtpPacketToSend::GetExtension<TransportSequenceNumber>,
+        EXPECT_EQ(packet->transport_sequence_number(),
-                        kStartTransportSequenceNumber)),
+                  kStartTransportSequenceNumber);
-                    _))
+        return false;
-      .WillOnce(Return(true));
+      });
   packet_router_.SendPacket(std::move(packet), PacedPacketInfo());
 
   packet_router_.OnBatchComplete();

modules/rtp_rtcp/source/rtp_packet_to_send.h

@@ -136,11 +136,20 @@ class RtpPacketToSend : public RtpPacket {
   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_;

modules/rtp_rtcp/source/rtp_sender_egress.cc

@@ -209,6 +209,11 @@ void RtpSenderEgress::SendPacket(std::unique_ptr<RtpPacketToSend> packet,
   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_) {

modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc

@@ -370,6 +370,23 @@ TEST_F(RtpSenderEgressTest, WritesNetwork2ToTimingExtension) {
   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,