diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc index 0ad64f212d..382ffcd090 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 @@ void PacketRouter::SendPacket(std::unique_ptr packet, bool assign_transport_sequence_number = packet->HasExtension(); if (assign_transport_sequence_number) { - packet->SetExtension((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 b91c309eec..434cdccafa 100644 --- a/modules/pacing/packet_router_unittest.cc +++ b/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()); - EXPECT_CALL( - rtp_1, - TrySendPacket(Pointee(Property( - &RtpPacketToSend::GetExtension, - transport_sequence_number)), - _)) - .WillOnce(Return(true)); + EXPECT_CALL(rtp_1, TrySendPacket) + .WillOnce([&](std::unique_ptr 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()); - EXPECT_CALL( - rtp_2, - TrySendPacket(Pointee(Property( - &RtpPacketToSend::GetExtension, - transport_sequence_number)), - _)) - .WillOnce(Return(true)); + EXPECT_CALL(rtp_2, TrySendPacket) + + .WillOnce([&](std::unique_ptr 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 @@ TEST_F(PacketRouterTest, DoesNotIncrementTransportSequenceNumberOnSendFailure) { // Return failure status code to make sure sequence number is not incremented. auto packet = BuildRtpPacket(kSsrc); EXPECT_TRUE(packet->ReserveExtension()); - EXPECT_CALL( - rtp, - TrySendPacket(Pointee(Property( - &RtpPacketToSend::GetExtension, - kStartTransportSequenceNumber)), - _)) - .WillOnce(Return(false)); + EXPECT_CALL(rtp, TrySendPacket) + + .WillOnce([&](std::unique_ptr 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 @@ TEST_F(PacketRouterTest, DoesNotIncrementTransportSequenceNumberOnSendFailure) { packet = BuildRtpPacket(kSsrc); EXPECT_TRUE(packet->ReserveExtension()); - EXPECT_CALL( - rtp, - TrySendPacket(Pointee(Property( - &RtpPacketToSend::GetExtension, - kStartTransportSequenceNumber)), - _)) - .WillOnce(Return(true)); + EXPECT_CALL(rtp, TrySendPacket) + .WillOnce([&](std::unique_ptr 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 64f9ee1ab1..515fdc2867 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 @@ class RtpPacketToSend : public RtpPacket { absl::optional 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 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 packet_type_; absl::optional original_packet_type_; + absl::optional transport_sequence_number_; bool allow_retransmission_ = false; absl::optional retransmitted_sequence_number_; rtc::scoped_refptr additional_data_; diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc index 7d7a30de2e..46b66103c0 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc @@ -209,6 +209,11 @@ void RtpSenderEgress::SendPacket(std::unique_ptr packet, if (packet->HasExtension()) { packet->SetExtension(AbsoluteSendTime::To24Bits(now)); } + if (packet->HasExtension() && + packet->transport_sequence_number()) { + packet->SetExtension( + *packet->transport_sequence_number() & 0xFFFF); + } if (packet->HasExtension()) { 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 b278ea2f06..f648cb4b07 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 @@ 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 packet = BuildRtpPacket(); + ASSERT_TRUE(packet->HasExtension()); + 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(), + kTransportSequenceNumber & 0xFFFF); +} + TEST_F(RtpSenderEgressTest, OnSendPacketUpdated) { std::unique_ptr sender = CreateRtpSenderEgress(); header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId,