Added cluster id to PacedSender::Callback::TimeToSendPacket.

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 29 matching lines @@
 namespace webrtc {
 namespace paced_sender {
 struct Packet {
   Packet(RtpPacketSender::Priority priority,
          uint32_t ssrc,
          uint16_t seq_number,
          int64_t capture_time_ms,
          int64_t enqueue_time_ms,
          size_t length_in_bytes,
          bool retransmission,
-         uint64_t enqueue_order)
+         uint64_t enqueue_order,
+         int cluster_id = -1)

[danilchap, 2016/05/10 15:31:39]

try to avoid default arguments. Here it is easy: constructor is called just once
and the -1 value could be added there, into InsertPacket function.
Or, if there is no plan to know cluster_id when you create Packet object, just
do not have this parameter and initialized cluster_id member with a constant.

[philipel, 2016/05/11 09:39:16]

Removed default argument, |cluster_id| is not initialized to -1.

       : priority(priority),
         ssrc(ssrc),
         sequence_number(seq_number),
         capture_time_ms(capture_time_ms),
         enqueue_time_ms(enqueue_time_ms),
         bytes(length_in_bytes),
         retransmission(retransmission),
-        enqueue_order(enqueue_order) {}
+        enqueue_order(enqueue_order),
+        cluster_id(cluster_id) {}
 
   RtpPacketSender::Priority priority;
   uint32_t ssrc;
   uint16_t sequence_number;
   int64_t capture_time_ms;
   int64_t enqueue_time_ms;
   size_t bytes;
   bool retransmission;
   uint64_t enqueue_order;
   std::list<Packet>::iterator this_it;
+  int cluster_id;

[danilchap, 2016/05/10 15:31:39]

may be instead of modifying Packet structure, using non-const Pop, unrecommended
default argument, etc. it would be easier to add an argument to SendPacket
function: cluster_id is not really property of a packet but property of the
current state when packet is sent.

[philipel, 2016/05/11 09:39:16]

I agree, removed |cluster_id| From Packet class.

 };
 
 // Used by priority queue to sort packets.
 struct Comparator {
   bool operator()(const Packet* first, const Packet* second) {
     // Highest prio = 0.
     if (first->priority != second->priority)
       return first->priority > second->priority;
 
     // Retransmissions go first.
@@ skipping 27 matching lines @@
     // 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;
   }
 
-  const Packet& BeginPop() {
-    const Packet& packet = *prio_queue_.top();
+  Packet& BeginPop() {

[danilchap, 2016/05/10 15:31:39]

If you want return a non-const object, return a pointer instead of reference.

[philipel, 2016/05/11 09:39:16]

True, but now it's const again :)

+    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);
@@ skipping 254 matching lines @@
     int64_t delta_time_ms = std::min(kMaxIntervalTimeMs, elapsed_time_ms);
     UpdateBytesPerInterval(delta_time_ms);
   }
   while (!packets_->Empty()) {
     if (media_budget_->bytes_remaining() == 0 && !prober_->IsProbing())
       return;
 
     // Since we need to release the lock in order to send, we first pop the
     // element from the priority queue but keep it in storage, so that we can
     // reinsert it if send fails.
-    const paced_sender::Packet& packet = packets_->BeginPop();
+    paced_sender::Packet& packet = packets_->BeginPop();
+    if (prober_->IsProbing())
+      packet.cluster_id = prober_->CurrentClusterId();

[danilchap, 2016/05/10 15:31:39]

should you clear cluster_id if not probing? Either here or before calling
CancelPop.

 
     if (SendPacket(packet)) {
       // Send succeeded, remove it from the queue.
       packets_->FinalizePop(packet);
       if (prober_->IsProbing())
         return;
     } else {
       // Send failed, put it back into the queue.
       packets_->CancelPop(packet);
       return;
@@ skipping 15 matching lines @@
     SendPadding(static_cast<size_t>(padding_needed));
 }
 
 bool PacedSender::SendPacket(const paced_sender::Packet& packet) {
   // TODO(holmer): Because of this bug issue 5307 we have to send audio
   // packets even when the pacer is paused. Here we assume audio packets are
   // always high priority and that they are the only high priority packets.
   if (paused_ && packet.priority != kHighPriority)
     return false;
   critsect_->Leave();
-  const bool success = callback_->TimeToSendPacket(packet.ssrc,
-                                                   packet.sequence_number,
-                                                   packet.capture_time_ms,
-                                                   packet.retransmission);
+  const bool success = callback_->TimeToSendPacket(
+      packet.ssrc, packet.sequence_number, packet.capture_time_ms,
+      packet.retransmission, packet.cluster_id);
   critsect_->Enter();
 
   if (success) {
     prober_->PacketSent(clock_->TimeInMilliseconds(), packet.bytes);
     // TODO(holmer): High priority packets should only be accounted for if we
     // are allocating bandwidth for audio.
     if (packet.priority != kHighPriority) {
       // Update media bytes sent.
       media_budget_->UseBudget(packet.bytes);
       padding_budget_->UseBudget(packet.bytes);
@@ skipping 13 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