Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(941)

Unified Diff: webrtc/modules/pacing/paced_sender.cc

Issue 1474533006: Fix bug in calculation of averge queue time in paced sender. (Closed) Base URL: https://chromium.googlesource.com/external/webrtc.git@master
Patch Set: Added a todo comment Created 5 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: webrtc/modules/pacing/paced_sender.cc
diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc
index e38405a6a24faa71145f89eb36b88a6b4c988493..96458b8a1abf4133490ca729fdd3012dbf21051f 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -32,6 +32,9 @@ const int64_t kMaxIntervalTimeMs = 30;
} // namespace
+// TODO(sprang): Move at least PacketQueue and MediaBudget out to separate
+// files, so that we can more easily test them.
+
namespace webrtc {
namespace paced_sender {
struct Packet {
@@ -93,9 +96,11 @@ class PacketQueue {
virtual ~PacketQueue() {}
void Push(const Packet& packet) {
- if (!AddToDupeSet(packet)) {
+ if (!AddToDupeSet(packet))
return;
- }
+
+ UpdateQueueTime(packet.enqueue_time_ms);
+
// Store packet in list, use pointers in priority queue for cheaper moves.
// Packets have a handle to its own iterator in the list, for easy removal
// when popping from queue.
@@ -134,12 +139,16 @@ class PacketQueue {
return it->enqueue_time_ms;
}
- int64_t AverageQueueTimeMs() {
- int64_t now = clock_->TimeInMilliseconds();
- RTC_DCHECK_GE(now, time_last_updated_);
- int64_t delta = now - time_last_updated_;
+ void UpdateQueueTime(int64_t timestamp_ms) {
+ RTC_DCHECK_GE(timestamp_ms, time_last_updated_);
+ int64_t delta = timestamp_ms - time_last_updated_;
queue_time_sum_ += delta * prio_queue_.size();
- time_last_updated_ = now;
+ time_last_updated_ = timestamp_ms;
+ }
+
+ int64_t AverageQueueTimeMs() const {
+ if (prio_queue_.empty())
+ return 0;
mflodman 2015/11/26 12:06:23 Or here if it's easier logically to check here.
return queue_time_sum_ / prio_queue_.size();
}
@@ -319,6 +328,12 @@ int64_t PacedSender::QueueInMs() const {
return clock_->TimeInMilliseconds() - oldest_packet;
}
+int64_t PacedSender::AverageQueueTimeMs() {
+ CriticalSectionScoped cs(critsect_.get());
+ packets_->UpdateQueueTime(clock_->TimeInMilliseconds());
+ return packets_->AverageQueueTimeMs();
+}
+
int64_t PacedSender::TimeUntilNextProcess() {
CriticalSectionScoped cs(critsect_.get());
if (prober_->IsProbing()) {
@@ -345,6 +360,7 @@ int32_t PacedSender::Process() {
// Assuming equal size packets and input/output rate, the average packet
// has avg_time_left_ms left to get queue_size_bytes out of the queue, if
// time constraint shall be met. Determine bitrate needed for that.
+ packets_->UpdateQueueTime(clock_->TimeInMilliseconds());
int64_t avg_time_left_ms = std::max<int64_t>(
1, kMaxQueueLengthMs - packets_->AverageQueueTimeMs());
int min_bitrate_needed_kbps =

Powered by Google App Engine
This is Rietveld 408576698