diff --git a/api/transport/network_types.h b/api/transport/network_types.h index 320a7c07c2..f45a35dbf0 100644 --- a/api/transport/network_types.h +++ b/api/transport/network_types.h @@ -98,6 +98,7 @@ struct PacedPacketInfo { int probe_cluster_id = kNotAProbe; int probe_cluster_min_probes = -1; int probe_cluster_min_bytes = -1; + int probe_cluster_bytes_sent = 0; }; struct SentPacket { diff --git a/modules/pacing/bitrate_prober.cc b/modules/pacing/bitrate_prober.cc index 99041dae4c..4192df956b 100644 --- a/modules/pacing/bitrate_prober.cc +++ b/modules/pacing/bitrate_prober.cc @@ -146,7 +146,9 @@ int BitrateProber::TimeUntilNextProbe(int64_t now_ms) { PacedPacketInfo BitrateProber::CurrentCluster() const { RTC_DCHECK(!clusters_.empty()); RTC_DCHECK(probing_state_ == ProbingState::kActive); - return clusters_.front().pace_info; + PacedPacketInfo info = clusters_.front().pace_info; + info.probe_cluster_bytes_sent = clusters_.front().sent_bytes; + return info; } // Probe size is recommended based on the probe bitrate required. We choose diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index 1633de96f9..85b9e05dc6 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -34,6 +34,8 @@ constexpr TimeDelta kMaxElapsedTime = TimeDelta::Seconds<2>(); // time. constexpr TimeDelta kMaxProcessingInterval = TimeDelta::Millis<30>(); +constexpr int kFirstPriority = 0; + bool IsDisabled(const WebRtcKeyValueConfig& field_trials, absl::string_view key) { return field_trials.Lookup(key).find("Disabled") == 0; @@ -45,24 +47,24 @@ bool IsEnabled(const WebRtcKeyValueConfig& field_trials, } int GetPriorityForType(RtpPacketToSend::Type type) { + // Lower number takes priority over higher. switch (type) { case RtpPacketToSend::Type::kAudio: // Audio is always prioritized over other packet types. - return 0; + return kFirstPriority + 1; case RtpPacketToSend::Type::kRetransmission: // Send retransmissions before new media. - return 1; + return kFirstPriority + 2; case RtpPacketToSend::Type::kVideo: - // Video has "normal" priority, in the old speak. - return 2; case RtpPacketToSend::Type::kForwardErrorCorrection: + // Video has "normal" priority, in the old speak. // Send redundancy concurrently to video. If it is delayed it might have a // lower chance of being useful. - return 2; + return kFirstPriority + 3; case RtpPacketToSend::Type::kPadding: // Packets that are in themselves likely useless, only sent to keep the // BWE high. - return 3; + return kFirstPriority + 4; } } @@ -88,6 +90,8 @@ PacingController::PacingController(Clock* clock, send_padding_if_silent_( IsEnabled(*field_trials_, "WebRTC-Pacer-PadInSilence")), pace_audio_(!IsDisabled(*field_trials_, "WebRTC-Pacer-BlockAudio")), + small_first_probe_packet_( + IsEnabled(*field_trials_, "WebRTC-Pacer-SmallFirstProbePacket")), min_packet_limit_(kDefaultMinPacketLimit), last_timestamp_(clock_->CurrentTime()), paused_(false), @@ -187,17 +191,11 @@ void PacingController::SetPacingRates(DataRate pacing_rate, void PacingController::EnqueuePacket(std::unique_ptr packet) { RTC_DCHECK(pacing_bitrate_ > DataRate::Zero()) << "SetPacingRate must be called before InsertPacket."; - - Timestamp now = CurrentTime(); - prober_.OnIncomingPacket(packet->payload_size()); - - if (packet->capture_time_ms() < 0) { - packet->set_capture_time_ms(now.ms()); - } - RTC_CHECK(packet->packet_type()); - int priority = GetPriorityForType(*packet->packet_type()); - packet_queue_.Push(priority, now, packet_counter_++, std::move(packet)); + // Get priority first and store in temporary, to avoid chance of object being + // moved before GetPriorityForType() being called. + const int priority = GetPriorityForType(*packet->packet_type()); + EnqueuePacketInternal(std::move(packet), priority); } void PacingController::SetAccountForAudioPackets(bool account_for_audio) { @@ -232,6 +230,22 @@ TimeDelta PacingController::OldestPacketWaitTime() const { return CurrentTime() - oldest_packet; } +void PacingController::EnqueuePacketInternal( + std::unique_ptr packet, + int priority) { + prober_.OnIncomingPacket(packet->payload_size()); + + Timestamp now = CurrentTime(); + prober_.OnIncomingPacket(packet->payload_size()); + + // TODO(sprang): Make sure tests respect this, replace with DCHECK. + if (packet->capture_time_ms() < 0) { + packet->set_capture_time_ms(now.ms()); + } + + packet_queue_.Push(priority, now, packet_counter_++, std::move(packet)); +} + TimeDelta PacingController::UpdateTimeAndGetElapsed(Timestamp now) { TimeDelta elapsed_time = now - time_last_process_; time_last_process_ = now; @@ -322,11 +336,13 @@ void PacingController::ProcessPackets() { UpdateBudgetWithElapsedTime(elapsed_time); } + bool first_packet_in_probe = false; bool is_probing = prober_.IsProbing(); PacedPacketInfo pacing_info; absl::optional recommended_probe_size; if (is_probing) { pacing_info = prober_.CurrentCluster(); + first_packet_in_probe = pacing_info.probe_cluster_bytes_sent == 0; recommended_probe_size = DataSize::bytes(prober_.RecommendedMinProbeSize()); } @@ -334,6 +350,22 @@ void PacingController::ProcessPackets() { // 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 (!paused_) { + if (small_first_probe_packet_ && first_packet_in_probe) { + // If first packet in probe, insert a small padding packet so we have a + // more reliable start window for the rate estimation. + auto padding = packet_sender_->GeneratePadding(DataSize::bytes(1)); + // If no RTP modules sending media are registered, we may not get a + // padding packet back. + if (!padding.empty()) { + // Insert with high priority so larger media packets don't preempt it. + EnqueuePacketInternal(std::move(padding[0]), kFirstPriority); + // We should never get more than one padding packets with a requested + // size of 1 byte. + RTC_DCHECK_EQ(padding.size(), 1u); + } + first_packet_in_probe = false; + } + auto* packet = GetPendingPacket(pacing_info); if (packet == nullptr) { // No packet available to send, check if we should send padding. diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h index 1b05444c3b..d0e68a9a71 100644 --- a/modules/pacing/pacing_controller.h +++ b/modules/pacing/pacing_controller.h @@ -135,6 +135,8 @@ class PacingController { bool Congested() const; private: + void EnqueuePacketInternal(std::unique_ptr packet, + int priority); TimeDelta UpdateTimeAndGetElapsed(Timestamp now); bool ShouldSendKeepalive(Timestamp now) const; @@ -160,6 +162,7 @@ class PacingController { const bool drain_large_queues_; const bool send_padding_if_silent_; const bool pace_audio_; + const bool small_first_probe_packet_; TimeDelta min_packet_limit_; // TODO(webrtc:9716): Remove this when we are certain clocks are monotonic. diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc index bcd4384b66..caec575233 100644 --- a/modules/pacing/pacing_controller_unittest.cc +++ b/modules/pacing/pacing_controller_unittest.cc @@ -1265,5 +1265,47 @@ TEST_F(PacingControllerTest, OwnedPacketPrioritizedOnType) { clock_.AdvanceTimeMilliseconds(200); pacer_->ProcessPackets(); } + +TEST_F(PacingControllerTest, SmallFirstProbePacket) { + ScopedFieldTrials trial("WebRTC-Pacer-SmallFirstProbePacket/Enabled/"); + MockPacketSender callback; + pacer_ = + std::make_unique(&clock_, &callback, nullptr, nullptr); + pacer_->CreateProbeCluster(kFirstClusterRate, /*cluster_id=*/0); + pacer_->SetPacingRates(kTargetRate * kPaceMultiplier, DataRate::Zero()); + + // Add high prio media. + pacer_->EnqueuePacket(BuildRtpPacket(RtpPacketToSend::Type::kAudio)); + + // Expect small padding packet to be requested. + EXPECT_CALL(callback, GeneratePadding(DataSize::bytes(1))) + .WillOnce([&](DataSize padding_size) { + std::vector> padding_packets; + padding_packets.emplace_back( + BuildPacket(RtpPacketToSend::Type::kPadding, kAudioSsrc, 1, + clock_.TimeInMilliseconds(), 1)); + return padding_packets; + }); + + size_t packets_sent = 0; + bool media_seen = false; + EXPECT_CALL(callback, SendRtpPacket) + .Times(::testing::AnyNumber()) + .WillRepeatedly([&](std::unique_ptr packet, + const PacedPacketInfo& cluster_info) { + if (packets_sent == 0) { + EXPECT_EQ(packet->packet_type(), RtpPacketToSend::Type::kPadding); + } else { + if (packet->packet_type() == RtpPacketToSend::Type::kAudio) { + media_seen = true; + } + } + packets_sent++; + }); + while (!media_seen) { + pacer_->ProcessPackets(); + clock_.AdvanceTimeMilliseconds(5); + } +} } // namespace test } // namespace webrtc