From 71a77c4b3b314a5e3b4e6b2f12d4886cff1b60d7 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Tue, 28 Jan 2020 15:51:50 +0100 Subject: [PATCH] Adds trial to use correct overhead calculation in pacer. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:9883 Change-Id: I1f25a235468678bf823ee1399ba31d94acf33be9 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166534 Reviewed-by: Erik Språng Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#30399} --- call/rtp_transport_controller_send.cc | 3 ++ modules/pacing/paced_sender.cc | 5 ++++ modules/pacing/paced_sender.h | 1 + modules/pacing/pacing_controller.cc | 21 +++++++++++--- modules/pacing/pacing_controller.h | 5 ++++ modules/pacing/round_robin_packet_queue.cc | 32 ++++++++++++++-------- modules/pacing/round_robin_packet_queue.h | 4 ++- modules/pacing/rtp_packet_pacer.h | 1 + modules/pacing/task_queue_paced_sender.cc | 7 +++++ modules/pacing/task_queue_paced_sender.h | 2 ++ 10 files changed, 65 insertions(+), 16 deletions(-) diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index c2946adbaf..20f3a996e5 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -421,6 +421,9 @@ void RtpTransportControllerSend::OnTransportOverheadChanged( return; } + pacer()->SetTransportOverhead( + DataSize::bytes(transport_overhead_bytes_per_packet)); + // TODO(holmer): Call AudioRtpSenders when they have been moved to // RtpTransportControllerSend. for (auto& rtp_video_sender : video_rtp_senders_) { diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index 6dc47b6892..3646952728 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -131,6 +131,11 @@ void PacedSender::SetIncludeOverhead() { pacing_controller_.SetIncludeOverhead(); } +void PacedSender::SetTransportOverhead(DataSize overhead_per_packet) { + rtc::CritScope cs(&critsect_); + pacing_controller_.SetTransportOverhead(overhead_per_packet); +} + TimeDelta PacedSender::ExpectedQueueTime() const { rtc::CritScope cs(&critsect_); return pacing_controller_.ExpectedQueueTime(); diff --git a/modules/pacing/paced_sender.h b/modules/pacing/paced_sender.h index 36913080e0..16137dfcd6 100644 --- a/modules/pacing/paced_sender.h +++ b/modules/pacing/paced_sender.h @@ -98,6 +98,7 @@ class PacedSender : public Module, void SetAccountForAudioPackets(bool account_for_audio) override; void SetIncludeOverhead() override; + void SetTransportOverhead(DataSize overhead_per_packet) override; // Returns the time since the oldest queued packet was enqueued. TimeDelta OldestPacketWaitTime() const override; diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index 09b76301fb..f2b21492de 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -99,7 +99,10 @@ PacingController::PacingController(Clock* clock, pace_audio_(IsEnabled(*field_trials_, "WebRTC-Pacer-BlockAudio")), small_first_probe_packet_( IsEnabled(*field_trials_, "WebRTC-Pacer-SmallFirstProbePacket")), + ignore_transport_overhead_( + !IsDisabled(*field_trials_, "WebRTC-Pacer-IgnoreTransportOverhead")), min_packet_limit_(kDefaultMinPacketLimit), + transport_overhead_per_packet_(DataSize::Zero()), last_timestamp_(clock_->CurrentTime()), paused_(false), media_budget_(0), @@ -230,6 +233,13 @@ void PacingController::SetIncludeOverhead() { packet_queue_.SetIncludeOverhead(); } +void PacingController::SetTransportOverhead(DataSize overhead_per_packet) { + if (ignore_transport_overhead_) + return; + transport_overhead_per_packet_ = overhead_per_packet; + packet_queue_.SetTransportOverhead(overhead_per_packet); +} + TimeDelta PacingController::ExpectedQueueTime() const { RTC_DCHECK_GT(pacing_bitrate_, DataRate::Zero()); return TimeDelta::ms( @@ -521,10 +531,13 @@ void PacingController::ProcessPackets() { 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(include_overhead_ ? rtp_packet->size() - : rtp_packet->payload_size() + - rtp_packet->padding_size()); + 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_->SendRtpPacket(std::move(rtp_packet), pacing_info); data_sent += packet_size; diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h index 12e3612684..45cab24269 100644 --- a/modules/pacing/pacing_controller.h +++ b/modules/pacing/pacing_controller.h @@ -109,6 +109,8 @@ class PacingController { void SetAccountForAudioPackets(bool account_for_audio); void SetIncludeOverhead(); + void SetTransportOverhead(DataSize overhead_per_packet); + // Returns the time since the oldest queued packet was enqueued. TimeDelta OldestPacketWaitTime() const; @@ -177,9 +179,12 @@ class PacingController { const bool send_padding_if_silent_; const bool pace_audio_; const bool small_first_probe_packet_; + const bool ignore_transport_overhead_; TimeDelta min_packet_limit_; + DataSize transport_overhead_per_packet_; + // TODO(webrtc:9716): Remove this when we are certain clocks are monotonic. // The last millisecond timestamp returned by |clock_|. mutable Timestamp last_timestamp_; diff --git a/modules/pacing/round_robin_packet_queue.cc b/modules/pacing/round_robin_packet_queue.cc index 754ff5888a..32f288c209 100644 --- a/modules/pacing/round_robin_packet_queue.cc +++ b/modules/pacing/round_robin_packet_queue.cc @@ -73,12 +73,6 @@ uint64_t RoundRobinPacketQueue::QueuedPacket::EnqueueOrder() const { return enqueue_order_; } -DataSize RoundRobinPacketQueue::QueuedPacket::Size(bool count_overhead) const { - return DataSize::bytes(count_overhead ? owned_packet_->size() - : owned_packet_->payload_size() + - owned_packet_->padding_size()); -} - RtpPacketToSend* RoundRobinPacketQueue::QueuedPacket::RtpPacket() const { return owned_packet_; } @@ -117,7 +111,8 @@ bool IsEnabled(const WebRtcKeyValueConfig* field_trials, const char* name) { RoundRobinPacketQueue::RoundRobinPacketQueue( Timestamp start_time, const WebRtcKeyValueConfig* field_trials) - : time_last_updated_(start_time), + : transport_overhead_per_packet_(DataSize::Zero()), + time_last_updated_(start_time), paused_(false), size_packets_(0), size_(DataSize::Zero()), @@ -167,7 +162,13 @@ std::unique_ptr RoundRobinPacketQueue::Pop() { // case a "budget" will be built up for the stream sending at the lower // rate. To avoid building a too large budget we limit |bytes| to be within // kMaxLeading bytes of the stream that has sent the most amount of bytes. - DataSize packet_size = queued_packet.Size(include_overhead_); + DataSize packet_size = + DataSize::bytes(queued_packet.RtpPacket()->payload_size() + + queued_packet.RtpPacket()->padding_size()); + if (include_overhead_) { + packet_size += DataSize::bytes(queued_packet.RtpPacket()->headers_size()) + + transport_overhead_per_packet_; + } stream->size = std::max(stream->size + packet_size, max_size_ - kMaxLeadingSize); max_size_ = std::max(max_size_, stream->size); @@ -250,14 +251,18 @@ void RoundRobinPacketQueue::SetPauseState(bool paused, Timestamp now) { void RoundRobinPacketQueue::SetIncludeOverhead() { include_overhead_ = true; // We need to update the size to reflect overhead for existing packets. - size_ = DataSize::Zero(); for (const auto& stream : streams_) { for (const QueuedPacket& packet : stream.second.packet_queue) { - size_ += packet.Size(include_overhead_); + size_ += DataSize::bytes(packet.RtpPacket()->headers_size()) + + transport_overhead_per_packet_; } } } +void RoundRobinPacketQueue::SetTransportOverhead(DataSize overhead_per_packet) { + transport_overhead_per_packet_ = overhead_per_packet; +} + TimeDelta RoundRobinPacketQueue::AverageQueueTime() const { if (Empty()) return TimeDelta::Zero(); @@ -299,7 +304,12 @@ void RoundRobinPacketQueue::Push(QueuedPacket packet) { packet.SubtractPauseTime(pause_time_sum_); size_packets_ += 1; - size_ += packet.Size(include_overhead_); + size_ += DataSize::bytes(packet.RtpPacket()->payload_size() + + packet.RtpPacket()->padding_size()); + if (include_overhead_) { + size_ += DataSize::bytes(packet.RtpPacket()->headers_size()) + + transport_overhead_per_packet_; + } stream->packet_queue.push(packet); } diff --git a/modules/pacing/round_robin_packet_queue.h b/modules/pacing/round_robin_packet_queue.h index d0a2f7cb72..225e137753 100644 --- a/modules/pacing/round_robin_packet_queue.h +++ b/modules/pacing/round_robin_packet_queue.h @@ -53,6 +53,7 @@ class RoundRobinPacketQueue { void UpdateQueueTime(Timestamp now); void SetPauseState(bool paused, Timestamp now); void SetIncludeOverhead(); + void SetTransportOverhead(DataSize overhead_per_packet); private: struct QueuedPacket { @@ -73,7 +74,6 @@ class RoundRobinPacketQueue { Timestamp EnqueueTime() const; bool IsRetransmission() const; uint64_t EnqueueOrder() const; - DataSize Size(bool count_overhead) const; RtpPacketToSend* RtpPacket() const; std::multiset::iterator EnqueueTimeIterator() const; @@ -137,6 +137,8 @@ class RoundRobinPacketQueue { // Just used to verify correctness. bool IsSsrcScheduled(uint32_t ssrc) const; + DataSize transport_overhead_per_packet_; + Timestamp time_last_updated_; bool paused_; diff --git a/modules/pacing/rtp_packet_pacer.h b/modules/pacing/rtp_packet_pacer.h index 2f11c1f5d6..d826eddd87 100644 --- a/modules/pacing/rtp_packet_pacer.h +++ b/modules/pacing/rtp_packet_pacer.h @@ -65,6 +65,7 @@ class RtpPacketPacer { // at high priority. virtual void SetAccountForAudioPackets(bool account_for_audio) = 0; virtual void SetIncludeOverhead() = 0; + virtual void SetTransportOverhead(DataSize overhead_per_packet) = 0; }; } // namespace webrtc diff --git a/modules/pacing/task_queue_paced_sender.cc b/modules/pacing/task_queue_paced_sender.cc index 54d2d844ca..646af4e95a 100644 --- a/modules/pacing/task_queue_paced_sender.cc +++ b/modules/pacing/task_queue_paced_sender.cc @@ -143,6 +143,13 @@ void TaskQueuePacedSender::SetIncludeOverhead() { }); } +void TaskQueuePacedSender::SetTransportOverhead(DataSize overhead_per_packet) { + task_queue_.PostTask([this, overhead_per_packet]() { + RTC_DCHECK_RUN_ON(&task_queue_); + pacing_controller_.SetTransportOverhead(overhead_per_packet); + }); +} + void TaskQueuePacedSender::SetQueueTimeLimit(TimeDelta limit) { task_queue_.PostTask([this, limit]() { RTC_DCHECK_RUN_ON(&task_queue_); diff --git a/modules/pacing/task_queue_paced_sender.h b/modules/pacing/task_queue_paced_sender.h index a50ffa2784..8b47f5ee3d 100644 --- a/modules/pacing/task_queue_paced_sender.h +++ b/modules/pacing/task_queue_paced_sender.h @@ -80,6 +80,8 @@ class TaskQueuePacedSender : public RtpPacketPacer, void SetAccountForAudioPackets(bool account_for_audio) override; void SetIncludeOverhead() override; + void SetTransportOverhead(DataSize overhead_per_packet) override; + // Returns the time since the oldest queued packet was enqueued. TimeDelta OldestPacketWaitTime() const override;