From b0df593e56b5d23e60de026783e208bf360127cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Mon, 18 Nov 2019 13:40:24 +0100 Subject: [PATCH] Reland "Prepares PacingController for simplified packet queue." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a reland of acdc22d7845c5dde7c23366110e54e5d26127c85 Original change's description: > Prepares PacingController for simplified packet queue. > > This CL removes references to RoundRobinPacketQueue::QueuedPacket, > other than the method to release an RtpPacketToSend. It also moves > both the BeginPop() and FinalizePop() to within a single helper > method. > > A follow-up cleanup of the packet queue will stop exposing the > QueuedPacket struct and replaces the the pop-methods with a single > new one that just returns an RtpPacketToSend. > > Bug: webrtc:10809 > Change-Id: I5208a93e12e6b56714d483cc12d2a37225ea8e5e > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/159889 > Commit-Queue: Erik Språng > Reviewed-by: Philip Eliasson > Cr-Commit-Position: refs/heads/master@{#29820} TBR=philipel@webrtc.org Bug: webrtc:10809 Change-Id: Id8196d9348d7fa69a5e410367b8a88e6039ef1b8 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160205 Commit-Queue: Erik Språng Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/master@{#29867} --- modules/pacing/pacing_controller.cc | 42 +++++++++++++++++++---------- modules/pacing/pacing_controller.h | 6 +++-- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index 16d1f36b95..18a4f88693 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -99,6 +99,8 @@ PacingController::PacingController(Clock* clock, pace_audio_(!IsDisabled(*field_trials_, "WebRTC-Pacer-BlockAudio")), small_first_probe_packet_( IsEnabled(*field_trials_, "WebRTC-Pacer-SmallFirstProbePacket")), + send_side_bwe_with_overhead_( + IsEnabled(*field_trials_, "WebRTC-SendSideBwe-WithOverhead")), min_packet_limit_(kDefaultMinPacketLimit), last_timestamp_(clock_->CurrentTime()), paused_(false), @@ -463,8 +465,10 @@ void PacingController::ProcessPackets() { // Fetch the next packet, so long as queue is not empty or budget is not // exhausted. - auto* packet = GetPendingPacket(pacing_info, target_send_time, now); - if (packet == nullptr) { + std::unique_ptr rtp_packet = + GetPendingPacket(pacing_info, target_send_time, now); + + if (rtp_packet == nullptr) { // No packet available to send, check if we should send padding. DataSize padding_to_add = PaddingToAdd(recommended_probe_size, data_sent); if (padding_to_add > DataSize::Zero()) { @@ -485,14 +489,19 @@ void PacingController::ProcessPackets() { break; } - std::unique_ptr rtp_packet = packet->ReleasePacket(); RTC_DCHECK(rtp_packet); + RTC_DCHECK(rtp_packet->packet_type().has_value()); + const RtpPacketToSend::Type packet_type = *rtp_packet->packet_type(); + const DataSize packet_size = DataSize::bytes( + send_side_bwe_with_overhead_ + ? rtp_packet->size() + : rtp_packet->payload_size() + rtp_packet->padding_size()); packet_sender_->SendRtpPacket(std::move(rtp_packet), pacing_info); - data_sent += packet->size(); - // Send succeeded, remove it from the queue and update send/process time to - // the target send time. - OnPacketSent(packet, target_send_time); + data_sent += packet_size; + + // Send done, update send/process time to the target send time. + OnPacketSent(packet_type, packet_size, target_send_time); if (recommended_probe_size && data_sent > *recommended_probe_size) break; @@ -551,7 +560,7 @@ DataSize PacingController::PaddingToAdd( return DataSize::Zero(); } -RoundRobinPacketQueue::QueuedPacket* PacingController::GetPendingPacket( +std::unique_ptr PacingController::GetPendingPacket( const PacedPacketInfo& pacing_info, Timestamp target_send_time, Timestamp now) { @@ -593,23 +602,28 @@ RoundRobinPacketQueue::QueuedPacket* PacingController::GetPendingPacket( } } - return packet_queue_.BeginPop(); + auto* queued_packet = packet_queue_.BeginPop(); + std::unique_ptr rtp_packet; + if (queued_packet != nullptr) { + rtp_packet = queued_packet->ReleasePacket(); + packet_queue_.FinalizePop(); + } + return rtp_packet; } -void PacingController::OnPacketSent(RoundRobinPacketQueue::QueuedPacket* packet, +void PacingController::OnPacketSent(RtpPacketToSend::Type packet_type, + DataSize packet_size, Timestamp send_time) { if (!first_sent_packet_time_) { first_sent_packet_time_ = send_time; } - bool audio_packet = packet->type() == RtpPacketToSend::Type::kAudio; + bool audio_packet = packet_type == RtpPacketToSend::Type::kAudio; if (!audio_packet || account_for_audio_) { // Update media bytes sent. - UpdateBudgetWithSentData(packet->size()); + UpdateBudgetWithSentData(packet_size); } last_send_time_ = send_time; last_process_time_ = send_time; - // Send succeeded, remove it from the queue. - packet_queue_.FinalizePop(); } void PacingController::OnPaddingSent(DataSize data_sent) { diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h index d6b5abfdf4..6a05eac438 100644 --- a/modules/pacing/pacing_controller.h +++ b/modules/pacing/pacing_controller.h @@ -150,11 +150,12 @@ class PacingController { DataSize PaddingToAdd(absl::optional recommended_probe_size, DataSize data_sent) const; - RoundRobinPacketQueue::QueuedPacket* GetPendingPacket( + std::unique_ptr GetPendingPacket( const PacedPacketInfo& pacing_info, Timestamp target_send_time, Timestamp now); - void OnPacketSent(RoundRobinPacketQueue::QueuedPacket* packet, + void OnPacketSent(RtpPacketToSend::Type packet_type, + DataSize packet_size, Timestamp send_time); void OnPaddingSent(DataSize padding_sent); @@ -170,6 +171,7 @@ class PacingController { const bool send_padding_if_silent_; const bool pace_audio_; const bool small_first_probe_packet_; + const bool send_side_bwe_with_overhead_; TimeDelta min_packet_limit_;