diff --git a/webrtc/modules/audio_coding/neteq/comfort_noise.cc b/webrtc/modules/audio_coding/neteq/comfort_noise.cc index 504a0bcd4c..9493d0a41c 100644 --- a/webrtc/modules/audio_coding/neteq/comfort_noise.cc +++ b/webrtc/modules/audio_coding/neteq/comfort_noise.cc @@ -24,17 +24,14 @@ void ComfortNoise::Reset() { first_call_ = true; } -int ComfortNoise::UpdateParameters(Packet* packet) { - assert(packet); // Existence is verified by caller. +int ComfortNoise::UpdateParameters(const Packet& packet) { // Get comfort noise decoder. - if (decoder_database_->SetActiveCngDecoder(packet->payload_type) != kOK) { - delete packet; + if (decoder_database_->SetActiveCngDecoder(packet.payload_type) != kOK) { return kUnknownPayloadType; } ComfortNoiseDecoder* cng_decoder = decoder_database_->GetActiveCngDecoder(); RTC_DCHECK(cng_decoder); - cng_decoder->UpdateSid(packet->payload); - delete packet; + cng_decoder->UpdateSid(packet.payload); return kOK; } diff --git a/webrtc/modules/audio_coding/neteq/comfort_noise.h b/webrtc/modules/audio_coding/neteq/comfort_noise.h index f877bf63ef..7476784791 100644 --- a/webrtc/modules/audio_coding/neteq/comfort_noise.h +++ b/webrtc/modules/audio_coding/neteq/comfort_noise.h @@ -45,8 +45,7 @@ class ComfortNoise { void Reset(); // Update the comfort noise generator with the parameters in |packet|. - // Will delete the packet. - int UpdateParameters(Packet* packet); + int UpdateParameters(const Packet& packet); // Generates |requested_length| samples of comfort noise and writes to // |output|. If this is the first in call after Reset (or first after creating diff --git a/webrtc/modules/audio_coding/neteq/decoder_database.cc b/webrtc/modules/audio_coding/neteq/decoder_database.cc index 299cedcd32..483c9b96ba 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database.cc +++ b/webrtc/modules/audio_coding/neteq/decoder_database.cc @@ -310,10 +310,10 @@ bool DecoderDatabase::IsRed(uint8_t rtp_payload_type) const { int DecoderDatabase::CheckPayloadTypes(const PacketList& packet_list) const { PacketList::const_iterator it; for (it = packet_list.begin(); it != packet_list.end(); ++it) { - if (!GetDecoderInfo((*it)->payload_type)) { + if (!GetDecoderInfo(it->payload_type)) { // Payload type is not found. LOG(LS_WARNING) << "CheckPayloadTypes: unknown RTP payload type " - << static_cast((*it)->payload_type); + << static_cast(it->payload_type); return kDecoderNotFound; } } diff --git a/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc b/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc index a23816e770..6c3b6c9b94 100644 --- a/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/decoder_database_unittest.cc @@ -175,16 +175,15 @@ TEST(DecoderDatabase, CheckPayloadTypes) { for (int i = 0; i < kNumPayloads + 1; ++i) { // Create packet with payload type |i|. The last packet will have a payload // type that is not registered in the decoder database. - Packet* packet = new Packet; - packet->payload_type = i; - packet_list.push_back(packet); + Packet packet; + packet.payload_type = i; + packet_list.push_back(std::move(packet)); } // Expect to return false, since the last packet is of an unknown type. EXPECT_EQ(DecoderDatabase::kDecoderNotFound, db.CheckPayloadTypes(packet_list)); - delete packet_list.back(); packet_list.pop_back(); // Remove the unknown one. EXPECT_EQ(DecoderDatabase::kOK, db.CheckPayloadTypes(packet_list)); @@ -192,7 +191,6 @@ TEST(DecoderDatabase, CheckPayloadTypes) { // Delete all packets. PacketList::iterator it = packet_list.begin(); while (it != packet_list.end()) { - delete packet_list.front(); it = packet_list.erase(it); } } diff --git a/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h b/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h index ae1ab7a386..5a2f7e11e1 100644 --- a/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h +++ b/webrtc/modules/audio_coding/neteq/mock/mock_packet_buffer.h @@ -27,7 +27,13 @@ class MockPacketBuffer : public PacketBuffer { void()); MOCK_CONST_METHOD0(Empty, bool()); - MOCK_METHOD1(InsertPacket, + int InsertPacket(Packet&& packet) { + return InsertPacketWrapped(&packet); + } + // Since gtest does not properly support move-only types, InsertPacket is + // implemented as a wrapper. You'll have to implement InsertPacketWrapped + // instead and move from |*packet|. + MOCK_METHOD1(InsertPacketWrapped, int(Packet* packet)); MOCK_METHOD4(InsertPacketList, int(PacketList* packet_list, @@ -40,8 +46,8 @@ class MockPacketBuffer : public PacketBuffer { int(uint32_t timestamp, uint32_t* next_timestamp)); MOCK_CONST_METHOD0(PeekNextPacket, const Packet*()); - MOCK_METHOD1(GetNextPacket, - Packet*(size_t* discard_count)); + MOCK_METHOD0(GetNextPacket, + rtc::Optional()); MOCK_METHOD0(DiscardNextPacket, int()); MOCK_METHOD2(DiscardOldPackets, diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc index 8439c5c971..e5cab165ba 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc @@ -581,21 +581,18 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header, } PacketList packet_list; - { + // Insert packet in a packet list. + packet_list.push_back([&rtp_header, &payload] { // Convert to Packet. - // Create |packet| within this separate scope, since it should not be used - // directly once it's been inserted in the packet list. This way, |packet| - // is not defined outside of this block. - Packet* packet = new Packet; - packet->payload_type = rtp_header.header.payloadType; - packet->sequence_number = rtp_header.header.sequenceNumber; - packet->timestamp = rtp_header.header.timestamp; - packet->payload.SetData(payload.data(), payload.size()); + Packet packet; + packet.payload_type = rtp_header.header.payloadType; + packet.sequence_number = rtp_header.header.sequenceNumber; + packet.timestamp = rtp_header.header.timestamp; + packet.payload.SetData(payload.data(), payload.size()); // Waiting time will be set upon inserting the packet in the buffer. - RTC_DCHECK(!packet->waiting_time); - // Insert packet in a packet list. - packet_list.push_back(packet); - } + RTC_DCHECK(!packet.waiting_time); + return packet; + }()); bool update_sample_rate_and_channels = false; // Reinitialize NetEq if it's needed (changed SSRC or first call). @@ -641,7 +638,6 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header, // Check for RED payload type, and separate payloads into several packets. if (decoder_database_->IsRed(rtp_header.header.payloadType)) { if (!red_payload_splitter_->SplitRed(&packet_list)) { - PacketBuffer::DeleteAllPackets(&packet_list); return kRedundancySplitError; } // Only accept a few RED payloads of the same type as the main data, @@ -652,16 +648,15 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header, // Check payload types. if (decoder_database_->CheckPayloadTypes(packet_list) == DecoderDatabase::kDecoderNotFound) { - PacketBuffer::DeleteAllPackets(&packet_list); return kUnknownRtpPayloadType; } RTC_DCHECK(!packet_list.empty()); // Store these for later use, since the first packet may very well disappear // before we need these values. - const uint32_t main_timestamp = packet_list.front()->timestamp; - const uint8_t main_payload_type = packet_list.front()->payload_type; - const uint16_t main_sequence_number = packet_list.front()->sequence_number; + const uint32_t main_timestamp = packet_list.front().timestamp; + const uint8_t main_payload_type = packet_list.front().payload_type; + const uint16_t main_sequence_number = packet_list.front().sequence_number; // Scale timestamp to internal domain (only for some codecs). timestamp_scaler_->ToInternal(&packet_list); @@ -670,23 +665,19 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header, // DTMF payloads found. PacketList::iterator it = packet_list.begin(); while (it != packet_list.end()) { - Packet* current_packet = (*it); - assert(current_packet); - assert(!current_packet->payload.empty()); - if (decoder_database_->IsDtmf(current_packet->payload_type)) { + const Packet& current_packet = (*it); + RTC_DCHECK(!current_packet.payload.empty()); + if (decoder_database_->IsDtmf(current_packet.payload_type)) { DtmfEvent event; - int ret = DtmfBuffer::ParseEvent(current_packet->timestamp, - current_packet->payload.data(), - current_packet->payload.size(), &event); + int ret = DtmfBuffer::ParseEvent(current_packet.timestamp, + current_packet.payload.data(), + current_packet.payload.size(), &event); if (ret != DtmfBuffer::kOK) { - PacketBuffer::DeleteAllPackets(&packet_list); return kDtmfParsingError; } if (dtmf_buffer_->InsertEvent(event) != DtmfBuffer::kOK) { - PacketBuffer::DeleteAllPackets(&packet_list); return kDtmfInsertError; } - delete current_packet; it = packet_list.erase(it); } else { ++it; @@ -700,19 +691,18 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header, AudioDecoder* decoder = decoder_database_->GetDecoder(main_payload_type); RTC_DCHECK(decoder); // Should always get a valid object, since we have // already checked that the payload types are known. - decoder->IncomingPacket(packet_list.front()->payload.data(), - packet_list.front()->payload.size(), - packet_list.front()->sequence_number, - packet_list.front()->timestamp, + decoder->IncomingPacket(packet_list.front().payload.data(), + packet_list.front().payload.size(), + packet_list.front().sequence_number, + packet_list.front().timestamp, receive_timestamp); } PacketList parsed_packet_list; while (!packet_list.empty()) { - std::unique_ptr packet(packet_list.front()); - packet_list.pop_front(); + Packet& packet = packet_list.front(); const DecoderDatabase::DecoderInfo* info = - decoder_database_->GetDecoderInfo(packet->payload_type); + decoder_database_->GetDecoderInfo(packet.payload_type); if (!info) { LOG(LS_WARNING) << "SplitAudio unknown payload type"; return kUnknownRtpPayloadType; @@ -720,28 +710,43 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header, if (info->IsComfortNoise()) { // Carry comfort noise packets along. - parsed_packet_list.push_back(packet.release()); + parsed_packet_list.splice(parsed_packet_list.end(), packet_list, + packet_list.begin()); } else { + const auto sequence_number = packet.sequence_number; + const auto payload_type = packet.payload_type; + const Packet::Priority original_priority = packet.priority; + auto packet_from_result = [&] (AudioDecoder::ParseResult& result) { + Packet new_packet; + new_packet.sequence_number = sequence_number; + new_packet.payload_type = payload_type; + new_packet.timestamp = result.timestamp; + new_packet.priority.codec_level = result.priority; + new_packet.priority.red_level = original_priority.red_level; + new_packet.frame = std::move(result.frame); + return new_packet; + }; + std::vector results = - info->GetDecoder()->ParsePayload(std::move(packet->payload), - packet->timestamp); - const auto sequence_number = packet->sequence_number; - const auto payload_type = packet->payload_type; - const Packet::Priority original_priority = packet->priority; - for (auto& result : results) { - RTC_DCHECK(result.frame); - // Reuse the packet if possible. - if (!packet) { - packet.reset(new Packet); - packet->sequence_number = sequence_number; - packet->payload_type = payload_type; + info->GetDecoder()->ParsePayload(std::move(packet.payload), + packet.timestamp); + if (results.empty()) { + packet_list.pop_front(); + } else { + bool first = true; + for (auto& result : results) { + RTC_DCHECK(result.frame); + RTC_DCHECK_GE(result.priority, 0); + if (first) { + // Re-use the node and move it to parsed_packet_list. + packet_list.front() = packet_from_result(result); + parsed_packet_list.splice(parsed_packet_list.end(), packet_list, + packet_list.begin()); + first = false; + } else { + parsed_packet_list.push_back(packet_from_result(result)); + } } - packet->timestamp = result.timestamp; - RTC_DCHECK_GE(result.priority, 0); - packet->priority.codec_level = result.priority; - packet->priority.red_level = original_priority.red_level; - packet->frame = std::move(result.frame); - parsed_packet_list.push_back(packet.release()); } } } @@ -757,7 +762,6 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header, new_codec_ = true; update_sample_rate_and_channels = true; } else if (ret != PacketBuffer::kOK) { - PacketBuffer::DeleteAllPackets(&parsed_packet_list); return kOtherError; } @@ -1344,15 +1348,15 @@ int NetEqImpl::Decode(PacketList* packet_list, Operations* operation, AudioDecoder* decoder = decoder_database_->GetActiveDecoder(); if (!packet_list->empty()) { - const Packet* packet = packet_list->front(); - uint8_t payload_type = packet->payload_type; + const Packet& packet = packet_list->front(); + uint8_t payload_type = packet.payload_type; if (!decoder_database_->IsComfortNoise(payload_type)) { decoder = decoder_database_->GetDecoder(payload_type); assert(decoder); if (!decoder) { LOG(LS_WARNING) << "Unknown payload type " << static_cast(payload_type); - PacketBuffer::DeleteAllPackets(packet_list); + packet_list->clear(); return kDecoderNotFound; } bool decoder_changed; @@ -1365,7 +1369,7 @@ int NetEqImpl::Decode(PacketList* packet_list, Operations* operation, if (!decoder_info) { LOG(LS_WARNING) << "Unknown payload type " << static_cast(payload_type); - PacketBuffer::DeleteAllPackets(packet_list); + packet_list->clear(); return kDecoderNotFound; } // If sampling rate or number of channels has changed, we need to make @@ -1475,13 +1479,10 @@ int NetEqImpl::DecodeCng(AudioDecoder* decoder, int* decoded_length, int NetEqImpl::DecodeLoop(PacketList* packet_list, const Operations& operation, AudioDecoder* decoder, int* decoded_length, AudioDecoder::SpeechType* speech_type) { - Packet* packet = NULL; - if (!packet_list->empty()) { - packet = packet_list->front(); - } - // Do decoding. - while (packet && !decoder_database_->IsComfortNoise(packet->payload_type)) { + while ( + !packet_list->empty() && + !decoder_database_->IsComfortNoise(packet_list->front().payload_type)) { assert(decoder); // At this point, we must have a decoder object. // The number of channels in the |sync_buffer_| should be the same as the // number decoder channels. @@ -1490,12 +1491,11 @@ int NetEqImpl::DecodeLoop(PacketList* packet_list, const Operations& operation, assert(operation == kNormal || operation == kAccelerate || operation == kFastAccelerate || operation == kMerge || operation == kPreemptiveExpand); - packet_list->pop_front(); - auto opt_result = packet->frame->Decode( + + auto opt_result = packet_list->front().frame->Decode( rtc::ArrayView(&decoded_buffer_[*decoded_length], decoded_buffer_length_ - *decoded_length)); - delete packet; - packet = NULL; + packet_list->pop_front(); if (opt_result) { const auto& result = *opt_result; *speech_type = result.speech_type; @@ -1510,27 +1510,23 @@ int NetEqImpl::DecodeLoop(PacketList* packet_list, const Operations& operation, // TODO(ossu): What to put here? LOG(LS_WARNING) << "Decode error"; *decoded_length = -1; - PacketBuffer::DeleteAllPackets(packet_list); + packet_list->clear(); break; } if (*decoded_length > rtc::checked_cast(decoded_buffer_length_)) { // Guard against overflow. LOG(LS_WARNING) << "Decoded too much."; - PacketBuffer::DeleteAllPackets(packet_list); + packet_list->clear(); return kDecodedTooMuch; } - if (!packet_list->empty()) { - packet = packet_list->front(); - } else { - packet = NULL; - } } // End of decode loop. // If the list is not empty at this point, either a decoding error terminated // the while-loop, or list must hold exactly one CNG packet. - assert(packet_list->empty() || *decoded_length < 0 || - (packet_list->size() == 1 && packet && - decoder_database_->IsComfortNoise(packet->payload_type))); + assert( + packet_list->empty() || *decoded_length < 0 || + (packet_list->size() == 1 && + decoder_database_->IsComfortNoise(packet_list->front().payload_type))); return 0; } @@ -1770,13 +1766,11 @@ int NetEqImpl::DoRfc3389Cng(PacketList* packet_list, bool play_dtmf) { if (!packet_list->empty()) { // Must have exactly one SID frame at this point. assert(packet_list->size() == 1); - Packet* packet = packet_list->front(); - packet_list->pop_front(); - if (!decoder_database_->IsComfortNoise(packet->payload_type)) { + const Packet& packet = packet_list->front(); + if (!decoder_database_->IsComfortNoise(packet.payload_type)) { LOG(LS_ERROR) << "Trying to decode non-CNG payload as CNG."; return kOtherError; } - // UpdateParameters() deletes |packet|. if (comfort_noise_->UpdateParameters(packet) == ComfortNoise::kInternalError) { algorithm_buffer_->Zeros(output_size_samples_); @@ -1960,19 +1954,16 @@ int NetEqImpl::ExtractPackets(size_t required_samples, // Packet extraction loop. do { timestamp_ = next_packet->timestamp; - size_t discard_count = 0; - Packet* packet = packet_buffer_->GetNextPacket(&discard_count); + rtc::Optional packet = packet_buffer_->GetNextPacket(); // |next_packet| may be invalid after the |packet_buffer_| operation. - next_packet = NULL; + next_packet = nullptr; if (!packet) { LOG(LS_ERROR) << "Should always be able to extract a packet here"; assert(false); // Should always be able to extract a packet here. return -1; } - stats_.PacketsDiscarded(discard_count); stats_.StoreWaitingTime(packet->waiting_time->ElapsedMs()); RTC_DCHECK(!packet->empty()); - packet_list->push_back(packet); // Store packet in list. if (first_packet) { first_packet = false; @@ -2008,6 +1999,9 @@ int NetEqImpl::ExtractPackets(size_t required_samples, } extracted_samples = packet->timestamp - first_timestamp + packet_duration; + packet_list->push_back(std::move(*packet)); // Store packet in list. + packet = rtc::Optional(); // Ensure it's never used after the move. + // Check what packet is available next. next_packet = packet_buffer_->PeekNextPacket(); next_packet_available = false; diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc index d520301c0d..5178938947 100644 --- a/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -52,7 +52,7 @@ namespace webrtc { // buffer. The purpose is to delete all inserted packets properly, to avoid // memory leaks in the test. int DeletePacketsAndReturnOk(PacketList* packet_list) { - PacketBuffer::DeleteAllPackets(packet_list); + packet_list->clear(); return PacketBuffer::kOK; } diff --git a/webrtc/modules/audio_coding/neteq/packet.cc b/webrtc/modules/audio_coding/neteq/packet.cc index 8a19fe4d59..f25f81a241 100644 --- a/webrtc/modules/audio_coding/neteq/packet.cc +++ b/webrtc/modules/audio_coding/neteq/packet.cc @@ -13,7 +13,23 @@ namespace webrtc { Packet::Packet() = default; +Packet::Packet(Packet&& b) = default; Packet::~Packet() = default; +Packet& Packet::operator=(Packet&& b) = default; + +Packet Packet::Clone() const { + RTC_CHECK(!frame); + + Packet clone; + clone.timestamp = timestamp; + clone.sequence_number = sequence_number; + clone.payload_type = payload_type; + clone.payload.SetData(payload.data(), payload.size()); + clone.priority = priority; + + return clone; +} + } // namespace webrtc diff --git a/webrtc/modules/audio_coding/neteq/packet.h b/webrtc/modules/audio_coding/neteq/packet.h index dc90495da8..74da83f435 100644 --- a/webrtc/modules/audio_coding/neteq/packet.h +++ b/webrtc/modules/audio_coding/neteq/packet.h @@ -75,8 +75,17 @@ struct Packet { std::unique_ptr frame; Packet(); + Packet(Packet&& b); ~Packet(); + // Packets should generally be moved around but sometimes it's useful to make + // a copy, for example for testing purposes. NOTE: Will only work for + // un-parsed packets, i.e. |frame| must be unset. The payload will, however, + // be copied. |waiting_time| will also not be copied. + Packet Clone() const; + + Packet& operator=(Packet&& b); + // Comparison operators. Establish a packet ordering based on (1) timestamp, // (2) sequence number and (3) redundancy. // Timestamp and sequence numbers are compared taking wrap-around into @@ -109,7 +118,7 @@ struct Packet { }; // A list of packets. -typedef std::list PacketList; +typedef std::list PacketList; } // namespace webrtc #endif // WEBRTC_MODULES_AUDIO_CODING_NETEQ_PACKET_H_ diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer.cc b/webrtc/modules/audio_coding/neteq/packet_buffer.cc index ee1439809b..a9a396d7be 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()(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 @@ 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) { 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) { 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* current_rtp_payload_type, rtc::Optional* 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(); Flush(); flushed = true; } *current_cng_rtp_payload_type = - rtc::Optional(packet->payload_type); - } else if (!decoder_database.IsDtmf(packet->payload_type)) { + rtc::Optional(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(); Flush(); flushed = true; } - *current_rtp_payload_type = rtc::Optional(packet->payload_type); + *current_rtp_payload_type = rtc::Optional(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) { // Found a packet matching the search. - *next_timestamp = (*it)->timestamp; + *next_timestamp = it->timestamp; 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 PacketBuffer::GetNextPacket() { if (Empty()) { // Buffer is empty. - return NULL; + return rtc::Optional(); } - Packet* packet = buffer_.front(); + rtc::Optional 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; + const Packet& packet = *it; + 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(buffer_.size()); *max_num_packets = static_cast(max_number_of_packets_); diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer.h b/webrtc/modules/audio_coding/neteq/packet_buffer.h index 63e006c1d5..a26d6c5c67 100644 --- a/webrtc/modules/audio_coding/neteq/packet_buffer.h +++ b/webrtc/modules/audio_coding/neteq/packet_buffer.h @@ -51,7 +51,7 @@ class PacketBuffer { // the packet object. // Returns PacketBuffer::kOK on success, PacketBuffer::kFlushed if the buffer // was flushed due to overfilling. - virtual int InsertPacket(Packet* packet); + virtual int InsertPacket(Packet&& packet); // Inserts a list of packets into the buffer. The buffer will take over // ownership of the packet objects. @@ -85,13 +85,9 @@ class PacketBuffer { // NULL if the buffer is empty. virtual const Packet* PeekNextPacket() const; - // Extracts the first packet in the buffer and returns a pointer to it. - // Returns NULL if the buffer is empty. The caller is responsible for deleting - // the packet. - // Subsequent packets with the same timestamp as the one extracted will be - // discarded and properly deleted. The number of discarded packets will be - // written to the output variable |discard_count|. - virtual Packet* GetNextPacket(size_t* discard_count); + // Extracts the first packet in the buffer and returns it. + // Returns an empty optional if the buffer is empty. + virtual rtc::Optional GetNextPacket(); // Discards the first packet in the buffer. The packet is deleted. // Returns PacketBuffer::kBufferEmpty if the buffer is empty, @@ -123,15 +119,6 @@ class PacketBuffer { virtual void BufferStat(int* num_packets, int* max_num_packets) const; - // Static method that properly deletes the first packet, and its payload - // array, in |packet_list|. Returns false if |packet_list| already was empty, - // otherwise true. - static bool DeleteFirstPacket(PacketList* packet_list); - - // Static method that properly deletes all packets, and their payload arrays, - // in |packet_list|. - static void DeleteAllPackets(PacketList* packet_list); - // Static method returning true if |timestamp| is older than |timestamp_limit| // but less than |horizon_samples| behind |timestamp_limit|. For instance, // with timestamp_limit = 100 and horizon_samples = 10, a timestamp in the diff --git a/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc b/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc index 2f918e6c95..dcb4f1a20e 100644 --- a/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/packet_buffer_unittest.cc @@ -30,7 +30,7 @@ class PacketGenerator { PacketGenerator(uint16_t seq_no, uint32_t ts, uint8_t pt, int frame_size); virtual ~PacketGenerator() {} void Reset(uint16_t seq_no, uint32_t ts, uint8_t pt, int frame_size); - Packet* NextPacket(int payload_size_bytes); + Packet NextPacket(int payload_size_bytes); uint16_t seq_no_; uint32_t ts_; @@ -51,12 +51,12 @@ void PacketGenerator::Reset(uint16_t seq_no, uint32_t ts, uint8_t pt, frame_size_ = frame_size; } -Packet* PacketGenerator::NextPacket(int payload_size_bytes) { - Packet* packet = new Packet; - packet->sequence_number = seq_no_; - packet->timestamp = ts_; - packet->payload_type = pt_; - packet->payload.SetSize(payload_size_bytes); +Packet PacketGenerator::NextPacket(int payload_size_bytes) { + Packet packet; + packet.sequence_number = seq_no_; + packet.timestamp = ts_; + packet.payload_type = pt_; + packet.payload.SetSize(payload_size_bytes); ++seq_no_; ts_ += frame_size_; return packet; @@ -88,16 +88,15 @@ TEST(PacketBuffer, InsertPacket) { PacketGenerator gen(17u, 4711u, 0, 10); const int payload_len = 100; - Packet* packet = gen.NextPacket(payload_len); - - EXPECT_EQ(0, buffer.InsertPacket(packet)); + const Packet packet = gen.NextPacket(payload_len); + EXPECT_EQ(0, buffer.InsertPacket(packet.Clone())); uint32_t next_ts; EXPECT_EQ(PacketBuffer::kOK, buffer.NextTimestamp(&next_ts)); EXPECT_EQ(4711u, next_ts); EXPECT_FALSE(buffer.Empty()); EXPECT_EQ(1u, buffer.NumPacketsInBuffer()); const Packet* next_packet = buffer.PeekNextPacket(); - EXPECT_EQ(packet, next_packet); // Compare pointer addresses. + EXPECT_EQ(packet, *next_packet); // Compare contents. // Do not explicitly flush buffer or delete packet to test that it is deleted // with the buffer. (Tested with Valgrind or similar tool.) @@ -112,8 +111,8 @@ TEST(PacketBuffer, FlushBuffer) { // Insert 10 small packets; should be ok. for (int i = 0; i < 10; ++i) { - Packet* packet = gen.NextPacket(payload_len); - EXPECT_EQ(PacketBuffer::kOK, buffer.InsertPacket(packet)); + EXPECT_EQ(PacketBuffer::kOK, + buffer.InsertPacket(gen.NextPacket(payload_len))); } EXPECT_EQ(10u, buffer.NumPacketsInBuffer()); EXPECT_FALSE(buffer.Empty()); @@ -134,21 +133,21 @@ TEST(PacketBuffer, OverfillBuffer) { const int payload_len = 10; int i; for (i = 0; i < 10; ++i) { - Packet* packet = gen.NextPacket(payload_len); - EXPECT_EQ(PacketBuffer::kOK, buffer.InsertPacket(packet)); + EXPECT_EQ(PacketBuffer::kOK, + buffer.InsertPacket(gen.NextPacket(payload_len))); } EXPECT_EQ(10u, buffer.NumPacketsInBuffer()); uint32_t next_ts; EXPECT_EQ(PacketBuffer::kOK, buffer.NextTimestamp(&next_ts)); EXPECT_EQ(0u, next_ts); // Expect first inserted packet to be first in line. + const Packet packet = gen.NextPacket(payload_len); // Insert 11th packet; should flush the buffer and insert it after flushing. - Packet* packet = gen.NextPacket(payload_len); - EXPECT_EQ(PacketBuffer::kFlushed, buffer.InsertPacket(packet)); + EXPECT_EQ(PacketBuffer::kFlushed, buffer.InsertPacket(packet.Clone())); EXPECT_EQ(1u, buffer.NumPacketsInBuffer()); EXPECT_EQ(PacketBuffer::kOK, buffer.NextTimestamp(&next_ts)); // Expect last inserted packet to be first in line. - EXPECT_EQ(packet->timestamp, next_ts); + EXPECT_EQ(packet.timestamp, next_ts); // Flush buffer to delete all packets. buffer.Flush(); @@ -164,8 +163,7 @@ TEST(PacketBuffer, InsertPacketList) { // Insert 10 small packets. for (int i = 0; i < 10; ++i) { - Packet* packet = gen.NextPacket(payload_len); - list.push_back(packet); + list.push_back(gen.NextPacket(payload_len)); } MockDecoderDatabase decoder_database; @@ -202,14 +200,14 @@ TEST(PacketBuffer, InsertPacketListChangePayloadType) { // Insert 10 small packets. for (int i = 0; i < 10; ++i) { - Packet* packet = gen.NextPacket(payload_len); - list.push_back(packet); + list.push_back(gen.NextPacket(payload_len)); } // Insert 11th packet of another payload type (not CNG). - Packet* packet = gen.NextPacket(payload_len); - packet->payload_type = 1; - list.push_back(packet); - + { + Packet packet = gen.NextPacket(payload_len); + packet.payload_type = 1; + list.push_back(std::move(packet)); + } MockDecoderDatabase decoder_database; auto factory = CreateBuiltinAudioDecoderFactory(); @@ -266,7 +264,7 @@ TEST(PacketBuffer, ExtractOrderRedundancy) { const size_t kExpectPacketsInBuffer = 9; - std::vector expect_order(kExpectPacketsInBuffer); + std::vector expect_order(kExpectPacketsInBuffer); PacketGenerator gen(0, 0, 0, kFrameSize); @@ -275,22 +273,19 @@ TEST(PacketBuffer, ExtractOrderRedundancy) { packet_facts[i].timestamp, packet_facts[i].payload_type, kFrameSize); - Packet* packet = gen.NextPacket(kPayloadLength); - packet->priority.red_level = packet_facts[i].primary ? 0 : 1; - EXPECT_EQ(PacketBuffer::kOK, buffer.InsertPacket(packet)); + Packet packet = gen.NextPacket(kPayloadLength); + packet.priority.red_level = packet_facts[i].primary ? 0 : 1; + EXPECT_EQ(PacketBuffer::kOK, buffer.InsertPacket(packet.Clone())); if (packet_facts[i].extract_order >= 0) { - expect_order[packet_facts[i].extract_order] = packet; + expect_order[packet_facts[i].extract_order] = std::move(packet); } } EXPECT_EQ(kExpectPacketsInBuffer, buffer.NumPacketsInBuffer()); - size_t drop_count; for (size_t i = 0; i < kExpectPacketsInBuffer; ++i) { - Packet* packet = buffer.GetNextPacket(&drop_count); - EXPECT_EQ(0u, drop_count); - EXPECT_EQ(packet, expect_order[i]); // Compare pointer addresses. - delete packet; + const rtc::Optional packet = buffer.GetNextPacket(); + EXPECT_EQ(packet, expect_order[i]); // Compare contents. } EXPECT_TRUE(buffer.Empty()); } @@ -307,8 +302,7 @@ TEST(PacketBuffer, DiscardPackets) { // Insert 10 small packets. for (int i = 0; i < 10; ++i) { - Packet* packet = gen.NextPacket(payload_len); - buffer.InsertPacket(packet); + buffer.InsertPacket(gen.NextPacket(payload_len)); } EXPECT_EQ(10u, buffer.NumPacketsInBuffer()); @@ -339,11 +333,11 @@ TEST(PacketBuffer, Reordering) { // a (rather strange) reordering. PacketList list; for (int i = 0; i < 10; ++i) { - Packet* packet = gen.NextPacket(payload_len); + Packet packet = gen.NextPacket(payload_len); if (i % 2) { - list.push_front(packet); + list.push_front(std::move(packet)); } else { - list.push_back(packet); + list.push_back(std::move(packet)); } } @@ -364,11 +358,10 @@ TEST(PacketBuffer, Reordering) { // Extract them and make sure that come out in the right order. uint32_t current_ts = start_ts; for (int i = 0; i < 10; ++i) { - Packet* packet = buffer.GetNextPacket(NULL); - ASSERT_FALSE(packet == NULL); + const rtc::Optional packet = buffer.GetNextPacket(); + ASSERT_TRUE(packet); EXPECT_EQ(current_ts, packet->timestamp); current_ts += ts_increment; - delete packet; } EXPECT_TRUE(buffer.Empty()); @@ -415,9 +408,11 @@ TEST(PacketBuffer, CngFirstThenSpeechWithNewSampleRate) { current_cng_pt); // CNG payload type set. // Insert second packet, which is wide-band speech. - Packet* packet = gen.NextPacket(kPayloadLen); - packet->payload_type = kSpeechPt; - list.push_back(packet); + { + Packet packet = gen.NextPacket(kPayloadLen); + packet.payload_type = kSpeechPt; + list.push_back(std::move(packet)); + } // Expect the buffer to flush out the CNG packet, since it does not match the // new speech sample rate. EXPECT_EQ(PacketBuffer::kFlushed, @@ -445,26 +440,25 @@ TEST(PacketBuffer, Failures) { TickTimer tick_timer; PacketBuffer* buffer = new PacketBuffer(100, &tick_timer); // 100 packets. - Packet* packet = NULL; - EXPECT_EQ(PacketBuffer::kInvalidPacket, buffer->InsertPacket(packet)); - packet = gen.NextPacket(payload_len); - packet->payload.Clear(); - EXPECT_EQ(PacketBuffer::kInvalidPacket, buffer->InsertPacket(packet)); - // Packet is deleted by the PacketBuffer. - + { + Packet packet = gen.NextPacket(payload_len); + packet.payload.Clear(); + EXPECT_EQ(PacketBuffer::kInvalidPacket, + buffer->InsertPacket(std::move(packet))); + } // Buffer should still be empty. Test all empty-checks. uint32_t temp_ts; EXPECT_EQ(PacketBuffer::kBufferEmpty, buffer->NextTimestamp(&temp_ts)); EXPECT_EQ(PacketBuffer::kBufferEmpty, buffer->NextHigherTimestamp(0, &temp_ts)); EXPECT_EQ(NULL, buffer->PeekNextPacket()); - EXPECT_EQ(NULL, buffer->GetNextPacket(NULL)); + EXPECT_FALSE(buffer->GetNextPacket()); EXPECT_EQ(PacketBuffer::kBufferEmpty, buffer->DiscardNextPacket()); EXPECT_EQ(0, buffer->DiscardAllOldPackets(0)); // 0 packets discarded. // Insert one packet to make the buffer non-empty. - packet = gen.NextPacket(payload_len); - EXPECT_EQ(PacketBuffer::kOK, buffer->InsertPacket(packet)); + EXPECT_EQ(PacketBuffer::kOK, + buffer->InsertPacket(gen.NextPacket(payload_len))); EXPECT_EQ(PacketBuffer::kInvalidPointer, buffer->NextTimestamp(NULL)); EXPECT_EQ(PacketBuffer::kInvalidPointer, buffer->NextHigherTimestamp(0, NULL)); @@ -476,9 +470,11 @@ TEST(PacketBuffer, Failures) { buffer = new PacketBuffer(100, &tick_timer); // 100 packets. PacketList list; list.push_back(gen.NextPacket(payload_len)); // Valid packet. - packet = gen.NextPacket(payload_len); - packet->payload.Clear(); // Invalid. - list.push_back(packet); + { + Packet packet = gen.NextPacket(payload_len); + packet.payload.Clear(); // Invalid. + list.push_back(std::move(packet)); + } list.push_back(gen.NextPacket(payload_len)); // Valid packet. MockDecoderDatabase decoder_database; auto factory = CreateBuiltinAudioDecoderFactory(); @@ -502,127 +498,109 @@ TEST(PacketBuffer, Failures) { // The function should return true if the first packet "goes before" the second. TEST(PacketBuffer, ComparePackets) { PacketGenerator gen(0, 0, 0, 10); - std::unique_ptr a(gen.NextPacket(10)); // SN = 0, TS = 0. - std::unique_ptr b(gen.NextPacket(10)); // SN = 1, TS = 10. - EXPECT_FALSE(*a == *b); - EXPECT_TRUE(*a != *b); - EXPECT_TRUE(*a < *b); - EXPECT_FALSE(*a > *b); - EXPECT_TRUE(*a <= *b); - EXPECT_FALSE(*a >= *b); + Packet a(gen.NextPacket(10)); // SN = 0, TS = 0. + Packet b(gen.NextPacket(10)); // SN = 1, TS = 10. + EXPECT_FALSE(a == b); + EXPECT_TRUE(a != b); + EXPECT_TRUE(a < b); + EXPECT_FALSE(a > b); + EXPECT_TRUE(a <= b); + EXPECT_FALSE(a >= b); // Testing wrap-around case; 'a' is earlier but has a larger timestamp value. - a->timestamp = 0xFFFFFFFF - 10; - EXPECT_FALSE(*a == *b); - EXPECT_TRUE(*a != *b); - EXPECT_TRUE(*a < *b); - EXPECT_FALSE(*a > *b); - EXPECT_TRUE(*a <= *b); - EXPECT_FALSE(*a >= *b); + a.timestamp = 0xFFFFFFFF - 10; + EXPECT_FALSE(a == b); + EXPECT_TRUE(a != b); + EXPECT_TRUE(a < b); + EXPECT_FALSE(a > b); + EXPECT_TRUE(a <= b); + EXPECT_FALSE(a >= b); // Test equal packets. - EXPECT_TRUE(*a == *a); - EXPECT_FALSE(*a != *a); - EXPECT_FALSE(*a < *a); - EXPECT_FALSE(*a > *a); - EXPECT_TRUE(*a <= *a); - EXPECT_TRUE(*a >= *a); + EXPECT_TRUE(a == a); + EXPECT_FALSE(a != a); + EXPECT_FALSE(a < a); + EXPECT_FALSE(a > a); + EXPECT_TRUE(a <= a); + EXPECT_TRUE(a >= a); // Test equal timestamps but different sequence numbers (0 and 1). - a->timestamp = b->timestamp; - EXPECT_FALSE(*a == *b); - EXPECT_TRUE(*a != *b); - EXPECT_TRUE(*a < *b); - EXPECT_FALSE(*a > *b); - EXPECT_TRUE(*a <= *b); - EXPECT_FALSE(*a >= *b); + a.timestamp = b.timestamp; + EXPECT_FALSE(a == b); + EXPECT_TRUE(a != b); + EXPECT_TRUE(a < b); + EXPECT_FALSE(a > b); + EXPECT_TRUE(a <= b); + EXPECT_FALSE(a >= b); // Test equal timestamps but different sequence numbers (32767 and 1). - a->sequence_number = 0xFFFF; - EXPECT_FALSE(*a == *b); - EXPECT_TRUE(*a != *b); - EXPECT_TRUE(*a < *b); - EXPECT_FALSE(*a > *b); - EXPECT_TRUE(*a <= *b); - EXPECT_FALSE(*a >= *b); + a.sequence_number = 0xFFFF; + EXPECT_FALSE(a == b); + EXPECT_TRUE(a != b); + EXPECT_TRUE(a < b); + EXPECT_FALSE(a > b); + EXPECT_TRUE(a <= b); + EXPECT_FALSE(a >= b); // Test equal timestamps and sequence numbers, but differing priorities. - a->sequence_number = b->sequence_number; - a->priority = {1, 0}; - b->priority = {0, 0}; + a.sequence_number = b.sequence_number; + a.priority = {1, 0}; + b.priority = {0, 0}; // a after b - EXPECT_FALSE(*a == *b); - EXPECT_TRUE(*a != *b); - EXPECT_FALSE(*a < *b); - EXPECT_TRUE(*a > *b); - EXPECT_FALSE(*a <= *b); - EXPECT_TRUE(*a >= *b); + EXPECT_FALSE(a == b); + EXPECT_TRUE(a != b); + EXPECT_FALSE(a < b); + EXPECT_TRUE(a > b); + EXPECT_FALSE(a <= b); + EXPECT_TRUE(a >= b); - std::unique_ptr c(gen.NextPacket(0)); // SN = 2, TS = 20. - std::unique_ptr d(gen.NextPacket(0)); // SN = 3, TS = 20. - c->timestamp = b->timestamp; - d->timestamp = b->timestamp; - c->sequence_number = b->sequence_number; - d->sequence_number = b->sequence_number; - c->priority = {1, 1}; - d->priority = {0, 1}; + Packet c(gen.NextPacket(0)); // SN = 2, TS = 20. + Packet d(gen.NextPacket(0)); // SN = 3, TS = 20. + c.timestamp = b.timestamp; + d.timestamp = b.timestamp; + c.sequence_number = b.sequence_number; + d.sequence_number = b.sequence_number; + c.priority = {1, 1}; + d.priority = {0, 1}; // c after d - EXPECT_FALSE(*c == *d); - EXPECT_TRUE(*c != *d); - EXPECT_FALSE(*c < *d); - EXPECT_TRUE(*c > *d); - EXPECT_FALSE(*c <= *d); - EXPECT_TRUE(*c >= *d); + EXPECT_FALSE(c == d); + EXPECT_TRUE(c != d); + EXPECT_FALSE(c < d); + EXPECT_TRUE(c > d); + EXPECT_FALSE(c <= d); + EXPECT_TRUE(c >= d); // c after a - EXPECT_FALSE(*c == *a); - EXPECT_TRUE(*c != *a); - EXPECT_FALSE(*c < *a); - EXPECT_TRUE(*c > *a); - EXPECT_FALSE(*c <= *a); - EXPECT_TRUE(*c >= *a); + EXPECT_FALSE(c == a); + EXPECT_TRUE(c != a); + EXPECT_FALSE(c < a); + EXPECT_TRUE(c > a); + EXPECT_FALSE(c <= a); + EXPECT_TRUE(c >= a); // c after b - EXPECT_FALSE(*c == *b); - EXPECT_TRUE(*c != *b); - EXPECT_FALSE(*c < *b); - EXPECT_TRUE(*c > *b); - EXPECT_FALSE(*c <= *b); - EXPECT_TRUE(*c >= *b); + EXPECT_FALSE(c == b); + EXPECT_TRUE(c != b); + EXPECT_FALSE(c < b); + EXPECT_TRUE(c > b); + EXPECT_FALSE(c <= b); + EXPECT_TRUE(c >= b); // a after d - EXPECT_FALSE(*a == *d); - EXPECT_TRUE(*a != *d); - EXPECT_FALSE(*a < *d); - EXPECT_TRUE(*a > *d); - EXPECT_FALSE(*a <= *d); - EXPECT_TRUE(*a >= *d); + EXPECT_FALSE(a == d); + EXPECT_TRUE(a != d); + EXPECT_FALSE(a < d); + EXPECT_TRUE(a > d); + EXPECT_FALSE(a <= d); + EXPECT_TRUE(a >= d); // d after b - EXPECT_FALSE(*d == *b); - EXPECT_TRUE(*d != *b); - EXPECT_FALSE(*d < *b); - EXPECT_TRUE(*d > *b); - EXPECT_FALSE(*d <= *b); - EXPECT_TRUE(*d >= *b); -} - -// Test the DeleteFirstPacket DeleteAllPackets methods. -TEST(PacketBuffer, DeleteAllPackets) { - PacketGenerator gen(0, 0, 0, 10); - PacketList list; - const int payload_len = 10; - - // Insert 10 small packets. - for (int i = 0; i < 10; ++i) { - Packet* packet = gen.NextPacket(payload_len); - list.push_back(packet); - } - EXPECT_TRUE(PacketBuffer::DeleteFirstPacket(&list)); - EXPECT_EQ(9u, list.size()); - PacketBuffer::DeleteAllPackets(&list); - EXPECT_TRUE(list.empty()); - EXPECT_FALSE(PacketBuffer::DeleteFirstPacket(&list)); + EXPECT_FALSE(d == b); + EXPECT_TRUE(d != b); + EXPECT_FALSE(d < b); + EXPECT_TRUE(d > b); + EXPECT_FALSE(d <= b); + EXPECT_TRUE(d >= b); } namespace { diff --git a/webrtc/modules/audio_coding/neteq/red_payload_splitter.cc b/webrtc/modules/audio_coding/neteq/red_payload_splitter.cc index c051aaae98..774832c416 100644 --- a/webrtc/modules/audio_coding/neteq/red_payload_splitter.cc +++ b/webrtc/modules/audio_coding/neteq/red_payload_splitter.cc @@ -34,9 +34,9 @@ bool RedPayloadSplitter::SplitRed(PacketList* packet_list) { bool ret = true; PacketList::iterator it = packet_list->begin(); while (it != packet_list->end()) { - const Packet* red_packet = (*it); - assert(!red_packet->payload.empty()); - const uint8_t* payload_ptr = red_packet->payload.data(); + const Packet& red_packet = *it; + assert(!red_packet.payload.empty()); + const uint8_t* payload_ptr = red_packet.payload.data(); // Read RED headers (according to RFC 2198): // @@ -69,14 +69,14 @@ bool RedPayloadSplitter::SplitRed(PacketList* packet_list) { if (last_block) { // No more header data to read. ++sum_length; // Account for RED header size of 1 byte. - new_header.timestamp = red_packet->timestamp; - new_header.payload_length = red_packet->payload.size() - sum_length; + new_header.timestamp = red_packet.timestamp; + new_header.payload_length = red_packet.payload.size() - sum_length; payload_ptr += 1; // Advance to first payload byte. } else { // Bits 8 through 21 are timestamp offset. int timestamp_offset = (payload_ptr[1] << 6) + ((payload_ptr[2] & 0xFC) >> 2); - new_header.timestamp = red_packet->timestamp - timestamp_offset; + new_header.timestamp = red_packet.timestamp - timestamp_offset; // Bits 22 through 31 are payload length. new_header.payload_length = ((payload_ptr[2] & 0x03) << 8) + payload_ptr[3]; @@ -96,7 +96,7 @@ bool RedPayloadSplitter::SplitRed(PacketList* packet_list) { const auto& new_header = new_headers[i]; size_t payload_length = new_header.payload_length; if (payload_ptr + payload_length > - red_packet->payload.data() + red_packet->payload.size()) { + red_packet.payload.data() + red_packet.payload.size()) { // The block lengths in the RED headers do not match the overall // packet length. Something is corrupt. Discard this and the remaining // payloads from this packet. @@ -105,26 +105,23 @@ bool RedPayloadSplitter::SplitRed(PacketList* packet_list) { break; } - Packet* new_packet = new Packet; - new_packet->timestamp = new_header.timestamp; - new_packet->payload_type = new_header.payload_type; - new_packet->sequence_number = red_packet->sequence_number; - new_packet->priority.red_level = + Packet new_packet; + new_packet.timestamp = new_header.timestamp; + new_packet.payload_type = new_header.payload_type; + new_packet.sequence_number = red_packet.sequence_number; + new_packet.priority.red_level = rtc::checked_cast((new_headers.size() - 1) - i); - new_packet->payload.SetData(payload_ptr, payload_length); - new_packets.push_front(new_packet); + new_packet.payload.SetData(payload_ptr, payload_length); + new_packets.push_front(std::move(new_packet)); payload_ptr += payload_length; } // Insert new packets into original list, before the element pointed to by // iterator |it|. - packet_list->splice(it, new_packets, new_packets.begin(), - new_packets.end()); + packet_list->splice(it, std::move(new_packets)); } else { LOG(LS_WARNING) << "SplitRed too many blocks: " << new_headers.size(); ret = false; } - // Delete old packet payload. - delete (*it); // Remove |it| from the packet list. This operation effectively moves the // iterator |it| to the next packet in the list. Thus, we do not have to // increment it manually. @@ -136,11 +133,10 @@ bool RedPayloadSplitter::SplitRed(PacketList* packet_list) { int RedPayloadSplitter::CheckRedPayloads( PacketList* packet_list, const DecoderDatabase& decoder_database) { - PacketList::iterator it = packet_list->begin(); int main_payload_type = -1; int num_deleted_packets = 0; - while (it != packet_list->end()) { - uint8_t this_payload_type = (*it)->payload_type; + for (auto it = packet_list->begin(); it != packet_list->end(); /* */) { + uint8_t this_payload_type = it->payload_type; if (!decoder_database.IsDtmf(this_payload_type) && !decoder_database.IsComfortNoise(this_payload_type)) { if (main_payload_type == -1) { @@ -149,8 +145,6 @@ int RedPayloadSplitter::CheckRedPayloads( } else { if (this_payload_type != main_payload_type) { // We do not allow redundant payloads of a different type. - // Discard this payload. - delete (*it); // Remove |it| from the packet list. This operation effectively // moves the iterator |it| to the next packet in the list. Thus, we // do not have to increment it manually. diff --git a/webrtc/modules/audio_coding/neteq/red_payload_splitter_unittest.cc b/webrtc/modules/audio_coding/neteq/red_payload_splitter_unittest.cc index ababe1c576..cd85f0329f 100644 --- a/webrtc/modules/audio_coding/neteq/red_payload_splitter_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/red_payload_splitter_unittest.cc @@ -75,18 +75,18 @@ void CreateOpusFecPayload(uint8_t* payload, // by the values in array |payload_types| (which must be of length // |num_payloads|). Each redundant payload is |timestamp_offset| samples // "behind" the the previous payload. -Packet* CreateRedPayload(size_t num_payloads, - uint8_t* payload_types, - int timestamp_offset, - bool embed_opus_fec = false) { - Packet* packet = new Packet; - packet->payload_type = kRedPayloadType; - packet->timestamp = kBaseTimestamp; - packet->sequence_number = kSequenceNumber; - packet->payload.SetSize((kPayloadLength + 1) + - (num_payloads - 1) * - (kPayloadLength + kRedHeaderLength)); - uint8_t* payload_ptr = packet->payload.data(); +Packet CreateRedPayload(size_t num_payloads, + uint8_t* payload_types, + int timestamp_offset, + bool embed_opus_fec = false) { + Packet packet; + packet.payload_type = kRedPayloadType; + packet.timestamp = kBaseTimestamp; + packet.sequence_number = kSequenceNumber; + packet.payload.SetSize((kPayloadLength + 1) + + (num_payloads - 1) * + (kPayloadLength + kRedHeaderLength)); + uint8_t* payload_ptr = packet.payload.data(); for (size_t i = 0; i < num_payloads; ++i) { // Write the RED headers. if (i == num_payloads - 1) { @@ -122,44 +122,44 @@ Packet* CreateRedPayload(size_t num_payloads, } // Create a packet with all payload bytes set to |payload_value|. -Packet* CreatePacket(uint8_t payload_type, - size_t payload_length, - uint8_t payload_value, - bool opus_fec = false) { - Packet* packet = new Packet; - packet->payload_type = payload_type; - packet->timestamp = kBaseTimestamp; - packet->sequence_number = kSequenceNumber; - packet->payload.SetSize(payload_length); +Packet CreatePacket(uint8_t payload_type, + size_t payload_length, + uint8_t payload_value, + bool opus_fec = false) { + Packet packet; + packet.payload_type = payload_type; + packet.timestamp = kBaseTimestamp; + packet.sequence_number = kSequenceNumber; + packet.payload.SetSize(payload_length); if (opus_fec) { - CreateOpusFecPayload(packet->payload.data(), packet->payload.size(), + CreateOpusFecPayload(packet.payload.data(), packet.payload.size(), payload_value); } else { - memset(packet->payload.data(), payload_value, packet->payload.size()); + memset(packet.payload.data(), payload_value, packet.payload.size()); } return packet; } // Checks that |packet| has the attributes given in the remaining parameters. -void VerifyPacket(const Packet* packet, +void VerifyPacket(const Packet& packet, size_t payload_length, uint8_t payload_type, uint16_t sequence_number, uint32_t timestamp, uint8_t payload_value, Packet::Priority priority) { - EXPECT_EQ(payload_length, packet->payload.size()); - EXPECT_EQ(payload_type, packet->payload_type); - EXPECT_EQ(sequence_number, packet->sequence_number); - EXPECT_EQ(timestamp, packet->timestamp); - EXPECT_EQ(priority, packet->priority); - ASSERT_FALSE(packet->payload.empty()); - for (size_t i = 0; i < packet->payload.size(); ++i) { - ASSERT_EQ(payload_value, packet->payload.data()[i]); + EXPECT_EQ(payload_length, packet.payload.size()); + EXPECT_EQ(payload_type, packet.payload_type); + EXPECT_EQ(sequence_number, packet.sequence_number); + EXPECT_EQ(timestamp, packet.timestamp); + EXPECT_EQ(priority, packet.priority); + ASSERT_FALSE(packet.payload.empty()); + for (size_t i = 0; i < packet.payload.size(); ++i) { + ASSERT_EQ(payload_value, packet.payload.data()[i]); } } -void VerifyPacket(const Packet* packet, +void VerifyPacket(const Packet& packet, size_t payload_length, uint8_t payload_type, uint16_t sequence_number, @@ -182,23 +182,18 @@ TEST(RedPayloadSplitter, CreateAndDestroy) { TEST(RedPayloadSplitter, OnePacketTwoPayloads) { uint8_t payload_types[] = {0, 0}; const int kTimestampOffset = 160; - Packet* packet = CreateRedPayload(2, payload_types, kTimestampOffset); PacketList packet_list; - packet_list.push_back(packet); + packet_list.push_back(CreateRedPayload(2, payload_types, kTimestampOffset)); RedPayloadSplitter splitter; EXPECT_TRUE(splitter.SplitRed(&packet_list)); ASSERT_EQ(2u, packet_list.size()); // Check first packet. The first in list should always be the primary payload. - packet = packet_list.front(); - VerifyPacket(packet, kPayloadLength, payload_types[1], kSequenceNumber, - kBaseTimestamp, 1, true); - delete packet; + VerifyPacket(packet_list.front(), kPayloadLength, payload_types[1], + kSequenceNumber, kBaseTimestamp, 1, true); packet_list.pop_front(); // Check second packet. - packet = packet_list.front(); - VerifyPacket(packet, kPayloadLength, payload_types[0], kSequenceNumber, - kBaseTimestamp - kTimestampOffset, 0, false); - delete packet; + VerifyPacket(packet_list.front(), kPayloadLength, payload_types[0], + kSequenceNumber, kBaseTimestamp - kTimestampOffset, 0, false); } // Packets A and B are not split at all. Only the RED header in each packet is @@ -207,29 +202,26 @@ TEST(RedPayloadSplitter, TwoPacketsOnePayload) { uint8_t payload_types[] = {0}; const int kTimestampOffset = 160; // Create first packet, with a single RED payload. - Packet* packet = CreateRedPayload(1, payload_types, kTimestampOffset); PacketList packet_list; - packet_list.push_back(packet); + packet_list.push_back(CreateRedPayload(1, payload_types, kTimestampOffset)); // Create second packet, with a single RED payload. - packet = CreateRedPayload(1, payload_types, kTimestampOffset); - // Manually change timestamp and sequence number of second packet. - packet->timestamp += kTimestampOffset; - packet->sequence_number++; - packet_list.push_back(packet); + { + Packet packet = CreateRedPayload(1, payload_types, kTimestampOffset); + // Manually change timestamp and sequence number of second packet. + packet.timestamp += kTimestampOffset; + packet.sequence_number++; + packet_list.push_back(std::move(packet)); + } RedPayloadSplitter splitter; EXPECT_TRUE(splitter.SplitRed(&packet_list)); ASSERT_EQ(2u, packet_list.size()); // Check first packet. - packet = packet_list.front(); - VerifyPacket(packet, kPayloadLength, payload_types[0], kSequenceNumber, - kBaseTimestamp, 0, true); - delete packet; + VerifyPacket(packet_list.front(), kPayloadLength, payload_types[0], + kSequenceNumber, kBaseTimestamp, 0, true); packet_list.pop_front(); // Check second packet. - packet = packet_list.front(); - VerifyPacket(packet, kPayloadLength, payload_types[0], kSequenceNumber + 1, - kBaseTimestamp + kTimestampOffset, 0, true); - delete packet; + VerifyPacket(packet_list.front(), kPayloadLength, payload_types[0], + kSequenceNumber + 1, kBaseTimestamp + kTimestampOffset, 0, true); } // Packets A and B are split into packets A1, A2, A3, B1, B2, B3, with @@ -245,53 +237,45 @@ TEST(RedPayloadSplitter, TwoPacketsThreePayloads) { uint8_t payload_types[] = {2, 1, 0}; // Primary is the last one. const int kTimestampOffset = 160; // Create first packet, with 3 RED payloads. - Packet* packet = CreateRedPayload(3, payload_types, kTimestampOffset); PacketList packet_list; - packet_list.push_back(packet); + packet_list.push_back(CreateRedPayload(3, payload_types, kTimestampOffset)); // Create first packet, with 3 RED payloads. - packet = CreateRedPayload(3, payload_types, kTimestampOffset); - // Manually change timestamp and sequence number of second packet. - packet->timestamp += kTimestampOffset; - packet->sequence_number++; - packet_list.push_back(packet); + { + Packet packet = CreateRedPayload(3, payload_types, kTimestampOffset); + // Manually change timestamp and sequence number of second packet. + packet.timestamp += kTimestampOffset; + packet.sequence_number++; + packet_list.push_back(std::move(packet)); + } RedPayloadSplitter splitter; EXPECT_TRUE(splitter.SplitRed(&packet_list)); ASSERT_EQ(6u, packet_list.size()); // Check first packet, A1. - packet = packet_list.front(); - VerifyPacket(packet, kPayloadLength, payload_types[2], kSequenceNumber, - kBaseTimestamp, 2, {0, 0}); - delete packet; + VerifyPacket(packet_list.front(), kPayloadLength, payload_types[2], + kSequenceNumber, kBaseTimestamp, 2, {0, 0}); packet_list.pop_front(); // Check second packet, A2. - packet = packet_list.front(); - VerifyPacket(packet, kPayloadLength, payload_types[1], kSequenceNumber, - kBaseTimestamp - kTimestampOffset, 1, {0, 1}); - delete packet; + VerifyPacket(packet_list.front(), kPayloadLength, payload_types[1], + kSequenceNumber, kBaseTimestamp - kTimestampOffset, 1, {0, 1}); packet_list.pop_front(); // Check third packet, A3. - packet = packet_list.front(); - VerifyPacket(packet, kPayloadLength, payload_types[0], kSequenceNumber, - kBaseTimestamp - 2 * kTimestampOffset, 0, {0, 2}); - delete packet; + VerifyPacket(packet_list.front(), kPayloadLength, payload_types[0], + kSequenceNumber, kBaseTimestamp - 2 * kTimestampOffset, 0, + {0, 2}); packet_list.pop_front(); // Check fourth packet, B1. - packet = packet_list.front(); - VerifyPacket(packet, kPayloadLength, payload_types[2], kSequenceNumber + 1, - kBaseTimestamp + kTimestampOffset, 2, {0, 0}); - delete packet; + VerifyPacket(packet_list.front(), kPayloadLength, payload_types[2], + kSequenceNumber + 1, kBaseTimestamp + kTimestampOffset, 2, + {0, 0}); packet_list.pop_front(); // Check fifth packet, B2. - packet = packet_list.front(); - VerifyPacket(packet, kPayloadLength, payload_types[1], kSequenceNumber + 1, - kBaseTimestamp, 1, {0, 1}); - delete packet; + VerifyPacket(packet_list.front(), kPayloadLength, payload_types[1], + kSequenceNumber + 1, kBaseTimestamp, 1, {0, 1}); packet_list.pop_front(); // Check sixth packet, B3. - packet = packet_list.front(); - VerifyPacket(packet, kPayloadLength, payload_types[0], kSequenceNumber + 1, - kBaseTimestamp - kTimestampOffset, 0, {0, 2}); - delete packet; + VerifyPacket(packet_list.front(), kPayloadLength, payload_types[0], + kSequenceNumber + 1, kBaseTimestamp - kTimestampOffset, 0, + {0, 2}); } // Creates a list with 4 packets with these payload types: @@ -306,8 +290,7 @@ TEST(RedPayloadSplitter, CheckRedPayloads) { PacketList packet_list; for (uint8_t i = 0; i <= 3; ++i) { // Create packet with payload type |i|, payload length 10 bytes, all 0. - Packet* packet = CreatePacket(i, 10, 0); - packet_list.push_back(packet); + packet_list.push_back(CreatePacket(i, 10, 0)); } // Use a real DecoderDatabase object here instead of a mock, since it is @@ -327,9 +310,8 @@ TEST(RedPayloadSplitter, CheckRedPayloads) { // Verify packets. The loop verifies that payload types 0, 1, and 2 are in the // list. for (int i = 0; i <= 2; ++i) { - Packet* packet = packet_list.front(); - VerifyPacket(packet, 10, i, kSequenceNumber, kBaseTimestamp, 0, true); - delete packet; + VerifyPacket(packet_list.front(), 10, i, kSequenceNumber, kBaseTimestamp, 0, + true); packet_list.pop_front(); } EXPECT_TRUE(packet_list.empty()); @@ -340,21 +322,22 @@ TEST(RedPayloadSplitter, CheckRedPayloads) { TEST(RedPayloadSplitter, WrongPayloadLength) { uint8_t payload_types[] = {0, 0, 0}; const int kTimestampOffset = 160; - Packet* packet = CreateRedPayload(3, payload_types, kTimestampOffset); - // Manually tamper with the payload length of the packet. - // This is one byte too short for the second payload (out of three). - // We expect only the first payload to be returned. - packet->payload.SetSize(packet->payload.size() - (kPayloadLength + 1)); PacketList packet_list; - packet_list.push_back(packet); + { + Packet packet = CreateRedPayload(3, payload_types, kTimestampOffset); + // Manually tamper with the payload length of the packet. + // This is one byte too short for the second payload (out of three). + // We expect only the first payload to be returned. + packet.payload.SetSize(packet.payload.size() - (kPayloadLength + 1)); + packet_list.push_back(std::move(packet)); + } RedPayloadSplitter splitter; EXPECT_FALSE(splitter.SplitRed(&packet_list)); ASSERT_EQ(1u, packet_list.size()); // Check first packet. - packet = packet_list.front(); - VerifyPacket(packet, kPayloadLength, payload_types[0], kSequenceNumber, - kBaseTimestamp - 2 * kTimestampOffset, 0, {0, 2}); - delete packet; + VerifyPacket(packet_list.front(), kPayloadLength, payload_types[0], + kSequenceNumber, kBaseTimestamp - 2 * kTimestampOffset, 0, + {0, 2}); packet_list.pop_front(); } diff --git a/webrtc/modules/audio_coding/neteq/timestamp_scaler.cc b/webrtc/modules/audio_coding/neteq/timestamp_scaler.cc index cdc30a79e8..7a1b91754a 100644 --- a/webrtc/modules/audio_coding/neteq/timestamp_scaler.cc +++ b/webrtc/modules/audio_coding/neteq/timestamp_scaler.cc @@ -29,7 +29,7 @@ void TimestampScaler::ToInternal(Packet* packet) { void TimestampScaler::ToInternal(PacketList* packet_list) { PacketList::iterator it; for (it = packet_list->begin(); it != packet_list->end(); ++it) { - ToInternal(*it); + ToInternal(&(*it)); } } diff --git a/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc b/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc index 7345e17d3e..41c33426ed 100644 --- a/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc +++ b/webrtc/modules/audio_coding/neteq/timestamp_scaler_unittest.cc @@ -209,19 +209,22 @@ TEST(TimestampScaler, TestG722PacketList) { // Test both sides of the timestamp wrap-around. uint32_t external_timestamp = 0xFFFFFFFF - 5; uint32_t internal_timestamp = external_timestamp; - Packet packet1; - packet1.payload_type = kRtpPayloadType; - packet1.timestamp = external_timestamp; - Packet packet2; - packet2.payload_type = kRtpPayloadType; - packet2.timestamp = external_timestamp + 10; PacketList packet_list; - packet_list.push_back(&packet1); - packet_list.push_back(&packet2); + { + Packet packet1; + packet1.payload_type = kRtpPayloadType; + packet1.timestamp = external_timestamp; + Packet packet2; + packet2.payload_type = kRtpPayloadType; + packet2.timestamp = external_timestamp + 10; + packet_list.push_back(std::move(packet1)); + packet_list.push_back(std::move(packet2)); + } scaler.ToInternal(&packet_list); - EXPECT_EQ(internal_timestamp, packet1.timestamp); - EXPECT_EQ(internal_timestamp + 20, packet2.timestamp); + EXPECT_EQ(internal_timestamp, packet_list.front().timestamp); + packet_list.pop_front(); + EXPECT_EQ(internal_timestamp + 20, packet_list.front().timestamp); EXPECT_CALL(db, Die()); // Called when database object is deleted. }