NetEq now works with packets as values, rather than pointers.

webrtc/modules/audio_coding/neteq/packet_buffer.cc

Index: webrtc/modules/audio_coding/neteq/packet_buffer.cc
diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer.cc b/webrtc/modules/audio_coding/neteq/packet_buffer.cc
index ee1439809b599688a7560ba1497bd513a3c986ec..c1f2791a1d37689a271df78e5eb706a839a93600 100644
--- a/webrtc/modules/audio_coding/neteq/packet_buffer.cc
+++ b/webrtc/modules/audio_coding/neteq/packet_buffer.cc
@@ -27,15 +27,15 @@ namespace {
 // Operator() returns true when |packet| goes before |new_packet|.
 class NewTimestampIsLarger {
  public:
-  explicit NewTimestampIsLarger(const Packet* new_packet)
+  explicit NewTimestampIsLarger(const Packet& new_packet)
       : new_packet_(new_packet) {
   }
-  bool operator()(Packet* packet) {
-    return (*new_packet_ >= *packet);
+  bool operator()(Packet& packet) {

[kwiberg-webrtc, 2016/10/20 22:39:22]

const?

[ossu, 2016/10/21 12:54:41]

yas!

+    return (new_packet_ >= packet);
   }
 
  private:
-  const Packet* new_packet_;
+  const Packet& new_packet_;
 };
 
 // Returns true if both payload types are known to the decoder database, and
@@ -60,28 +60,25 @@ PacketBuffer::~PacketBuffer() {
 
 // Flush the buffer. All packets in the buffer will be destroyed.
 void PacketBuffer::Flush() {
-  DeleteAllPackets(&buffer_);
+  buffer_.clear();
 }
 
 bool PacketBuffer::Empty() const {
   return buffer_.empty();
 }
 
-int PacketBuffer::InsertPacket(Packet* packet) {
-  if (!packet || packet->empty()) {
-    if (packet) {
-      delete packet;
-    }
+int PacketBuffer::InsertPacket(Packet&& packet) {
+  if (packet.empty()) {
     LOG(LS_WARNING) << "InsertPacket invalid packet";
     return kInvalidPacket;
   }
 
-  RTC_DCHECK_GE(packet->priority.codec_level, 0);
-  RTC_DCHECK_GE(packet->priority.red_level, 0);
+  RTC_DCHECK_GE(packet.priority.codec_level, 0);
+  RTC_DCHECK_GE(packet.priority.red_level, 0);
 
   int return_val = kOK;
 
-  packet->waiting_time = tick_timer_->GetNewStopwatch();
+  packet.waiting_time = tick_timer_->GetNewStopwatch();
 
   if (buffer_.size() >= max_number_of_packets_) {
     // Buffer is full. Flush it.
@@ -100,8 +97,7 @@ int PacketBuffer::InsertPacket(Packet* packet) {
   // The new packet is to be inserted to the right of |rit|. If it has the same
   // timestamp as |rit|, which has a higher priority, do not insert the new
   // packet to list.
-  if (rit != buffer_.rend() && packet->timestamp == (*rit)->timestamp) {
-    delete packet;
+  if (rit != buffer_.rend() && packet.timestamp == (*rit).timestamp) {

[kwiberg-webrtc, 2016/10/20 22:39:23]

(*x).foo can be written as x->foo...

[ossu, 2016/10/21 12:54:41]

Done.

     return return_val;
   }
 
@@ -109,11 +105,10 @@ int PacketBuffer::InsertPacket(Packet* packet) {
   // timestamp as |it|, which has a lower priority, replace |it| with the new
   // packet.
   PacketList::iterator it = rit.base();
-  if (it != buffer_.end() && packet->timestamp == (*it)->timestamp) {
-    delete *it;
+  if (it != buffer_.end() && packet.timestamp == (*it).timestamp) {

[kwiberg-webrtc, 2016/10/20 22:39:23]

Same here.

[ossu, 2016/10/21 12:54:41]

Done.

     it = buffer_.erase(it);
   }
-  buffer_.insert(it, packet);  // Insert the packet at that position.
+  buffer_.insert(it, std::move(packet));  // Insert the packet at that position.
 
   return return_val;
 }
@@ -124,43 +119,42 @@ int PacketBuffer::InsertPacketList(
     rtc::Optional<uint8_t>* current_rtp_payload_type,
     rtc::Optional<uint8_t>* current_cng_rtp_payload_type) {
   bool flushed = false;
-  while (!packet_list->empty()) {
-    Packet* packet = packet_list->front();
-    if (decoder_database.IsComfortNoise(packet->payload_type)) {
+  for (auto& packet : *packet_list) {
+    if (decoder_database.IsComfortNoise(packet.payload_type)) {
       if (*current_cng_rtp_payload_type &&
-          **current_cng_rtp_payload_type != packet->payload_type) {
+          **current_cng_rtp_payload_type != packet.payload_type) {
         // New CNG payload type implies new codec type.
         *current_rtp_payload_type = rtc::Optional<uint8_t>();
         Flush();
         flushed = true;
       }
       *current_cng_rtp_payload_type =
-          rtc::Optional<uint8_t>(packet->payload_type);
-    } else if (!decoder_database.IsDtmf(packet->payload_type)) {
+          rtc::Optional<uint8_t>(packet.payload_type);
+    } else if (!decoder_database.IsDtmf(packet.payload_type)) {
       // This must be speech.
       if ((*current_rtp_payload_type &&
-           **current_rtp_payload_type != packet->payload_type) ||
+           **current_rtp_payload_type != packet.payload_type) ||
           (*current_cng_rtp_payload_type &&
-           !EqualSampleRates(packet->payload_type,
+           !EqualSampleRates(packet.payload_type,
                              **current_cng_rtp_payload_type,
                              decoder_database))) {
         *current_cng_rtp_payload_type = rtc::Optional<uint8_t>();
         Flush();
         flushed = true;
       }
-      *current_rtp_payload_type = rtc::Optional<uint8_t>(packet->payload_type);
+      *current_rtp_payload_type = rtc::Optional<uint8_t>(packet.payload_type);
     }
-    int return_val = InsertPacket(packet);
-    packet_list->pop_front();
+    int return_val = InsertPacket(std::move(packet));
     if (return_val == kFlushed) {
       // The buffer flushed, but this is not an error. We can still continue.
       flushed = true;
     } else if (return_val != kOK) {
       // An error occurred. Delete remaining packets in list and return.
-      DeleteAllPackets(packet_list);
+      packet_list->clear();
       return return_val;
     }
   }
+  packet_list->clear();
   return flushed ? kFlushed : kOK;
 }
 
@@ -171,7 +165,7 @@ int PacketBuffer::NextTimestamp(uint32_t* next_timestamp) const {
   if (!next_timestamp) {
     return kInvalidPointer;
   }
-  *next_timestamp = buffer_.front()->timestamp;
+  *next_timestamp = buffer_.front().timestamp;
   return kOK;
 }
 
@@ -185,9 +179,9 @@ int PacketBuffer::NextHigherTimestamp(uint32_t timestamp,
   }
   PacketList::const_iterator it;
   for (it = buffer_.begin(); it != buffer_.end(); ++it) {
-    if ((*it)->timestamp >= timestamp) {
+    if ((*it).timestamp >= timestamp) {

[kwiberg-webrtc, 2016/10/20 22:39:23]

->

[ossu, 2016/10/21 12:54:41]

Done.

       // Found a packet matching the search.
-      *next_timestamp = (*it)->timestamp;
+      *next_timestamp = (*it).timestamp;

[kwiberg-webrtc, 2016/10/20 22:39:22]

->

[ossu, 2016/10/21 12:54:41]

Done.

       return kOK;
     }
   }
@@ -195,36 +189,20 @@ int PacketBuffer::NextHigherTimestamp(uint32_t timestamp,
 }
 
 const Packet* PacketBuffer::PeekNextPacket() const {
-  return buffer_.empty() ? nullptr : buffer_.front();
+  return buffer_.empty() ? nullptr : &buffer_.front();
 }
 
-Packet* PacketBuffer::GetNextPacket(size_t* discard_count) {
+rtc::Optional<Packet> PacketBuffer::GetNextPacket() {
   if (Empty()) {
     // Buffer is empty.
-    return NULL;
+    return rtc::Optional<Packet>();
   }
 
-  Packet* packet = buffer_.front();
+  rtc::Optional<Packet> packet(std::move(buffer_.front()));
   // Assert that the packet sanity checks in InsertPacket method works.
-  RTC_DCHECK(packet && !packet->empty());
+  RTC_DCHECK(!packet->empty());
   buffer_.pop_front();
 
-  // Discard other packets with the same timestamp. These are duplicates or
-  // redundant payloads that should not be used.
-  size_t discards = 0;
-
-  while (!Empty() && buffer_.front()->timestamp == packet->timestamp) {
-    if (DiscardNextPacket() != kOK) {
-      assert(false);  // Must be ok by design.
-    }
-    ++discards;
-  }
-  // The way of inserting packet should not cause any packet discarding here.
-  // TODO(minyue): remove |discard_count|.
-  assert(discards == 0);
-  if (discard_count)
-    *discard_count = discards;
-
   return packet;
 }
 
@@ -233,16 +211,15 @@ int PacketBuffer::DiscardNextPacket() {
     return kBufferEmpty;
   }
   // Assert that the packet sanity checks in InsertPacket method works.
-  RTC_DCHECK(buffer_.front());
-  RTC_DCHECK(!buffer_.front()->empty());
-  DeleteFirstPacket(&buffer_);
+  RTC_DCHECK(!buffer_.front().empty());
+  buffer_.pop_front();
   return kOK;
 }
 
 int PacketBuffer::DiscardOldPackets(uint32_t timestamp_limit,
                                     uint32_t horizon_samples) {
-  while (!Empty() && timestamp_limit != buffer_.front()->timestamp &&
-         IsObsoleteTimestamp(buffer_.front()->timestamp, timestamp_limit,
+  while (!Empty() && timestamp_limit != buffer_.front().timestamp &&
+         IsObsoleteTimestamp(buffer_.front().timestamp, timestamp_limit,
                              horizon_samples)) {
     if (DiscardNextPacket() != kOK) {
       assert(false);  // Must be ok by design.
@@ -257,9 +234,8 @@ int PacketBuffer::DiscardAllOldPackets(uint32_t timestamp_limit) {
 
 void PacketBuffer::DiscardPacketsWithPayloadType(uint8_t payload_type) {
   for (auto it = buffer_.begin(); it != buffer_.end(); /* */) {
-    Packet* packet = *it;
-    if (packet->payload_type == payload_type) {
-      delete packet;
+    Packet& packet = *it;

[kwiberg-webrtc, 2016/10/20 22:39:22]

const? And can you use cbegin/cend so that the iterator is a const_iterator?

[ossu, 2016/10/21 12:54:41]

Nope! Can't erase from a const_iterator (which makes sense, imo). Made the
Packet& const, tho.

[kwiberg-webrtc, 2016/10/24 11:42:25]

Acknowledged.

+    if (packet.payload_type == payload_type) {
       it = buffer_.erase(it);
     } else {
       ++it;
@@ -274,14 +250,14 @@ size_t PacketBuffer::NumPacketsInBuffer() const {
 size_t PacketBuffer::NumSamplesInBuffer(size_t last_decoded_length) const {
   size_t num_samples = 0;
   size_t last_duration = last_decoded_length;
-  for (Packet* packet : buffer_) {
-    if (packet->frame) {
+  for (const Packet& packet : buffer_) {
+    if (packet.frame) {
       // TODO(hlundin): Verify that it's fine to count all packets and remove
       // this check.
-      if (packet->priority != Packet::Priority(0, 0)) {
+      if (packet.priority != Packet::Priority(0, 0)) {
         continue;
       }
-      size_t duration = packet->frame->Duration();
+      size_t duration = packet.frame->Duration();
       if (duration > 0) {
         last_duration = duration;  // Save the most up-to-date (valid) duration.
       }
@@ -291,22 +267,6 @@ size_t PacketBuffer::NumSamplesInBuffer(size_t last_decoded_length) const {
   return num_samples;
 }
 
-bool PacketBuffer::DeleteFirstPacket(PacketList* packet_list) {
-  if (packet_list->empty()) {
-    return false;
-  }
-  Packet* first_packet = packet_list->front();
-  delete first_packet;
-  packet_list->pop_front();
-  return true;
-}
-
-void PacketBuffer::DeleteAllPackets(PacketList* packet_list) {
-  while (DeleteFirstPacket(packet_list)) {
-    // Continue while the list is not empty.
-  }
-}
-
 void PacketBuffer::BufferStat(int* num_packets, int* max_num_packets) const {
   *num_packets = static_cast<int>(buffer_.size());
   *max_num_packets = static_cast<int>(max_number_of_packets_);