Revert "Adds trial to use correct overhead calculation in pacer."
This reverts commit 71a77c4b3b314a5e3b4e6b2f12d4886cff1b60d7.
Reason for revert: https://webrtc-review.googlesource.com/c/src/+/167524 needs to be reverted and this CL causes a merge conflict.
Original change's description:
> Adds trial to use correct overhead calculation in pacer.
>
> Bug: webrtc:9883
> Change-Id: I1f25a235468678bf823ee1399ba31d94acf33be9
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166534
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Commit-Queue: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#30399}
TBR=sprang@webrtc.org,srte@webrtc.org
Change-Id: I7d3efa29f70aa0363311766980acae6d88bbcaaa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:9883
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/167880
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30409}
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index 20f3a99..c2946ad 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -421,9 +421,6 @@
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 3646952..6dc47b6 100644
--- a/modules/pacing/paced_sender.cc
+++ b/modules/pacing/paced_sender.cc
@@ -131,11 +131,6 @@
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 16137df..3691308 100644
--- a/modules/pacing/paced_sender.h
+++ b/modules/pacing/paced_sender.h
@@ -98,7 +98,6 @@
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 f2b2149..09b7630 100644
--- a/modules/pacing/pacing_controller.cc
+++ b/modules/pacing/pacing_controller.cc
@@ -99,10 +99,7 @@
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),
@@ -233,13 +230,6 @@
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(
@@ -531,13 +521,10 @@
RTC_DCHECK(rtp_packet);
RTC_DCHECK(rtp_packet->packet_type().has_value());
const RtpPacketToSend::Type 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_;
- }
+ const DataSize packet_size =
+ DataSize::bytes(include_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;
diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h
index c1b3942..fb4d9d3 100644
--- a/modules/pacing/pacing_controller.h
+++ b/modules/pacing/pacing_controller.h
@@ -109,8 +109,6 @@
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;
@@ -179,12 +177,9 @@
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 32f288c..754ff58 100644
--- a/modules/pacing/round_robin_packet_queue.cc
+++ b/modules/pacing/round_robin_packet_queue.cc
@@ -73,6 +73,12 @@
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_;
}
@@ -111,8 +117,7 @@
RoundRobinPacketQueue::RoundRobinPacketQueue(
Timestamp start_time,
const WebRtcKeyValueConfig* field_trials)
- : transport_overhead_per_packet_(DataSize::Zero()),
- time_last_updated_(start_time),
+ : time_last_updated_(start_time),
paused_(false),
size_packets_(0),
size_(DataSize::Zero()),
@@ -162,13 +167,7 @@
// 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 =
- 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_;
- }
+ DataSize packet_size = queued_packet.Size(include_overhead_);
stream->size =
std::max(stream->size + packet_size, max_size_ - kMaxLeadingSize);
max_size_ = std::max(max_size_, stream->size);
@@ -251,18 +250,14 @@
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_ += DataSize::bytes(packet.RtpPacket()->headers_size()) +
- transport_overhead_per_packet_;
+ size_ += packet.Size(include_overhead_);
}
}
}
-void RoundRobinPacketQueue::SetTransportOverhead(DataSize overhead_per_packet) {
- transport_overhead_per_packet_ = overhead_per_packet;
-}
-
TimeDelta RoundRobinPacketQueue::AverageQueueTime() const {
if (Empty())
return TimeDelta::Zero();
@@ -304,12 +299,7 @@
packet.SubtractPauseTime(pause_time_sum_);
size_packets_ += 1;
- 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_;
- }
+ size_ += packet.Size(include_overhead_);
stream->packet_queue.push(packet);
}
diff --git a/modules/pacing/round_robin_packet_queue.h b/modules/pacing/round_robin_packet_queue.h
index 225e137..d0a2f7c 100644
--- a/modules/pacing/round_robin_packet_queue.h
+++ b/modules/pacing/round_robin_packet_queue.h
@@ -53,7 +53,6 @@
void UpdateQueueTime(Timestamp now);
void SetPauseState(bool paused, Timestamp now);
void SetIncludeOverhead();
- void SetTransportOverhead(DataSize overhead_per_packet);
private:
struct QueuedPacket {
@@ -74,6 +73,7 @@
Timestamp EnqueueTime() const;
bool IsRetransmission() const;
uint64_t EnqueueOrder() const;
+ DataSize Size(bool count_overhead) const;
RtpPacketToSend* RtpPacket() const;
std::multiset<Timestamp>::iterator EnqueueTimeIterator() const;
@@ -137,8 +137,6 @@
// 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 d826edd..2f11c1f 100644
--- a/modules/pacing/rtp_packet_pacer.h
+++ b/modules/pacing/rtp_packet_pacer.h
@@ -65,7 +65,6 @@
// 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 646af4e..54d2d84 100644
--- a/modules/pacing/task_queue_paced_sender.cc
+++ b/modules/pacing/task_queue_paced_sender.cc
@@ -143,13 +143,6 @@
});
}
-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 8b47f5e..a50ffa2 100644
--- a/modules/pacing/task_queue_paced_sender.h
+++ b/modules/pacing/task_queue_paced_sender.h
@@ -80,8 +80,6 @@
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;