diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index 665b070339..83a8da3e84 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -78,11 +78,6 @@ void PacedSender::UpdateOutstandingData(DataSize outstanding_data) { pacing_controller_.UpdateOutstandingData(outstanding_data); } -void PacedSender::SetProbingEnabled(bool enabled) { - rtc::CritScope cs(&critsect_); - pacing_controller_.SetProbingEnabled(enabled); -} - void PacedSender::SetPacingRates(DataRate pacing_rate, DataRate padding_rate) { rtc::CritScope cs(&critsect_); pacing_controller_.SetPacingRates(pacing_rate, padding_rate); @@ -186,29 +181,4 @@ std::vector> PacedSender::GeneratePadding( critsect_.Enter(); return padding_packets; } - -RtpPacketSendResult PacedSender::TimeToSendPacket( - uint32_t ssrc, - uint16_t sequence_number, - int64_t capture_timestamp, - bool retransmission, - const PacedPacketInfo& packet_info) { - RtpPacketSendResult result; - critsect_.Leave(); - result = packet_router_->TimeToSendPacket( - ssrc, sequence_number, capture_timestamp, retransmission, packet_info); - critsect_.Enter(); - return result; -} - -DataSize PacedSender::TimeToSendPadding(DataSize size, - const PacedPacketInfo& pacing_info) { - size_t padding_bytes_sent; - critsect_.Leave(); - padding_bytes_sent = - packet_router_->TimeToSendPadding(size.bytes(), pacing_info); - critsect_.Enter(); - return DataSize::bytes(padding_bytes_sent); -} - } // namespace webrtc diff --git a/modules/pacing/paced_sender.h b/modules/pacing/paced_sender.h index 71e826d394..7b14480ed9 100644 --- a/modules/pacing/paced_sender.h +++ b/modules/pacing/paced_sender.h @@ -117,12 +117,6 @@ class PacedSender : public Module, // Below are methods specific to this implementation, such as things related // to module processing thread specifics or methods exposed for test. - // TODO(bugs.webrtc.org/10809): Remove when cleanup up unit tests. - // Enable bitrate probing. Enabled by default, mostly here to simplify - // testing. Must be called before any packets are being sent to have an - // effect. - void SetProbingEnabled(bool enabled); - // Methods implementing Module. // Returns the number of milliseconds until the module want a worker thread @@ -145,17 +139,6 @@ class PacedSender : public Module, std::vector> GeneratePadding( DataSize size) override RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_); - // TODO(bugs.webrtc.org/10633): Remove these when old code path is gone. - RtpPacketSendResult TimeToSendPacket(uint32_t ssrc, - uint16_t sequence_number, - int64_t capture_timestamp, - bool retransmission, - const PacedPacketInfo& packet_info) - override RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_); - DataSize TimeToSendPadding(DataSize size, - const PacedPacketInfo& pacing_info) override - RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_); - rtc::CriticalSection critsect_; PacingController pacing_controller_ RTC_GUARDED_BY(critsect_); diff --git a/modules/pacing/paced_sender_unittest.cc b/modules/pacing/paced_sender_unittest.cc index c49d3700aa..8c5761d206 100644 --- a/modules/pacing/paced_sender_unittest.cc +++ b/modules/pacing/paced_sender_unittest.cc @@ -28,7 +28,6 @@ using ::testing::_; using ::testing::Return; namespace { -static const int kTargetBitrateBps = 800000; constexpr uint32_t kAudioSsrc = 12345; constexpr uint32_t kVideoSsrc = 234565; constexpr uint32_t kVideoRtxSsrc = 34567; @@ -83,47 +82,6 @@ std::unique_ptr BuildRtpPacket(RtpPacketToSend::Type type) { return packet; } -TEST(PacedSenderTest, PacesPacketsLegacyWay) { - SimulatedClock clock(0); - MockCallback callback; - ScopedFieldTrials field_trials( - "WebRTC-Pacer-LegacyPacketReferencing/Enabled/"); - PacedSender pacer(&clock, &callback, nullptr, nullptr); - - // Insert a number of packets over one second. - static constexpr size_t kPacketsToSend = 42; - pacer.SetPacingRates(DataRate::bps(kDefaultPacketSize * 8 * kPacketsToSend), - DataRate::Zero()); - for (size_t i = 0; i < kPacketsToSend; ++i) { - pacer.InsertPacket(RtpPacketSender::Priority::kNormalPriority, kVideoSsrc, - i, clock.TimeInMilliseconds(), kDefaultPacketSize, - false); - } - - // Expect all of them to be sent. - size_t packets_sent = 0; - clock.AdvanceTimeMilliseconds(pacer.TimeUntilNextProcess()); - EXPECT_CALL(callback, TimeToSendPacket) - .WillRepeatedly([&](uint32_t ssrc, uint16_t sequence_number, - int64_t capture_time_ms, bool retransmission, - const PacedPacketInfo& pacing_info) { - ++packets_sent; - return RtpPacketSendResult::kSuccess; - }); - - const Timestamp start_time = clock.CurrentTime(); - - while (packets_sent < kPacketsToSend) { - clock.AdvanceTimeMilliseconds(pacer.TimeUntilNextProcess()); - pacer.Process(); - } - - // Packets should be sent over a period of close to 1s. Expect a little lower - // than this since initial probing is a bit quicker. - TimeDelta duration = clock.CurrentTime() - start_time; - EXPECT_GT(duration, TimeDelta::ms(900)); -} - TEST(PacedSenderTest, PacesPackets) { SimulatedClock clock(0); MockCallback callback; @@ -158,49 +116,5 @@ TEST(PacedSenderTest, PacesPackets) { EXPECT_GT(duration, TimeDelta::ms(900)); } -TEST(PacedSenderTest, AvoidBusyLoopOnSendFailure) { - // This test only makes sense for legacy packet referencing mode, since we - // don't handle send failure and more. - - ScopedFieldTrials field_trials( - "WebRTC-Pacer-LegacyPacketReferencing/Enabled/"); - MockCallback callback; - SimulatedClock clock(0); - PacedSender pacer(&clock, &callback, nullptr, nullptr); - - // Configure up to full target bitrate of padding. - pacer.SetPacingRates(DataRate::bps(kTargetBitrateBps), - DataRate::bps(kTargetBitrateBps)); - - // Insert a number of packets, covering the initial probe. - static constexpr size_t kPacketsToSend = 8; - for (size_t i = 0; i < kPacketsToSend; ++i) { - pacer.EnqueuePacket(BuildRtpPacket(RtpPacketToSend::Type::kVideo)); - } - - // Expect all of them to be sent. - size_t packets_sent = 0; - clock.AdvanceTimeMilliseconds(pacer.TimeUntilNextProcess()); - EXPECT_CALL(callback, SendPacket) - .WillRepeatedly( - [&](std::unique_ptr packet, - const PacedPacketInfo& cluster_info) { ++packets_sent; }); - while (packets_sent < kPacketsToSend) { - clock.AdvanceTimeMilliseconds(pacer.TimeUntilNextProcess()); - pacer.Process(); - } - - // Make sure we have budget for padding. - clock.AdvanceTimeMilliseconds(500); - - // If sending padding fails, wait the standard 5ms until trying again. - EXPECT_CALL(callback, TimeToSendPadding).Times(2).WillRepeatedly(Return(0)); - pacer.Process(); - EXPECT_EQ(5, pacer.TimeUntilNextProcess()); - clock.AdvanceTimeMilliseconds(5); - pacer.Process(); - EXPECT_EQ(5, pacer.TimeUntilNextProcess()); -} - } // namespace test } // namespace webrtc diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index 233a3facf0..3c97163234 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -104,9 +104,7 @@ PacingController::PacingController(Clock* clock, congestion_window_size_(DataSize::PlusInfinity()), outstanding_data_(DataSize::Zero()), queue_time_limit(kMaxExpectedQueueLength), - account_for_audio_(false), - legacy_packet_referencing_( - IsEnabled(*field_trials_, "WebRTC-Pacer-LegacyPacketReferencing")) { + account_for_audio_(false) { if (!drain_large_queues_) { RTC_LOG(LS_WARNING) << "Pacer queues will not be drained," "pushback experiment must be enabled."; @@ -192,29 +190,7 @@ void PacingController::InsertPacket(RtpPacketSender::Priority priority, int64_t capture_time_ms, size_t bytes, bool retransmission) { - RTC_DCHECK(pacing_bitrate_ > DataRate::Zero()) - << "SetPacingRate must be called before InsertPacket."; - - Timestamp now = CurrentTime(); - prober_.OnIncomingPacket(bytes); - - if (capture_time_ms < 0) - capture_time_ms = now.ms(); - - RtpPacketToSend::Type type; - switch (priority) { - case RtpPacketSender::kHighPriority: - type = RtpPacketToSend::Type::kAudio; - break; - case RtpPacketSender::kNormalPriority: - type = RtpPacketToSend::Type::kRetransmission; - break; - default: - type = RtpPacketToSend::Type::kVideo; - } - packet_queue_.Push(GetPriorityForType(type), type, ssrc, sequence_number, - capture_time_ms, now, DataSize::bytes(bytes), - retransmission, packet_counter_++); + RTC_NOTREACHED(); } void PacingController::EnqueuePacket(std::unique_ptr packet) { @@ -316,20 +292,15 @@ void PacingController::ProcessPackets() { Timestamp now = CurrentTime(); TimeDelta elapsed_time = UpdateTimeAndGetElapsed(now); if (ShouldSendKeepalive(now)) { - if (legacy_packet_referencing_) { - OnPaddingSent(packet_sender_->TimeToSendPadding(DataSize::bytes(1), - PacedPacketInfo())); - } else { - DataSize keepalive_data_sent = DataSize::Zero(); - std::vector> keepalive_packets = - packet_sender_->GeneratePadding(DataSize::bytes(1)); - for (auto& packet : keepalive_packets) { - keepalive_data_sent += - DataSize::bytes(packet->payload_size() + packet->padding_size()); - packet_sender_->SendRtpPacket(std::move(packet), PacedPacketInfo()); - } - OnPaddingSent(keepalive_data_sent); + DataSize keepalive_data_sent = DataSize::Zero(); + std::vector> keepalive_packets = + packet_sender_->GeneratePadding(DataSize::bytes(1)); + for (auto& packet : keepalive_packets) { + keepalive_data_sent += + DataSize::bytes(packet->payload_size() + packet->padding_size()); + packet_sender_->SendRtpPacket(std::move(packet), PacedPacketInfo()); } + OnPaddingSent(keepalive_data_sent); } if (paused_) @@ -375,22 +346,19 @@ void PacingController::ProcessPackets() { auto* packet = GetPendingPacket(pacing_info); if (packet == nullptr) { // No packet available to send, check if we should send padding. - if (!legacy_packet_referencing_) { - DataSize padding_to_add = - PaddingToAdd(recommended_probe_size, data_sent); - if (padding_to_add > DataSize::Zero()) { - std::vector> padding_packets = - packet_sender_->GeneratePadding(padding_to_add); - 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; + DataSize padding_to_add = PaddingToAdd(recommended_probe_size, data_sent); + if (padding_to_add > DataSize::Zero()) { + std::vector> padding_packets = + packet_sender_->GeneratePadding(padding_to_add); + 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. @@ -398,55 +366,14 @@ void PacingController::ProcessPackets() { } std::unique_ptr rtp_packet = packet->ReleasePacket(); - const bool owned_rtp_packet = rtp_packet != nullptr; - RtpPacketSendResult success; + RTC_DCHECK(rtp_packet); + packet_sender_->SendRtpPacket(std::move(rtp_packet), pacing_info); - if (rtp_packet != nullptr) { - packet_sender_->SendRtpPacket(std::move(rtp_packet), pacing_info); - success = RtpPacketSendResult::kSuccess; - } else { - success = packet_sender_->TimeToSendPacket( - packet->ssrc(), packet->sequence_number(), packet->capture_time_ms(), - packet->is_retransmission(), pacing_info); - } - - if (success == RtpPacketSendResult::kSuccess || - success == RtpPacketSendResult::kPacketNotFound) { - // Packet sent or invalid packet, remove it from queue. - // TODO(webrtc:8052): Don't consume media budget on kInvalid. - data_sent += packet->size(); - // Send succeeded, remove it from the queue. - OnPacketSent(packet); - if (recommended_probe_size && data_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 - // consuming budget. - packet_queue_.FinalizePop(); + data_sent += packet->size(); + // Send succeeded, remove it from the queue. + OnPacketSent(packet); + if (recommended_probe_size && data_sent > *recommended_probe_size) break; - } else { - // Send failed, put it back into the queue. - packet_queue_.CancelPop(); - break; - } - } - - if (legacy_packet_referencing_ && packet_queue_.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) { - DataSize padding_needed = - (recommended_probe_size && *recommended_probe_size > data_sent) - ? (*recommended_probe_size - data_sent) - : DataSize::bytes(padding_budget_.bytes_remaining()); - if (padding_needed > DataSize::Zero()) { - DataSize padding_sent = DataSize::Zero(); - padding_sent = - packet_sender_->TimeToSendPadding(padding_needed, pacing_info); - data_sent += padding_sent; - OnPaddingSent(padding_sent); - } - } } if (is_probing) { diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h index 0948616919..50d0de030e 100644 --- a/modules/pacing/pacing_controller.h +++ b/modules/pacing/pacing_controller.h @@ -51,16 +51,6 @@ class PacingController { const PacedPacketInfo& cluster_info) = 0; virtual std::vector> GeneratePadding( DataSize size) = 0; - - // TODO(bugs.webrtc.org/10633): Remove these when old code path is gone. - virtual RtpPacketSendResult TimeToSendPacket( - uint32_t ssrc, - uint16_t sequence_number, - int64_t capture_timestamp, - bool retransmission, - const PacedPacketInfo& packet_info) = 0; - virtual DataSize TimeToSendPadding(DataSize size, - const PacedPacketInfo& pacing_info) = 0; }; // Expected max pacer delay. If ExpectedQueueTime() is higher than @@ -210,11 +200,6 @@ class PacingController { TimeDelta queue_time_limit; bool account_for_audio_; - - // 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 diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc index a092e01cf9..e07e8c85ab 100644 --- a/modules/pacing/pacing_controller_unittest.cc +++ b/modules/pacing/pacing_controller_unittest.cc @@ -51,40 +51,6 @@ constexpr uint32_t kFlexFecSsrc = 45678; constexpr DataRate kTargetRate = DataRate::KilobitsPerSec<800>(); -enum class PacerMode { kReferencePackets, kOwnPackets }; -std::string GetFieldTrialStirng(PacerMode mode) { - std::string field_trial = "WebRTC-Pacer-LegacyPacketReferencing/"; - switch (mode) { - case PacerMode::kOwnPackets: - field_trial += "Disabled"; - break; - case PacerMode::kReferencePackets: - field_trial += "Enabled"; - break; - } - field_trial += "/"; - return field_trial; -} - -// TODO(bugs.webrtc.org/10633): Remove when packets are always owned by pacer. -RtpPacketSender::Priority PacketTypeToPriority(RtpPacketToSend::Type type) { - switch (type) { - case RtpPacketToSend::Type::kAudio: - return RtpPacketSender::Priority::kHighPriority; - case RtpPacketToSend::Type::kVideo: - return RtpPacketSender::Priority::kLowPriority; - case RtpPacketToSend::Type::kRetransmission: - return RtpPacketSender::Priority::kNormalPriority; - case RtpPacketToSend::Type::kForwardErrorCorrection: - return RtpPacketSender::Priority::kLowPriority; - break; - case RtpPacketToSend::Type::kPadding: - RTC_NOTREACHED() << "Unexpected type for legacy path: kPadding"; - break; - } - return RtpPacketSender::Priority::kLowPriority; -} - std::unique_ptr BuildPacket(RtpPacketToSend::Type type, uint32_t ssrc, uint16_t sequence_number, @@ -104,15 +70,6 @@ std::unique_ptr BuildPacket(RtpPacketToSend::Type type, // methods that focus on core aspects. class MockPacingControllerCallback : public PacingController::PacketSender { public: - RtpPacketSendResult TimeToSendPacket(uint32_t ssrc, - uint16_t sequence_number, - int64_t capture_timestamp, - bool retransmission, - const PacedPacketInfo& packet_info) { - SendPacket(ssrc, sequence_number, capture_timestamp, retransmission, false); - return RtpPacketSendResult::kSuccess; - } - void SendRtpPacket(std::unique_ptr packet, const PacedPacketInfo& cluster_info) override { SendPacket(packet->Ssrc(), packet->SequenceNumber(), @@ -121,11 +78,6 @@ class MockPacingControllerCallback : public PacingController::PacketSender { packet->packet_type() == RtpPacketToSend::Type::kPadding); } - DataSize TimeToSendPadding(DataSize size, - const PacedPacketInfo& packet_info) override { - return DataSize::bytes(SendPadding(size.bytes())); - } - std::vector> GeneratePadding( DataSize target_size) override { std::vector> ret; @@ -151,15 +103,6 @@ class MockPacingControllerCallback : public PacingController::PacketSender { // Mock callback implementing the raw api. class MockPacketSender : public PacingController::PacketSender { public: - MOCK_METHOD5(TimeToSendPacket, - RtpPacketSendResult(uint32_t ssrc, - uint16_t sequence_number, - int64_t capture_time_ms, - bool retransmission, - const PacedPacketInfo& pacing_info)); - MOCK_METHOD2(TimeToSendPadding, - DataSize(DataSize size, const PacedPacketInfo& pacing_info)); - MOCK_METHOD2(SendRtpPacket, void(std::unique_ptr packet, const PacedPacketInfo& cluster_info)); @@ -174,26 +117,9 @@ class PacingControllerPadding : public PacingController::PacketSender { PacingControllerPadding() : padding_sent_(0) {} - RtpPacketSendResult TimeToSendPacket( - uint32_t ssrc, - uint16_t sequence_number, - int64_t capture_time_ms, - bool retransmission, - const PacedPacketInfo& pacing_info) override { - return RtpPacketSendResult::kSuccess; - } - void SendRtpPacket(std::unique_ptr packet, const PacedPacketInfo& pacing_info) override {} - DataSize TimeToSendPadding(DataSize size, - const PacedPacketInfo& pacing_info) override { - size_t num_packets = - (size.bytes() + kPaddingPacketSize - 1) / kPaddingPacketSize; - padding_sent_ += kPaddingPacketSize * num_packets; - return DataSize::bytes(kPaddingPacketSize * num_packets); - } - std::vector> GeneratePadding( DataSize target_size) override { size_t num_packets = @@ -218,16 +144,6 @@ class PacingControllerProbing : public PacingController::PacketSender { public: PacingControllerProbing() : packets_sent_(0), padding_sent_(0) {} - RtpPacketSendResult TimeToSendPacket( - uint32_t ssrc, - uint16_t sequence_number, - int64_t capture_time_ms, - bool retransmission, - const PacedPacketInfo& pacing_info) override { - ++packets_sent_; - return RtpPacketSendResult::kSuccess; - } - void SendRtpPacket(std::unique_ptr packet, const PacedPacketInfo& pacing_info) override { if (packet->packet_type() != RtpPacketToSend::Type::kPadding) { @@ -235,12 +151,6 @@ class PacingControllerProbing : public PacingController::PacketSender { } } - DataSize TimeToSendPadding(DataSize size, - const PacedPacketInfo& pacing_info) override { - padding_sent_ += size.bytes(); - return DataSize::bytes(padding_sent_); - } - std::vector> GeneratePadding( DataSize target_size) override { std::vector> packets; @@ -260,10 +170,9 @@ class PacingControllerProbing : public PacingController::PacketSender { int padding_sent_; }; -class PacingControllerTest : public ::testing::TestWithParam { +class PacingControllerTest : public ::testing::Test { protected: - PacingControllerTest() - : clock_(123456), field_trial_(GetFieldTrialStirng(GetParam())) { + PacingControllerTest() : clock_(123456) { srand(0); // Need to initialize PacingController after we initialize clock. pacer_ = absl::make_unique(&clock_, &callback_, nullptr, @@ -288,14 +197,8 @@ class PacingControllerTest : public ::testing::TestWithParam { uint16_t sequence_number, int64_t capture_time_ms, size_t size) { - if (GetParam() == PacerMode::kReferencePackets) { - pacer_->InsertPacket(PacketTypeToPriority(type), ssrc, sequence_number, - capture_time_ms, size, - type == RtpPacketToSend::Type::kRetransmission); - } else { - pacer_->EnqueuePacket( - BuildPacket(type, ssrc, sequence_number, capture_time_ms, size)); - } + pacer_->EnqueuePacket( + BuildPacket(type, ssrc, sequence_number, capture_time_ms, size)); } void SendAndExpectPacket(RtpPacketToSend::Type type, @@ -311,12 +214,6 @@ class PacingControllerTest : public ::testing::TestWithParam { .Times(1); } - void ExpectSendPadding() { - if (GetParam() == PacerMode::kOwnPackets) { - EXPECT_CALL(callback_, SendPacket(_, _, _, _, true)).Times(1); - } - } - std::unique_ptr BuildRtpPacket(RtpPacketToSend::Type type) { auto packet = absl::make_unique(nullptr); packet->set_packet_type(type); @@ -360,13 +257,11 @@ class PacingControllerTest : public ::testing::TestWithParam { } SimulatedClock clock_; - ScopedFieldTrials field_trial_; MockPacingControllerCallback callback_; std::unique_ptr pacer_; }; -class PacingControllerFieldTrialTest - : public ::testing::TestWithParam { +class PacingControllerFieldTrialTest : public ::testing::Test { protected: struct MediaStream { const RtpPacketToSend::Type type; @@ -379,15 +274,9 @@ class PacingControllerFieldTrialTest PacingControllerFieldTrialTest() : clock_(123456) {} void InsertPacket(PacingController* pacer, MediaStream* stream) { - if (GetParam() == PacerMode::kReferencePackets) { - pacer->InsertPacket(PacketTypeToPriority(stream->type), stream->ssrc, - stream->seq_num++, clock_.TimeInMilliseconds(), - stream->packet_size, false); - } else { - pacer->EnqueuePacket( - BuildPacket(stream->type, stream->ssrc, stream->seq_num++, - clock_.TimeInMilliseconds(), stream->packet_size)); - } + pacer->EnqueuePacket( + BuildPacket(stream->type, stream->ssrc, stream->seq_num++, + clock_.TimeInMilliseconds(), stream->packet_size)); } void ProcessNext(PacingController* pacer) { clock_.AdvanceTimeMilliseconds(5); @@ -401,7 +290,7 @@ class PacingControllerFieldTrialTest MockPacingControllerCallback callback_; }; -TEST_P(PacingControllerFieldTrialTest, DefaultNoPaddingInSilence) { +TEST_F(PacingControllerFieldTrialTest, DefaultNoPaddingInSilence) { PacingController pacer(&clock_, &callback_, nullptr, nullptr); pacer.SetPacingRates(kTargetRate, DataRate::Zero()); // Video packet to reset last send time and provide padding data. @@ -415,19 +304,13 @@ TEST_P(PacingControllerFieldTrialTest, DefaultNoPaddingInSilence) { pacer.ProcessPackets(); } -TEST_P(PacingControllerFieldTrialTest, PaddingInSilenceWithTrial) { - ScopedFieldTrials trial(GetFieldTrialStirng(GetParam()) + - "WebRTC-Pacer-PadInSilence/Enabled/"); +TEST_F(PacingControllerFieldTrialTest, PaddingInSilenceWithTrial) { + ScopedFieldTrials trial("WebRTC-Pacer-PadInSilence/Enabled/"); PacingController pacer(&clock_, &callback_, nullptr, nullptr); pacer.SetPacingRates(kTargetRate, DataRate::Zero()); // Video packet to reset last send time and provide padding data. InsertPacket(&pacer, &video); - if (GetParam() == PacerMode::kReferencePackets) { - // Only payload, not padding, sent by pacer in legacy mode. - EXPECT_CALL(callback_, SendPacket).Times(1); - } else { - EXPECT_CALL(callback_, SendPacket).Times(2); - } + EXPECT_CALL(callback_, SendPacket).Times(2); clock_.AdvanceTimeMilliseconds(5); pacer.ProcessPackets(); EXPECT_CALL(callback_, SendPadding).WillOnce(Return(1000)); @@ -436,7 +319,7 @@ TEST_P(PacingControllerFieldTrialTest, PaddingInSilenceWithTrial) { pacer.ProcessPackets(); } -TEST_P(PacingControllerFieldTrialTest, DefaultCongestionWindowAffectsAudio) { +TEST_F(PacingControllerFieldTrialTest, DefaultCongestionWindowAffectsAudio) { EXPECT_CALL(callback_, SendPadding).Times(0); PacingController pacer(&clock_, &callback_, nullptr, nullptr); pacer.SetPacingRates(DataRate::bps(10000000), DataRate::Zero()); @@ -458,10 +341,9 @@ TEST_P(PacingControllerFieldTrialTest, DefaultCongestionWindowAffectsAudio) { ProcessNext(&pacer); } -TEST_P(PacingControllerFieldTrialTest, +TEST_F(PacingControllerFieldTrialTest, CongestionWindowDoesNotAffectAudioInTrial) { - ScopedFieldTrials trial(GetFieldTrialStirng(GetParam()) + - "WebRTC-Pacer-BlockAudio/Disabled/"); + ScopedFieldTrials trial("WebRTC-Pacer-BlockAudio/Disabled/"); EXPECT_CALL(callback_, SendPadding).Times(0); PacingController pacer(&clock_, &callback_, nullptr, nullptr); pacer.SetPacingRates(DataRate::bps(10000000), DataRate::Zero()); @@ -477,7 +359,7 @@ TEST_P(PacingControllerFieldTrialTest, ProcessNext(&pacer); } -TEST_P(PacingControllerFieldTrialTest, DefaultBudgetAffectsAudio) { +TEST_F(PacingControllerFieldTrialTest, DefaultBudgetAffectsAudio) { PacingController pacer(&clock_, &callback_, nullptr, nullptr); pacer.SetPacingRates( DataRate::bps(video.packet_size / 3 * 8 * kProcessIntervalsPerSecond), @@ -498,9 +380,8 @@ TEST_P(PacingControllerFieldTrialTest, DefaultBudgetAffectsAudio) { ProcessNext(&pacer); } -TEST_P(PacingControllerFieldTrialTest, BudgetDoesNotAffectAudioInTrial) { - ScopedFieldTrials trial(GetFieldTrialStirng(GetParam()) + - "WebRTC-Pacer-BlockAudio/Disabled/"); +TEST_F(PacingControllerFieldTrialTest, BudgetDoesNotAffectAudioInTrial) { + ScopedFieldTrials trial("WebRTC-Pacer-BlockAudio/Disabled/"); EXPECT_CALL(callback_, SendPadding).Times(0); PacingController pacer(&clock_, &callback_, nullptr, nullptr); pacer.SetPacingRates( @@ -516,12 +397,7 @@ TEST_P(PacingControllerFieldTrialTest, BudgetDoesNotAffectAudioInTrial) { ProcessNext(&pacer); } -INSTANTIATE_TEST_SUITE_P(ReferencingAndOwningPackets, - PacingControllerFieldTrialTest, - ::testing::Values(PacerMode::kReferencePackets, - PacerMode::kOwnPackets)); - -TEST_P(PacingControllerTest, FirstSentPacketTimeIsSet) { +TEST_F(PacingControllerTest, FirstSentPacketTimeIsSet) { uint16_t sequence_number = 1234; const uint32_t kSsrc = 12345; const size_t kSizeBytes = 250; @@ -540,7 +416,7 @@ TEST_P(PacingControllerTest, FirstSentPacketTimeIsSet) { EXPECT_EQ(kStartTime, pacer_->FirstSentPacketTime()); } -TEST_P(PacingControllerTest, QueuePacket) { +TEST_F(PacingControllerTest, QueuePacket) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; // Due to the multiplicative factor we can send 5 packets during a send @@ -581,7 +457,7 @@ TEST_P(PacingControllerTest, QueuePacket) { EXPECT_EQ(1u, pacer_->QueueSizePackets()); } -TEST_P(PacingControllerTest, PaceQueuedPackets) { +TEST_F(PacingControllerTest, PaceQueuedPackets) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; @@ -625,7 +501,7 @@ TEST_P(PacingControllerTest, PaceQueuedPackets) { EXPECT_EQ(1u, pacer_->QueueSizePackets()); } -TEST_P(PacingControllerTest, RepeatedRetransmissionsAllowed) { +TEST_F(PacingControllerTest, RepeatedRetransmissionsAllowed) { // Send one packet, then two retransmissions of that packet. for (size_t i = 0; i < 3; i++) { constexpr uint32_t ssrc = 333; @@ -641,7 +517,7 @@ TEST_P(PacingControllerTest, RepeatedRetransmissionsAllowed) { pacer_->ProcessPackets(); } -TEST_P(PacingControllerTest, +TEST_F(PacingControllerTest, CanQueuePacketsWithSameSequenceNumberOnDifferentSsrcs) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; @@ -657,7 +533,7 @@ TEST_P(PacingControllerTest, pacer_->ProcessPackets(); } -TEST_P(PacingControllerTest, Padding) { +TEST_F(PacingControllerTest, Padding) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; @@ -685,12 +561,12 @@ TEST_P(PacingControllerTest, Padding) { // 5 milliseconds later we have enough budget to send some padding. EXPECT_CALL(callback_, SendPadding(250)).WillOnce(Return(250)); - ExpectSendPadding(); + EXPECT_CALL(callback_, SendPacket(_, _, _, _, true)).Times(1); clock_.AdvanceTime(TimeUntilNextProcess()); pacer_->ProcessPackets(); } -TEST_P(PacingControllerTest, NoPaddingBeforeNormalPacket) { +TEST_F(PacingControllerTest, NoPaddingBeforeNormalPacket) { pacer_->SetPacingRates(kTargetRate * kPaceMultiplier, kTargetRate); EXPECT_CALL(callback_, SendPadding).Times(0); @@ -707,11 +583,11 @@ TEST_P(PacingControllerTest, NoPaddingBeforeNormalPacket) { SendAndExpectPacket(RtpPacketToSend::Type::kVideo, ssrc, sequence_number++, capture_time_ms, 250); EXPECT_CALL(callback_, SendPadding(250)).WillOnce(Return(250)); - ExpectSendPadding(); + EXPECT_CALL(callback_, SendPacket(_, _, _, _, true)).Times(1); pacer_->ProcessPackets(); } -TEST_P(PacingControllerTest, VerifyPaddingUpToBitrate) { +TEST_F(PacingControllerTest, VerifyPaddingUpToBitrate) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; int64_t capture_time_ms = 56789; @@ -724,13 +600,13 @@ TEST_P(PacingControllerTest, VerifyPaddingUpToBitrate) { SendAndExpectPacket(RtpPacketToSend::Type::kVideo, ssrc, sequence_number++, capture_time_ms, 250); EXPECT_CALL(callback_, SendPadding(250)).WillOnce(Return(250)); - ExpectSendPadding(); + EXPECT_CALL(callback_, SendPacket(_, _, _, _, true)).Times(1); pacer_->ProcessPackets(); clock_.AdvanceTimeMilliseconds(kTimeStep); } } -TEST_P(PacingControllerTest, VerifyAverageBitrateVaryingMediaPayload) { +TEST_F(PacingControllerTest, VerifyAverageBitrateVaryingMediaPayload) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; int64_t capture_time_ms = 56789; @@ -759,7 +635,7 @@ TEST_P(PacingControllerTest, VerifyAverageBitrateVaryingMediaPayload) { 1); } -TEST_P(PacingControllerTest, Priority) { +TEST_F(PacingControllerTest, Priority) { uint32_t ssrc_low_priority = 12345; uint32_t ssrc = 12346; uint16_t sequence_number = 1234; @@ -806,7 +682,7 @@ TEST_P(PacingControllerTest, Priority) { pacer_->ProcessPackets(); } -TEST_P(PacingControllerTest, RetransmissionPriority) { +TEST_F(PacingControllerTest, RetransmissionPriority) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; int64_t capture_time_ms = 45678; @@ -853,7 +729,7 @@ TEST_P(PacingControllerTest, RetransmissionPriority) { EXPECT_EQ(0u, pacer_->QueueSizePackets()); } -TEST_P(PacingControllerTest, HighPrioDoesntAffectBudget) { +TEST_F(PacingControllerTest, HighPrioDoesntAffectBudget) { uint32_t ssrc = 12346; uint16_t sequence_number = 1234; int64_t capture_time_ms = 56789; @@ -888,7 +764,7 @@ TEST_P(PacingControllerTest, HighPrioDoesntAffectBudget) { EXPECT_EQ(0u, pacer_->QueueSizePackets()); } -TEST_P(PacingControllerTest, SendsOnlyPaddingWhenCongested) { +TEST_F(PacingControllerTest, SendsOnlyPaddingWhenCongested) { uint32_t ssrc = 202020; uint16_t sequence_number = 1000; int kPacketSize = 250; @@ -920,13 +796,13 @@ TEST_P(PacingControllerTest, SendsOnlyPaddingWhenCongested) { } ::testing::Mock::VerifyAndClearExpectations(&callback_); EXPECT_CALL(callback_, SendPadding(1)).WillOnce(Return(1)); - ExpectSendPadding(); + EXPECT_CALL(callback_, SendPacket(_, _, _, _, true)).Times(1); clock_.AdvanceTimeMilliseconds(5); pacer_->ProcessPackets(); EXPECT_EQ(blocked_packets, pacer_->QueueSizePackets()); } -TEST_P(PacingControllerTest, DoesNotAllowOveruseAfterCongestion) { +TEST_F(PacingControllerTest, DoesNotAllowOveruseAfterCongestion) { uint32_t ssrc = 202020; uint16_t seq_num = 1000; int size = 1000; @@ -967,7 +843,7 @@ TEST_P(PacingControllerTest, DoesNotAllowOveruseAfterCongestion) { pacer_->ProcessPackets(); } -TEST_P(PacingControllerTest, ResumesSendingWhenCongestionEnds) { +TEST_F(PacingControllerTest, ResumesSendingWhenCongestionEnds) { uint32_t ssrc = 202020; uint16_t sequence_number = 1000; int64_t kPacketSize = 250; @@ -1022,7 +898,7 @@ TEST_P(PacingControllerTest, ResumesSendingWhenCongestionEnds) { } } -TEST_P(PacingControllerTest, Pause) { +TEST_F(PacingControllerTest, Pause) { uint32_t ssrc_low_priority = 12345; uint32_t ssrc = 12346; uint32_t ssrc_high_priority = 12347; @@ -1069,7 +945,7 @@ TEST_P(PacingControllerTest, Pause) { pacer_->OldestPacketWaitTime()); EXPECT_CALL(callback_, SendPadding(1)).WillOnce(Return(1)); - ExpectSendPadding(); + EXPECT_CALL(callback_, SendPacket(_, _, _, _, true)).Times(1); pacer_->ProcessPackets(); int64_t expected_time_until_send = 500; @@ -1082,7 +958,7 @@ TEST_P(PacingControllerTest, Pause) { ::testing::Mock::VerifyAndClearExpectations(&callback_); EXPECT_CALL(callback_, SendPadding(1)).WillOnce(Return(1)); - ExpectSendPadding(); + EXPECT_CALL(callback_, SendPacket(_, _, _, _, true)).Times(1); clock_.AdvanceTimeMilliseconds(5); pacer_->ProcessPackets(); ::testing::Mock::VerifyAndClearExpectations(&callback_); @@ -1131,68 +1007,7 @@ TEST_P(PacingControllerTest, Pause) { EXPECT_EQ(TimeDelta::Zero(), pacer_->OldestPacketWaitTime()); } -TEST_P(PacingControllerTest, ResendPacket) { - if (GetParam() == PacerMode::kOwnPackets) { - // This test only makes sense when re-sending is supported. - return; - } - - MockPacketSender callback; - - // Need to initialize PacedSender after we initialize clock. - pacer_ = - absl::make_unique(&clock_, &callback, nullptr, nullptr); - Init(); - - uint32_t ssrc = 12346; - uint16_t sequence_number = 1234; - int64_t capture_time_ms = clock_.TimeInMilliseconds(); - EXPECT_EQ(TimeDelta::Zero(), pacer_->OldestPacketWaitTime()); - - pacer_->InsertPacket(RtpPacketSender::kNormalPriority, ssrc, sequence_number, - capture_time_ms, 250, false); - clock_.AdvanceTimeMilliseconds(1); - pacer_->InsertPacket(RtpPacketSender::kNormalPriority, ssrc, - sequence_number + 1, capture_time_ms + 1, 250, false); - clock_.AdvanceTimeMilliseconds(9999); - EXPECT_EQ(TimeDelta::ms(clock_.TimeInMilliseconds() - capture_time_ms), - pacer_->OldestPacketWaitTime()); - // Fails to send first packet so only one call. - EXPECT_CALL(callback, TimeToSendPacket(ssrc, sequence_number, capture_time_ms, - false, _)) - .Times(1) - .WillOnce(Return(RtpPacketSendResult::kTransportUnavailable)); - clock_.AdvanceTimeMilliseconds(10000); - pacer_->ProcessPackets(); - - // Queue remains unchanged. - EXPECT_EQ(TimeDelta::ms(clock_.TimeInMilliseconds() - capture_time_ms), - pacer_->OldestPacketWaitTime()); - - // Fails to send second packet. - EXPECT_CALL(callback, TimeToSendPacket(ssrc, sequence_number, capture_time_ms, - false, _)) - .WillOnce(Return(RtpPacketSendResult::kSuccess)); - EXPECT_CALL(callback, TimeToSendPacket(ssrc, sequence_number + 1, - capture_time_ms + 1, false, _)) - .WillOnce(Return(RtpPacketSendResult::kTransportUnavailable)); - clock_.AdvanceTimeMilliseconds(10000); - pacer_->ProcessPackets(); - - // Queue is reduced by 1 packet. - EXPECT_EQ(TimeDelta::ms(clock_.TimeInMilliseconds() - capture_time_ms - 1), - pacer_->OldestPacketWaitTime()); - - // Send second packet and queue becomes empty. - EXPECT_CALL(callback, TimeToSendPacket(ssrc, sequence_number + 1, - capture_time_ms + 1, false, _)) - .WillOnce(Return(RtpPacketSendResult::kSuccess)); - clock_.AdvanceTimeMilliseconds(10000); - pacer_->ProcessPackets(); - EXPECT_EQ(TimeDelta::Zero(), pacer_->OldestPacketWaitTime()); -} - -TEST_P(PacingControllerTest, ExpectedQueueTimeMs) { +TEST_F(PacingControllerTest, ExpectedQueueTimeMs) { uint32_t ssrc = 12346; uint16_t sequence_number = 1234; const size_t kNumPackets = 60; @@ -1228,7 +1043,7 @@ TEST_P(PacingControllerTest, ExpectedQueueTimeMs) { TimeDelta::ms(1000 * kPacketSize * 8 / kMaxBitrate)); } -TEST_P(PacingControllerTest, QueueTimeGrowsOverTime) { +TEST_F(PacingControllerTest, QueueTimeGrowsOverTime) { uint32_t ssrc = 12346; uint16_t sequence_number = 1234; EXPECT_EQ(TimeDelta::Zero(), pacer_->OldestPacketWaitTime()); @@ -1244,7 +1059,7 @@ TEST_P(PacingControllerTest, QueueTimeGrowsOverTime) { EXPECT_EQ(TimeDelta::Zero(), pacer_->OldestPacketWaitTime()); } -TEST_P(PacingControllerTest, ProbingWithInsertedPackets) { +TEST_F(PacingControllerTest, ProbingWithInsertedPackets) { const size_t kPacketSize = 1200; const int kInitialBitrateBps = 300000; uint32_t ssrc = 12346; @@ -1291,7 +1106,7 @@ TEST_P(PacingControllerTest, ProbingWithInsertedPackets) { kSecondClusterRate.bps(), kProbingErrorMargin.bps()); } -TEST_P(PacingControllerTest, ProbingWithPaddingSupport) { +TEST_F(PacingControllerTest, ProbingWithPaddingSupport) { const size_t kPacketSize = 1200; const int kInitialBitrateBps = 300000; uint32_t ssrc = 12346; @@ -1328,7 +1143,7 @@ TEST_P(PacingControllerTest, ProbingWithPaddingSupport) { kFirstClusterRate.bps(), kProbingErrorMargin.bps()); } -TEST_P(PacingControllerTest, PaddingOveruse) { +TEST_F(PacingControllerTest, PaddingOveruse) { uint32_t ssrc = 12346; uint16_t sequence_number = 1234; const size_t kPacketSize = 1200; @@ -1355,7 +1170,7 @@ TEST_P(PacingControllerTest, PaddingOveruse) { pacer_->ProcessPackets(); } -TEST_P(PacingControllerTest, ProbeClusterId) { +TEST_F(PacingControllerTest, ProbeClusterId) { MockPacketSender callback; pacer_ = @@ -1374,17 +1189,9 @@ TEST_P(PacingControllerTest, ProbeClusterId) { } // First probing cluster. - if (GetParam() == PacerMode::kReferencePackets) { - EXPECT_CALL(callback, - TimeToSendPacket(_, _, _, _, - Field(&PacedPacketInfo::probe_cluster_id, 0))) - .Times(5) - .WillRepeatedly(Return(RtpPacketSendResult::kSuccess)); - } else { - EXPECT_CALL(callback, - SendRtpPacket(_, Field(&PacedPacketInfo::probe_cluster_id, 0))) - .Times(5); - } + EXPECT_CALL(callback, + SendRtpPacket(_, Field(&PacedPacketInfo::probe_cluster_id, 0))) + .Times(5); for (int i = 0; i < 5; ++i) { clock_.AdvanceTimeMilliseconds(20); @@ -1392,18 +1199,9 @@ TEST_P(PacingControllerTest, ProbeClusterId) { } // Second probing cluster. - if (GetParam() == PacerMode::kReferencePackets) { - EXPECT_CALL(callback, - TimeToSendPacket(_, _, _, _, - Field(&PacedPacketInfo::probe_cluster_id, 1))) - .Times(5) - .WillRepeatedly(Return(RtpPacketSendResult::kSuccess)); - EXPECT_CALL(callback, TimeToSendPadding).WillOnce(Return(DataSize::Zero())); - } else { - EXPECT_CALL(callback, - SendRtpPacket(_, Field(&PacedPacketInfo::probe_cluster_id, 1))) - .Times(5); - } + EXPECT_CALL(callback, + SendRtpPacket(_, Field(&PacedPacketInfo::probe_cluster_id, 1))) + .Times(5); for (int i = 0; i < 5; ++i) { clock_.AdvanceTimeMilliseconds(20); @@ -1413,33 +1211,21 @@ TEST_P(PacingControllerTest, ProbeClusterId) { // Needed for the Field comparer below. const int kNotAProbe = PacedPacketInfo::kNotAProbe; // No more probing packets. - if (GetParam() == PacerMode::kReferencePackets) { - EXPECT_CALL(callback, - TimeToSendPadding( - _, Field(&PacedPacketInfo::probe_cluster_id, kNotAProbe))) - .WillOnce(Return(DataSize::bytes(500))); - } else { - EXPECT_CALL(callback, GeneratePadding).WillOnce([&](DataSize padding_size) { - std::vector> padding_packets; - padding_packets.emplace_back( - BuildPacket(RtpPacketToSend::Type::kPadding, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), padding_size.bytes())); - return padding_packets; - }); - EXPECT_CALL( - callback, - SendRtpPacket(_, Field(&PacedPacketInfo::probe_cluster_id, kNotAProbe))) - .Times(1); - } + EXPECT_CALL(callback, GeneratePadding).WillOnce([&](DataSize padding_size) { + std::vector> padding_packets; + padding_packets.emplace_back( + BuildPacket(RtpPacketToSend::Type::kPadding, ssrc, sequence_number++, + clock_.TimeInMilliseconds(), padding_size.bytes())); + return padding_packets; + }); + EXPECT_CALL( + callback, + SendRtpPacket(_, Field(&PacedPacketInfo::probe_cluster_id, kNotAProbe))) + .Times(1); pacer_->ProcessPackets(); } -TEST_P(PacingControllerTest, OwnedPacketPrioritizedOnType) { - if (GetParam() != PacerMode::kOwnPackets) { - // This test only makes sense when using the new code path. - return; - } - +TEST_F(PacingControllerTest, OwnedPacketPrioritizedOnType) { MockPacketSender callback; pacer_ = absl::make_unique(&clock_, &callback, nullptr, nullptr); @@ -1480,11 +1266,5 @@ TEST_P(PacingControllerTest, OwnedPacketPrioritizedOnType) { clock_.AdvanceTimeMilliseconds(200); pacer_->ProcessPackets(); } - -INSTANTIATE_TEST_SUITE_P(ReferencingAndOwningPackets, - PacingControllerTest, - ::testing::Values(PacerMode::kReferencePackets, - PacerMode::kOwnPackets)); - } // namespace test } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index f7ee2634e5..08858e2f6a 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -46,8 +46,6 @@ constexpr uint16_t kMaxInitRtpSeqNumber = 32767; // 2^15 -1. constexpr uint32_t kTimestampTicksPerMs = 90; constexpr int kBitrateStatisticsWindowMs = 1000; -constexpr size_t kMinFlexfecPacketsToStoreForPacing = 50; - // Min size needed to get payload padding from packet history. constexpr int kMinPayloadPaddingBytes = 50; @@ -89,42 +87,6 @@ constexpr RtpExtensionSize kVideoExtensionSizes[] = { RtpGenericFrameDescriptorExtension01::kMaxSizeBytes}, }; -// TODO(bugs.webrtc.org/10633): Remove when downstream code stops using -// priority. At the time of writing, the priority can be directly mapped to a -// packet type. This is only for a transition period. -RtpPacketToSend::Type PacketPriorityToType(RtpPacketSender::Priority priority) { - switch (priority) { - case RtpPacketSender::Priority::kLowPriority: - return RtpPacketToSend::Type::kVideo; - case RtpPacketSender::Priority::kNormalPriority: - return RtpPacketToSend::Type::kRetransmission; - case RtpPacketSender::Priority::kHighPriority: - return RtpPacketToSend::Type::kAudio; - default: - RTC_NOTREACHED() << "Unexpected priority: " << priority; - return RtpPacketToSend::Type::kVideo; - } -} - -// TODO(bugs.webrtc.org/10633): Remove when packets are always owned by pacer. -RtpPacketSender::Priority PacketTypeToPriority(RtpPacketToSend::Type type) { - switch (type) { - case RtpPacketToSend::Type::kAudio: - return RtpPacketSender::Priority::kHighPriority; - case RtpPacketToSend::Type::kVideo: - return RtpPacketSender::Priority::kLowPriority; - case RtpPacketToSend::Type::kRetransmission: - return RtpPacketSender::Priority::kNormalPriority; - case RtpPacketToSend::Type::kForwardErrorCorrection: - return RtpPacketSender::Priority::kLowPriority; - break; - case RtpPacketToSend::Type::kPadding: - RTC_NOTREACHED() << "Unexpected type for legacy path: kPadding"; - break; - } - return RtpPacketSender::Priority::kLowPriority; -} - bool IsEnabled(absl::string_view name, const WebRtcKeyValueConfig* field_trials) { FieldTrialBasedConfig default_trials; @@ -159,7 +121,6 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config) last_payload_type_(-1), rtp_header_extension_map_(config.extmap_allow_mixed), packet_history_(clock_), - flexfec_packet_history_(clock_), // Statistics send_delays_(), max_delay_it_(send_delays_.end()), @@ -192,23 +153,12 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config) overhead_observer_(config.overhead_observer), populate_network2_timestamp_(config.populate_network2_timestamp), send_side_bwe_with_overhead_( - IsEnabled("WebRTC-SendSideBwe-WithOverhead", config.field_trials)), - pacer_legacy_packet_referencing_( - IsEnabled("WebRTC-Pacer-LegacyPacketReferencing", - config.field_trials)) { + IsEnabled("WebRTC-SendSideBwe-WithOverhead", config.field_trials)) { // This random initialization is not intended to be cryptographic strong. timestamp_offset_ = random_.Rand(); // Random start, 16 bits. Can't be 0. sequence_number_rtx_ = random_.Rand(1, kMaxInitRtpSeqNumber); sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber); - - // Store FlexFEC packets in the packet history data structure, so they can - // be found when paced. - if (flexfec_ssrc_) { - flexfec_packet_history_.SetStorePacketsStatus( - RtpPacketHistory::StorageMode::kStoreAndCull, - kMinFlexfecPacketsToStoreForPacing); - } } RTPSender::RTPSender( @@ -244,7 +194,6 @@ RTPSender::RTPSender( last_payload_type_(-1), rtp_header_extension_map_(extmap_allow_mixed), packet_history_(clock), - flexfec_packet_history_(clock), // Statistics send_delays_(), max_delay_it_(send_delays_.end()), @@ -276,23 +225,12 @@ RTPSender::RTPSender( populate_network2_timestamp_(populate_network2_timestamp), send_side_bwe_with_overhead_( field_trials.Lookup("WebRTC-SendSideBwe-WithOverhead") - .find("Enabled") == 0), - pacer_legacy_packet_referencing_( - field_trials.Lookup("WebRTC-Pacer-LegacyPacketReferencing") .find("Enabled") == 0) { // This random initialization is not intended to be cryptographic strong. timestamp_offset_ = random_.Rand(); // Random start, 16 bits. Can't be 0. sequence_number_rtx_ = random_.Rand(1, kMaxInitRtpSeqNumber); sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber); - - // Store FlexFEC packets in the packet history data structure, so they can - // be found when paced. - if (flexfec_ssrc_) { - flexfec_packet_history_.SetStorePacketsStatus( - RtpPacketHistory::StorageMode::kStoreAndCull, - kMinFlexfecPacketsToStoreForPacing); - } } RTPSender::~RTPSender() { @@ -406,158 +344,6 @@ void RTPSender::SetRtxPayloadType(int payload_type, rtx_payload_type_map_[associated_payload_type] = payload_type; } -size_t RTPSender::TrySendRedundantPayloads(size_t bytes_to_send, - const PacedPacketInfo& pacing_info) { - { - rtc::CritScope lock(&send_critsect_); - if (!sending_media_) - return 0; - if ((rtx_ & kRtxRedundantPayloads) == 0) - return 0; - } - - int bytes_left = static_cast(bytes_to_send); - while (bytes_left >= kMinPayloadPaddingBytes) { - std::unique_ptr packet = - packet_history_.GetPayloadPaddingPacket(); - - if (!packet) - break; - size_t payload_size = packet->payload_size(); - if (!PrepareAndSendPacket(std::move(packet), true, false, pacing_info)) - break; - bytes_left -= payload_size; - } - return bytes_to_send - bytes_left; -} - -size_t RTPSender::SendPadData(size_t bytes, - const PacedPacketInfo& pacing_info) { - size_t padding_bytes_in_packet; - size_t max_payload_size = max_packet_size_ - RtpHeaderLength(); - - if (audio_configured_) { - // Allow smaller padding packets for audio. - padding_bytes_in_packet = - rtc::SafeClamp(bytes, kMinAudioPaddingLength, - rtc::SafeMin(max_payload_size, kMaxPaddingLength)); - } else { - // Always send full padding packets. This is accounted for by the - // RtpPacketSender, which will make sure we don't send too much padding even - // if a single packet is larger than requested. - // We do this to avoid frequently sending small packets on higher bitrates. - padding_bytes_in_packet = rtc::SafeMin(max_payload_size, kMaxPaddingLength); - } - size_t bytes_sent = 0; - while (bytes_sent < bytes) { - int64_t now_ms = clock_->TimeInMilliseconds(); - uint32_t ssrc; - uint32_t timestamp; - int64_t capture_time_ms; - uint16_t sequence_number; - int payload_type; - bool over_rtx; - { - rtc::CritScope lock(&send_critsect_); - if (!sending_media_) - break; - timestamp = last_rtp_timestamp_; - capture_time_ms = capture_time_ms_; - if (rtx_ == kRtxOff) { - if (last_payload_type_ == -1) - break; - // Without RTX we can't send padding in the middle of frames. - // For audio marker bits doesn't mark the end of a frame and frames - // are usually a single packet, so for now we don't apply this rule - // for audio. - if (!audio_configured_ && !last_packet_marker_bit_) { - break; - } - if (!ssrc_) { - RTC_LOG(LS_ERROR) << "SSRC unset."; - return 0; - } - - RTC_DCHECK(ssrc_); - ssrc = *ssrc_; - - sequence_number = sequence_number_; - ++sequence_number_; - payload_type = last_payload_type_; - over_rtx = false; - } else { - // Without abs-send-time or transport sequence number a media packet - // must be sent before padding so that the timestamps used for - // estimation are correct. - if (!media_has_been_sent_ && - !(rtp_header_extension_map_.IsRegistered(AbsoluteSendTime::kId) || - (rtp_header_extension_map_.IsRegistered( - TransportSequenceNumber::kId) && - transport_sequence_number_allocator_))) { - break; - } - // Only change change the timestamp of padding packets sent over RTX. - // Padding only packets over RTP has to be sent as part of a media - // frame (and therefore the same timestamp). - if (last_timestamp_time_ms_ > 0) { - timestamp += - (now_ms - last_timestamp_time_ms_) * kTimestampTicksPerMs; - capture_time_ms += (now_ms - last_timestamp_time_ms_); - } - if (!ssrc_rtx_) { - RTC_LOG(LS_ERROR) << "RTX SSRC unset."; - return 0; - } - RTC_DCHECK(ssrc_rtx_); - ssrc = *ssrc_rtx_; - sequence_number = sequence_number_rtx_; - ++sequence_number_rtx_; - payload_type = rtx_payload_type_map_.begin()->second; - over_rtx = true; - } - } - - RtpPacketToSend padding_packet(&rtp_header_extension_map_); - padding_packet.SetPayloadType(payload_type); - padding_packet.SetMarker(false); - padding_packet.SetSequenceNumber(sequence_number); - padding_packet.SetTimestamp(timestamp); - padding_packet.SetSsrc(ssrc); - - if (capture_time_ms > 0) { - padding_packet.SetExtension( - (now_ms - capture_time_ms) * kTimestampTicksPerMs); - } - padding_packet.SetExtension( - AbsoluteSendTime::MsTo24Bits(now_ms)); - PacketOptions options; - // Padding packets are never retransmissions. - options.is_retransmit = false; - bool has_transport_seq_num; - { - rtc::CritScope lock(&send_critsect_); - has_transport_seq_num = - UpdateTransportSequenceNumber(&padding_packet, &options.packet_id); - options.included_in_allocation = - has_transport_seq_num || force_part_of_allocation_; - options.included_in_feedback = has_transport_seq_num; - } - padding_packet.SetPadding(padding_bytes_in_packet); - if (has_transport_seq_num) { - AddPacketToTransportFeedback(options.packet_id, padding_packet, - pacing_info); - } - - if (!SendPacketToNetwork(padding_packet, options, pacing_info)) - break; - - bytes_sent += padding_bytes_in_packet; - UpdateRtpStats(padding_packet, over_rtx, false); - } - - return bytes_sent; -} - void RTPSender::SetStorePacketsStatus(bool enable, uint16_t number_to_store) { packet_history_.SetStorePacketsStatus( enable ? RtpPacketHistory::StorageMode::kStoreAndCull @@ -584,54 +370,34 @@ int32_t RTPSender::ReSendPacket(uint16_t packet_id) { const bool rtx = (RtxStatus() & kRtxRetransmitted) > 0; if (paced_sender_) { - 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); - } - if (retransmit_packet) { - retransmit_packet->set_retransmitted_sequence_number( - stored_packet.SequenceNumber()); - } + 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 (!packet) { - return -1; - } - packet->set_packet_type(RtpPacketToSend::Type::kRetransmission); - paced_sender_->EnqueuePacket(std::move(packet)); + } + if (rtx) { + retransmit_packet = BuildRtxPacket(stored_packet); + } else { + retransmit_packet = + absl::make_unique(stored_packet); + } + if (retransmit_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)); return packet_size; } @@ -712,28 +478,8 @@ RtpPacketSendResult RTPSender::TimeToSendPacket( int64_t capture_time_ms, bool retransmission, const PacedPacketInfo& pacing_info) { - if (!SendingMedia()) { - return RtpPacketSendResult::kPacketNotFound; - } - - std::unique_ptr packet; - if (ssrc == SSRC()) { - packet = packet_history_.GetPacketAndSetSendTime(sequence_number); - } else if (ssrc == FlexfecSsrc()) { - packet = flexfec_packet_history_.GetPacketAndSetSendTime(sequence_number); - } - - if (!packet) { - // Packet cannot be found or was resent too recently. - return RtpPacketSendResult::kPacketNotFound; - } - - return PrepareAndSendPacket( - std::move(packet), - retransmission && (RtxStatus() & kRtxRetransmitted) > 0, - retransmission, pacing_info) - ? RtpPacketSendResult::kSuccess - : RtpPacketSendResult::kTransportUnavailable; + RTC_NOTREACHED(); + return RtpPacketSendResult::kSuccess; } // Called from pacer when we can send the packet. @@ -971,12 +717,15 @@ void RTPSender::UpdateRtpStats(const RtpPacketToSend& packet, size_t RTPSender::TimeToSendPadding(size_t bytes, const PacedPacketInfo& pacing_info) { - if (bytes == 0) - return 0; - size_t bytes_sent = TrySendRedundantPayloads(bytes, pacing_info); - if (bytes_sent < bytes) - bytes_sent += SendPadData(bytes - bytes_sent, pacing_info); - return bytes_sent; + // TODO(bugs.webrtc.org/10633): Remove when downstream test usage is gone. + size_t padding_bytes_sent = 0; + for (auto& packet : GeneratePadding(bytes)) { + const size_t packet_size = packet->payload_size() + packet->padding_size(); + if (TrySendPacket(packet.get(), pacing_info)) { + padding_bytes_sent += packet_size; + } + } + return padding_bytes_sent; } std::vector> RTPSender::GeneratePadding( @@ -1101,10 +850,6 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, uint32_t ssrc = packet->Ssrc(); if (paced_sender_) { - uint16_t seq_no = packet->SequenceNumber(); - int64_t capture_time_ms = packet->capture_time_ms(); - size_t packet_size = - send_side_bwe_with_overhead_ ? packet->size() : packet->payload_size(); auto packet_type = packet->packet_type(); RTC_CHECK(packet_type) << "Packet type must be set before sending."; @@ -1112,25 +857,9 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, packet->set_capture_time_ms(now_ms); } - 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->set_allow_retransmission(storage == - StorageType::kAllowRetransmission); - paced_sender_->EnqueuePacket(std::move(packet)); - } + packet->set_allow_retransmission(storage == + StorageType::kAllowRetransmission); + paced_sender_->EnqueuePacket(std::move(packet)); return true; } @@ -1192,13 +921,6 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, return sent; } -bool RTPSender::SendToNetwork(std::unique_ptr packet, - StorageType storage, - RtpPacketSender::Priority priority) { - packet->set_packet_type(PacketPriorityToType(priority)); - return SendToNetwork(std::move(packet), storage); -} - void RTPSender::RecomputeMaxSendDelay() { max_delay_it_ = send_delays_.begin(); for (auto it = send_delays_.begin(); it != send_delays_.end(); ++it) { @@ -1730,7 +1452,6 @@ int64_t RTPSender::LastTimestampTimeMs() const { void RTPSender::SetRtt(int64_t rtt_ms) { packet_history_.SetRtt(rtt_ms); - flexfec_packet_history_.SetRtt(rtt_ms); } void RTPSender::OnPacketsAcknowledged( diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 08e8f42528..f831e1b6d7 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -176,11 +176,6 @@ class RTPSender { bool SendToNetwork(std::unique_ptr packet, StorageType storage); - // Fallback that infers PacketType from Priority. - bool SendToNetwork(std::unique_ptr packet, - StorageType storage, - RtpPacketSender::Priority priority); - // Called on update of RTP statistics. void RegisterRtpStatisticsCallback(StreamDataCountersCallback* callback); StreamDataCountersCallback* GetRtpStatisticsCallback() const; @@ -204,18 +199,11 @@ class RTPSender { // time. typedef std::map SendDelayMap; - size_t SendPadData(size_t bytes, const PacedPacketInfo& pacing_info); - bool PrepareAndSendPacket(std::unique_ptr packet, bool send_over_rtx, bool is_retransmit, const PacedPacketInfo& pacing_info); - // Return the number of bytes sent. Note that both of these functions may - // return a larger value that their argument. - size_t TrySendRedundantPayloads(size_t bytes, - const PacedPacketInfo& pacing_info); - std::unique_ptr BuildRtxPacket( const RtpPacketToSend& packet); @@ -269,9 +257,6 @@ class RTPSender { RTC_GUARDED_BY(send_critsect_); RtpPacketHistory packet_history_; - // TODO(brandtr): Remove |flexfec_packet_history_| when the FlexfecSender - // is hooked up to the PacedSender. - RtpPacketHistory flexfec_packet_history_; // Statistics rtc::CriticalSection statistics_crit_; @@ -327,11 +312,6 @@ class RTPSender { const bool send_side_bwe_with_overhead_; - // 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 d50528093f..484711d179 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -144,11 +144,8 @@ MATCHER_P(SameRtcEventTypeAs, value, "") { } struct TestConfig { - TestConfig(bool with_overhead, bool pacer_references_packets) - : with_overhead(with_overhead), - pacer_references_packets(pacer_references_packets) {} + explicit TestConfig(bool with_overhead) : with_overhead(with_overhead) {} bool with_overhead = false; - bool pacer_references_packets = false; }; std::string ToFieldTrialString(TestConfig config) { @@ -156,11 +153,6 @@ std::string ToFieldTrialString(TestConfig config) { 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; } @@ -734,30 +726,21 @@ TEST_P(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) { Field(&RtpPacketSendInfo::pacing_info, PacedPacketInfo())))) .Times(1); - 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()); - } + 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; - EXPECT_TRUE(packet.GetExtension(&transport_seq_no)); + EXPECT_TRUE( + transport_.last_sent_packet().GetExtension( + &transport_seq_no)); EXPECT_EQ(kTransportSequenceNumber, transport_seq_no); EXPECT_EQ(transport_.last_options_.packet_id, transport_seq_no); } @@ -778,26 +761,13 @@ 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()); - } + 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()); EXPECT_EQ(1, transport_.packets_sent()); EXPECT_EQ(packet_size, transport_.last_sent_packet().size()); @@ -826,27 +796,14 @@ TEST_P(RtpSenderTest, WritesNetwork2ToTimingExtensionWithPacer) { 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)); + absl::make_unique(*packet), kAllowRetransmission)); fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); - } EXPECT_EQ(1, transport_.packets_sent()); EXPECT_EQ(packet_size, transport_.last_sent_packet().size()); @@ -872,9 +829,8 @@ TEST_P(RtpSenderTest, WritesNetwork2ToTimingExtensionWithoutPacer) { const int kPropagateTimeMs = 10; fake_clock_.AdvanceTimeMilliseconds(kPropagateTimeMs); - EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet), - kAllowRetransmission, - RtpPacketSender::kNormalPriority)); + EXPECT_TRUE( + rtp_sender_->SendToNetwork(std::move(packet), kAllowRetransmission)); EXPECT_EQ(1, transport_.packets_sent()); absl::optional video_timing = @@ -900,19 +856,6 @@ TEST_P(RtpSenderTest, TrafficSmoothingWithExtensions) { size_t packet_size = packet->size(); const int kStoredTimeInMs = 100; - 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( @@ -924,7 +867,6 @@ TEST_P(RtpSenderTest, TrafficSmoothingWithExtensions) { 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()); @@ -957,17 +899,6 @@ TEST_P(RtpSenderTest, TrafficSmoothingRetransmits) { size_t packet_size = packet->size(); // Packet should be stored in a send bucket. - 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( @@ -979,7 +910,6 @@ TEST_P(RtpSenderTest, TrafficSmoothingRetransmits) { absl::make_unique(*packet), kAllowRetransmission)); // Immediately process send bucket and send packet. rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); - } EXPECT_EQ(1, transport_.packets_sent()); @@ -989,16 +919,6 @@ TEST_P(RtpSenderTest, TrafficSmoothingRetransmits) { EXPECT_CALL(mock_rtc_event_log_, LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))); - 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( @@ -1010,7 +930,6 @@ TEST_P(RtpSenderTest, TrafficSmoothingRetransmits) { 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()); @@ -1059,18 +978,6 @@ TEST_P(RtpSenderTest, SendPadding) { const int kStoredTimeInMs = 100; // Packet should be stored in a send bucket. - 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( @@ -1084,7 +991,6 @@ TEST_P(RtpSenderTest, SendPadding) { fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); ++seq_num; - } // Packet should now be sent. This test doesn't verify the regular video // packet, since it is tested in another test. @@ -1127,17 +1033,6 @@ TEST_P(RtpSenderTest, SendPadding) { packet = BuildRtpPacket(kPayload, kMarkerBit, timestamp, capture_time_ms); packet_size = packet->size(); - 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_, @@ -1147,7 +1042,6 @@ TEST_P(RtpSenderTest, SendPadding) { EXPECT_TRUE(rtp_sender_->SendToNetwork( absl::make_unique(*packet), kAllowRetransmission)); rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); - } // Process send bucket. EXPECT_EQ(++total_packets_sent, transport_.packets_sent()); @@ -1174,16 +1068,6 @@ TEST_P(RtpSenderTest, OnSendPacketUpdated) { OnSendPacket(kTransportSequenceNumber, _, _)) .Times(1); - 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( @@ -1193,7 +1077,6 @@ TEST_P(RtpSenderTest, OnSendPacketUpdated) { packet->set_packet_type(RtpPacketToSend::Type::kVideo); packet->SetExtension(kTransportSequenceNumber); rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo()); - } EXPECT_EQ(1, transport_.packets_sent()); } @@ -1206,16 +1089,6 @@ TEST_P(RtpSenderTest, OnSendPacketNotUpdatedForRetransmits) { EXPECT_CALL(send_packet_observer_, OnSendPacket(_, _, _)).Times(0); - 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( @@ -1225,150 +1098,11 @@ TEST_P(RtpSenderTest, OnSendPacketNotUpdatedForRetransmits) { 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_; - config.paced_sender = &mock_paced_sender_; - config.local_media_ssrc = kSsrc; - config.send_packet_observer = &send_packet_observer_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); - - rtp_sender_->SetSequenceNumber(kSeqNum); - EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( - kRtpExtensionTransportSequenceNumber, - kTransportSequenceNumberExtensionId)); - rtp_sender_->SetSequenceNumber(kSeqNum); - rtp_sender_->SetStorePacketsStatus(true, 10); - - EXPECT_CALL(send_packet_observer_, OnSendPacket(_, _, _)).Times(0); - - 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()); -} - -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; - } - - MockTransport transport; - RtpRtcp::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport; - config.paced_sender = &mock_paced_sender_; - config.local_media_ssrc = kSsrc; - config.rtx_send_ssrc = kRtxSsrc; - config.event_log = &mock_rtc_event_log_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_ = absl::make_unique(config); - - rtp_sender_->SetSequenceNumber(kSeqNum); - rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); - - uint16_t seq_num = kSeqNum; - rtp_sender_->SetStorePacketsStatus(true, 10); - int32_t rtp_header_len = kRtpHeaderSize; - EXPECT_EQ( - 0, rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionAbsoluteSendTime, - kAbsoluteSendTimeExtensionId)); - rtp_header_len += 4; // 4 bytes extension. - rtp_header_len += 4; // 4 extra bytes common to all extension headers. - - rtp_sender_->SetRtxStatus(kRtxRetransmitted | kRtxRedundantPayloads); - - const size_t kNumPayloadSizes = 10; - 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_rtc_event_log_, - LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) - .Times(kNumPayloadSizes); - - // 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(::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); - } - - EXPECT_CALL(mock_rtc_event_log_, - LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) - .Times(AtLeast(4)); - - // The amount of padding to send it too small to send a payload packet. - EXPECT_CALL(transport, SendRtp(_, kMaxPaddingSize + rtp_header_len, _)) - .WillOnce(Return(true)); - EXPECT_EQ(kMaxPaddingSize, - rtp_sender_->TimeToSendPadding(49, PacedPacketInfo())); - - // Payload padding will prefer packets with lower transmit count first and - // lower age second. - EXPECT_CALL(transport, SendRtp(_, - kPayloadSizes[kNumPayloadSizes - 1] + - rtp_header_len + kRtxHeaderSize, - Field(&PacketOptions::is_retransmit, true))) - .WillOnce(Return(true)); - EXPECT_EQ(kPayloadSizes[kNumPayloadSizes - 1], - rtp_sender_->TimeToSendPadding(500, PacedPacketInfo())); - - EXPECT_CALL(transport, SendRtp(_, - kPayloadSizes[kNumPayloadSizes - 2] + - rtp_header_len + kRtxHeaderSize, - _)) - .WillOnce(Return(true)); - - EXPECT_CALL(transport, SendRtp(_, kMaxPaddingSize + rtp_header_len, - Field(&PacketOptions::is_retransmit, false))) - .WillOnce(Return(true)); - EXPECT_EQ(kPayloadSizes[kNumPayloadSizes - 2] + kMaxPaddingSize, - rtp_sender_->TimeToSendPadding( - kPayloadSizes[kNumPayloadSizes - 2] + 49, PacedPacketInfo())); -} - TEST_P(RtpSenderTestWithoutPacer, SendGenericVideo) { const char payload_name[] = "GENERIC"; const uint8_t payload_type = 127; @@ -1476,27 +1210,6 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { uint16_t flexfec_seq_num; RTPVideoHeader video_header; - 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; @@ -1525,17 +1238,16 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { 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()); - EXPECT_EQ(kSeqNum, media_packet.SequenceNumber()); - EXPECT_EQ(kSsrc, media_packet.Ssrc()); - const RtpPacketReceived& flexfec_packet = transport_.sent_packets_[1]; - EXPECT_EQ(kFlexfecPayloadType, flexfec_packet.PayloadType()); - EXPECT_EQ(flexfec_seq_num, flexfec_packet.SequenceNumber()); - EXPECT_EQ(kFlexFecSsrc, flexfec_packet.Ssrc()); + const RtpPacketReceived& sent_media_packet = transport_.sent_packets_[0]; + EXPECT_EQ(kMediaPayloadType, sent_media_packet.PayloadType()); + EXPECT_EQ(kSeqNum, sent_media_packet.SequenceNumber()); + EXPECT_EQ(kSsrc, sent_media_packet.Ssrc()); + const RtpPacketReceived& sent_flexfec_packet = transport_.sent_packets_[1]; + EXPECT_EQ(kFlexfecPayloadType, sent_flexfec_packet.PayloadType()); + EXPECT_EQ(flexfec_seq_num, sent_flexfec_packet.SequenceNumber()); + EXPECT_EQ(kFlexFecSsrc, sent_flexfec_packet.Ssrc()); } // TODO(ilnik): because of webrtc:7859. Once FEC moved below pacer, this test @@ -1591,23 +1303,6 @@ TEST_P(RtpSenderTest, NoFlexfecForTimingFrames) { 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_, @@ -1630,13 +1325,12 @@ TEST_P(RtpSenderTest, NoFlexfecForTimingFrames) { 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()); + const RtpPacketReceived& sent_media_packet1 = transport_.sent_packets_[0]; + EXPECT_EQ(kMediaPayloadType, sent_media_packet1.PayloadType()); + EXPECT_EQ(kSeqNum, sent_media_packet1.SequenceNumber()); + EXPECT_EQ(kSsrc, sent_media_packet1.Ssrc()); // Now try to send not a timing frame. uint16_t flexfec_seq_num; @@ -1644,65 +1338,42 @@ TEST_P(RtpSenderTest, NoFlexfecForTimingFrames) { 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)); + std::unique_ptr media_packet2; + std::unique_ptr fec_packet; - 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_packet2 = std::move(packet); + } else { + EXPECT_EQ(packet->packet_type(), + RtpPacketToSend::Type::kForwardErrorCorrection); + EXPECT_EQ(packet->Ssrc(), kFlexFecSsrc); + fec_packet = std::move(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)); - 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_packet2 != nullptr); + ASSERT_TRUE(fec_packet != nullptr); - 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()); - } + flexfec_seq_num = fec_packet->SequenceNumber(); + rtp_sender_->TrySendPacket(media_packet2.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& sent_media_packet2 = transport_.sent_packets_[1]; + EXPECT_EQ(kMediaPayloadType, sent_media_packet2.PayloadType()); + EXPECT_EQ(kSeqNum + 1, sent_media_packet2.SequenceNumber()); + EXPECT_EQ(kSsrc, sent_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()); @@ -2018,13 +1689,8 @@ TEST_P(RtpSenderTest, FecOverheadRate) { constexpr size_t kNumMediaPackets = 10; constexpr size_t kNumFecPackets = kNumMediaPackets; constexpr int64_t kTimeBetweenPacketsMs = 10; - 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; @@ -2812,24 +2478,6 @@ TEST_P(RtpSenderTest, SetsCaptureTimeAndPopulatesTransmissionOffset) { const uint32_t kTimestampTicksPerMs = 90; const int64_t kOffsetMs = 10; - if (GetParam().pacer_references_packets) { - EXPECT_CALL(mock_paced_sender_, InsertPacket); - - auto packet = - BuildRtpPacket(kPayload, kMarkerBit, fake_clock_.TimeInMilliseconds(), - kMissingCaptureTimeMs); - packet->set_packet_type(RtpPacketToSend::Type::kVideo); - packet->ReserveExtension(); - packet->AllocatePayload(sizeof(kPayloadData)); - EXPECT_TRUE( - rtp_sender_->SendToNetwork(std::move(packet), kAllowRetransmission)); - - fake_clock_.AdvanceTimeMilliseconds(kOffsetMs); - - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, - fake_clock_.TimeInMilliseconds(), false, - PacedPacketInfo()); - } else { auto packet = BuildRtpPacket(kPayload, kMarkerBit, fake_clock_.TimeInMilliseconds(), kMissingCaptureTimeMs); @@ -2850,7 +2498,6 @@ TEST_P(RtpSenderTest, SetsCaptureTimeAndPopulatesTransmissionOffset) { fake_clock_.AdvanceTimeMilliseconds(kOffsetMs); rtp_sender_->TrySendPacket(packet_to_pace.get(), PacedPacketInfo()); - } EXPECT_EQ(1, transport_.packets_sent()); absl::optional transmission_time_extension = @@ -2862,13 +2509,6 @@ TEST_P(RtpSenderTest, SetsCaptureTimeAndPopulatesTransmissionOffset) { // original packet, so offset is delta from original packet to now. fake_clock_.AdvanceTimeMilliseconds(kOffsetMs); - if (GetParam().pacer_references_packets) { - EXPECT_CALL(mock_paced_sender_, InsertPacket); - EXPECT_GT(rtp_sender_->ReSendPacket(kSeqNum), 0); - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, - fake_clock_.TimeInMilliseconds(), true, - PacedPacketInfo()); - } else { std::unique_ptr rtx_packet_to_pace; EXPECT_CALL(mock_paced_sender_, EnqueuePacket) .WillOnce([&](std::unique_ptr packet) { @@ -2878,7 +2518,6 @@ TEST_P(RtpSenderTest, SetsCaptureTimeAndPopulatesTransmissionOffset) { EXPECT_GT(rtp_sender_->ReSendPacket(kSeqNum), 0); rtp_sender_->TrySendPacket(rtx_packet_to_pace.get(), PacedPacketInfo()); - } EXPECT_EQ(2, transport_.packets_sent()); transmission_time_extension = @@ -2953,28 +2592,6 @@ TEST_P(RtpSenderTest, IgnoresNackAfterDisablingMedia) { rtp_sender_->SetRtt(kRtt); // Send a packet so it is in the packet history. - if (GetParam().pacer_references_packets) { - EXPECT_CALL(mock_paced_sender_, InsertPacket); - SendGenericPacket(); - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, - fake_clock_.TimeInMilliseconds(), false, - PacedPacketInfo()); - ASSERT_EQ(1u, transport_.sent_packets_.size()); - - // Disable media sending and try to retransmit the packet, it should be put - // in the pacer queue. - rtp_sender_->SetSendingMediaStatus(false); - fake_clock_.AdvanceTimeMilliseconds(kRtt); - EXPECT_CALL(mock_paced_sender_, InsertPacket); - EXPECT_GT(rtp_sender_->ReSendPacket(kSeqNum), 0); - - // Time to send the retransmission. It should fail and the send packet - // counter should not increase. - rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, - fake_clock_.TimeInMilliseconds(), true, - PacedPacketInfo()); - ASSERT_EQ(1u, transport_.sent_packets_.size()); - } else { std::unique_ptr packet_to_pace; EXPECT_CALL(mock_paced_sender_, EnqueuePacket) .WillOnce([&](std::unique_ptr packet) { @@ -2990,19 +2607,16 @@ TEST_P(RtpSenderTest, IgnoresNackAfterDisablingMedia) { rtp_sender_->SetSendingMediaStatus(false); fake_clock_.AdvanceTimeMilliseconds(kRtt); EXPECT_LT(rtp_sender_->ReSendPacket(kSeqNum), 0); - } } INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderTest, - ::testing::Values(TestConfig{false, false}, - TestConfig{false, true}, - TestConfig{true, false}, - TestConfig{true, true})); + ::testing::Values(TestConfig{false}, + TestConfig{true})); INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderTestWithoutPacer, - ::testing::Values(TestConfig{false, false}, - TestConfig{true, false})); + ::testing::Values(TestConfig{false}, + TestConfig{true})); } // namespace webrtc