From acded164242a95263c9364798487d81f40b0fad7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Fri, 22 Apr 2022 15:59:06 +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 fc5871cb98481937351bee2f46b2a2e3987d177a. Reason for revert: I rage quit. Original change's description: > Reland "Send first probe packet directly instead of enqueuing it." > > This is a reland of commit 8088aad5ac0154d8fdc252de9e8ab4e0172d7da2 > > Patchset 1 is original CL, patchset 2 contains a fix to make behavior > exactly identical to original. It fixes a difference when the first > small probe packet comprises the entire probe. > > 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: I7d2f476a7043edd03f44a4a3de31f13c3c946a8d > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259776 > Reviewed-by: Emil Lundmark > Commit-Queue: Erik Språng > Cr-Commit-Position: refs/heads/main@{#36626} Bug: webrtc:11340 Change-Id: Ife22cff8f5fbf9b70dc24926b84caad82d2738b8 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259778 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/main@{#36628} --- modules/pacing/pacing_controller.cc | 65 +++++++++++------------------ modules/pacing/pacing_controller.h | 3 -- 2 files changed, 24 insertions(+), 44 deletions(-) diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index 28d78734f3..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,21 +479,11 @@ void PacingController::ProcessPackets() { // 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); - - // Send packet immediately to avoid priority inversions. - data_sent += SendPacket(std::move(padding[0]), pacing_info, now); - - // If we are currently probing, we need to stop the send sending when - // we have reached the send target. - if (is_probing && data_sent >= recommended_probe_size) { - RTC_DCHECK(!data_sent.IsZero()); - probing_send_failure_ = false; - prober_.ProbeSent(CurrentTime(), data_sent); - return; - } } } } else { @@ -503,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; @@ -535,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; } @@ -657,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);