Reland "Adds trial to use correct overhead calculation in pacer."

This reverts commit 7affd9bcbb7a778408942d8afa4fe3ce29a8fc0b.

Reason for revert: The perf issue has been addressed in the reland (https://webrtc-review.googlesource.com/c/src/+/167883).

Original change's description:
> 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}

TBR=mbonadei@webrtc.org,sprang@webrtc.org,srte@webrtc.org

Change-Id: Iafdef81d08078000dc368e001f67bee660e2f5bc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:9883
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/167861
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30414}
This commit is contained in:
Mirko Bonadei
2020-01-29 18:45:00 +00:00
committed by Commit Bot
parent c3eb9fd49f
commit e7bc3a3477
10 changed files with 65 additions and 16 deletions

View File

@ -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_) {

View File

@ -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();

View File

@ -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;

View File

@ -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() +
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;

View File

@ -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_;

View File

@ -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<RtpPacketToSend> 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);
}

View File

@ -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<Timestamp>::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_;

View File

@ -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

View File

@ -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_);

View File

@ -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;