NetEq now works with packets as values, rather than pointers.
PacketList is now list<Packet> instead of list<Packet*>.
Splicing the lists in NetEqImpl::InsertPacketInternal instead of
moving packets. Avoid moving the packet when doing Rfc3389Cng.
Removed PacketBuffer::DeleteFirstPacket and DeleteAllPackets.
BUG=chromium:657300
Review-Url: https://codereview.webrtc.org/2425223002
Cr-Commit-Position: refs/heads/master@{#14747}
diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer.cc b/webrtc/modules/audio_coding/neteq/packet_buffer.cc
index ee14398..a9a396d 100644
--- a/webrtc/modules/audio_coding/neteq/packet_buffer.cc
+++ b/webrtc/modules/audio_coding/neteq/packet_buffer.cc
@@ -27,15 +27,15 @@
// 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()(const Packet& packet) {
+ 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 @@
// 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 @@
// 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) {
return return_val;
}
@@ -109,11 +105,10 @@
// 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) {
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 @@
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 @@
if (!next_timestamp) {
return kInvalidPointer;
}
- *next_timestamp = buffer_.front()->timestamp;
+ *next_timestamp = buffer_.front().timestamp;
return kOK;
}
@@ -185,9 +179,9 @@
}
PacketList::const_iterator it;
for (it = buffer_.begin(); it != buffer_.end(); ++it) {
- if ((*it)->timestamp >= timestamp) {
+ if (it->timestamp >= timestamp) {
// Found a packet matching the search.
- *next_timestamp = (*it)->timestamp;
+ *next_timestamp = it->timestamp;
return kOK;
}
}
@@ -195,36 +189,20 @@
}
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 @@
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 @@
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;
+ const Packet& packet = *it;
+ if (packet.payload_type == payload_type) {
it = buffer_.erase(it);
} else {
++it;
@@ -274,14 +250,14 @@
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 @@
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_);