diff --git a/modules/pacing/BUILD.gn b/modules/pacing/BUILD.gn index 9c9f7d91f8..f93d400faf 100644 --- a/modules/pacing/BUILD.gn +++ b/modules/pacing/BUILD.gn @@ -28,6 +28,7 @@ rtc_static_library("pacing") { deps = [ ":interval_budget", "..:module_api", + "../../api:function_view", "../../api/transport:field_trial_based_config", "../../api/transport:network_control", "../../api/transport:webrtc_key_value_config", diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index 6177ca61fb..f99d43cbad 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -12,6 +12,7 @@ #include #include +#include #include "absl/memory/memory.h" #include "logging/rtc_event_log/rtc_event_log.h" @@ -97,7 +98,9 @@ PacedSender::PacedSender(Clock* clock, packets_(clock->TimeInMicroseconds()), packet_counter_(0), queue_time_limit(kMaxQueueLengthMs), - account_for_audio_(false) { + account_for_audio_(false), + legacy_packet_referencing_( + !IsDisabled(*field_trials_, "WebRTC-Pacer-LegacyPacketReferencing")) { if (!drain_large_queues_) { RTC_LOG(LS_WARNING) << "Pacer queues will not be drained," "pushback experiment must be enabled."; @@ -328,10 +331,21 @@ void PacedSender::Process() { int64_t now_us = clock_->TimeInMicroseconds(); int64_t elapsed_time_ms = UpdateTimeAndGetElapsedMs(now_us); if (ShouldSendKeepalive(now_us)) { - critsect_.Leave(); - size_t bytes_sent = packet_router_->TimeToSendPadding(1, PacedPacketInfo()); - critsect_.Enter(); - OnPaddingSent(bytes_sent); + if (legacy_packet_referencing_) { + critsect_.Leave(); + size_t bytes_sent = + packet_router_->TimeToSendPadding(1, PacedPacketInfo()); + critsect_.Enter(); + OnPaddingSent(bytes_sent); + } else { + critsect_.Leave(); + std::vector> keepalive_packets = + packet_router_->GeneratePadding(1); + critsect_.Enter(); + for (auto& packet : keepalive_packets) { + EnqueuePacket(std::move(packet)); + } + } } if (paused_) @@ -364,35 +378,60 @@ void PacedSender::Process() { bool is_probing = prober_.IsProbing(); PacedPacketInfo pacing_info; - size_t bytes_sent = 0; - size_t recommended_probe_size = 0; + absl::optional recommended_probe_size; if (is_probing) { pacing_info = prober_.CurrentCluster(); recommended_probe_size = prober_.RecommendedMinProbeSize(); } + + size_t bytes_sent = 0; // The paused state is checked in the loop since it leaves the critical // section allowing the paused state to be changed from other code. - while (!packets_.Empty() && !paused_) { + while (!paused_) { auto* packet = GetPendingPacket(pacing_info); - if (packet == nullptr) + if (packet == nullptr) { + // No packet available to send, check if we should send padding. + if (!legacy_packet_referencing_) { + size_t padding_bytes_to_add = + PaddingBytesToAdd(recommended_probe_size, bytes_sent); + if (padding_bytes_to_add > 0) { + critsect_.Leave(); + std::vector> padding_packets = + packet_router_->GeneratePadding(padding_bytes_to_add); + critsect_.Enter(); + if (padding_packets.empty()) { + // No padding packets were generated, quite send loop. + break; + } + for (auto& packet : padding_packets) { + EnqueuePacket(std::move(packet)); + } + // Continue loop to send the padding that was just added. + continue; + } + } + + // Can't fetch new packet and no padding to send, exit send loop. break; + } std::unique_ptr rtp_packet = packet->ReleasePacket(); const bool owned_rtp_packet = rtp_packet != nullptr; - - critsect_.Leave(); - RtpPacketSendResult success; + if (rtp_packet != nullptr) { + critsect_.Leave(); packet_router_->SendPacket(std::move(rtp_packet), pacing_info); + critsect_.Enter(); success = RtpPacketSendResult::kSuccess; } else { + critsect_.Leave(); success = packet_router_->TimeToSendPacket( packet->ssrc(), packet->sequence_number(), packet->capture_time_ms(), packet->is_retransmission(), pacing_info); + critsect_.Enter(); } - critsect_.Enter(); if (success == RtpPacketSendResult::kSuccess || success == RtpPacketSendResult::kPacketNotFound) { // Packet sent or invalid packet, remove it from queue. @@ -400,7 +439,7 @@ void PacedSender::Process() { bytes_sent += packet->size_in_bytes(); // Send succeeded, remove it from the queue. OnPacketSent(packet); - if (is_probing && bytes_sent > recommended_probe_size) + if (recommended_probe_size && bytes_sent > *recommended_probe_size) break; } else if (owned_rtp_packet) { // Send failed, but we can't put it back in the queue, remove it without @@ -414,16 +453,17 @@ void PacedSender::Process() { } } - if (packets_.Empty() && !Congested()) { + if (legacy_packet_referencing_ && packets_.Empty() && !Congested()) { // We can not send padding unless a normal packet has first been sent. If we // do, timestamps get messed up. if (packet_counter_ > 0) { - int padding_needed = - static_cast(is_probing ? (recommended_probe_size - bytes_sent) - : padding_budget_.bytes_remaining()); + int padding_needed = static_cast( + recommended_probe_size ? (*recommended_probe_size - bytes_sent) + : padding_budget_.bytes_remaining()); if (padding_needed > 0) { + size_t padding_sent = 0; critsect_.Leave(); - size_t padding_sent = + padding_sent = packet_router_->TimeToSendPadding(padding_needed, pacing_info); critsect_.Enter(); bytes_sent += padding_sent; @@ -431,6 +471,7 @@ void PacedSender::Process() { } } } + if (is_probing) { probing_send_failure_ = bytes_sent == 0; if (!probing_send_failure_) @@ -444,8 +485,41 @@ void PacedSender::ProcessThreadAttached(ProcessThread* process_thread) { process_thread_ = process_thread; } +size_t PacedSender::PaddingBytesToAdd( + absl::optional recommended_probe_size, + size_t bytes_sent) { + if (!packets_.Empty()) { + // Actual payload available, no need to add padding. + return 0; + } + + if (Congested()) { + // Don't add padding if congested, even if requested for probing. + return 0; + } + + if (packet_counter_ == 0) { + // We can not send padding unless a normal packet has first been sent. If we + // do, timestamps get messed up. + return 0; + } + + if (recommended_probe_size) { + if (*recommended_probe_size > bytes_sent) { + return *recommended_probe_size - bytes_sent; + } + return 0; + } + + return padding_budget_.bytes_remaining(); +} + RoundRobinPacketQueue::QueuedPacket* PacedSender::GetPendingPacket( const PacedPacketInfo& pacing_info) { + if (packets_.Empty()) { + return nullptr; + } + // Since we need to release the lock in order to send, we first pop the // element from the priority queue but keep it in storage, so that we can // reinsert it if send fails. diff --git a/modules/pacing/paced_sender.h b/modules/pacing/paced_sender.h index c67e162d4a..0cdb068a65 100644 --- a/modules/pacing/paced_sender.h +++ b/modules/pacing/paced_sender.h @@ -17,6 +17,7 @@ #include #include "absl/types/optional.h" +#include "api/function_view.h" #include "api/transport/field_trial_based_config.h" #include "api/transport/network_types.h" #include "api/transport/webrtc_key_value_config.h" @@ -135,6 +136,10 @@ class PacedSender : public Module, public RtpPacketPacer { void UpdateBudgetWithBytesSent(size_t bytes) RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_); + size_t PaddingBytesToAdd(absl::optional recommended_probe_size, + size_t bytes_sent) + RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_); + RoundRobinPacketQueue::QueuedPacket* GetPendingPacket( const PacedPacketInfo& pacing_info) RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_); @@ -195,6 +200,11 @@ class PacedSender : public Module, public RtpPacketPacer { int64_t queue_time_limit RTC_GUARDED_BY(critsect_); bool account_for_audio_ RTC_GUARDED_BY(critsect_); + + // If true, PacedSender should only reference packets as in legacy mode. + // If false, PacedSender may have direct ownership of RtpPacketToSend objects. + // Defaults to true, will be changed to default false soon. + const bool legacy_packet_referencing_; }; } // namespace webrtc #endif // MODULES_PACING_PACED_SENDER_H_ diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc index e73516f90e..7492f1309e 100644 --- a/modules/pacing/packet_router.cc +++ b/modules/pacing/packet_router.cc @@ -146,6 +146,12 @@ RtpRtcp* PacketRouter::FindRtpModule(uint32_t ssrc) { void PacketRouter::SendPacket(std::unique_ptr packet, const PacedPacketInfo& cluster_info) { rtc::CritScope cs(&modules_crit_); + // With the new pacer code path, transport sequence numbers are only set here, + // on the pacer thread. Therefore we don't need atomics/synchronization. + if (packet->IsExtensionReserved() && + packet->SetExtension(transport_seq_)) { + ++transport_seq_; + } for (auto* rtp_module : rtp_send_modules_) { if (rtp_module->TrySendPacket(packet.get(), cluster_info)) { const bool can_send_padding = @@ -200,7 +206,8 @@ size_t PacketRouter::TimeToSendPadding(size_t bytes_to_send, return total_bytes_sent; } -void PacketRouter::GeneratePadding(size_t target_size_bytes) { +std::vector> PacketRouter::GeneratePadding( + size_t target_size_bytes) { rtc::CritScope cs(&modules_crit_); // First try on the last rtp module to have sent media. This increases the // the chance that any payload based padding will be useful as it will be @@ -212,17 +219,17 @@ void PacketRouter::GeneratePadding(size_t target_size_bytes) { RTC_DCHECK(std::find(rtp_send_modules_.begin(), rtp_send_modules_.end(), last_send_module_) != rtp_send_modules_.end()); RTC_DCHECK(last_send_module_->HasBweExtensions()); - last_send_module_->GeneratePadding(target_size_bytes); - return; + return last_send_module_->GeneratePadding(target_size_bytes); } // Rtp modules are ordered by which stream can most benefit from padding. for (RtpRtcp* rtp_module : rtp_send_modules_) { if (rtp_module->SendingMedia() && rtp_module->HasBweExtensions()) { - rtp_module->GeneratePadding(target_size_bytes); - return; + return rtp_module->GeneratePadding(target_size_bytes); } } + + return {}; } void PacketRouter::SetTransportWideSequenceNumber(uint16_t sequence_number) { diff --git a/modules/pacing/packet_router.h b/modules/pacing/packet_router.h index a14a55e922..a03dc4cdd2 100644 --- a/modules/pacing/packet_router.h +++ b/modules/pacing/packet_router.h @@ -65,7 +65,8 @@ class PacketRouter : public TransportSequenceNumberAllocator, virtual size_t TimeToSendPadding(size_t bytes, const PacedPacketInfo& packet_info); - virtual void GeneratePadding(size_t target_size_bytes); + virtual std::vector> GeneratePadding( + size_t target_size_bytes); void SetTransportWideSequenceNumber(uint16_t sequence_number); uint16_t AllocateSequenceNumber() override; diff --git a/modules/pacing/packet_router_unittest.cc b/modules/pacing/packet_router_unittest.cc index 92ecdd6a36..de2f342324 100644 --- a/modules/pacing/packet_router_unittest.cc +++ b/modules/pacing/packet_router_unittest.cc @@ -10,9 +10,12 @@ #include #include +#include +#include "absl/memory/memory.h" #include "api/units/time_delta.h" #include "modules/pacing/packet_router.h" +#include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "rtc_base/checks.h" @@ -36,6 +39,7 @@ using ::testing::Field; using ::testing::Gt; using ::testing::Le; using ::testing::NiceMock; +using ::testing::Property; using ::testing::Return; using ::testing::ReturnPointee; using ::testing::SaveArg; @@ -296,9 +300,15 @@ TEST(PacketRouterTest, GeneratePaddingPicksCorrectModule) { packet_router.AddSendRtpModule(&rtp_2, false); const size_t kPaddingSize = 123; + const size_t kExpectedPaddingPackets = 1; EXPECT_CALL(rtp_1, GeneratePadding(_)).Times(0); - EXPECT_CALL(rtp_2, GeneratePadding(kPaddingSize)).Times(1); - packet_router.GeneratePadding(kPaddingSize); + EXPECT_CALL(rtp_2, GeneratePadding(kPaddingSize)) + .WillOnce([&](size_t padding_size) { + return std::vector>( + kExpectedPaddingPackets); + }); + auto generated_padding = packet_router.GeneratePadding(kPaddingSize); + EXPECT_EQ(generated_padding.size(), kExpectedPaddingPackets); packet_router.RemoveSendRtpModule(&rtp_1); packet_router.RemoveSendRtpModule(&rtp_2); @@ -938,4 +948,86 @@ TEST(PacketRouterRembTest, ReceiveModuleTakesOverWhenLastSendModuleRemoved) { packet_router.RemoveReceiveRtpModule(&receive_module); } +TEST(PacketRouterTest, SendPacketWithoutTransportSequenceNumbers) { + PacketRouter packet_router; + NiceMock rtp_1; + packet_router.AddSendRtpModule(&rtp_1, false); + + const uint16_t kSsrc1 = 1234; + ON_CALL(rtp_1, SendingMedia).WillByDefault(Return(true)); + ON_CALL(rtp_1, SSRC).WillByDefault(Return(kSsrc1)); + + // Send a packet without TransportSequenceNumber extension registered, + // packets sent should not have the extension set. + RtpHeaderExtensionMap extension_manager; + auto packet = absl::make_unique(&extension_manager); + packet->SetSsrc(kSsrc1); + EXPECT_CALL( + rtp_1, + TrySendPacket( + Property(&RtpPacketToSend::HasExtension, + false), + _)) + .WillOnce(Return(true)); + packet_router.SendPacket(std::move(packet), PacedPacketInfo()); + + packet_router.RemoveSendRtpModule(&rtp_1); +} + +TEST(PacketRouterTest, SendPacketAssignsTransportSequenceNumbers) { + PacketRouter packet_router; + NiceMock rtp_1; + NiceMock rtp_2; + + packet_router.AddSendRtpModule(&rtp_1, false); + packet_router.AddSendRtpModule(&rtp_2, false); + + const uint16_t kSsrc1 = 1234; + const uint16_t kSsrc2 = 2345; + + ON_CALL(rtp_1, SendingMedia).WillByDefault(Return(true)); + ON_CALL(rtp_1, SSRC).WillByDefault(Return(kSsrc1)); + ON_CALL(rtp_2, SendingMedia).WillByDefault(Return(true)); + ON_CALL(rtp_2, SSRC).WillByDefault(Return(kSsrc2)); + + RtpHeaderExtensionMap extension_manager; + const int kTransportSequenceNumberExtensionId = 1; + extension_manager.Register(kRtpExtensionTransportSequenceNumber, + kTransportSequenceNumberExtensionId); + + uint16_t transport_sequence_number = 0; + + auto packet = absl::make_unique(&extension_manager); + EXPECT_TRUE(packet->ReserveExtension()); + packet->SetSsrc(kSsrc1); + EXPECT_CALL( + rtp_1, + TrySendPacket( + Property(&RtpPacketToSend::GetExtension, + transport_sequence_number), + _)) + .WillOnce(Return(true)); + packet_router.SendPacket(std::move(packet), PacedPacketInfo()); + + ++transport_sequence_number; + packet = absl::make_unique(&extension_manager); + EXPECT_TRUE(packet->ReserveExtension()); + packet->SetSsrc(kSsrc2); + + // There will be a failed attempt to send on kSsrc1 before trying + // the correct RTP module. + EXPECT_CALL(rtp_1, TrySendPacket).WillOnce(Return(false)); + EXPECT_CALL( + rtp_2, + TrySendPacket( + Property(&RtpPacketToSend::GetExtension, + transport_sequence_number), + _)) + .WillOnce(Return(true)); + packet_router.SendPacket(std::move(packet), PacedPacketInfo()); + + packet_router.RemoveSendRtpModule(&rtp_1); + packet_router.RemoveSendRtpModule(&rtp_2); +} + } // namespace webrtc diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index 25be175cd7..42dd27dbcd 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -287,7 +287,8 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { virtual size_t TimeToSendPadding(size_t bytes, const PacedPacketInfo& pacing_info) = 0; - virtual void GeneratePadding(size_t target_size_bytes) = 0; + virtual std::vector> GeneratePadding( + size_t target_size_bytes) = 0; // Called on generation of new statistics after an RTP send. virtual void RegisterSendChannelRtpStatisticsCallback( diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index fc2bb36d81..f3812ffb9b 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -95,7 +95,9 @@ class MockRtpRtcp : public RtpRtcp { const PacedPacketInfo& pacing_info)); MOCK_METHOD2(TimeToSendPadding, size_t(size_t bytes, const PacedPacketInfo& pacing_info)); - MOCK_METHOD1(GeneratePadding, void(size_t target_size_bytes)); + MOCK_METHOD1( + GeneratePadding, + std::vector>(size_t target_size_bytes)); MOCK_METHOD2(RegisterRtcpObservers, void(RtcpIntraFrameObserver* intra_frame_callback, RtcpBandwidthObserver* bandwidth_callback)); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 21b85a19d5..aaf1822c92 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -409,8 +409,9 @@ size_t ModuleRtpRtcpImpl::TimeToSendPadding( return rtp_sender_->TimeToSendPadding(bytes, pacing_info); } -void ModuleRtpRtcpImpl::GeneratePadding(size_t target_size_bytes) { - rtp_sender_->GeneratePadding(target_size_bytes); +std::vector> +ModuleRtpRtcpImpl::GeneratePadding(size_t target_size_bytes) { + return rtp_sender_->GeneratePadding(target_size_bytes); } size_t ModuleRtpRtcpImpl::MaxRtpPacketSize() const { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 60ac5fd604..e22126c3c4 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -148,7 +148,8 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { size_t TimeToSendPadding(size_t bytes, const PacedPacketInfo& pacing_info) override; - void GeneratePadding(size_t target_size_bytes) override; + std::vector> GeneratePadding( + size_t target_size_bytes) override; // RTCP part. diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index a932fab24f..0fa719e11c 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -195,6 +195,9 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config) config.field_trials)), payload_padding_prefer_useful_packets_( !IsDisabled("WebRTC-PayloadPadding-UseMostUsefulPacket", + config.field_trials)), + pacer_legacy_packet_referencing_( + !IsDisabled("WebRTC-Pacer-LegacyPacketReferencing", config.field_trials)) { // This random initialization is not intended to be cryptographic strong. timestamp_offset_ = random_.Rand(); @@ -283,6 +286,9 @@ RTPSender::RTPSender( .find("Enabled") == 0), payload_padding_prefer_useful_packets_( field_trials.Lookup("WebRTC-PayloadPadding-UseMostUsefulPacket") + .find("Disabled") != 0), + pacer_legacy_packet_referencing_( + field_trials.Lookup("WebRTC-Pacer-LegacyPacketReferencing") .find("Disabled") != 0) { // This random initialization is not intended to be cryptographic strong. timestamp_offset_ = random_.Rand(); @@ -592,31 +598,67 @@ int32_t RTPSender::ReSendPacket(uint16_t packet_id) { } const int32_t packet_size = static_cast(stored_packet->packet_size); - - // Skip retransmission rate check if not configured. - if (retransmission_rate_limiter_) { - // Check if we're overusing retransmission bitrate. - // TODO(sprang): Add histograms for nack success or failure reasons. - if (!retransmission_rate_limiter_->TryUseRate(packet_size)) { - return -1; - } - } + const bool rtx = (RtxStatus() & kRtxRetransmitted) > 0; if (paced_sender_) { - // Mark packet as being in pacer queue again, to prevent duplicates. - if (!packet_history_.SetPendingTransmission(packet_id)) { - // Packet has already been removed from history, return early. - return 0; + if (pacer_legacy_packet_referencing_) { + // Check if we're overusing retransmission bitrate. + // TODO(sprang): Add histograms for nack success or failure reasons. + if (retransmission_rate_limiter_ && + !retransmission_rate_limiter_->TryUseRate(packet_size)) { + return -1; + } + + // Mark packet as being in pacer queue again, to prevent duplicates. + if (!packet_history_.SetPendingTransmission(packet_id)) { + // Packet has already been removed from history, return early. + return 0; + } + + paced_sender_->InsertPacket( + RtpPacketSender::kNormalPriority, stored_packet->ssrc, + stored_packet->rtp_sequence_number, stored_packet->capture_time_ms, + stored_packet->packet_size, true); + } else { + std::unique_ptr packet = + packet_history_.GetPacketAndMarkAsPending( + packet_id, [&](const RtpPacketToSend& stored_packet) { + // Check if we're overusing retransmission bitrate. + // TODO(sprang): Add histograms for nack success or failure + // reasons. + std::unique_ptr retransmit_packet; + if (retransmission_rate_limiter_ && + !retransmission_rate_limiter_->TryUseRate(packet_size)) { + return retransmit_packet; + } + if (rtx) { + retransmit_packet = BuildRtxPacket(stored_packet); + } else { + retransmit_packet = + absl::make_unique(stored_packet); + } + retransmit_packet->set_retransmitted_sequence_number( + stored_packet.SequenceNumber()); + return retransmit_packet; + }); + if (!packet) { + return -1; + } + packet->set_packet_type(RtpPacketToSend::Type::kRetransmission); + paced_sender_->EnqueuePacket(std::move(packet)); } - paced_sender_->InsertPacket( - RtpPacketSender::kNormalPriority, stored_packet->ssrc, - stored_packet->rtp_sequence_number, stored_packet->capture_time_ms, - stored_packet->packet_size, true); - return packet_size; } + // TODO(sprang): Replace this whole code-path with a pass-through pacer. + // Check if we're overusing retransmission bitrate. + // TODO(sprang): Add histograms for nack success or failure reasons. + if (retransmission_rate_limiter_ && + !retransmission_rate_limiter_->TryUseRate(packet_size)) { + return -1; + } + std::unique_ptr packet = packet_history_.GetPacketAndSetSendTime(packet_id); if (!packet) { @@ -624,7 +666,6 @@ int32_t RTPSender::ReSendPacket(uint16_t packet_id) { return 0; } - const bool rtx = (RtxStatus() & kRtxRetransmitted) > 0; if (!PrepareAndSendPacket(std::move(packet), rtx, true, PacedPacketInfo())) return -1; @@ -926,15 +967,18 @@ size_t RTPSender::TimeToSendPadding(size_t bytes, return bytes_sent; } -void RTPSender::GeneratePadding(size_t target_size_bytes) { +std::vector> RTPSender::GeneratePadding( + size_t target_size_bytes) { // This method does not actually send packets, it just generates // them and puts them in the pacer queue. Since this should incur // low overhead, keep the lock for the scope of the method in order // to make the code more readable. rtc::CritScope lock(&send_critsect_); - if (!sending_media_) - return; + if (!sending_media_) { + return {}; + } + std::vector> padding_packets; size_t bytes_left = target_size_bytes; if ((rtx_ & kRtxRedundantPayloads) != 0) { while (bytes_left >= 0) { @@ -953,7 +997,7 @@ void RTPSender::GeneratePadding(size_t target_size_bytes) { bytes_left -= std::min(bytes_left, packet->payload_size()); packet->set_packet_type(RtpPacketToSend::Type::kPadding); - paced_sender_->EnqueuePacket(std::move(packet)); + padding_packets.push_back(std::move(packet)); } } @@ -1022,10 +1066,15 @@ void RTPSender::GeneratePadding(size_t target_size_bytes) { padding_packet->SetPayloadType(rtx_payload_type_map_.begin()->second); } + if (rtp_header_extension_map_.IsRegistered(TransportSequenceNumber::kId)) { + padding_packet->ReserveExtension(); + } padding_packet->SetPadding(padding_bytes_in_packet); bytes_left -= std::min(bytes_left, padding_bytes_in_packet); - paced_sender_->EnqueuePacket(std::move(padding_packet)); + padding_packets.push_back(std::move(padding_packet)); } + + return padding_packets; } bool RTPSender::SendToNetwork(std::unique_ptr packet, @@ -1040,18 +1089,28 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, size_t packet_size = send_side_bwe_with_overhead_ ? packet->size() : packet->payload_size(); auto packet_type = packet->packet_type(); - RTC_DCHECK(packet_type.has_value()); - if (ssrc == FlexfecSsrc()) { - // Store FlexFEC packets in the history here, so they can be found - // when the pacer calls TimeToSendPacket. - flexfec_packet_history_.PutRtpPacket(std::move(packet), storage, - absl::nullopt); + RTC_CHECK(packet_type) << "Packet type must be set before sending."; + + if (pacer_legacy_packet_referencing_) { + // If |pacer_reference_packets_| then pacer needs to find the packet in + // the history when it is time to send, so move packet there. + if (ssrc == FlexfecSsrc()) { + // Store FlexFEC packets in a separate history since they are on a + // separate SSRC. + flexfec_packet_history_.PutRtpPacket(std::move(packet), storage, + absl::nullopt); + } else { + packet_history_.PutRtpPacket(std::move(packet), storage, absl::nullopt); + } + + paced_sender_->InsertPacket(PacketTypeToPriority(*packet_type), ssrc, + seq_no, capture_time_ms, packet_size, false); } else { - packet_history_.PutRtpPacket(std::move(packet), storage, absl::nullopt); + packet->set_allow_retransmission(storage == + StorageType::kAllowRetransmission); + paced_sender_->EnqueuePacket(std::move(packet)); } - paced_sender_->InsertPacket(PacketTypeToPriority(*packet_type), ssrc, - seq_no, capture_time_ms, packet_size, false); return true; } diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index c191694de5..8e505750a5 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -119,7 +119,8 @@ class RTPSender { bool TrySendPacket(RtpPacketToSend* packet, const PacedPacketInfo& pacing_info); size_t TimeToSendPadding(size_t bytes, const PacedPacketInfo& pacing_info); - void GeneratePadding(size_t target_size_bytes); + std::vector> GeneratePadding( + size_t target_size_bytes); // NACK. void OnReceivedNack(const std::vector& nack_sequence_numbers, @@ -322,6 +323,11 @@ class RTPSender { // packet_history_.GetBestFittingPacket() in TrySendRedundantPayloads(). const bool payload_padding_prefer_useful_packets_; + // If true, PacedSender should only reference packets as in legacy mode. + // If false, PacedSender may have direct ownership of RtpPacketToSend objects. + // Defaults to true, will be changed to default false soon. + const bool pacer_legacy_packet_referencing_; + RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RTPSender); }; diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index dad0d74102..02462b40d9 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -142,6 +142,27 @@ MATCHER_P(SameRtcEventTypeAs, value, "") { return value == arg->GetType(); } +struct TestConfig { + TestConfig(bool with_overhead, bool pacer_references_packets) + : with_overhead(with_overhead), + pacer_references_packets(pacer_references_packets) {} + bool with_overhead = false; + bool pacer_references_packets = false; +}; + +std::string ToFieldTrialString(TestConfig config) { + std::string field_trials; + if (config.with_overhead) { + field_trials += "WebRTC-SendSideBwe-WithOverhead/Enabled/"; + } + if (config.pacer_references_packets) { + field_trials += "WebRTC-Pacer-LegacyPacketReferencing/Enabled/"; + } else { + field_trials += "WebRTC-Pacer-LegacyPacketReferencing/Disabled/"; + } + return field_trials; +} + } // namespace class MockRtpPacketPacer : public RtpPacketPacer { @@ -188,7 +209,7 @@ class MockOverheadObserver : public OverheadObserver { MOCK_METHOD1(OnOverheadChanged, void(size_t overhead_bytes_per_packet)); }; -class RtpSenderTest : public ::testing::TestWithParam { +class RtpSenderTest : public ::testing::TestWithParam { protected: RtpSenderTest() : fake_clock_(kStartTime), @@ -206,8 +227,7 @@ class RtpSenderTest : public ::testing::TestWithParam { rtp_sender_(), transport_(), kMarkerBit(true), - field_trials_(GetParam() ? "WebRTC-SendSideBwe-WithOverhead/Enabled/" - : "") {} + field_trials_(ToFieldTrialString(GetParam())) {} void SetUp() override { SetUpRtpSender(true, false); } @@ -255,21 +275,23 @@ class RtpSenderTest : public ::testing::TestWithParam { return packet; } - void SendPacket(int64_t capture_time_ms, int payload_length) { + std::unique_ptr SendPacket(int64_t capture_time_ms, + int payload_length) { uint32_t timestamp = capture_time_ms * 90; auto packet = BuildRtpPacket(kPayload, kMarkerBit, timestamp, capture_time_ms); packet->AllocatePayload(payload_length); // Packet should be stored in a send bucket. - EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), - kAllowRetransmission, - RtpPacketSender::kNormalPriority)); + EXPECT_TRUE(rtp_sender_->SendToNetwork( + absl::make_unique(*packet), kAllowRetransmission, + RtpPacketSender::kNormalPriority)); + return packet; } - void SendGenericPacket() { + std::unique_ptr SendGenericPacket() { const int64_t kCaptureTimeMs = fake_clock_.TimeInMilliseconds(); - SendPacket(kCaptureTimeMs, sizeof(kPayloadData)); + return SendPacket(kCaptureTimeMs, sizeof(kPayloadData)); } }; @@ -429,8 +451,9 @@ TEST_P(RtpSenderTestWithoutPacer, .WillOnce(Return(kTransportSequenceNumber)); const size_t expected_bytes = - GetParam() ? sizeof(kPayloadData) + kRtpOverheadBytesPerPacket - : sizeof(kPayloadData); + GetParam().with_overhead + ? sizeof(kPayloadData) + kRtpOverheadBytesPerPacket + : sizeof(kPayloadData); EXPECT_CALL(feedback_observer_, OnAddPacket(AllOf( @@ -658,9 +681,6 @@ TEST_P(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) { kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); - EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)); - EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) - .WillOnce(Return(kTransportSequenceNumber)); EXPECT_CALL(send_packet_observer_, OnSendPacket(kTransportSequenceNumber, _, _)) .Times(1); @@ -674,10 +694,26 @@ TEST_P(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) { Field(&RtpPacketSendInfo::pacing_info, PacedPacketInfo())))) .Times(1); - SendGenericPacket(); - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, - fake_clock_.TimeInMilliseconds(), false, - PacedPacketInfo()); + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)); + SendGenericPacket(); + EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) + .WillOnce(Return(kTransportSequenceNumber)); + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, + fake_clock_.TimeInMilliseconds(), false, + PacedPacketInfo()); + } else { + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), + Pointee(Property(&RtpPacketToSend::SequenceNumber, kSeqNum))))); + auto packet = SendGenericPacket(); + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + // Transport sequence number is set by PacketRouter, before TrySendPacket(). + packet->SetExtension(kTransportSequenceNumber); + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); + } const auto& packet = transport_.last_sent_packet(); uint16_t transport_seq_no; @@ -702,17 +738,26 @@ TEST_P(RtpSenderTest, WritesPacerExitToTimingExtension) { size_t packet_size = packet->size(); const int kStoredTimeInMs = 100; - { + if (GetParam().pacer_references_packets) { EXPECT_CALL( mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, kSsrc, _, _, _, _)); EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), kAllowRetransmission, RtpPacketSender::kNormalPriority)); + fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, + PacedPacketInfo()); + } else { + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)))); + EXPECT_TRUE(rtp_sender_->SendToNetwork( + absl::make_unique(*packet), kAllowRetransmission)); + fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); } - fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, - PacedPacketInfo()); EXPECT_EQ(1, transport_.packets_sent()); EXPECT_EQ(packet_size, transport_.last_sent_packet().size()); @@ -740,17 +785,29 @@ TEST_P(RtpSenderTest, WritesNetwork2ToTimingExtensionWithPacer) { size_t packet_size = packet->size(); const int kStoredTimeInMs = 100; - { + + if (GetParam().pacer_references_packets) { EXPECT_CALL( mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, kSsrc, _, _, _, _)); EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), kAllowRetransmission, RtpPacketSender::kNormalPriority)); + fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, + PacedPacketInfo()); + } else { + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)))); + EXPECT_TRUE(rtp_sender_->SendToNetwork( + absl::make_unique(*packet), kAllowRetransmission, + RtpPacketSender::kNormalPriority)); + fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); } - fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, - PacedPacketInfo()); + EXPECT_EQ(1, transport_.packets_sent()); EXPECT_EQ(packet_size, transport_.last_sent_packet().size()); @@ -787,8 +844,6 @@ TEST_P(RtpSenderTest, WritesNetwork2ToTimingExtensionWithoutPacer) { } TEST_P(RtpSenderTest, TrafficSmoothingWithExtensions) { - EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, - kSsrc, kSeqNum, _, _, _)); EXPECT_CALL(mock_rtc_event_log_, LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))); @@ -804,18 +859,32 @@ TEST_P(RtpSenderTest, TrafficSmoothingWithExtensions) { BuildRtpPacket(kPayload, kMarkerBit, kTimestamp, capture_time_ms); size_t packet_size = packet->size(); - // Packet should be stored in a send bucket. - EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), - kAllowRetransmission, - RtpPacketSender::kNormalPriority)); - - EXPECT_EQ(0, transport_.packets_sent()); - const int kStoredTimeInMs = 100; - fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); - - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, - PacedPacketInfo()); + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, + InsertPacket(RtpPacketSender::kNormalPriority, kSsrc, kSeqNum, + _, _, _)); + // Packet should be stored in a send bucket. + EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), + kAllowRetransmission, + RtpPacketSender::kNormalPriority)); + EXPECT_EQ(0, transport_.packets_sent()); + fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, + PacedPacketInfo()); + } else { + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), + Pointee(Property(&RtpPacketToSend::SequenceNumber, kSeqNum))))); + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + EXPECT_TRUE(rtp_sender_->SendToNetwork( + absl::make_unique(*packet), kAllowRetransmission)); + EXPECT_EQ(0, transport_.packets_sent()); + fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); + } // Process send bucket. Packet should now be sent. EXPECT_EQ(1, transport_.packets_sent()); @@ -832,8 +901,6 @@ TEST_P(RtpSenderTest, TrafficSmoothingWithExtensions) { } TEST_P(RtpSenderTest, TrafficSmoothingRetransmits) { - EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, - kSsrc, kSeqNum, _, _, _)); EXPECT_CALL(mock_rtc_event_log_, LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))); @@ -850,28 +917,60 @@ TEST_P(RtpSenderTest, TrafficSmoothingRetransmits) { size_t packet_size = packet->size(); // Packet should be stored in a send bucket. - EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), - kAllowRetransmission, - RtpPacketSender::kNormalPriority)); - // Immediately process send bucket and send packet. - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, - PacedPacketInfo()); + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, + InsertPacket(RtpPacketSender::kNormalPriority, kSsrc, kSeqNum, + _, _, _)); + EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), + kAllowRetransmission, + RtpPacketSender::kNormalPriority)); + // Immediately process send bucket and send packet. + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, + PacedPacketInfo()); + } else { + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), + Pointee(Property(&RtpPacketToSend::SequenceNumber, kSeqNum))))); + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + packet->set_allow_retransmission(true); + EXPECT_TRUE(rtp_sender_->SendToNetwork( + absl::make_unique(*packet), kAllowRetransmission)); + // Immediately process send bucket and send packet. + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); + } + EXPECT_EQ(1, transport_.packets_sent()); // Retransmit packet. const int kStoredTimeInMs = 100; fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); - EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, - kSsrc, kSeqNum, _, _, _)); EXPECT_CALL(mock_rtc_event_log_, LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))); - - EXPECT_EQ(static_cast(packet_size), rtp_sender_->ReSendPacket(kSeqNum)); - EXPECT_EQ(1, transport_.packets_sent()); - - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, - PacedPacketInfo()); + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, + InsertPacket(RtpPacketSender::kNormalPriority, kSsrc, kSeqNum, + _, _, _)); + EXPECT_EQ(static_cast(packet_size), + rtp_sender_->ReSendPacket(kSeqNum)); + EXPECT_EQ(1, transport_.packets_sent()); + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, true, + PacedPacketInfo()); + } else { + packet->set_packet_type(RtpPacketToSend::Type::kRetransmission); + packet->set_retransmitted_sequence_number(kSeqNum); + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), + Pointee(Property(&RtpPacketToSend::SequenceNumber, kSeqNum))))); + EXPECT_EQ(static_cast(packet_size), + rtp_sender_->ReSendPacket(kSeqNum)); + EXPECT_EQ(1, transport_.packets_sent()); + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); + } // Process send bucket. Packet should now be sent. EXPECT_EQ(2, transport_.packets_sent()); @@ -891,8 +990,6 @@ TEST_P(RtpSenderTest, TrafficSmoothingRetransmits) { // 1 more regular packet. TEST_P(RtpSenderTest, SendPadding) { // Make all (non-padding) packets go to send queue. - EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, - kSsrc, kSeqNum, _, _, _)); EXPECT_CALL(mock_rtc_event_log_, LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) .Times(1 + 4 + 1); @@ -918,19 +1015,37 @@ TEST_P(RtpSenderTest, SendPadding) { BuildRtpPacket(kPayload, kMarkerBit, timestamp, capture_time_ms); const uint32_t media_packet_timestamp = timestamp; size_t packet_size = packet->size(); + int total_packets_sent = 0; + const int kStoredTimeInMs = 100; // Packet should be stored in a send bucket. - EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), - kAllowRetransmission, - RtpPacketSender::kNormalPriority)); + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, + InsertPacket(RtpPacketSender::kNormalPriority, kSsrc, kSeqNum, + _, _, _)); + EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), + kAllowRetransmission, + RtpPacketSender::kNormalPriority)); + EXPECT_EQ(total_packets_sent, transport_.packets_sent()); + fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); + rtp_sender_->TimeToSendPacket(kSsrc, seq_num++, capture_time_ms, false, + PacedPacketInfo()); + } else { + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), + Pointee(Property(&RtpPacketToSend::SequenceNumber, kSeqNum))))); + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + packet->set_allow_retransmission(true); + EXPECT_TRUE(rtp_sender_->SendToNetwork( + absl::make_unique(*packet), kAllowRetransmission)); + EXPECT_EQ(total_packets_sent, transport_.packets_sent()); + fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); + ++seq_num; + } - int total_packets_sent = 0; - EXPECT_EQ(total_packets_sent, transport_.packets_sent()); - - const int kStoredTimeInMs = 100; - fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); - rtp_sender_->TimeToSendPacket(kSsrc, seq_num++, capture_time_ms, false, - PacedPacketInfo()); // Packet should now be sent. This test doesn't verify the regular video // packet, since it is tested in another test. EXPECT_EQ(++total_packets_sent, transport_.packets_sent()); @@ -972,16 +1087,28 @@ TEST_P(RtpSenderTest, SendPadding) { packet = BuildRtpPacket(kPayload, kMarkerBit, timestamp, capture_time_ms); packet_size = packet->size(); - EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority, - kSsrc, seq_num, _, _, _)); + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, + InsertPacket(RtpPacketSender::kNormalPriority, kSsrc, seq_num, + _, _, _)); + // Packet should be stored in a send bucket. + EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), + kAllowRetransmission, + RtpPacketSender::kNormalPriority)); + rtp_sender_->TimeToSendPacket(kSsrc, seq_num, capture_time_ms, false, + PacedPacketInfo()); + } else { + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), + Pointee(Property(&RtpPacketToSend::SequenceNumber, seq_num))))); + EXPECT_TRUE(rtp_sender_->SendToNetwork( + absl::make_unique(*packet), kAllowRetransmission)); + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); + } - // Packet should be stored in a send bucket. - EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), - kAllowRetransmission, - RtpPacketSender::kNormalPriority)); - - rtp_sender_->TimeToSendPacket(kSsrc, seq_num, capture_time_ms, false, - PacedPacketInfo()); // Process send bucket. EXPECT_EQ(++total_packets_sent, transport_.packets_sent()); EXPECT_EQ(packet_size, transport_.last_sent_packet().size()); @@ -1006,16 +1133,28 @@ TEST_P(RtpSenderTest, OnSendPacketUpdated) { EXPECT_CALL(send_packet_observer_, OnSendPacket(kTransportSequenceNumber, _, _)) .Times(1); - EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) - .WillOnce(Return(kTransportSequenceNumber)); - EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)) - .Times(1); - SendGenericPacket(); // Packet passed to pacer. - const bool kIsRetransmit = false; - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, - fake_clock_.TimeInMilliseconds(), kIsRetransmit, - PacedPacketInfo()); + if (GetParam().pacer_references_packets) { + const bool kIsRetransmit = false; + EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)); + SendGenericPacket(); // Packet passed to pacer. + EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) + .WillOnce(::testing::Return(kTransportSequenceNumber)); + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, + fake_clock_.TimeInMilliseconds(), + kIsRetransmit, PacedPacketInfo()); + } else { + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), + Pointee(Property(&RtpPacketToSend::SequenceNumber, kSeqNum))))); + auto packet = SendGenericPacket(); + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + packet->SetExtension(kTransportSequenceNumber); + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); + } + EXPECT_EQ(1, transport_.packets_sent()); } @@ -1026,21 +1165,41 @@ TEST_P(RtpSenderTest, OnSendPacketNotUpdatedForRetransmits) { rtp_sender_->SetStorePacketsStatus(true, 10); EXPECT_CALL(send_packet_observer_, OnSendPacket(_, _, _)).Times(0); - EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) - .WillOnce(Return(kTransportSequenceNumber)); - EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)) - .Times(1); - SendGenericPacket(); // Packet passed to pacer. - const bool kIsRetransmit = true; - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, - fake_clock_.TimeInMilliseconds(), kIsRetransmit, - PacedPacketInfo()); + if (GetParam().pacer_references_packets) { + const bool kIsRetransmit = true; + EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)); + SendGenericPacket(); // Packet passed to pacer. + EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) + .WillOnce(Return(kTransportSequenceNumber)); + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, + fake_clock_.TimeInMilliseconds(), + kIsRetransmit, PacedPacketInfo()); + } else { + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), + Pointee(Property(&RtpPacketToSend::SequenceNumber, kSeqNum))))); + auto packet = SendGenericPacket(); + packet->set_packet_type(RtpPacketToSend::Type::kRetransmission); + packet->SetExtension(kTransportSequenceNumber); + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); + } + EXPECT_EQ(1, transport_.packets_sent()); EXPECT_TRUE(transport_.last_options_.is_retransmit); } TEST_P(RtpSenderTest, OnSendPacketNotUpdatedWithoutSeqNumAllocator) { + if (!GetParam().pacer_references_packets) { + // When PacedSender owns packets, there is no + // TransportSequenceNumberAllocator callback, so this test does not make any + // sense. + // TODO(bugs.webrtc.org/10633): Remove this test once old code is gone. + return; + } + RtpRtcp::Configuration config; config.clock = &fake_clock_; config.outgoing_transport = &transport_; @@ -1058,20 +1217,26 @@ TEST_P(RtpSenderTest, OnSendPacketNotUpdatedWithoutSeqNumAllocator) { rtp_sender_->SetStorePacketsStatus(true, 10); EXPECT_CALL(send_packet_observer_, OnSendPacket(_, _, _)).Times(0); - EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)) - .Times(1); - SendGenericPacket(); // Packet passed to pacer. const bool kIsRetransmit = false; + EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, kSeqNum, _, _, _)); + SendGenericPacket(); // Packet passed to pacer. rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, fake_clock_.TimeInMilliseconds(), kIsRetransmit, PacedPacketInfo()); + EXPECT_EQ(1, transport_.packets_sent()); } // TODO(bugs.webrtc.org/8975): Remove this test when non-useful padding is // removed. TEST_P(RtpSenderTest, SendRedundantPayloads) { + if (!GetParam().pacer_references_packets) { + // If PacedSender owns the RTP packets, GeneratePadding() family of methods + // will be called instead and this test makes no sense. + return; + } + test::ScopedFieldTrials field_trials( "WebRTC-PayloadPadding-UseMostUsefulPacket/Disabled/"); MockTransport transport; @@ -1103,9 +1268,6 @@ TEST_P(RtpSenderTest, SendRedundantPayloads) { const size_t kPayloadSizes[kNumPayloadSizes] = {500, 550, 600, 650, 700, 750, 800, 850, 900, 950}; // Expect all packets go through the pacer. - EXPECT_CALL(mock_paced_sender_, - InsertPacket(RtpPacketSender::kNormalPriority, kSsrc, _, _, _, _)) - .Times(kNumPayloadSizes); EXPECT_CALL(mock_rtc_event_log_, LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) .Times(kNumPayloadSizes); @@ -1113,10 +1275,27 @@ TEST_P(RtpSenderTest, SendRedundantPayloads) { // Send 10 packets of increasing size. for (size_t i = 0; i < kNumPayloadSizes; ++i) { int64_t capture_time_ms = fake_clock_.TimeInMilliseconds(); - EXPECT_CALL(transport, SendRtp(_, _, _)).WillOnce(Return(true)); - SendPacket(capture_time_ms, kPayloadSizes[i]); - rtp_sender_->TimeToSendPacket(kSsrc, seq_num++, capture_time_ms, false, - PacedPacketInfo()); + + EXPECT_CALL(transport, SendRtp(_, _, _)).WillOnce(::testing::Return(true)); + + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, seq_num, _, _, _)); + SendPacket(capture_time_ms, kPayloadSizes[i]); + rtp_sender_->TimeToSendPacket(kSsrc, seq_num, + fake_clock_.TimeInMilliseconds(), false, + PacedPacketInfo()); + } else { + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), + Pointee(Property(&RtpPacketToSend::SequenceNumber, seq_num))))); + auto packet = SendPacket(capture_time_ms, kPayloadSizes[i]); + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); + } + + ++seq_num; fake_clock_.AdvanceTimeMilliseconds(33); } @@ -1153,6 +1332,12 @@ TEST_P(RtpSenderTest, SendRedundantPayloads) { } TEST_P(RtpSenderTest, SendRedundantPayloadsUsefulPadding) { + if (!GetParam().pacer_references_packets) { + // If PacedSender owns the RTP packets, GeneratePadding() family of methods + // will be called instead and this test makes no sense. + return; + } + test::ScopedFieldTrials field_trials( "WebRTC-PayloadPadding-UseMostUsefulPacket/Enabled/"); MockTransport transport; @@ -1184,22 +1369,34 @@ TEST_P(RtpSenderTest, SendRedundantPayloadsUsefulPadding) { const size_t kPayloadSizes[kNumPayloadSizes] = {500, 550, 600, 650, 700, 750, 800, 850, 900, 950}; // Expect all packets go through the pacer. - EXPECT_CALL(mock_paced_sender_, - InsertPacket(RtpPacketSender::kNormalPriority, kSsrc, _, _, _, _)) - .Times(kNumPayloadSizes); EXPECT_CALL(mock_rtc_event_log_, LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) .Times(kNumPayloadSizes); // Send 10 packets of increasing size. - EXPECT_CALL(transport, SendRtp) - .Times(kNumPayloadSizes) - .WillRepeatedly(Return(true)); for (size_t i = 0; i < kNumPayloadSizes; ++i) { int64_t capture_time_ms = fake_clock_.TimeInMilliseconds(); - SendPacket(capture_time_ms, kPayloadSizes[i]); - rtp_sender_->TimeToSendPacket(kSsrc, seq_num++, capture_time_ms, false, - PacedPacketInfo()); + + EXPECT_CALL(transport, SendRtp(_, _, _)).WillOnce(::testing::Return(true)); + + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, InsertPacket(_, kSsrc, seq_num, _, _, _)); + SendPacket(capture_time_ms, kPayloadSizes[i]); + rtp_sender_->TimeToSendPacket(kSsrc, seq_num, + fake_clock_.TimeInMilliseconds(), false, + PacedPacketInfo()); + } else { + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), + Pointee(Property(&RtpPacketToSend::SequenceNumber, seq_num))))); + auto packet = SendPacket(capture_time_ms, kPayloadSizes[i]); + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); + } + + ++seq_num; fake_clock_.AdvanceTimeMilliseconds(33); } @@ -1341,30 +1538,60 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { params.fec_mask_type = kFecMaskRandom; rtp_sender_video.SetFecParameters(params, params); - EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority, - kSsrc, kSeqNum, _, _, false)); uint16_t flexfec_seq_num; - EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority, - kFlexFecSsrc, _, _, _, false)) - .WillOnce(SaveArg<2>(&flexfec_seq_num)); - RTPVideoHeader video_header; - EXPECT_TRUE(rtp_sender_video.SendVideo( - VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp, - fake_clock_.TimeInMilliseconds(), kPayloadData, sizeof(kPayloadData), - nullptr, &video_header, kDefaultExpectedRetransmissionTimeMs)); - EXPECT_CALL(mock_rtc_event_log_, - LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) - .Times(2); - EXPECT_EQ(RtpPacketSendResult::kSuccess, - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, - fake_clock_.TimeInMilliseconds(), - false, PacedPacketInfo())); - EXPECT_EQ(RtpPacketSendResult::kSuccess, - rtp_sender_->TimeToSendPacket(kFlexFecSsrc, flexfec_seq_num, - fake_clock_.TimeInMilliseconds(), - false, PacedPacketInfo())); + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority, + kSsrc, kSeqNum, _, _, false)); + EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority, + kFlexFecSsrc, _, _, _, false)) + .WillOnce(::testing::SaveArg<2>(&flexfec_seq_num)); + + EXPECT_TRUE(rtp_sender_video.SendVideo( + VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp, + fake_clock_.TimeInMilliseconds(), kPayloadData, sizeof(kPayloadData), + nullptr, &video_header, kDefaultExpectedRetransmissionTimeMs)); + + EXPECT_EQ(RtpPacketSendResult::kSuccess, + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, + fake_clock_.TimeInMilliseconds(), + false, PacedPacketInfo())); + EXPECT_EQ(RtpPacketSendResult::kSuccess, + rtp_sender_->TimeToSendPacket(kFlexFecSsrc, flexfec_seq_num, + fake_clock_.TimeInMilliseconds(), + false, PacedPacketInfo())); + } else { + std::unique_ptr media_packet; + std::unique_ptr fec_packet; + + EXPECT_CALL(mock_paced_sender_, EnqueuePacket) + .Times(2) + .WillRepeatedly([&](std::unique_ptr packet) { + if (packet->packet_type() == RtpPacketToSend::Type::kVideo) { + EXPECT_EQ(packet->Ssrc(), kSsrc); + EXPECT_EQ(packet->SequenceNumber(), kSeqNum); + media_packet = std::move(packet); + } else { + EXPECT_EQ(packet->packet_type(), + RtpPacketToSend::Type::kForwardErrorCorrection); + EXPECT_EQ(packet->Ssrc(), kFlexFecSsrc); + fec_packet = std::move(packet); + } + }); + + EXPECT_TRUE(rtp_sender_video.SendVideo( + VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp, + fake_clock_.TimeInMilliseconds(), kPayloadData, sizeof(kPayloadData), + nullptr, &video_header, kDefaultExpectedRetransmissionTimeMs)); + ASSERT_TRUE(media_packet != nullptr); + ASSERT_TRUE(fec_packet != nullptr); + + flexfec_seq_num = fec_packet->SequenceNumber(); + rtp_sender_->TrySendPacket(media_packet.get(), PacedPacketInfo()); + rtp_sender_->TrySendPacket(fec_packet.get(), PacedPacketInfo()); + } + ASSERT_EQ(2, transport_.packets_sent()); const RtpPacketReceived& media_packet = transport_.sent_packets_[0]; EXPECT_EQ(kMediaPayloadType, media_packet.PayloadType()); @@ -1376,14 +1603,180 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { EXPECT_EQ(kFlexFecSsrc, flexfec_packet.Ssrc()); } +// TODO(ilnik): because of webrtc:7859. Once FEC moved below pacer, this test +// should be removed. +TEST_P(RtpSenderTest, NoFlexfecForTimingFrames) { + constexpr uint32_t kTimestamp = 1234; + const int64_t kCaptureTimeMs = fake_clock_.TimeInMilliseconds(); + constexpr int kMediaPayloadType = 127; + constexpr int kFlexfecPayloadType = 118; + const std::vector kNoRtpExtensions; + const std::vector kNoRtpExtensionSizes; + + FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexFecSsrc, kSsrc, kNoMid, + kNoRtpExtensions, kNoRtpExtensionSizes, + nullptr /* rtp_state */, &fake_clock_); + + // Reset |rtp_sender_| to use FlexFEC. + rtp_sender_.reset(new RTPSender( + false, &fake_clock_, &transport_, &mock_paced_sender_, + flexfec_sender.ssrc(), &seq_num_allocator_, nullptr, nullptr, nullptr, + &mock_rtc_event_log_, &send_packet_observer_, + &retransmission_rate_limiter_, nullptr, false, nullptr, false, false, + FieldTrialBasedConfig())); + rtp_sender_->SetSSRC(kSsrc); + rtp_sender_->SetSequenceNumber(kSeqNum); + rtp_sender_->SetStorePacketsStatus(true, 10); + + PlayoutDelayOracle playout_delay_oracle; + RTPSenderVideo rtp_sender_video( + &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle, + nullptr, false, false, FieldTrialBasedConfig()); + rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC", + /*raw_payload=*/false); + + // Need extension to be registered for timing frames to be sent. + ASSERT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( + kRtpExtensionVideoTiming, kVideoTimingExtensionId)); + + // Parameters selected to generate a single FEC packet per media packet. + FecProtectionParams params; + params.fec_rate = 15; + params.max_fec_frames = 1; + params.fec_mask_type = kFecMaskRandom; + rtp_sender_video.SetFecParameters(params, params); + + RTPVideoHeader video_header; + video_header.video_timing.flags = VideoSendTiming::kTriggeredByTimer; + + EXPECT_CALL(mock_rtc_event_log_, + LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) + .Times(1); + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority, + kSsrc, kSeqNum, _, _, false)); + EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority, + kFlexFecSsrc, _, _, _, false)) + .Times(0); // Not called because packet should not be protected. + + EXPECT_TRUE(rtp_sender_video.SendVideo( + VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp, + kCaptureTimeMs, kPayloadData, sizeof(kPayloadData), nullptr, + &video_header, kDefaultExpectedRetransmissionTimeMs)); + + EXPECT_EQ(RtpPacketSendResult::kSuccess, + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, + fake_clock_.TimeInMilliseconds(), + false, PacedPacketInfo())); + } else { + std::unique_ptr rtp_packet; + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(AllOf( + Pointee(Property(&RtpPacketToSend::Ssrc, kSsrc)), + Pointee(Property(&RtpPacketToSend::SequenceNumber, kSeqNum))))) + .WillOnce([&rtp_packet](std::unique_ptr packet) { + rtp_packet = std::move(packet); + }); + + EXPECT_CALL( + mock_paced_sender_, + EnqueuePacket(Pointee(Property(&RtpPacketToSend::Ssrc, kFlexFecSsrc)))) + .Times(0); // Not called because packet should not be protected. + + EXPECT_TRUE(rtp_sender_video.SendVideo( + VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp, + kCaptureTimeMs, kPayloadData, sizeof(kPayloadData), nullptr, + &video_header, kDefaultExpectedRetransmissionTimeMs)); + + EXPECT_TRUE( + rtp_sender_->TrySendPacket(rtp_packet.get(), PacedPacketInfo())); + } + + ASSERT_EQ(1, transport_.packets_sent()); + const RtpPacketReceived& media_packet = transport_.sent_packets_[0]; + EXPECT_EQ(kMediaPayloadType, media_packet.PayloadType()); + EXPECT_EQ(kSeqNum, media_packet.SequenceNumber()); + EXPECT_EQ(kSsrc, media_packet.Ssrc()); + + // Now try to send not a timing frame. + uint16_t flexfec_seq_num; + + EXPECT_CALL(mock_rtc_event_log_, + LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) + .Times(2); + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority, + kFlexFecSsrc, _, _, _, false)) + .WillOnce(::testing::SaveArg<2>(&flexfec_seq_num)); + EXPECT_CALL(mock_paced_sender_, + InsertPacket(RtpPacketSender::kLowPriority, kSsrc, kSeqNum + 1, + _, _, false)); + video_header.video_timing.flags = VideoSendTiming::kInvalid; + EXPECT_TRUE(rtp_sender_video.SendVideo( + VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp + 1, + kCaptureTimeMs + 1, kPayloadData, sizeof(kPayloadData), nullptr, + &video_header, kDefaultExpectedRetransmissionTimeMs)); + + EXPECT_EQ(RtpPacketSendResult::kSuccess, + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum + 1, + fake_clock_.TimeInMilliseconds(), + false, PacedPacketInfo())); + EXPECT_EQ(RtpPacketSendResult::kSuccess, + rtp_sender_->TimeToSendPacket(kFlexFecSsrc, flexfec_seq_num, + fake_clock_.TimeInMilliseconds(), + false, PacedPacketInfo())); + } else { + std::unique_ptr media_packet; + std::unique_ptr fec_packet; + + EXPECT_CALL(mock_paced_sender_, EnqueuePacket) + .Times(2) + .WillRepeatedly([&](std::unique_ptr packet) { + if (packet->packet_type() == RtpPacketToSend::Type::kVideo) { + EXPECT_EQ(packet->Ssrc(), kSsrc); + EXPECT_EQ(packet->SequenceNumber(), kSeqNum + 1); + media_packet = std::move(packet); + } else { + EXPECT_EQ(packet->packet_type(), + RtpPacketToSend::Type::kForwardErrorCorrection); + EXPECT_EQ(packet->Ssrc(), kFlexFecSsrc); + fec_packet = std::move(packet); + } + }); + + video_header.video_timing.flags = VideoSendTiming::kInvalid; + EXPECT_TRUE(rtp_sender_video.SendVideo( + VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp + 1, + kCaptureTimeMs + 1, kPayloadData, sizeof(kPayloadData), nullptr, + &video_header, kDefaultExpectedRetransmissionTimeMs)); + + ASSERT_TRUE(media_packet != nullptr); + ASSERT_TRUE(fec_packet != nullptr); + + flexfec_seq_num = fec_packet->SequenceNumber(); + rtp_sender_->TrySendPacket(media_packet.get(), PacedPacketInfo()); + rtp_sender_->TrySendPacket(fec_packet.get(), PacedPacketInfo()); + } + + ASSERT_EQ(3, transport_.packets_sent()); + const RtpPacketReceived& media_packet2 = transport_.sent_packets_[1]; + EXPECT_EQ(kMediaPayloadType, media_packet2.PayloadType()); + EXPECT_EQ(kSeqNum + 1, media_packet2.SequenceNumber()); + EXPECT_EQ(kSsrc, media_packet2.Ssrc()); + const RtpPacketReceived& flexfec_packet = transport_.sent_packets_[2]; + EXPECT_EQ(kFlexfecPayloadType, flexfec_packet.PayloadType()); + EXPECT_EQ(flexfec_seq_num, flexfec_packet.SequenceNumber()); + EXPECT_EQ(kFlexFecSsrc, flexfec_packet.Ssrc()); +} + TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) { constexpr uint32_t kTimestamp = 1234; constexpr int kMediaPayloadType = 127; constexpr int kFlexfecPayloadType = 118; - constexpr uint32_t kFlexfecSsrc = 5678; const std::vector kNoRtpExtensions; const std::vector kNoRtpExtensionSizes; - FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexfecSsrc, kSsrc, kNoMid, + FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexFecSsrc, kSsrc, kNoMid, kNoRtpExtensions, kNoRtpExtensionSizes, nullptr /* rtp_state */, &fake_clock_); @@ -1430,7 +1823,7 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) { EXPECT_EQ(kSsrc, media_packet.Ssrc()); const RtpPacketReceived& flexfec_packet = transport_.sent_packets_[1]; EXPECT_EQ(kFlexfecPayloadType, flexfec_packet.PayloadType()); - EXPECT_EQ(kFlexfecSsrc, flexfec_packet.Ssrc()); + EXPECT_EQ(kFlexFecSsrc, flexfec_packet.Ssrc()); } // Test that the MID header extension is included on sent packets when @@ -1550,8 +1943,13 @@ TEST_P(RtpSenderTest, FecOverheadRate) { constexpr size_t kNumMediaPackets = 10; constexpr size_t kNumFecPackets = kNumMediaPackets; constexpr int64_t kTimeBetweenPacketsMs = 10; - EXPECT_CALL(mock_paced_sender_, InsertPacket(_, _, _, _, _, false)) - .Times(kNumMediaPackets + kNumFecPackets); + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, InsertPacket(_, _, _, _, _, false)) + .Times(kNumMediaPackets + kNumFecPackets); + } else { + EXPECT_CALL(mock_paced_sender_, EnqueuePacket) + .Times(kNumMediaPackets + kNumFecPackets); + } for (size_t i = 0; i < kNumMediaPackets; ++i) { RTPVideoHeader video_header; @@ -2166,26 +2564,33 @@ TEST_P(RtpSenderTest, GeneratePaddingResendsOldPacketsWithRtx) { // Generated padding has large enough budget that the video packet should be // retransmitted as padding. - EXPECT_CALL(mock_paced_sender_, - EnqueuePacket(AllOf( - Pointee(Property(&RtpPacketToSend::packet_type, - RtpPacketToSend::Type::kPadding)), - Pointee(Property(&RtpPacketToSend::Ssrc, kRtxSsrc)), - Pointee(Property(&RtpPacketToSend::payload_size, - kPayloadPacketSize + kRtxHeaderSize))))) - .Times(1); - rtp_sender_->GeneratePadding(kPayloadPacketSize + kRtxHeaderSize); + std::vector> generated_packets = + rtp_sender_->GeneratePadding(kPayloadPacketSize + kRtxHeaderSize); + ASSERT_EQ(generated_packets.size(), 1u); + auto& padding_packet = generated_packets.front(); + EXPECT_EQ(padding_packet->packet_type(), RtpPacketToSend::Type::kPadding); + EXPECT_EQ(padding_packet->Ssrc(), kRtxSsrc); + EXPECT_EQ(padding_packet->payload_size(), + kPayloadPacketSize + kRtxHeaderSize); // Not enough budged for payload padding, use plain padding instead. - EXPECT_CALL(mock_paced_sender_, - EnqueuePacket(AllOf( - Pointee(Property(&RtpPacketToSend::packet_type, - RtpPacketToSend::Type::kPadding)), - Pointee(Property(&RtpPacketToSend::Ssrc, kRtxSsrc)), - Pointee(Property(&RtpPacketToSend::payload_size, 0)), - Pointee(Property(&RtpPacketToSend::padding_size, Gt(0u)))))) - .Times((kPayloadPacketSize + kMaxPaddingSize - 1) / kMaxPaddingSize); - rtp_sender_->GeneratePadding(kPayloadPacketSize + kRtxHeaderSize - 1); + const size_t kPaddingBytesRequested = kPayloadPacketSize + kRtxHeaderSize - 1; + const size_t kExpectedNumPaddingPackets = + (kPaddingBytesRequested + kMaxPaddingSize - 1) / kMaxPaddingSize; + + size_t padding_bytes_generated = 0; + generated_packets = rtp_sender_->GeneratePadding(kPaddingBytesRequested); + EXPECT_EQ(generated_packets.size(), kExpectedNumPaddingPackets); + for (auto& packet : generated_packets) { + EXPECT_EQ(packet->packet_type(), RtpPacketToSend::Type::kPadding); + EXPECT_EQ(packet->Ssrc(), kRtxSsrc); + EXPECT_EQ(packet->payload_size(), 0u); + EXPECT_GT(packet->padding_size(), 0u); + padding_bytes_generated += packet->padding_size(); + } + + EXPECT_EQ(padding_bytes_generated, + kExpectedNumPaddingPackets * kMaxPaddingSize); } TEST_P(RtpSenderTest, GeneratePaddingCreatesPurePaddingWithoutRtx) { @@ -2209,24 +2614,32 @@ TEST_P(RtpSenderTest, GeneratePaddingCreatesPurePaddingWithoutRtx) { const size_t kPaddingBytesRequested = kPayloadPacketSize + kRtxHeaderSize; const size_t kExpectedNumPaddingPackets = (kPaddingBytesRequested + kMaxPaddingSize - 1) / kMaxPaddingSize; - size_t padding_bytes_sent = 0; - EXPECT_CALL(mock_paced_sender_, EnqueuePacket) - .WillRepeatedly([&](std::unique_ptr packet) { - EXPECT_EQ(packet->packet_type(), RtpPacketToSend::Type::kPadding); - EXPECT_EQ(packet->Ssrc(), kSsrc); - EXPECT_EQ(packet->payload_size(), 0u); - EXPECT_GT(packet->padding_size(), 0u); - padding_bytes_sent += packet->padding_size(); - }); - rtp_sender_->GeneratePadding(kPaddingBytesRequested); - EXPECT_EQ(padding_bytes_sent, kExpectedNumPaddingPackets * kMaxPaddingSize); + size_t padding_bytes_generated = 0; + std::vector> padding_packets = + rtp_sender_->GeneratePadding(kPaddingBytesRequested); + EXPECT_EQ(padding_packets.size(), kExpectedNumPaddingPackets); + for (auto& packet : padding_packets) { + EXPECT_EQ(packet->packet_type(), RtpPacketToSend::Type::kPadding); + EXPECT_EQ(packet->Ssrc(), kSsrc); + EXPECT_EQ(packet->payload_size(), 0u); + EXPECT_GT(packet->padding_size(), 0u); + padding_bytes_generated += packet->padding_size(); + } + + EXPECT_EQ(padding_bytes_generated, + kExpectedNumPaddingPackets * kMaxPaddingSize); } INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderTest, - ::testing::Bool()); + ::testing::Values(TestConfig{false, false}, + TestConfig{false, true}, + TestConfig{true, false}, + TestConfig{true, true})); + INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderTestWithoutPacer, - ::testing::Bool()); + ::testing::Values(TestConfig{false, false}, + TestConfig{true, false})); } // namespace webrtc