From 89fd1e8e99b47544f5fa36bc7fbde1d089536b0b Mon Sep 17 00:00:00 2001 From: "stefan@webrtc.org" Date: Tue, 15 Jul 2014 16:40:38 +0000 Subject: [PATCH] Improvements to the pacer where it lost some budget due to truncation errors. With this CL the resolution is increased to microseconds and proper rounding is done in the Process() function. This means that we will be allowed to send more than prior to r6664 as we previously truncated away parts of our budget. We will also not lose budget due to inaccurate calculations in TimeUntilNextProcess(), which was a regression in r6664. BUG=cr/393950 TEST=out/Debug/webrtc_perf_tests --gtest_filter=RampUpTest.Simulcast R=pbos@webrtc.org Review URL: https://webrtc-codereview.appspot.com/20949004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6694 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/modules/pacing/include/paced_sender.h | 4 +-- webrtc/modules/pacing/paced_sender.cc | 27 ++++++++------------ 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/webrtc/modules/pacing/include/paced_sender.h b/webrtc/modules/pacing/include/paced_sender.h index b9151a5fc0..ddd8e53b7e 100644 --- a/webrtc/modules/pacing/include/paced_sender.h +++ b/webrtc/modules/pacing/include/paced_sender.h @@ -146,8 +146,8 @@ class PacedSender : public Module { scoped_ptr padding_budget_ GUARDED_BY(critsect_); - int64_t time_last_update_ GUARDED_BY(critsect_); - int64_t time_last_send_ GUARDED_BY(critsect_); + int64_t time_last_update_us_ GUARDED_BY(critsect_); + int64_t time_last_send_us_ GUARDED_BY(critsect_); int64_t capture_time_ms_last_queued_ GUARDED_BY(critsect_); int64_t capture_time_ms_last_sent_ GUARDED_BY(critsect_); diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc index 52e9cfb421..6204a9a066 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -31,12 +31,11 @@ const int kMaxIntervalTimeMs = 30; // Max time that the first packet in the queue can sit in the queue if no // packets are sent, regardless of buffer state. In practice only in effect at // low bitrates (less than 320 kbits/s). -const int kMaxQueueTimeWithoutSendingMs = 30; +const int kMaxQueueTimeWithoutSendingUs = 30000; } // namespace namespace webrtc { - namespace paced_sender { struct Packet { Packet(uint32_t ssrc, @@ -142,7 +141,7 @@ PacedSender::PacedSender(Clock* clock, max_queue_length_ms_(kDefaultMaxQueueLengthMs), media_budget_(new paced_sender::IntervalBudget(max_bitrate_kbps)), padding_budget_(new paced_sender::IntervalBudget(min_bitrate_kbps)), - time_last_update_(clock->TimeInMilliseconds()), + time_last_update_us_(clock->TimeInMicroseconds()), capture_time_ms_last_queued_(0), capture_time_ms_last_sent_(0), high_priority_packets_(new paced_sender::PacketList), @@ -151,8 +150,7 @@ PacedSender::PacedSender(Clock* clock, UpdateBytesPerInterval(kMinPacketLimitMs); } -PacedSender::~PacedSender() { -} +PacedSender::~PacedSender() {} void PacedSender::Pause() { CriticalSectionScoped cs(critsect_.get()); @@ -248,7 +246,8 @@ int PacedSender::QueueInMs() const { int32_t PacedSender::TimeUntilNextProcess() { CriticalSectionScoped cs(critsect_.get()); - int64_t elapsed_time_ms = clock_->TimeInMilliseconds() - time_last_update_; + int64_t elapsed_time_ms = (clock_->TimeInMicroseconds() - + time_last_update_us_ + 500) / 1000; if (elapsed_time_ms <= 0) { return kMinPacketLimitMs; } @@ -259,10 +258,10 @@ int32_t PacedSender::TimeUntilNextProcess() { } int32_t PacedSender::Process() { - int64_t now = clock_->TimeInMilliseconds(); + int64_t now_us = clock_->TimeInMicroseconds(); CriticalSectionScoped cs(critsect_.get()); - int elapsed_time_ms = now - time_last_update_; - time_last_update_ = now; + int elapsed_time_ms = (now_us - time_last_update_us_ + 500) / 1000; + time_last_update_us_ = now_us; if (!enabled_) { return 0; } @@ -291,7 +290,6 @@ int32_t PacedSender::Process() { return 0; } -// MUST have critsect_ when calling. bool PacedSender::SendPacketFromList(paced_sender::PacketList* packet_list) EXCLUSIVE_LOCKS_REQUIRED(critsect_.get()) { paced_sender::Packet packet = GetNextPacketFromList(packet_list); @@ -322,20 +320,18 @@ bool PacedSender::SendPacketFromList(paced_sender::PacketList* packet_list) return true; } -// MUST have critsect_ when calling. void PacedSender::UpdateBytesPerInterval(uint32_t delta_time_ms) { media_budget_->IncreaseBudget(delta_time_ms); padding_budget_->IncreaseBudget(delta_time_ms); } -// MUST have critsect_ when calling. bool PacedSender::ShouldSendNextPacket(paced_sender::PacketList** packet_list) { *packet_list = NULL; if (media_budget_->bytes_remaining() <= 0) { // All bytes consumed for this interval. // Check if we have not sent in a too long time. - if (clock_->TimeInMilliseconds() - time_last_send_ > - kMaxQueueTimeWithoutSendingMs) { + if (clock_->TimeInMicroseconds() - time_last_send_us_ > + kMaxQueueTimeWithoutSendingUs) { if (!high_priority_packets_->empty()) { *packet_list = high_priority_packets_.get(); return true; @@ -386,9 +382,8 @@ paced_sender::Packet PacedSender::GetNextPacketFromList( return packet; } -// MUST have critsect_ when calling. void PacedSender::UpdateMediaBytesSent(int num_bytes) { - time_last_send_ = clock_->TimeInMilliseconds(); + time_last_send_us_ = clock_->TimeInMicroseconds(); media_budget_->UseBudget(num_bytes); padding_budget_->UseBudget(num_bytes); }