Fix bug in calculation of averge queue time in paced sender.

webrtc/modules/pacing/paced_sender.cc

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..f9d968d2296ecf3c8e047dec7c5ba59aba56d20a 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -93,9 +93,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 +136,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;
     return queue_time_sum_ / prio_queue_.size();
   }
 
@@ -319,6 +325,12 @@ int64_t PacedSender::QueueInMs() const {
   return clock_->TimeInMilliseconds() - oldest_packet;
 }
 
+int64_t PacedSender::AverageQueueTimeMs() const {

[stefan-webrtc, 2015/11/25 18:14:22]

Can this really be const if we call UpdateQueueTime below?

[sprang_webrtc, 2015/11/25 19:40:42]

Ehh, yeah.. What am I missing? Why does this compile?!

[stefan-webrtc, 2015/11/26 08:02:46]

Did you figure out why it works? Should we remove the const?

[sprang_webrtc, 2015/11/26 08:11:20]

Did not figure out why it works. Removed it anyways, cause is scares me.

+  CriticalSectionScoped cs(critsect_.get());
+  packets_->UpdateQueueTime(clock_->TimeInMilliseconds());
+  return packets_->AverageQueueTimeMs();
+}
+
 int64_t PacedSender::TimeUntilNextProcess() {
   CriticalSectionScoped cs(critsect_.get());
   if (prober_->IsProbing()) {
@@ -345,6 +357,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 =