From 660a4343079e9495ec136851bb7f2ec0ba355d82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Fri, 22 Apr 2022 13:00:38 +0000 Subject: [PATCH] Revert "Reland "Send first probe packet directly instead of enqueuing it."" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 1386fbca8b558f14336d9863c8a4e8f2654ab568. Reason for revert: Now another downstream test flakes :( Original change's description: > Reland "Send first probe packet directly instead of enqueuing it." > > This is a reland of commit 8088aad5ac0154d8fdc252de9e8ab4e0172d7da2 > > Original change's description: > > Send first probe packet directly instead of enqueuing it. > > > > This avoids potentially creating needless containers in the packet > > queue and removes usage of the packet prio, allowing it to be moved in > > an upcoming CL. > > > > Bug: webrtc:11340 > > Change-Id: Iddd9e7e4e73c97ab25a85e42bcc0094d61fd60d3 > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259524 > > Reviewed-by: Emil Lundmark > > Commit-Queue: Erik Språng > > Cr-Commit-Position: refs/heads/main@{#36602} > > Bug: webrtc:11340 > Change-Id: I3e0db452961f7f5a8d00ea3283e92ec43da2f66a > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259774 > Reviewed-by: Emil Lundmark > Commit-Queue: Erik Språng > Cr-Commit-Position: refs/heads/main@{#36619} Bug: webrtc:11340 Change-Id: I31b2c6636aca5debe8e4d71546b5a3b9eb746c9d No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259862 Auto-Submit: Erik Språng Reviewed-by: Erik Språng Commit-Queue: Erik Språng Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Commit-Queue: rubber-stamper@appspot.gserviceaccount.com Cr-Commit-Position: refs/heads/main@{#36620} --- modules/pacing/pacing_controller.cc | 55 +++++++++++++---------------- modules/pacing/pacing_controller.h | 3 -- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index 68bc2fe5e6..f2895cf4e2 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -463,7 +463,6 @@ void PacingController::ProcessPackets() { PacedPacketInfo pacing_info; DataSize recommended_probe_size = DataSize::Zero(); - DataSize data_sent = DataSize::Zero(); bool is_probing = prober_.is_probing(); if (is_probing) { // Probe timing is sensitive, and handled explicitly by BitrateProber, so @@ -480,8 +479,8 @@ void PacingController::ProcessPackets() { // If no RTP modules sending media are registered, we may not get a // padding packet back. if (!padding.empty()) { - // Send packet immediately to avoid priority inversions. - data_sent += SendPacket(std::move(padding[0]), pacing_info, now); + // 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); @@ -493,6 +492,7 @@ void PacingController::ProcessPackets() { } } + DataSize data_sent = DataSize::Zero(); // Circuit breaker, making sure main loop isn't forever. static constexpr int kMaxIterations = 1 << 16; int iteration = 0; @@ -525,11 +525,29 @@ void PacingController::ProcessPackets() { // Can't fetch new packet and no padding to send, exit send loop. break; } else { - data_sent += SendPacket(std::move(rtp_packet), pacing_info, now); + RTC_DCHECK(rtp_packet); + RTC_DCHECK(rtp_packet->packet_type().has_value()); + const RtpPacketMediaType packet_type = *rtp_packet->packet_type(); + DataSize packet_size = DataSize::Bytes(rtp_packet->payload_size() + + rtp_packet->padding_size()); + + if (include_overhead_) { + packet_size += DataSize::Bytes(rtp_packet->headers_size()) + + transport_overhead_per_packet_; + } + + packet_sender_->SendPacket(std::move(rtp_packet), pacing_info); + for (auto& packet : packet_sender_->FetchFec()) { + EnqueuePacket(std::move(packet)); + } + data_sent += packet_size; ++packets_sent; - // If we are currently probing, we need to stop the send loop when we have - // reached the send target. + // Send done, update send time. + OnPacketSent(packet_type, packet_size, now); + + // If we are currently probing, we need to stop the send loop when we + // have reached the send target. if (is_probing && data_sent >= recommended_probe_size) { break; } @@ -647,31 +665,6 @@ std::unique_ptr PacingController::GetPendingPacket( return packet_queue_.Pop(); } -DataSize PacingController::SendPacket(std::unique_ptr packet, - const PacedPacketInfo& pacing_info, - Timestamp now) { - RTC_DCHECK(packet); - RTC_DCHECK(packet->packet_type().has_value()); - const RtpPacketMediaType packet_type = *packet->packet_type(); - DataSize packet_size = - DataSize::Bytes(packet->payload_size() + packet->padding_size()); - - if (include_overhead_) { - packet_size += DataSize::Bytes(packet->headers_size()) + - transport_overhead_per_packet_; - } - - packet_sender_->SendPacket(std::move(packet), pacing_info); - for (std::unique_ptr& packet : packet_sender_->FetchFec()) { - EnqueuePacket(std::move(packet)); - } - - // Sending complete, update send time. - OnPacketSent(packet_type, packet_size, now); - - return packet_size; -} - void PacingController::OnPacketSent(RtpPacketMediaType packet_type, DataSize packet_size, Timestamp send_time) { diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h index 7c5b4a6d28..c3e1dde2fb 100644 --- a/modules/pacing/pacing_controller.h +++ b/modules/pacing/pacing_controller.h @@ -167,9 +167,6 @@ class PacingController { const PacedPacketInfo& pacing_info, Timestamp target_send_time, Timestamp now); - DataSize SendPacket(std::unique_ptr packet, - const PacedPacketInfo& pacing_info, - Timestamp now); void OnPacketSent(RtpPacketMediaType packet_type, DataSize packet_size, Timestamp send_time);