From 8b749e462b8fc4f2a367d37904cd536a351bbdea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Mon, 25 Apr 2022 13:21:27 +0200 Subject: [PATCH] Move priority handling from PacingController to packet queue. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will simplify upcoming changes to the queue. Bug: webrtc:11340 Change-Id: Id023618fdef8a8bc9fb50e477cc87e6ba20c5421 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/259875 Reviewed-by: Danil Chapovalov Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/main@{#36644} --- modules/pacing/pacing_controller.cc | 70 ++++++---------------- modules/pacing/pacing_controller.h | 2 - modules/pacing/round_robin_packet_queue.cc | 28 ++++++++- modules/pacing/round_robin_packet_queue.h | 3 +- 4 files changed, 45 insertions(+), 58 deletions(-) diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index ba5ba5a156..f762b96158 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -38,8 +38,6 @@ constexpr TimeDelta kMaxElapsedTime = TimeDelta::Seconds(2); // time. Applies only to periodic mode. constexpr TimeDelta kMaxProcessingInterval = TimeDelta::Millis(30); -constexpr int kFirstPriority = 0; - bool IsDisabled(const FieldTrialsView& field_trials, absl::string_view key) { return absl::StartsWith(field_trials.Lookup(key), "Disabled"); } @@ -56,29 +54,6 @@ TimeDelta GetDynamicPaddingTarget(const FieldTrialsView& field_trials) { return padding_target.Get(); } -int GetPriorityForType(RtpPacketMediaType type) { - // Lower number takes priority over higher. - switch (type) { - case RtpPacketMediaType::kAudio: - // Audio is always prioritized over other packet types. - return kFirstPriority + 1; - case RtpPacketMediaType::kRetransmission: - // Send retransmissions before new media. - return kFirstPriority + 2; - case RtpPacketMediaType::kVideo: - case RtpPacketMediaType::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 kFirstPriority + 3; - case RtpPacketMediaType::kPadding: - // Packets that are in themselves likely useless, only sent to keep the - // BWE high. - return kFirstPriority + 4; - } - RTC_CHECK_NOTREACHED(); -} - } // namespace const TimeDelta PacingController::kMaxExpectedQueueLength = @@ -217,10 +192,24 @@ void PacingController::EnqueuePacket(std::unique_ptr packet) { RTC_DCHECK(pacing_bitrate_ > DataRate::Zero()) << "SetPacingRate must be called before InsertPacket."; RTC_CHECK(packet->packet_type()); - // 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); + + prober_.OnIncomingPacket(DataSize::Bytes(packet->payload_size())); + + Timestamp now = CurrentTime(); + if (mode_ == ProcessMode::kDynamic && packet_queue_.Empty()) { + // If queue is empty, we need to "fast-forward" the last process time, + // so that we don't use passed time as budget for sending the first new + // packet. + Timestamp target_process_time = now; + Timestamp next_send_time = NextSendTime(); + if (next_send_time.IsFinite()) { + // There was already a valid planned send time, such as a keep-alive. + // Use that as last process time only if it's prior to now. + target_process_time = std::min(now, next_send_time); + } + UpdateBudgetWithElapsedTime(UpdateTimeAndGetElapsed(target_process_time)); + } + packet_queue_.Push(now, packet_counter_++, std::move(packet)); } void PacingController::SetAccountForAudioPackets(bool account_for_audio) { @@ -266,29 +255,6 @@ Timestamp PacingController::OldestPacketEnqueueTime() const { return packet_queue_.OldestEnqueueTime(); } -void PacingController::EnqueuePacketInternal( - std::unique_ptr packet, - int priority) { - prober_.OnIncomingPacket(DataSize::Bytes(packet->payload_size())); - - Timestamp now = CurrentTime(); - - if (mode_ == ProcessMode::kDynamic && packet_queue_.Empty()) { - // If queue is empty, we need to "fast-forward" the last process time, - // so that we don't use passed time as budget for sending the first new - // packet. - Timestamp target_process_time = now; - Timestamp next_send_time = NextSendTime(); - if (next_send_time.IsFinite()) { - // There was already a valid planned send time, such as a keep-alive. - // Use that as last process time only if it's prior to now. - target_process_time = std::min(now, next_send_time); - } - UpdateBudgetWithElapsedTime(UpdateTimeAndGetElapsed(target_process_time)); - } - packet_queue_.Push(priority, now, packet_counter_++, std::move(packet)); -} - TimeDelta PacingController::UpdateTimeAndGetElapsed(Timestamp now) { // If no previous processing, or last process was "in the future" because of // early probe processing, then there is no elapsed time to add budget for. diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h index c3e1dde2fb..c87154d1c2 100644 --- a/modules/pacing/pacing_controller.h +++ b/modules/pacing/pacing_controller.h @@ -150,8 +150,6 @@ class PacingController { bool IsProbing() const; private: - void EnqueuePacketInternal(std::unique_ptr packet, - int priority); TimeDelta UpdateTimeAndGetElapsed(Timestamp now); bool ShouldSendKeepalive(Timestamp now) const; diff --git a/modules/pacing/round_robin_packet_queue.cc b/modules/pacing/round_robin_packet_queue.cc index 9014c30cb6..b372d66973 100644 --- a/modules/pacing/round_robin_packet_queue.cc +++ b/modules/pacing/round_robin_packet_queue.cc @@ -20,8 +20,32 @@ namespace webrtc { namespace { static constexpr DataSize kMaxLeadingSize = DataSize::Bytes(1400); + +int GetPriorityForType(RtpPacketMediaType type) { + // Lower number takes priority over higher. + switch (type) { + case RtpPacketMediaType::kAudio: + // Audio is always prioritized over other packet types. + return 0; + case RtpPacketMediaType::kRetransmission: + // Send retransmissions before new media. + return 1; + case RtpPacketMediaType::kVideo: + case RtpPacketMediaType::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; + case RtpPacketMediaType::kPadding: + // Packets that are in themselves likely useless, only sent to keep the + // BWE high. + return 3; + } + RTC_CHECK_NOTREACHED(); } +} // namespace + RoundRobinPacketQueue::QueuedPacket::QueuedPacket(const QueuedPacket& rhs) = default; RoundRobinPacketQueue::QueuedPacket::~QueuedPacket() = default; @@ -125,11 +149,11 @@ RoundRobinPacketQueue::~RoundRobinPacketQueue() { } } -void RoundRobinPacketQueue::Push(int priority, - Timestamp enqueue_time, +void RoundRobinPacketQueue::Push(Timestamp enqueue_time, uint64_t enqueue_order, std::unique_ptr packet) { RTC_DCHECK(packet->packet_type().has_value()); + int priority = GetPriorityForType(*packet->packet_type()); if (size_packets_ == 0) { // Single packet fast-path. single_packet_queue_.emplace( diff --git a/modules/pacing/round_robin_packet_queue.h b/modules/pacing/round_robin_packet_queue.h index f8255834b3..38149f40f0 100644 --- a/modules/pacing/round_robin_packet_queue.h +++ b/modules/pacing/round_robin_packet_queue.h @@ -36,8 +36,7 @@ class RoundRobinPacketQueue { explicit RoundRobinPacketQueue(Timestamp start_time); ~RoundRobinPacketQueue(); - void Push(int priority, - Timestamp enqueue_time, + void Push(Timestamp enqueue_time, uint64_t enqueue_order, std::unique_ptr packet); std::unique_ptr Pop();