Skip to content

Commit 7affd9b

Browse files
MirkoBonadeiCommit Bot
authored andcommitted
Revert "Adds trial to use correct overhead calculation in pacer."
This reverts commit 71a77c4. 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 <[email protected]> > Commit-Queue: Sebastian Jansson <[email protected]> > Cr-Commit-Position: refs/heads/master@{#30399} [email protected],[email protected] 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 <[email protected]> Commit-Queue: Mirko Bonadei <[email protected]> Cr-Commit-Position: refs/heads/master@{#30409}
1 parent 99d6d81 commit 7affd9b

10 files changed

+16
-65
lines changed

call/rtp_transport_controller_send.cc

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -421,9 +421,6 @@ void RtpTransportControllerSend::OnTransportOverheadChanged(
421421
return;
422422
}
423423

424-
pacer()->SetTransportOverhead(
425-
DataSize::bytes(transport_overhead_bytes_per_packet));
426-
427424
// TODO(holmer): Call AudioRtpSenders when they have been moved to
428425
// RtpTransportControllerSend.
429426
for (auto& rtp_video_sender : video_rtp_senders_) {

modules/pacing/paced_sender.cc

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,6 @@ void PacedSender::SetIncludeOverhead() {
131131
pacing_controller_.SetIncludeOverhead();
132132
}
133133

134-
void PacedSender::SetTransportOverhead(DataSize overhead_per_packet) {
135-
rtc::CritScope cs(&critsect_);
136-
pacing_controller_.SetTransportOverhead(overhead_per_packet);
137-
}
138-
139134
TimeDelta PacedSender::ExpectedQueueTime() const {
140135
rtc::CritScope cs(&critsect_);
141136
return pacing_controller_.ExpectedQueueTime();

modules/pacing/paced_sender.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,6 @@ class PacedSender : public Module,
9898
void SetAccountForAudioPackets(bool account_for_audio) override;
9999

100100
void SetIncludeOverhead() override;
101-
void SetTransportOverhead(DataSize overhead_per_packet) override;
102101

103102
// Returns the time since the oldest queued packet was enqueued.
104103
TimeDelta OldestPacketWaitTime() const override;

modules/pacing/pacing_controller.cc

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,7 @@ PacingController::PacingController(Clock* clock,
9999
pace_audio_(IsEnabled(*field_trials_, "WebRTC-Pacer-BlockAudio")),
100100
small_first_probe_packet_(
101101
IsEnabled(*field_trials_, "WebRTC-Pacer-SmallFirstProbePacket")),
102-
ignore_transport_overhead_(
103-
!IsDisabled(*field_trials_, "WebRTC-Pacer-IgnoreTransportOverhead")),
104102
min_packet_limit_(kDefaultMinPacketLimit),
105-
transport_overhead_per_packet_(DataSize::Zero()),
106103
last_timestamp_(clock_->CurrentTime()),
107104
paused_(false),
108105
media_budget_(0),
@@ -233,13 +230,6 @@ void PacingController::SetIncludeOverhead() {
233230
packet_queue_.SetIncludeOverhead();
234231
}
235232

236-
void PacingController::SetTransportOverhead(DataSize overhead_per_packet) {
237-
if (ignore_transport_overhead_)
238-
return;
239-
transport_overhead_per_packet_ = overhead_per_packet;
240-
packet_queue_.SetTransportOverhead(overhead_per_packet);
241-
}
242-
243233
TimeDelta PacingController::ExpectedQueueTime() const {
244234
RTC_DCHECK_GT(pacing_bitrate_, DataRate::Zero());
245235
return TimeDelta::ms(
@@ -531,13 +521,10 @@ void PacingController::ProcessPackets() {
531521
RTC_DCHECK(rtp_packet);
532522
RTC_DCHECK(rtp_packet->packet_type().has_value());
533523
const RtpPacketToSend::Type packet_type = *rtp_packet->packet_type();
534-
DataSize packet_size = DataSize::bytes(rtp_packet->payload_size() +
535-
rtp_packet->padding_size());
536-
537-
if (include_overhead_) {
538-
packet_size += DataSize::bytes(rtp_packet->headers_size()) +
539-
transport_overhead_per_packet_;
540-
}
524+
const DataSize packet_size =
525+
DataSize::bytes(include_overhead_ ? rtp_packet->size()
526+
: rtp_packet->payload_size() +
527+
rtp_packet->padding_size());
541528
packet_sender_->SendRtpPacket(std::move(rtp_packet), pacing_info);
542529

543530
data_sent += packet_size;

modules/pacing/pacing_controller.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,6 @@ class PacingController {
109109
void SetAccountForAudioPackets(bool account_for_audio);
110110
void SetIncludeOverhead();
111111

112-
void SetTransportOverhead(DataSize overhead_per_packet);
113-
114112
// Returns the time since the oldest queued packet was enqueued.
115113
TimeDelta OldestPacketWaitTime() const;
116114

@@ -179,12 +177,9 @@ class PacingController {
179177
const bool send_padding_if_silent_;
180178
const bool pace_audio_;
181179
const bool small_first_probe_packet_;
182-
const bool ignore_transport_overhead_;
183180

184181
TimeDelta min_packet_limit_;
185182

186-
DataSize transport_overhead_per_packet_;
187-
188183
// TODO(webrtc:9716): Remove this when we are certain clocks are monotonic.
189184
// The last millisecond timestamp returned by |clock_|.
190185
mutable Timestamp last_timestamp_;

modules/pacing/round_robin_packet_queue.cc

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ uint64_t RoundRobinPacketQueue::QueuedPacket::EnqueueOrder() const {
7373
return enqueue_order_;
7474
}
7575

76+
DataSize RoundRobinPacketQueue::QueuedPacket::Size(bool count_overhead) const {
77+
return DataSize::bytes(count_overhead ? owned_packet_->size()
78+
: owned_packet_->payload_size() +
79+
owned_packet_->padding_size());
80+
}
81+
7682
RtpPacketToSend* RoundRobinPacketQueue::QueuedPacket::RtpPacket() const {
7783
return owned_packet_;
7884
}
@@ -111,8 +117,7 @@ bool IsEnabled(const WebRtcKeyValueConfig* field_trials, const char* name) {
111117
RoundRobinPacketQueue::RoundRobinPacketQueue(
112118
Timestamp start_time,
113119
const WebRtcKeyValueConfig* field_trials)
114-
: transport_overhead_per_packet_(DataSize::Zero()),
115-
time_last_updated_(start_time),
120+
: time_last_updated_(start_time),
116121
paused_(false),
117122
size_packets_(0),
118123
size_(DataSize::Zero()),
@@ -162,13 +167,7 @@ std::unique_ptr<RtpPacketToSend> RoundRobinPacketQueue::Pop() {
162167
// case a "budget" will be built up for the stream sending at the lower
163168
// rate. To avoid building a too large budget we limit |bytes| to be within
164169
// kMaxLeading bytes of the stream that has sent the most amount of bytes.
165-
DataSize packet_size =
166-
DataSize::bytes(queued_packet.RtpPacket()->payload_size() +
167-
queued_packet.RtpPacket()->padding_size());
168-
if (include_overhead_) {
169-
packet_size += DataSize::bytes(queued_packet.RtpPacket()->headers_size()) +
170-
transport_overhead_per_packet_;
171-
}
170+
DataSize packet_size = queued_packet.Size(include_overhead_);
172171
stream->size =
173172
std::max(stream->size + packet_size, max_size_ - kMaxLeadingSize);
174173
max_size_ = std::max(max_size_, stream->size);
@@ -251,18 +250,14 @@ void RoundRobinPacketQueue::SetPauseState(bool paused, Timestamp now) {
251250
void RoundRobinPacketQueue::SetIncludeOverhead() {
252251
include_overhead_ = true;
253252
// We need to update the size to reflect overhead for existing packets.
253+
size_ = DataSize::Zero();
254254
for (const auto& stream : streams_) {
255255
for (const QueuedPacket& packet : stream.second.packet_queue) {
256-
size_ += DataSize::bytes(packet.RtpPacket()->headers_size()) +
257-
transport_overhead_per_packet_;
256+
size_ += packet.Size(include_overhead_);
258257
}
259258
}
260259
}
261260

262-
void RoundRobinPacketQueue::SetTransportOverhead(DataSize overhead_per_packet) {
263-
transport_overhead_per_packet_ = overhead_per_packet;
264-
}
265-
266261
TimeDelta RoundRobinPacketQueue::AverageQueueTime() const {
267262
if (Empty())
268263
return TimeDelta::Zero();
@@ -304,12 +299,7 @@ void RoundRobinPacketQueue::Push(QueuedPacket packet) {
304299
packet.SubtractPauseTime(pause_time_sum_);
305300

306301
size_packets_ += 1;
307-
size_ += DataSize::bytes(packet.RtpPacket()->payload_size() +
308-
packet.RtpPacket()->padding_size());
309-
if (include_overhead_) {
310-
size_ += DataSize::bytes(packet.RtpPacket()->headers_size()) +
311-
transport_overhead_per_packet_;
312-
}
302+
size_ += packet.Size(include_overhead_);
313303

314304
stream->packet_queue.push(packet);
315305
}

modules/pacing/round_robin_packet_queue.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ class RoundRobinPacketQueue {
5353
void UpdateQueueTime(Timestamp now);
5454
void SetPauseState(bool paused, Timestamp now);
5555
void SetIncludeOverhead();
56-
void SetTransportOverhead(DataSize overhead_per_packet);
5756

5857
private:
5958
struct QueuedPacket {
@@ -74,6 +73,7 @@ class RoundRobinPacketQueue {
7473
Timestamp EnqueueTime() const;
7574
bool IsRetransmission() const;
7675
uint64_t EnqueueOrder() const;
76+
DataSize Size(bool count_overhead) const;
7777
RtpPacketToSend* RtpPacket() const;
7878

7979
std::multiset<Timestamp>::iterator EnqueueTimeIterator() const;
@@ -137,8 +137,6 @@ class RoundRobinPacketQueue {
137137
// Just used to verify correctness.
138138
bool IsSsrcScheduled(uint32_t ssrc) const;
139139

140-
DataSize transport_overhead_per_packet_;
141-
142140
Timestamp time_last_updated_;
143141

144142
bool paused_;

modules/pacing/rtp_packet_pacer.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ class RtpPacketPacer {
6565
// at high priority.
6666
virtual void SetAccountForAudioPackets(bool account_for_audio) = 0;
6767
virtual void SetIncludeOverhead() = 0;
68-
virtual void SetTransportOverhead(DataSize overhead_per_packet) = 0;
6968
};
7069

7170
} // namespace webrtc

modules/pacing/task_queue_paced_sender.cc

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -143,13 +143,6 @@ void TaskQueuePacedSender::SetIncludeOverhead() {
143143
});
144144
}
145145

146-
void TaskQueuePacedSender::SetTransportOverhead(DataSize overhead_per_packet) {
147-
task_queue_.PostTask([this, overhead_per_packet]() {
148-
RTC_DCHECK_RUN_ON(&task_queue_);
149-
pacing_controller_.SetTransportOverhead(overhead_per_packet);
150-
});
151-
}
152-
153146
void TaskQueuePacedSender::SetQueueTimeLimit(TimeDelta limit) {
154147
task_queue_.PostTask([this, limit]() {
155148
RTC_DCHECK_RUN_ON(&task_queue_);

modules/pacing/task_queue_paced_sender.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,6 @@ class TaskQueuePacedSender : public RtpPacketPacer,
8080
void SetAccountForAudioPackets(bool account_for_audio) override;
8181

8282
void SetIncludeOverhead() override;
83-
void SetTransportOverhead(DataSize overhead_per_packet) override;
84-
8583
// Returns the time since the oldest queued packet was enqueued.
8684
TimeDelta OldestPacketWaitTime() const override;
8785

0 commit comments

Comments
 (0)