From cb838e2c4e92cf069ba55d4067214bf7f0fc6d59 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Fri, 5 May 2023 14:41:30 +0200 Subject: [PATCH] Move packets into RtpRtcpInterface and RtpSenderEgress. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL prepares for send packet batching support in later CLs. Bug: chromium:1439830 Change-Id: I0bbecfa895aa6d4317ef8049b3789272a440d032 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304282 Auto-Submit: Markus Handell Reviewed-by: Erik Språng Commit-Queue: Markus Handell Cr-Commit-Position: refs/heads/main@{#40009} --- modules/pacing/packet_router.cc | 2 +- modules/pacing/packet_router_unittest.cc | 57 ++++--- modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 3 +- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 4 +- modules/rtp_rtcp/source/rtp_rtcp_impl.h | 2 +- modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 4 +- modules/rtp_rtcp/source/rtp_rtcp_impl2.h | 2 +- .../source/rtp_rtcp_impl2_unittest.cc | 18 ++- .../rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 12 +- modules/rtp_rtcp/source/rtp_rtcp_interface.h | 2 +- modules/rtp_rtcp/source/rtp_sender_egress.cc | 4 +- modules/rtp_rtcp/source/rtp_sender_egress.h | 4 +- .../source/rtp_sender_egress_unittest.cc | 147 ++++++++++-------- 13 files changed, 140 insertions(+), 121 deletions(-) diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc index 85490ef82d..a25f69225a 100644 --- a/modules/pacing/packet_router.cc +++ b/modules/pacing/packet_router.cc @@ -162,7 +162,7 @@ void PacketRouter::SendPacket(std::unique_ptr packet, } RtpRtcpInterface* rtp_module = it->second; - if (!rtp_module->TrySendPacket(packet.get(), cluster_info)) { + if (!rtp_module->TrySendPacket(std::move(packet), cluster_info)) { RTC_LOG(LS_WARNING) << "Failed to send packet, rejected by RTP module."; return; } diff --git a/modules/pacing/packet_router_unittest.cc b/modules/pacing/packet_router_unittest.cc index 76d14c9dce..2528129d6e 100644 --- a/modules/pacing/packet_router_unittest.cc +++ b/modules/pacing/packet_router_unittest.cc @@ -41,6 +41,7 @@ using ::testing::Field; using ::testing::Gt; using ::testing::Le; using ::testing::NiceMock; +using ::testing::Pointee; using ::testing::Property; using ::testing::Return; using ::testing::SaveArg; @@ -194,10 +195,8 @@ TEST_F(PacketRouterTest, PadsOnLastActiveMediaStream) { EXPECT_CALL(rtp_1, SupportsPadding).WillRepeatedly(Return(true)); EXPECT_CALL(rtp_1, SupportsRtxPayloadPadding).WillRepeatedly(Return(true)); EXPECT_CALL(rtp_1, TrySendPacket).WillRepeatedly(Return(false)); - EXPECT_CALL( - rtp_1, - TrySendPacket( - ::testing::Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc1)), _)) + EXPECT_CALL(rtp_1, TrySendPacket( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc1)), _)) .WillRepeatedly(Return(true)); NiceMock rtp_2; @@ -205,10 +204,8 @@ TEST_F(PacketRouterTest, PadsOnLastActiveMediaStream) { EXPECT_CALL(rtp_2, SupportsPadding).WillRepeatedly(Return(true)); EXPECT_CALL(rtp_2, SupportsRtxPayloadPadding).WillRepeatedly(Return(true)); EXPECT_CALL(rtp_2, TrySendPacket).WillRepeatedly(Return(false)); - EXPECT_CALL( - rtp_2, - TrySendPacket( - ::testing::Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc2)), _)) + EXPECT_CALL(rtp_2, TrySendPacket( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc2)), _)) .WillRepeatedly(Return(true)); // Third module is sending media, but does not support rtx. @@ -217,10 +214,8 @@ TEST_F(PacketRouterTest, PadsOnLastActiveMediaStream) { EXPECT_CALL(rtp_3, SupportsPadding).WillRepeatedly(Return(true)); EXPECT_CALL(rtp_3, SupportsRtxPayloadPadding).WillRepeatedly(Return(false)); EXPECT_CALL(rtp_3, TrySendPacket).WillRepeatedly(Return(false)); - EXPECT_CALL( - rtp_3, - TrySendPacket( - ::testing::Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc3)), _)) + EXPECT_CALL(rtp_3, TrySendPacket( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc3)), _)) .WillRepeatedly(Return(true)); packet_router_.AddSendRtpModule(&rtp_1, false); @@ -346,8 +341,8 @@ TEST_F(PacketRouterTest, SendPacketWithoutTransportSequenceNumbers) { EXPECT_CALL( rtp_1, TrySendPacket( - Property(&RtpPacketToSend::HasExtension, - false), + Pointee(Property( + &RtpPacketToSend::HasExtension, false)), _)) .WillOnce(Return(true)); packet_router_.SendPacket(std::move(packet), PacedPacketInfo()); @@ -375,10 +370,10 @@ TEST_F(PacketRouterTest, SendPacketAssignsTransportSequenceNumbers) { EXPECT_TRUE(packet->ReserveExtension()); EXPECT_CALL( rtp_1, - TrySendPacket( - Property(&RtpPacketToSend::GetExtension, - transport_sequence_number), - _)) + TrySendPacket(Pointee(Property( + &RtpPacketToSend::GetExtension, + transport_sequence_number)), + _)) .WillOnce(Return(true)); packet_router_.SendPacket(std::move(packet), PacedPacketInfo()); @@ -388,10 +383,10 @@ TEST_F(PacketRouterTest, SendPacketAssignsTransportSequenceNumbers) { EXPECT_CALL( rtp_2, - TrySendPacket( - Property(&RtpPacketToSend::GetExtension, - transport_sequence_number), - _)) + TrySendPacket(Pointee(Property( + &RtpPacketToSend::GetExtension, + transport_sequence_number)), + _)) .WillOnce(Return(true)); packet_router_.SendPacket(std::move(packet), PacedPacketInfo()); @@ -413,10 +408,11 @@ TEST_F(PacketRouterTest, DoesNotIncrementTransportSequenceNumberOnSendFailure) { auto packet = BuildRtpPacket(kSsrc); EXPECT_TRUE(packet->ReserveExtension()); EXPECT_CALL( - rtp, TrySendPacket( - Property(&RtpPacketToSend::GetExtension, - kStartTransportSequenceNumber), - _)) + rtp, + TrySendPacket(Pointee(Property( + &RtpPacketToSend::GetExtension, + kStartTransportSequenceNumber)), + _)) .WillOnce(Return(false)); packet_router_.SendPacket(std::move(packet), PacedPacketInfo()); @@ -426,10 +422,11 @@ TEST_F(PacketRouterTest, DoesNotIncrementTransportSequenceNumberOnSendFailure) { EXPECT_TRUE(packet->ReserveExtension()); EXPECT_CALL( - rtp, TrySendPacket( - Property(&RtpPacketToSend::GetExtension, - kStartTransportSequenceNumber), - _)) + rtp, + TrySendPacket(Pointee(Property( + &RtpPacketToSend::GetExtension, + kStartTransportSequenceNumber)), + _)) .WillOnce(Return(true)); packet_router_.SendPacket(std::move(packet), PacedPacketInfo()); diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 029fc1bea3..52c3fe148c 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -85,7 +85,8 @@ class MockRtpRtcpInterface : public RtpRtcpInterface { (override)); MOCK_METHOD(bool, TrySendPacket, - (RtpPacketToSend * packet, const PacedPacketInfo& pacing_info), + (std::unique_ptr packet, + const PacedPacketInfo& pacing_info), (override)); MOCK_METHOD(void, SetFecProtectionParams, diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 75fee87deb..d87763ac41 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -350,7 +350,7 @@ bool ModuleRtpRtcpImpl::OnSendingRtpFrame(uint32_t timestamp, return true; } -bool ModuleRtpRtcpImpl::TrySendPacket(RtpPacketToSend* packet, +bool ModuleRtpRtcpImpl::TrySendPacket(std::unique_ptr packet, const PacedPacketInfo& pacing_info) { RTC_DCHECK(rtp_sender_); // TODO(sprang): Consider if we can remove this check. @@ -372,7 +372,7 @@ bool ModuleRtpRtcpImpl::TrySendPacket(RtpPacketToSend* packet, rtp_sender_->sequencer_.Sequence(*packet); } } - rtp_sender_->packet_sender.SendPacket(packet, pacing_info); + rtp_sender_->packet_sender.SendPacket(packet.get(), pacing_info); return true; } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index c8e90c25b0..f9d7c7234c 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -132,7 +132,7 @@ class ABSL_DEPRECATED("") ModuleRtpRtcpImpl int payload_type, bool force_sender_report) override; - bool TrySendPacket(RtpPacketToSend* packet, + bool TrySendPacket(std::unique_ptr packet, const PacedPacketInfo& pacing_info) override; void SetFecProtectionParams(const FecProtectionParams& delta_params, diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index d33ef86169..79ae08d2ea 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -320,7 +320,7 @@ bool ModuleRtpRtcpImpl2::OnSendingRtpFrame(uint32_t timestamp, return true; } -bool ModuleRtpRtcpImpl2::TrySendPacket(RtpPacketToSend* packet, +bool ModuleRtpRtcpImpl2::TrySendPacket(std::unique_ptr packet, const PacedPacketInfo& pacing_info) { RTC_DCHECK(rtp_sender_); RTC_DCHECK_RUN_ON(&rtp_sender_->sequencing_checker); @@ -340,7 +340,7 @@ bool ModuleRtpRtcpImpl2::TrySendPacket(RtpPacketToSend* packet, rtp_sender_->sequencer.Sequence(*packet); } - rtp_sender_->packet_sender.SendPacket(packet, pacing_info); + rtp_sender_->packet_sender.SendPacket(std::move(packet), pacing_info); return true; } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h index e6da54929b..214ef0406c 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h @@ -144,7 +144,7 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, int payload_type, bool force_sender_report) override; - bool TrySendPacket(RtpPacketToSend* packet, + bool TrySendPacket(std::unique_ptr packet, const PacedPacketInfo& pacing_info) override; void SetFecProtectionParams(const FecProtectionParams& delta_params, diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc index 2b81aa34b7..009bc3cfcd 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc @@ -728,7 +728,8 @@ TEST_F(RtpRtcpImpl2Test, StoresPacketInfoForSentPackets) { packet.SetTimestamp(1); packet.set_first_packet_of_frame(true); packet.SetMarker(true); - sender_.impl_->TrySendPacket(&packet, pacing_info); + sender_.impl_->TrySendPacket(std::make_unique(packet), + pacing_info); AdvanceTime(TimeDelta::Millis(1)); std::vector seqno_info = @@ -743,13 +744,16 @@ TEST_F(RtpRtcpImpl2Test, StoresPacketInfoForSentPackets) { packet.SetTimestamp(2); packet.set_first_packet_of_frame(true); packet.SetMarker(false); - sender_.impl_->TrySendPacket(&packet, pacing_info); + sender_.impl_->TrySendPacket(std::make_unique(packet), + pacing_info); packet.set_first_packet_of_frame(false); - sender_.impl_->TrySendPacket(&packet, pacing_info); + sender_.impl_->TrySendPacket(std::make_unique(packet), + pacing_info); packet.SetMarker(true); - sender_.impl_->TrySendPacket(&packet, pacing_info); + sender_.impl_->TrySendPacket(std::make_unique(packet), + pacing_info); AdvanceTime(TimeDelta::Millis(1)); @@ -907,7 +911,7 @@ TEST_F(RtpRtcpImpl2Test, PaddingNotAllowedInMiddleOfFrame) { packet->set_first_packet_of_frame(true); packet->SetMarker(false); // Marker false - not last packet of frame. - EXPECT_TRUE(sender_.impl_->TrySendPacket(packet.get(), pacing_info)); + EXPECT_TRUE(sender_.impl_->TrySendPacket(std::move(packet), pacing_info)); // Padding not allowed in middle of frame. EXPECT_THAT(sender_.impl_->GeneratePadding(kPaddingSize), SizeIs(0u)); @@ -917,7 +921,7 @@ TEST_F(RtpRtcpImpl2Test, PaddingNotAllowedInMiddleOfFrame) { packet->set_first_packet_of_frame(true); packet->SetMarker(true); - EXPECT_TRUE(sender_.impl_->TrySendPacket(packet.get(), pacing_info)); + EXPECT_TRUE(sender_.impl_->TrySendPacket(std::move(packet), pacing_info)); // Padding is OK again. EXPECT_THAT(sender_.impl_->GeneratePadding(kPaddingSize), SizeIs(Gt(0u))); @@ -936,7 +940,7 @@ TEST_F(RtpRtcpImpl2Test, PaddingTimestampMatchesMedia) { auto padding = sender_.impl_->GeneratePadding(kPaddingSize); ASSERT_FALSE(padding.empty()); for (auto& packet : padding) { - sender_.impl_->TrySendPacket(packet.get(), PacedPacketInfo()); + sender_.impl_->TrySendPacket(std::move(packet), PacedPacketInfo()); } // Verify we sent a new packet, but with the same timestamp. diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 2e0ce76388..057935e221 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -563,7 +563,8 @@ TEST_F(RtpRtcpImplTest, StoresPacketInfoForSentPackets) { packet.SetTimestamp(1); packet.set_first_packet_of_frame(true); packet.SetMarker(true); - sender_.impl_->TrySendPacket(&packet, pacing_info); + sender_.impl_->TrySendPacket(std::make_unique(packet), + pacing_info); std::vector seqno_info = sender_.impl_->GetSentRtpPacketInfos(std::vector{1}); @@ -577,13 +578,16 @@ TEST_F(RtpRtcpImplTest, StoresPacketInfoForSentPackets) { packet.SetTimestamp(2); packet.set_first_packet_of_frame(true); packet.SetMarker(false); - sender_.impl_->TrySendPacket(&packet, pacing_info); + sender_.impl_->TrySendPacket(std::make_unique(packet), + pacing_info); packet.set_first_packet_of_frame(false); - sender_.impl_->TrySendPacket(&packet, pacing_info); + sender_.impl_->TrySendPacket(std::make_unique(packet), + pacing_info); packet.SetMarker(true); - sender_.impl_->TrySendPacket(&packet, pacing_info); + sender_.impl_->TrySendPacket(std::make_unique(packet), + pacing_info); seqno_info = sender_.impl_->GetSentRtpPacketInfos(std::vector{2, 3, 4}); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index 66a29c9ddd..92f1a5bbe6 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h @@ -314,7 +314,7 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { // Try to send the provided packet. Returns true iff packet matches any of // the SSRCs for this module (media/rtx/fec etc) and was forwarded to the // transport. - virtual bool TrySendPacket(RtpPacketToSend* packet, + virtual bool TrySendPacket(std::unique_ptr packet, const PacedPacketInfo& pacing_info) = 0; // Update the FEC protection parameters to use for delta- and key-frames. diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc index 88f5d28c78..d6052f8db3 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc @@ -41,7 +41,7 @@ void RtpSenderEgress::NonPacedPacketSender::EnqueuePackets( std::vector> packets) { for (auto& packet : packets) { PrepareForSend(packet.get()); - sender_->SendPacket(packet.get(), PacedPacketInfo()); + sender_->SendPacket(std::move(packet), PacedPacketInfo()); } auto fec_packets = sender_->FetchFecPackets(); if (!fec_packets.empty()) { @@ -113,7 +113,7 @@ RtpSenderEgress::~RtpSenderEgress() { update_task_.Stop(); } -void RtpSenderEgress::SendPacket(RtpPacketToSend* packet, +void RtpSenderEgress::SendPacket(std::unique_ptr packet, const PacedPacketInfo& pacing_info) { RTC_DCHECK_RUN_ON(&pacer_checker_); RTC_DCHECK(packet); diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.h b/modules/rtp_rtcp/source/rtp_sender_egress.h index f76c42d4d0..4b7b50233b 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.h +++ b/modules/rtp_rtcp/source/rtp_sender_egress.h @@ -65,8 +65,8 @@ class RtpSenderEgress { RtpPacketHistory* packet_history); ~RtpSenderEgress(); - void SendPacket(RtpPacketToSend* packet, const PacedPacketInfo& pacing_info) - RTC_LOCKS_EXCLUDED(lock_); + void SendPacket(std::unique_ptr packet, + const PacedPacketInfo& pacing_info) RTC_LOCKS_EXCLUDED(lock_); uint32_t Ssrc() const { return ssrc_; } absl::optional RtxSsrc() const { return rtx_ssrc_; } absl::optional FlexFecSsrc() const { return flexfec_ssrc_; } diff --git a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc index cc1c8feb8d..504a467c9b 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc @@ -206,22 +206,22 @@ TEST_F(RtpSenderEgressTest, TransportFeedbackObserverGetsCorrectByteCount) { packet->AllocatePayload(kPayloadSize); std::unique_ptr sender = CreateRtpSenderEgress(); - sender->SendPacket(packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(packet), PacedPacketInfo()); } TEST_F(RtpSenderEgressTest, PacketOptionsIsRetransmitSetByPacketType) { std::unique_ptr sender = CreateRtpSenderEgress(); std::unique_ptr media_packet = BuildRtpPacket(); + auto sequence_number = media_packet->SequenceNumber(); media_packet->set_packet_type(RtpPacketMediaType::kVideo); - sender->SendPacket(media_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(media_packet), PacedPacketInfo()); EXPECT_FALSE(transport_.last_packet()->options.is_retransmit); std::unique_ptr retransmission = BuildRtpPacket(); retransmission->set_packet_type(RtpPacketMediaType::kRetransmission); - retransmission->set_retransmitted_sequence_number( - media_packet->SequenceNumber()); - sender->SendPacket(retransmission.get(), PacedPacketInfo()); + retransmission->set_retransmitted_sequence_number(sequence_number); + sender->SendPacket(std::move(retransmission), PacedPacketInfo()); EXPECT_TRUE(transport_.last_packet()->options.is_retransmit); } @@ -229,7 +229,7 @@ TEST_F(RtpSenderEgressTest, DoesnSetIncludedInAllocationByDefault) { std::unique_ptr sender = CreateRtpSenderEgress(); std::unique_ptr packet = BuildRtpPacket(); - sender->SendPacket(packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(packet), PacedPacketInfo()); EXPECT_FALSE(transport_.last_packet()->options.included_in_feedback); EXPECT_FALSE(transport_.last_packet()->options.included_in_allocation); } @@ -241,7 +241,7 @@ TEST_F(RtpSenderEgressTest, header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, TransportSequenceNumber::Uri()); std::unique_ptr packet = BuildRtpPacket(); - sender->SendPacket(packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(packet), PacedPacketInfo()); EXPECT_TRUE(transport_.last_packet()->options.included_in_feedback); } @@ -253,7 +253,7 @@ TEST_F( header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, TransportSequenceNumber::Uri()); std::unique_ptr packet = BuildRtpPacket(); - sender->SendPacket(packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(packet), PacedPacketInfo()); EXPECT_TRUE(transport_.last_packet()->options.included_in_allocation); } @@ -263,7 +263,7 @@ TEST_F(RtpSenderEgressTest, sender->ForceIncludeSendPacketsInAllocation(true); std::unique_ptr packet = BuildRtpPacket(); - sender->SendPacket(packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(packet), PacedPacketInfo()); EXPECT_FALSE(transport_.last_packet()->options.included_in_feedback); EXPECT_TRUE(transport_.last_packet()->options.included_in_allocation); } @@ -279,7 +279,7 @@ TEST_F(RtpSenderEgressTest, OnSendSideDelayUpdated) { EXPECT_CALL(send_side_delay_observer, SendSideDelayUpdated(10, 10, kSsrc)); int64_t capture_time_ms = clock_->TimeInMilliseconds(); time_controller_.AdvanceTime(TimeDelta::Millis(10)); - sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms).get(), + sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms), PacedPacketInfo()); // Send another packet with 20 ms delay. The average, max and total should be @@ -287,7 +287,7 @@ TEST_F(RtpSenderEgressTest, OnSendSideDelayUpdated) { EXPECT_CALL(send_side_delay_observer, SendSideDelayUpdated(15, 20, kSsrc)); capture_time_ms = clock_->TimeInMilliseconds(); time_controller_.AdvanceTime(TimeDelta::Millis(20)); - sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms).get(), + sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms), PacedPacketInfo()); // Send another packet at the same time, which replaces the last packet. @@ -296,7 +296,7 @@ TEST_F(RtpSenderEgressTest, OnSendSideDelayUpdated) { // TODO(terelius): Is is not clear that this is the right behavior. EXPECT_CALL(send_side_delay_observer, SendSideDelayUpdated(5, 10, kSsrc)); capture_time_ms = clock_->TimeInMilliseconds(); - sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms).get(), + sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms), PacedPacketInfo()); // Send a packet 1 second later. The earlier packets should have timed @@ -306,7 +306,7 @@ TEST_F(RtpSenderEgressTest, OnSendSideDelayUpdated) { EXPECT_CALL(send_side_delay_observer, SendSideDelayUpdated(1, 1, kSsrc)); capture_time_ms = clock_->TimeInMilliseconds(); time_controller_.AdvanceTime(TimeDelta::Millis(1)); - sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms).get(), + sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms), PacedPacketInfo()); } @@ -320,7 +320,7 @@ TEST_F(RtpSenderEgressTest, WritesPacerExitToTimingExtension) { const int kStoredTimeInMs = 100; time_controller_.AdvanceTime(TimeDelta::Millis(kStoredTimeInMs)); - sender->SendPacket(packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(packet), PacedPacketInfo()); ASSERT_TRUE(transport_.last_packet().has_value()); VideoSendTiming video_timing; @@ -345,7 +345,7 @@ TEST_F(RtpSenderEgressTest, WritesNetwork2ToTimingExtension) { const int kStoredTimeInMs = 100; time_controller_.AdvanceTime(TimeDelta::Millis(kStoredTimeInMs)); - sender->SendPacket(packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(packet), PacedPacketInfo()); ASSERT_TRUE(transport_.last_packet().has_value()); VideoSendTiming video_timing; @@ -367,7 +367,7 @@ TEST_F(RtpSenderEgressTest, OnSendPacketUpdated) { clock_->TimeInMilliseconds(), kSsrc)); std::unique_ptr packet = BuildRtpPacket(); packet->SetExtension(kTransportSequenceNumber); - sender->SendPacket(packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(packet), PacedPacketInfo()); } TEST_F(RtpSenderEgressTest, OnSendPacketNotUpdatedForRetransmits) { @@ -381,7 +381,7 @@ TEST_F(RtpSenderEgressTest, OnSendPacketNotUpdatedForRetransmits) { packet->SetExtension(kTransportSequenceNumber); packet->set_packet_type(RtpPacketMediaType::kRetransmission); packet->set_retransmitted_sequence_number(packet->SequenceNumber()); - sender->SendPacket(packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(packet), PacedPacketInfo()); } TEST_F(RtpSenderEgressTest, ReportsFecRate) { @@ -395,13 +395,14 @@ TEST_F(RtpSenderEgressTest, ReportsFecRate) { std::unique_ptr media_packet = BuildRtpPacket(); media_packet->set_packet_type(RtpPacketMediaType::kVideo); media_packet->SetPayloadSize(500); - sender->SendPacket(media_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(media_packet), PacedPacketInfo()); std::unique_ptr fec_packet = BuildRtpPacket(); fec_packet->set_packet_type(RtpPacketMediaType::kForwardErrorCorrection); fec_packet->SetPayloadSize(123); - sender->SendPacket(fec_packet.get(), PacedPacketInfo()); - total_fec_data_sent += DataSize::Bytes(fec_packet->size()); + auto fec_packet_size = fec_packet->size(); + sender->SendPacket(std::move(fec_packet), PacedPacketInfo()); + total_fec_data_sent += DataSize::Bytes(fec_packet_size); time_controller_.AdvanceTime(kTimeBetweenPackets); } @@ -454,7 +455,7 @@ TEST_F(RtpSenderEgressTest, BitrateCallbacks) { EXPECT_NEAR(retransmission_bitrate_bps, expected_bitrate_bps, 500); }); - sender->SendPacket(packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(packet), PacedPacketInfo()); time_controller_.AdvanceTime(kPacketInterval); } } @@ -466,8 +467,9 @@ TEST_F(RtpSenderEgressTest, DoesNotPutNotRetransmittablePacketsInHistory) { std::unique_ptr packet = BuildRtpPacket(); packet->set_allow_retransmission(false); - sender->SendPacket(packet.get(), PacedPacketInfo()); - EXPECT_FALSE(packet_history_.GetPacketState(packet->SequenceNumber())); + auto packet_sequence_number = packet->SequenceNumber(); + sender->SendPacket(std::move(packet), PacedPacketInfo()); + EXPECT_FALSE(packet_history_.GetPacketState(packet_sequence_number)); } TEST_F(RtpSenderEgressTest, PutsRetransmittablePacketsInHistory) { @@ -477,8 +479,9 @@ TEST_F(RtpSenderEgressTest, PutsRetransmittablePacketsInHistory) { std::unique_ptr packet = BuildRtpPacket(); packet->set_allow_retransmission(true); - sender->SendPacket(packet.get(), PacedPacketInfo()); - EXPECT_TRUE(packet_history_.GetPacketState(packet->SequenceNumber())); + auto packet_sequence_number = packet->SequenceNumber(); + sender->SendPacket(std::move(packet), PacedPacketInfo()); + EXPECT_TRUE(packet_history_.GetPacketState(packet_sequence_number)); } TEST_F(RtpSenderEgressTest, DoesNotPutNonMediaInHistory) { @@ -493,21 +496,23 @@ TEST_F(RtpSenderEgressTest, DoesNotPutNonMediaInHistory) { retransmission->set_packet_type(RtpPacketMediaType::kRetransmission); retransmission->set_retransmitted_sequence_number( retransmission->SequenceNumber()); - sender->SendPacket(retransmission.get(), PacedPacketInfo()); - EXPECT_FALSE( - packet_history_.GetPacketState(retransmission->SequenceNumber())); + auto retransmission_sequence_number = retransmission->SequenceNumber(); + sender->SendPacket(std::move(retransmission), PacedPacketInfo()); + EXPECT_FALSE(packet_history_.GetPacketState(retransmission_sequence_number)); std::unique_ptr fec = BuildRtpPacket(); fec->set_allow_retransmission(true); fec->set_packet_type(RtpPacketMediaType::kForwardErrorCorrection); - sender->SendPacket(fec.get(), PacedPacketInfo()); - EXPECT_FALSE(packet_history_.GetPacketState(fec->SequenceNumber())); + auto fec_sequence_number = fec->SequenceNumber(); + sender->SendPacket(std::move(fec), PacedPacketInfo()); + EXPECT_FALSE(packet_history_.GetPacketState(fec_sequence_number)); std::unique_ptr padding = BuildRtpPacket(); padding->set_allow_retransmission(true); padding->set_packet_type(RtpPacketMediaType::kPadding); - sender->SendPacket(padding.get(), PacedPacketInfo()); - EXPECT_FALSE(packet_history_.GetPacketState(padding->SequenceNumber())); + auto padding_sequence_number = padding->SequenceNumber(); + sender->SendPacket(std::move(padding), PacedPacketInfo()); + EXPECT_FALSE(packet_history_.GetPacketState(padding_sequence_number)); } TEST_F(RtpSenderEgressTest, UpdatesSendStatusOfRetransmittedPackets) { @@ -518,20 +523,21 @@ TEST_F(RtpSenderEgressTest, UpdatesSendStatusOfRetransmittedPackets) { // Send a packet, putting it in the history. std::unique_ptr media_packet = BuildRtpPacket(); media_packet->set_allow_retransmission(true); - sender->SendPacket(media_packet.get(), PacedPacketInfo()); - EXPECT_TRUE(packet_history_.GetPacketState(media_packet->SequenceNumber())); + auto media_packet_sequence_number = media_packet->SequenceNumber(); + sender->SendPacket(std::move(media_packet), PacedPacketInfo()); + EXPECT_TRUE(packet_history_.GetPacketState(media_packet_sequence_number)); // Simulate a retransmission, marking the packet as pending. std::unique_ptr retransmission = - packet_history_.GetPacketAndMarkAsPending(media_packet->SequenceNumber()); + packet_history_.GetPacketAndMarkAsPending(media_packet_sequence_number); retransmission->set_retransmitted_sequence_number( - media_packet->SequenceNumber()); + media_packet_sequence_number); retransmission->set_packet_type(RtpPacketMediaType::kRetransmission); - EXPECT_TRUE(packet_history_.GetPacketState(media_packet->SequenceNumber())); + EXPECT_TRUE(packet_history_.GetPacketState(media_packet_sequence_number)); // Simulate packet leaving pacer, the packet should be marked as non-pending. - sender->SendPacket(retransmission.get(), PacedPacketInfo()); - EXPECT_TRUE(packet_history_.GetPacketState(media_packet->SequenceNumber())); + sender->SendPacket(std::move(retransmission), PacedPacketInfo()); + EXPECT_TRUE(packet_history_.GetPacketState(media_packet_sequence_number)); } TEST_F(RtpSenderEgressTest, StreamDataCountersCallbacks) { @@ -559,7 +565,7 @@ TEST_F(RtpSenderEgressTest, StreamDataCountersCallbacks) { expected_retransmission_counter), Field(&StreamDataCounters::fec, kEmptyCounter)), kSsrc)); - sender->SendPacket(media_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(media_packet), PacedPacketInfo()); time_controller_.AdvanceTime(TimeDelta::Zero()); // Send a retransmission. Retransmissions are counted into both transmitted @@ -570,7 +576,6 @@ TEST_F(RtpSenderEgressTest, StreamDataCountersCallbacks) { retransmission_packet->set_retransmitted_sequence_number( kStartSequenceNumber); retransmission_packet->set_time_in_send_queue(TimeDelta::Millis(20)); - media_packet->SetPayloadSize(7); expected_transmitted_counter.packets += 1; expected_transmitted_counter.payload_bytes += retransmission_packet->payload_size(); @@ -593,7 +598,7 @@ TEST_F(RtpSenderEgressTest, StreamDataCountersCallbacks) { expected_retransmission_counter), Field(&StreamDataCounters::fec, kEmptyCounter)), kSsrc)); - sender->SendPacket(retransmission_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(retransmission_packet), PacedPacketInfo()); time_controller_.AdvanceTime(TimeDelta::Zero()); // Send a padding packet. @@ -615,7 +620,7 @@ TEST_F(RtpSenderEgressTest, StreamDataCountersCallbacks) { expected_retransmission_counter), Field(&StreamDataCounters::fec, kEmptyCounter)), kSsrc)); - sender->SendPacket(padding_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(padding_packet), PacedPacketInfo()); time_controller_.AdvanceTime(TimeDelta::Zero()); } @@ -641,7 +646,7 @@ TEST_F(RtpSenderEgressTest, StreamDataCountersCallbacksFec) { Field(&StreamDataCounters::retransmitted, kEmptyCounter), Field(&StreamDataCounters::fec, expected_fec_counter)), kSsrc)); - sender->SendPacket(media_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(media_packet), PacedPacketInfo()); time_controller_.AdvanceTime(TimeDelta::Zero()); // Send and FEC packet. FEC is counted into both transmitted and FEC packet @@ -665,7 +670,7 @@ TEST_F(RtpSenderEgressTest, StreamDataCountersCallbacksFec) { Field(&StreamDataCounters::retransmitted, kEmptyCounter), Field(&StreamDataCounters::fec, expected_fec_counter)), kSsrc)); - sender->SendPacket(fec_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(fec_packet), PacedPacketInfo()); time_controller_.AdvanceTime(TimeDelta::Zero()); } @@ -677,7 +682,11 @@ TEST_F(RtpSenderEgressTest, UpdatesDataCounters) { // Send a media packet. std::unique_ptr media_packet = BuildRtpPacket(); media_packet->SetPayloadSize(6); - sender->SendPacket(media_packet.get(), PacedPacketInfo()); + auto media_packet_sequence_number = media_packet->SequenceNumber(); + auto media_packet_payload_size = media_packet->payload_size(); + auto media_packet_padding_size = media_packet->padding_size(); + auto media_packet_headers_size = media_packet->headers_size(); + sender->SendPacket(std::move(media_packet), PacedPacketInfo()); time_controller_.AdvanceTime(TimeDelta::Zero()); // Send an RTX retransmission packet. @@ -685,8 +694,11 @@ TEST_F(RtpSenderEgressTest, UpdatesDataCounters) { rtx_packet->set_packet_type(RtpPacketMediaType::kRetransmission); rtx_packet->SetSsrc(kRtxSsrc); rtx_packet->SetPayloadSize(7); - rtx_packet->set_retransmitted_sequence_number(media_packet->SequenceNumber()); - sender->SendPacket(rtx_packet.get(), PacedPacketInfo()); + rtx_packet->set_retransmitted_sequence_number(media_packet_sequence_number); + auto rtx_packet_payload_size = rtx_packet->payload_size(); + auto rtx_packet_padding_size = rtx_packet->padding_size(); + auto rtx_packet_headers_size = rtx_packet->headers_size(); + sender->SendPacket(std::move(rtx_packet), PacedPacketInfo()); time_controller_.AdvanceTime(TimeDelta::Zero()); StreamDataCounters rtp_stats; @@ -694,18 +706,18 @@ TEST_F(RtpSenderEgressTest, UpdatesDataCounters) { sender->GetDataCounters(&rtp_stats, &rtx_stats); EXPECT_EQ(rtp_stats.transmitted.packets, 1u); - EXPECT_EQ(rtp_stats.transmitted.payload_bytes, media_packet->payload_size()); - EXPECT_EQ(rtp_stats.transmitted.padding_bytes, media_packet->padding_size()); - EXPECT_EQ(rtp_stats.transmitted.header_bytes, media_packet->headers_size()); + EXPECT_EQ(rtp_stats.transmitted.payload_bytes, media_packet_payload_size); + EXPECT_EQ(rtp_stats.transmitted.padding_bytes, media_packet_padding_size); + EXPECT_EQ(rtp_stats.transmitted.header_bytes, media_packet_headers_size); EXPECT_EQ(rtp_stats.retransmitted, kEmptyCounter); EXPECT_EQ(rtp_stats.fec, kEmptyCounter); // Retransmissions are counted both into transmitted and retransmitted // packet counts. EXPECT_EQ(rtx_stats.transmitted.packets, 1u); - EXPECT_EQ(rtx_stats.transmitted.payload_bytes, rtx_packet->payload_size()); - EXPECT_EQ(rtx_stats.transmitted.padding_bytes, rtx_packet->padding_size()); - EXPECT_EQ(rtx_stats.transmitted.header_bytes, rtx_packet->headers_size()); + EXPECT_EQ(rtx_stats.transmitted.payload_bytes, rtx_packet_payload_size); + EXPECT_EQ(rtx_stats.transmitted.padding_bytes, rtx_packet_padding_size); + EXPECT_EQ(rtx_stats.transmitted.header_bytes, rtx_packet_headers_size); EXPECT_EQ(rtx_stats.retransmitted, rtx_stats.transmitted); EXPECT_EQ(rtx_stats.fec, kEmptyCounter); } @@ -725,7 +737,7 @@ TEST_F(RtpSenderEgressTest, SendPacketUpdatesExtensions) { const int32_t kDiffMs = 10; time_controller_.AdvanceTime(TimeDelta::Millis(kDiffMs)); - sender->SendPacket(packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(packet), PacedPacketInfo()); RtpPacketReceived received_packet = transport_.last_packet()->packet; @@ -748,7 +760,8 @@ TEST_F(RtpSenderEgressTest, SendPacketSetsPacketOptions) { std::unique_ptr packet = BuildRtpPacket(); packet->SetExtension(kPacketId); EXPECT_CALL(send_packet_observer_, OnSendPacket); - sender->SendPacket(packet.get(), PacedPacketInfo()); + auto packet_sequence_number = packet->SequenceNumber(); + sender->SendPacket(std::move(packet), PacedPacketInfo()); PacketOptions packet_options = transport_.last_packet()->options; @@ -761,8 +774,8 @@ TEST_F(RtpSenderEgressTest, SendPacketSetsPacketOptions) { std::unique_ptr retransmission = BuildRtpPacket(); retransmission->SetExtension(kPacketId + 1); retransmission->set_packet_type(RtpPacketMediaType::kRetransmission); - retransmission->set_retransmitted_sequence_number(packet->SequenceNumber()); - sender->SendPacket(retransmission.get(), PacedPacketInfo()); + retransmission->set_retransmitted_sequence_number(packet_sequence_number); + sender->SendPacket(std::move(retransmission), PacedPacketInfo()); EXPECT_TRUE(transport_.last_packet()->options.is_retransmit); } @@ -812,15 +825,15 @@ TEST_F(RtpSenderEgressTest, SendPacketUpdatesStats) { EXPECT_CALL(send_packet_observer_, OnSendPacket(1, capture_time_ms, kSsrc)); - sender->SendPacket(video_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(video_packet), PacedPacketInfo()); // Send packet observer not called for padding/retransmissions. EXPECT_CALL(send_packet_observer_, OnSendPacket(2, _, _)).Times(0); - sender->SendPacket(rtx_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(rtx_packet), PacedPacketInfo()); EXPECT_CALL(send_packet_observer_, OnSendPacket(3, capture_time_ms, kFlexFecSsrc)); - sender->SendPacket(fec_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(fec_packet), PacedPacketInfo()); time_controller_.AdvanceTime(TimeDelta::Zero()); StreamDataCounters rtp_stats; @@ -850,7 +863,7 @@ TEST_F(RtpSenderEgressTest, TransportFeedbackObserverWithRetransmission) { Field(&RtpPacketSendInfo::rtp_sequence_number, retransmitted_seq), Field(&RtpPacketSendInfo::transport_sequence_number, kTransportSequenceNumber)))); - sender->SendPacket(retransmission.get(), PacedPacketInfo()); + sender->SendPacket(std::move(retransmission), PacedPacketInfo()); } TEST_F(RtpSenderEgressTest, TransportFeedbackObserverWithRtxRetransmission) { @@ -874,7 +887,7 @@ TEST_F(RtpSenderEgressTest, TransportFeedbackObserverWithRtxRetransmission) { Field(&RtpPacketSendInfo::rtp_sequence_number, rtx_retransmitted_seq), Field(&RtpPacketSendInfo::transport_sequence_number, kTransportSequenceNumber)))); - sender->SendPacket(rtx_retransmission.get(), PacedPacketInfo()); + sender->SendPacket(std::move(rtx_retransmission), PacedPacketInfo()); } TEST_F(RtpSenderEgressTest, TransportFeedbackObserverPadding) { @@ -892,7 +905,7 @@ TEST_F(RtpSenderEgressTest, TransportFeedbackObserverPadding) { OnAddPacket(AllOf(Field(&RtpPacketSendInfo::media_ssrc, absl::nullopt), Field(&RtpPacketSendInfo::transport_sequence_number, kTransportSequenceNumber)))); - sender->SendPacket(padding.get(), PacedPacketInfo()); + sender->SendPacket(std::move(padding), PacedPacketInfo()); } TEST_F(RtpSenderEgressTest, TransportFeedbackObserverRtxPadding) { @@ -912,7 +925,7 @@ TEST_F(RtpSenderEgressTest, TransportFeedbackObserverRtxPadding) { OnAddPacket(AllOf(Field(&RtpPacketSendInfo::media_ssrc, absl::nullopt), Field(&RtpPacketSendInfo::transport_sequence_number, kTransportSequenceNumber)))); - sender->SendPacket(rtx_padding.get(), PacedPacketInfo()); + sender->SendPacket(std::move(rtx_padding), PacedPacketInfo()); } TEST_F(RtpSenderEgressTest, TransportFeedbackObserverFec) { @@ -937,7 +950,7 @@ TEST_F(RtpSenderEgressTest, TransportFeedbackObserverFec) { OnAddPacket(AllOf(Field(&RtpPacketSendInfo::media_ssrc, absl::nullopt), Field(&RtpPacketSendInfo::transport_sequence_number, kTransportSequenceNumber)))); - sender->SendPacket(fec_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(fec_packet), PacedPacketInfo()); } TEST_F(RtpSenderEgressTest, SupportsAbortingRetransmissions) { @@ -950,7 +963,7 @@ TEST_F(RtpSenderEgressTest, SupportsAbortingRetransmissions) { media_packet->set_packet_type(RtpPacketMediaType::kVideo); media_packet->set_allow_retransmission(true); const uint16_t media_sequence_number = media_packet->SequenceNumber(); - sender->SendPacket(media_packet.get(), PacedPacketInfo()); + sender->SendPacket(std::move(media_packet), PacedPacketInfo()); // Fetch a retranmission packet from the history, this should mark the // media packets as pending so it is not available to grab again.