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

webrtc/modules/pacing/paced_sender.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 75 matching lines @@
 class PacketQueue {
  public:
   explicit PacketQueue(Clock* clock)
       : bytes_(0),
         clock_(clock),
         queue_time_sum_(0),
         time_last_updated_(clock_->TimeInMilliseconds()) {}
   virtual ~PacketQueue() {}
 
   void Push(const Packet& packet) {
-    if (!AddToDupeSet(packet)) {
+    if (!AddToDupeSet(packet))
       return;
-    }
+
     // 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.
     packet_list_.push_front(packet);
     std::list<Packet>::iterator it = packet_list_.begin();
     it->this_it = it;          // Handle for direct removal from list.
     prio_queue_.push(&(*it));  // Pointer into list.
     bytes_ += packet.bytes;
+    UpdateQueueTime();

[stefan-webrtc, 2015/11/25 15:28:52]

To me it makes more sense to call this before we add the packet. Btw, is there a
risk of getting some glitch here since we use different timestamps in
UpdateQueueTime and for enqueue_time_ms in Packet?

[sprang_webrtc, 2015/11/25 15:36:00]

Yes, I realized this just after uploading...
Yes, good catch. Updating to use packet.enqueue_time instead.

   }
 
   const Packet& BeginPop() {
     const Packet& packet = *prio_queue_.top();
     prio_queue_.pop();
     return packet;
   }
 
   void CancelPop(const Packet& packet) { prio_queue_.push(&(*packet.this_it)); }
 
   void FinalizePop(const Packet& packet) {
     RemoveFromDupeSet(packet);
     bytes_ -= packet.bytes;
     queue_time_sum_ -= (time_last_updated_ - packet.enqueue_time_ms);
     packet_list_.erase(packet.this_it);
+    UpdateQueueTime();
   }
 
   bool Empty() const { return prio_queue_.empty(); }
 
   size_t SizeInPackets() const { return prio_queue_.size(); }
 
   uint64_t SizeInBytes() const { return bytes_; }
 
   int64_t OldestEnqueueTimeMs() const {
     auto it = packet_list_.rbegin();
     if (it == packet_list_.rend())
       return 0;
     return it->enqueue_time_ms;
   }
 
-  int64_t AverageQueueTimeMs() {
+  void UpdateQueueTime() {
     int64_t now = clock_->TimeInMilliseconds();
     RTC_DCHECK_GE(now, time_last_updated_);
     int64_t delta = now - time_last_updated_;
     queue_time_sum_ += delta * prio_queue_.size();
     time_last_updated_ = now;
+  }
+
+  int64_t AverageQueueTimeMs() {

[stefan-webrtc, 2015/11/25 15:28:52]

Make this const...

[sprang_webrtc, 2015/11/25 15:36:00]

Done.

+    UpdateQueueTime();

[stefan-webrtc, 2015/11/25 15:28:52]

... and call this separately?

[sprang_webrtc, 2015/11/25 15:36:00]

Done.

+    if (prio_queue_.empty())
+      return 0;
     return queue_time_sum_ / prio_queue_.size();
   }
 
  private:
   // Try to add a packet to the set of ssrc/seqno identifiers currently in the
   // queue. Return true if inserted, false if this is a duplicate.
   bool AddToDupeSet(const Packet& packet) {
     SsrcSeqNoMap::iterator it = dupe_map_.find(packet.ssrc);
     if (it == dupe_map_.end()) {
       // First for this ssrc, just insert.
@@ skipping 268 matching lines @@
     media_budget_->UseBudget(bytes_sent);
     padding_budget_->UseBudget(bytes_sent);
   }
 }
 
 void PacedSender::UpdateBytesPerInterval(int64_t delta_time_ms) {
   media_budget_->IncreaseBudget(delta_time_ms);
   padding_budget_->IncreaseBudget(delta_time_ms);
 }
 }  // namespace webrtc