diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc b/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc index 29cde65b12..9a1d218cda 100644 --- a/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc @@ -11,6 +11,7 @@ #include "webrtc/modules/rtp_rtcp/source/fec_receiver_impl.h" #include +#include #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" @@ -29,10 +30,7 @@ FecReceiverImpl::FecReceiverImpl(RtpData* callback) fec_(new ForwardErrorCorrection()) {} FecReceiverImpl::~FecReceiverImpl() { - while (!received_packet_list_.empty()) { - delete received_packet_list_.front(); - received_packet_list_.pop_front(); - } + received_packet_list_.clear(); if (fec_ != NULL) { fec_->ResetState(&recovered_packet_list_); delete fec_; @@ -209,9 +207,9 @@ int32_t FecReceiverImpl::AddReceivedRedPacket( return 0; } - received_packet_list_.push_back(received_packet.release()); + received_packet_list_.push_back(std::move(received_packet)); if (second_received_packet) { - received_packet_list_.push_back(second_received_packet.release()); + received_packet_list_.push_back(std::move(second_received_packet)); } return 0; } @@ -237,7 +235,7 @@ int32_t FecReceiverImpl::ProcessReceivedFec() { RTC_DCHECK(received_packet_list_.empty()); } // Send any recovered media packets to VCM. - for(auto* recovered_packet : recovered_packet_list_) { + for (const auto& recovered_packet : recovered_packet_list_) { if (recovered_packet->returned) { // Already sent to the VCM and the jitter buffer. continue; diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc index 0c66dc1f52..32d4a73590 100644 --- a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc @@ -39,7 +39,7 @@ class ReceiverFecTest : public ::testing::Test { } void GenerateFec(ForwardErrorCorrection::PacketList* media_packets, - ForwardErrorCorrection::PacketList* fec_packets, + std::list* fec_packets, unsigned int num_fec_packets) { uint8_t protection_factor = num_fec_packets * 255 / media_packets->size(); EXPECT_EQ(0, fec_->GenerateFec(*media_packets, protection_factor, @@ -53,9 +53,10 @@ class ReceiverFecTest : public ::testing::Test { ForwardErrorCorrection::PacketList* media_packets) { generator_->NewFrame(num_media_packets); for (int i = 0; i < num_media_packets; ++i) { - media_rtp_packets->push_back( - generator_->NextPacket(frame_offset + i, kRtpHeaderSize + 10)); - media_packets->push_back(media_rtp_packets->back()); + std::unique_ptr next_packet( + generator_->NextPacket(frame_offset + i, kRtpHeaderSize + 10)); + media_rtp_packets->push_back(next_packet.get()); + media_packets->push_back(std::move(next_packet)); } } @@ -97,19 +98,12 @@ class ReceiverFecTest : public ::testing::Test { std::unique_ptr generator_; }; -void DeletePackets(ForwardErrorCorrection::PacketList* packets) { - while (!packets->empty()) { - delete packets->front(); - packets->pop_front(); - } -} - TEST_F(ReceiverFecTest, TwoMediaOneFec) { const unsigned int kNumFecPackets = 1u; std::list media_rtp_packets; ForwardErrorCorrection::PacketList media_packets; GenerateFrame(2, 0, &media_rtp_packets, &media_packets); - ForwardErrorCorrection::PacketList fec_packets; + std::list fec_packets; GenerateFec(&media_packets, &fec_packets, kNumFecPackets); // Recovery @@ -128,8 +122,6 @@ TEST_F(ReceiverFecTest, TwoMediaOneFec) { EXPECT_EQ(2U, counter.num_packets); EXPECT_EQ(1U, counter.num_fec_packets); EXPECT_EQ(1U, counter.num_recovered_packets); - - DeletePackets(&media_packets); } void ReceiverFecTest::InjectGarbagePacketLength(size_t fec_garbage_offset) { @@ -140,7 +132,7 @@ void ReceiverFecTest::InjectGarbagePacketLength(size_t fec_garbage_offset) { std::list media_rtp_packets; ForwardErrorCorrection::PacketList media_packets; GenerateFrame(2, 0, &media_rtp_packets, &media_packets); - ForwardErrorCorrection::PacketList fec_packets; + std::list fec_packets; GenerateFec(&media_packets, &fec_packets, kNumFecPackets); ByteWriter::WriteBigEndian( &fec_packets.front()->data[fec_garbage_offset], 0x4711); @@ -155,8 +147,6 @@ void ReceiverFecTest::InjectGarbagePacketLength(size_t fec_garbage_offset) { EXPECT_EQ(2u, counter.num_packets); EXPECT_EQ(1u, counter.num_fec_packets); EXPECT_EQ(0u, counter.num_recovered_packets); - - DeletePackets(&media_packets); } TEST_F(ReceiverFecTest, InjectGarbageFecHeaderLengthRecovery) { @@ -175,7 +165,7 @@ TEST_F(ReceiverFecTest, TwoMediaTwoFec) { std::list media_rtp_packets; ForwardErrorCorrection::PacketList media_packets; GenerateFrame(2, 0, &media_rtp_packets, &media_packets); - ForwardErrorCorrection::PacketList fec_packets; + std::list fec_packets; GenerateFec(&media_packets, &fec_packets, kNumFecPackets); // Recovery @@ -190,8 +180,6 @@ TEST_F(ReceiverFecTest, TwoMediaTwoFec) { ++it; VerifyReconstructedMediaPacket(*it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); - - DeletePackets(&media_packets); } TEST_F(ReceiverFecTest, TwoFramesOneFec) { @@ -200,7 +188,7 @@ TEST_F(ReceiverFecTest, TwoFramesOneFec) { ForwardErrorCorrection::PacketList media_packets; GenerateFrame(1, 0, &media_rtp_packets, &media_packets); GenerateFrame(1, 1, &media_rtp_packets, &media_packets); - ForwardErrorCorrection::PacketList fec_packets; + std::list fec_packets; GenerateFec(&media_packets, &fec_packets, kNumFecPackets); // Recovery @@ -213,8 +201,6 @@ TEST_F(ReceiverFecTest, TwoFramesOneFec) { ++it; VerifyReconstructedMediaPacket(*it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); - - DeletePackets(&media_packets); } TEST_F(ReceiverFecTest, OneCompleteOneUnrecoverableFrame) { @@ -224,7 +210,7 @@ TEST_F(ReceiverFecTest, OneCompleteOneUnrecoverableFrame) { GenerateFrame(1, 0, &media_rtp_packets, &media_packets); GenerateFrame(2, 1, &media_rtp_packets, &media_packets); - ForwardErrorCorrection::PacketList fec_packets; + std::list fec_packets; GenerateFec(&media_packets, &fec_packets, kNumFecPackets); // Recovery @@ -236,8 +222,6 @@ TEST_F(ReceiverFecTest, OneCompleteOneUnrecoverableFrame) { BuildAndAddRedMediaPacket(*it); // First packet of second frame. VerifyReconstructedMediaPacket(*it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); - - DeletePackets(&media_packets); } TEST_F(ReceiverFecTest, MaxFramesOneFec) { @@ -248,7 +232,7 @@ TEST_F(ReceiverFecTest, MaxFramesOneFec) { for (unsigned int i = 0; i < kNumMediaPackets; ++i) { GenerateFrame(1, i, &media_rtp_packets, &media_packets); } - ForwardErrorCorrection::PacketList fec_packets; + std::list fec_packets; GenerateFec(&media_packets, &fec_packets, kNumFecPackets); // Recovery @@ -263,8 +247,6 @@ TEST_F(ReceiverFecTest, MaxFramesOneFec) { it = media_rtp_packets.begin(); VerifyReconstructedMediaPacket(*it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); - - DeletePackets(&media_packets); } TEST_F(ReceiverFecTest, TooManyFrames) { @@ -275,12 +257,10 @@ TEST_F(ReceiverFecTest, TooManyFrames) { for (unsigned int i = 0; i < kNumMediaPackets; ++i) { GenerateFrame(1, i, &media_rtp_packets, &media_packets); } - ForwardErrorCorrection::PacketList fec_packets; + std::list fec_packets; EXPECT_EQ(-1, fec_->GenerateFec(media_packets, kNumFecPackets * 255 / kNumMediaPackets, 0, false, kFecMaskBursty, &fec_packets)); - - DeletePackets(&media_packets); } TEST_F(ReceiverFecTest, PacketNotDroppedTooEarly) { @@ -293,7 +273,7 @@ TEST_F(ReceiverFecTest, PacketNotDroppedTooEarly) { ForwardErrorCorrection::PacketList media_packets_batch1; GenerateFrame(kNumMediaPacketsBatch1, 0, &media_rtp_packets_batch1, &media_packets_batch1); - ForwardErrorCorrection::PacketList fec_packets; + std::list fec_packets; GenerateFec(&media_packets_batch1, &fec_packets, kNumFecPacketsBatch1); BuildAndAddRedMediaPacket(media_rtp_packets_batch1.front()); @@ -322,9 +302,6 @@ TEST_F(ReceiverFecTest, PacketNotDroppedTooEarly) { EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)) .Times(1).WillRepeatedly(Return(true)); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); - - DeletePackets(&media_packets_batch1); - DeletePackets(&media_packets_batch2); } TEST_F(ReceiverFecTest, PacketDroppedWhenTooOld) { @@ -337,7 +314,7 @@ TEST_F(ReceiverFecTest, PacketDroppedWhenTooOld) { ForwardErrorCorrection::PacketList media_packets_batch1; GenerateFrame(kNumMediaPacketsBatch1, 0, &media_rtp_packets_batch1, &media_packets_batch1); - ForwardErrorCorrection::PacketList fec_packets; + std::list fec_packets; GenerateFec(&media_packets_batch1, &fec_packets, kNumFecPacketsBatch1); BuildAndAddRedMediaPacket(media_rtp_packets_batch1.front()); @@ -367,9 +344,6 @@ TEST_F(ReceiverFecTest, PacketDroppedWhenTooOld) { EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)) .Times(0); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); - - DeletePackets(&media_packets_batch1); - DeletePackets(&media_packets_batch2); } TEST_F(ReceiverFecTest, OldFecPacketDropped) { @@ -381,7 +355,7 @@ TEST_F(ReceiverFecTest, OldFecPacketDropped) { for (unsigned int i = 0; i < kNumMediaPackets / 2; ++i) { std::list frame_media_rtp_packets; ForwardErrorCorrection::PacketList frame_media_packets; - ForwardErrorCorrection::PacketList fec_packets; + std::list fec_packets; GenerateFrame(2, 0, &frame_media_rtp_packets, &frame_media_packets); GenerateFec(&frame_media_packets, &fec_packets, 1); for (auto it = fec_packets.begin(); it != fec_packets.end(); ++it) { @@ -391,8 +365,10 @@ TEST_F(ReceiverFecTest, OldFecPacketDropped) { .Times(0); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); } - media_packets.insert(media_packets.end(), frame_media_packets.begin(), - frame_media_packets.end()); + // Move unique_ptr's to media_packets for lifetime management. + media_packets.insert(media_packets.end(), + std::make_move_iterator(frame_media_packets.begin()), + std::make_move_iterator(frame_media_packets.end())); media_rtp_packets.insert(media_rtp_packets.end(), frame_media_rtp_packets.begin(), frame_media_rtp_packets.end()); @@ -404,8 +380,6 @@ TEST_F(ReceiverFecTest, OldFecPacketDropped) { EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)) .Times(1).WillRepeatedly(Return(true)); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); - - DeletePackets(&media_packets); } void ReceiverFecTest::SurvivesMaliciousPacket(const uint8_t* data, diff --git a/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc b/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc index ecaa86e5d2..f8a2e6f7f7 100644 --- a/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc +++ b/webrtc/modules/rtp_rtcp/source/forward_error_correction.cc @@ -14,7 +14,7 @@ #include #include -#include +#include #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" @@ -36,6 +36,9 @@ constexpr size_t kUlpHeaderSizeLBitClear = (2 + kMaskSizeLBitClear); // Transport header size in bytes. Assume UDP/IPv4 as a reasonable minimum. constexpr size_t kTransportOverhead = 28; +// Maximum number of media packets that can be protected. +constexpr size_t ForwardErrorCorrection::kMaxMediaPackets; + // Maximum number of FEC packets stored internally. constexpr size_t kMaxFecPackets = ForwardErrorCorrection::kMaxMediaPackets; @@ -51,30 +54,14 @@ int32_t ForwardErrorCorrection::Packet::Release() { return ref_count; } -// Used to link media packets to their protecting FEC packets. -// -// TODO(holmer): Refactor into a proper class. -class ProtectedPacket : public ForwardErrorCorrection::SortablePacket { - public: - rtc::scoped_refptr pkt; -}; - -typedef std::list ProtectedPacketList; - -// -// Used for internal storage of FEC packets in a list. -// -// TODO(holmer): Refactor into a proper class. -class FecPacket : public ForwardErrorCorrection::SortablePacket { - public: - ProtectedPacketList protected_pkt_list; - uint32_t ssrc; // SSRC of the current frame. - rtc::scoped_refptr pkt; -}; - -bool ForwardErrorCorrection::SortablePacket::LessThan( - const SortablePacket* first, - const SortablePacket* second) { +// This comparator is used to compare std::unique_ptr's pointing to +// subclasses of SortablePackets. It needs to be parametric since +// the std::unique_ptr's are not covariant w.r.t. the types that +// they are pointing to. +template +bool ForwardErrorCorrection::SortablePacket::LessThan::operator() ( + const S& first, + const T& second) { return IsNewerSequenceNumber(second->seq_num, first->seq_num); } @@ -85,7 +72,7 @@ ForwardErrorCorrection::RecoveredPacket::RecoveredPacket() {} ForwardErrorCorrection::RecoveredPacket::~RecoveredPacket() {} ForwardErrorCorrection::ForwardErrorCorrection() - : generated_fec_packets_(kMaxMediaPackets), fec_packet_list_(), + : generated_fec_packets_(kMaxMediaPackets), received_fec_packets_(), packet_mask_(), tmp_packet_mask_() {} ForwardErrorCorrection::~ForwardErrorCorrection() {} @@ -109,22 +96,23 @@ ForwardErrorCorrection::~ForwardErrorCorrection() {} // // Note that any potential RED headers are added/removed before calling // GenerateFec() or DecodeFec(). -int ForwardErrorCorrection::GenerateFec(const PacketList& media_packet_list, +int ForwardErrorCorrection::GenerateFec(const PacketList& media_packets, uint8_t protection_factor, int num_important_packets, bool use_unequal_protection, FecMaskType fec_mask_type, - PacketList* fec_packet_list) { - const uint16_t num_media_packets = media_packet_list.size(); + std::list* fec_packets) { + const uint16_t num_media_packets = media_packets.size(); // Sanity check arguments. RTC_DCHECK_GT(num_media_packets, 0); RTC_DCHECK_GE(num_important_packets, 0); RTC_DCHECK_LE(num_important_packets, num_media_packets); - RTC_DCHECK(fec_packet_list->empty()); + RTC_DCHECK(fec_packets->empty()); if (num_media_packets > kMaxMediaPackets) { LOG(LS_WARNING) << "Can't protect " << num_media_packets - << " media packets per frame. Max is " << kMaxMediaPackets; + << " media packets per frame. Max is " << kMaxMediaPackets + << "."; return -1; } @@ -132,7 +120,7 @@ int ForwardErrorCorrection::GenerateFec(const PacketList& media_packet_list, int num_mask_bytes = l_bit ? kMaskSizeLBitSet : kMaskSizeLBitClear; // Do some error checking on the media packets. - for (Packet* media_packet : media_packet_list) { + for (const auto& media_packet : media_packets) { RTC_DCHECK(media_packet); if (media_packet->length < kRtpHeaderSize) { @@ -145,7 +133,8 @@ int ForwardErrorCorrection::GenerateFec(const PacketList& media_packet_list, if (media_packet->length + PacketOverhead() + kTransportOverhead > IP_PACKET_SIZE) { LOG(LS_WARNING) << "Media packet " << media_packet->length << " bytes " - << "with overhead is larger than " << IP_PACKET_SIZE; + << "with overhead is larger than " << IP_PACKET_SIZE + << " bytes."; } } @@ -155,12 +144,12 @@ int ForwardErrorCorrection::GenerateFec(const PacketList& media_packet_list, return 0; } - // Prepare FEC packets by setting them to 0. + // Prepare generated FEC packets by setting them to 0. for (int i = 0; i < num_fec_packets; ++i) { memset(generated_fec_packets_[i].data, 0, IP_PACKET_SIZE); - generated_fec_packets_[i].length = 0; // Use this as a marker for untouched - // packets. - fec_packet_list->push_back(&generated_fec_packets_[i]); + // Use this as a marker for untouched packets. + generated_fec_packets_[i].length = 0; + fec_packets->push_back(&generated_fec_packets_[i]); } const internal::PacketMaskTable mask_table(fec_mask_type, num_media_packets); @@ -172,7 +161,7 @@ int ForwardErrorCorrection::GenerateFec(const PacketList& media_packet_list, mask_table, packet_mask_); int num_mask_bits = InsertZerosInBitMasks( - media_packet_list, packet_mask_, num_mask_bytes, num_fec_packets); + media_packets, packet_mask_, num_mask_bytes, num_fec_packets); if (num_mask_bits < 0) { return -1; @@ -182,10 +171,8 @@ int ForwardErrorCorrection::GenerateFec(const PacketList& media_packet_list, num_mask_bytes = kMaskSizeLBitSet; } - GenerateFecBitStrings(media_packet_list, packet_mask_, - num_fec_packets, l_bit); - GenerateFecUlpHeaders(media_packet_list, packet_mask_, - num_fec_packets, l_bit); + GenerateFecBitStrings(media_packets, packet_mask_, num_fec_packets, l_bit); + GenerateFecUlpHeaders(media_packets, packet_mask_, num_fec_packets, l_bit); return 0; } @@ -203,11 +190,11 @@ int ForwardErrorCorrection::GetNumberOfFecPackets(int num_media_packets, } void ForwardErrorCorrection::GenerateFecBitStrings( - const PacketList& media_packet_list, + const PacketList& media_packets, uint8_t* packet_mask, int num_fec_packets, bool l_bit) { - RTC_DCHECK(!media_packet_list.empty()); + RTC_DCHECK(!media_packets.empty()); uint8_t media_payload_length[2]; const int num_mask_bytes = l_bit ? kMaskSizeLBitSet : kMaskSizeLBitClear; const uint16_t ulp_header_size = @@ -217,16 +204,16 @@ void ForwardErrorCorrection::GenerateFecBitStrings( for (int i = 0; i < num_fec_packets; ++i) { Packet* const fec_packet = &generated_fec_packets_[i]; - auto media_list_it = media_packet_list.cbegin(); + auto media_packets_it = media_packets.cbegin(); uint32_t pkt_mask_idx = i * num_mask_bytes; uint32_t media_pkt_idx = 0; uint16_t fec_packet_length = 0; - uint16_t prev_seq_num = ParseSequenceNumber((*media_list_it)->data); - while (media_list_it != media_packet_list.end()) { + uint16_t prev_seq_num = ParseSequenceNumber((*media_packets_it)->data); + while (media_packets_it != media_packets.end()) { // Each FEC packet has a multiple byte mask. Determine if this media // packet should be included in FEC packet i. if (packet_mask[pkt_mask_idx] & (1 << (7 - media_pkt_idx))) { - Packet* media_packet = *media_list_it; + Packet* media_packet = media_packets_it->get(); // Assign network-ordered media payload length. ByteWriter::WriteBigEndian( @@ -271,9 +258,9 @@ void ForwardErrorCorrection::GenerateFecBitStrings( fec_packet->length = fec_packet_length; } } - media_list_it++; - if (media_list_it != media_packet_list.end()) { - uint16_t seq_num = ParseSequenceNumber((*media_list_it)->data); + media_packets_it++; + if (media_packets_it != media_packets.end()) { + uint16_t seq_num = ParseSequenceNumber((*media_packets_it)->data); media_pkt_idx += static_cast(seq_num - prev_seq_num); prev_seq_num = seq_num; } @@ -308,14 +295,15 @@ int ForwardErrorCorrection::InsertZerosInBitMasks( return -1; // Allocate the new mask. int new_mask_bytes = kMaskSizeLBitClear; - if (media_packets.size() + total_missing_seq_nums > 8 * kMaskSizeLBitClear) { + if (media_packets.size() + + total_missing_seq_nums > 8 * kMaskSizeLBitClear) { new_mask_bytes = kMaskSizeLBitSet; } memset(tmp_packet_mask_, 0, num_fec_packets * kMaskSizeLBitSet); - auto it = media_packets.cbegin(); + auto media_packets_it = media_packets.cbegin(); uint16_t prev_seq_num = first_seq_num; - ++it; + ++media_packets_it; // Insert the first column. CopyColumn(tmp_packet_mask_, new_mask_bytes, packet_mask, num_mask_bytes, @@ -323,12 +311,12 @@ int ForwardErrorCorrection::InsertZerosInBitMasks( int new_bit_index = 1; int old_bit_index = 1; // Insert zeros in the bit mask for every hole in the sequence. - for (; it != media_packets.end(); ++it) { + while (media_packets_it != media_packets.end()) { if (new_bit_index == 8 * kMaskSizeLBitSet) { // We can only cover up to 48 packets. break; } - uint16_t seq_num = ParseSequenceNumber((*it)->data); + uint16_t seq_num = ParseSequenceNumber((*media_packets_it)->data); const int zeros_to_insert = static_cast(seq_num - prev_seq_num - 1); if (zeros_to_insert > 0) { @@ -341,6 +329,7 @@ int ForwardErrorCorrection::InsertZerosInBitMasks( ++new_bit_index; ++old_bit_index; prev_seq_num = seq_num; + ++media_packets_it; } if (new_bit_index % 8 != 0) { // We didn't fill the last byte. Shift bits to correct position. @@ -387,7 +376,7 @@ void ForwardErrorCorrection::CopyColumn(uint8_t* new_mask, } void ForwardErrorCorrection::GenerateFecUlpHeaders( - const PacketList& media_packet_list, + const PacketList& media_packets, uint8_t* packet_mask, int num_fec_packets, bool l_bit) { @@ -416,8 +405,8 @@ void ForwardErrorCorrection::GenerateFecUlpHeaders( const uint16_t ulp_header_size = l_bit ? kUlpHeaderSizeLBitSet : kUlpHeaderSizeLBitClear; - RTC_DCHECK(!media_packet_list.empty()); - Packet* first_media_packet = media_packet_list.front(); + RTC_DCHECK(!media_packets.empty()); + Packet* first_media_packet = media_packets.front().get(); RTC_DCHECK(first_media_packet); uint16_t seq_num = ParseSequenceNumber(first_media_packet->data); for (int i = 0; i < num_fec_packets; ++i) { @@ -448,67 +437,51 @@ void ForwardErrorCorrection::GenerateFecUlpHeaders( } void ForwardErrorCorrection::ResetState( - RecoveredPacketList* recovered_packet_list) { + RecoveredPacketList* recovered_packets) { // Free the memory for any existing recovered packets, if the user hasn't. - while (!recovered_packet_list->empty()) { - delete recovered_packet_list->front(); - recovered_packet_list->pop_front(); - } - RTC_DCHECK(recovered_packet_list->empty()); - - // Free the FEC packet list. - while (!fec_packet_list_.empty()) { - FecPacket* fec_packet = fec_packet_list_.front(); - auto protected_packet_list_it = fec_packet->protected_pkt_list.begin(); - while (protected_packet_list_it != fec_packet->protected_pkt_list.end()) { - delete *protected_packet_list_it; - protected_packet_list_it = - fec_packet->protected_pkt_list.erase(protected_packet_list_it); - } - RTC_DCHECK(fec_packet->protected_pkt_list.empty()); - delete fec_packet; - fec_packet_list_.pop_front(); - } - RTC_DCHECK(fec_packet_list_.empty()); + recovered_packets->clear(); + received_fec_packets_.clear(); } void ForwardErrorCorrection::InsertMediaPacket( ReceivedPacket* rx_packet, - RecoveredPacketList* recovered_packet_list) { - auto recovered_packet_list_it = recovered_packet_list->cbegin(); + RecoveredPacketList* recovered_packets) { // Search for duplicate packets. - while (recovered_packet_list_it != recovered_packet_list->end()) { - if (rx_packet->seq_num == (*recovered_packet_list_it)->seq_num) { + for (const auto& recovered_packet : *recovered_packets) { + if (rx_packet->seq_num == recovered_packet->seq_num) { // Duplicate packet, no need to add to list. // Delete duplicate media packet data. rx_packet->pkt = nullptr; return; } - ++recovered_packet_list_it; } - RecoveredPacket* recovered_packet_to_insert = new RecoveredPacket(); + std::unique_ptr recovered_packet_to_insert( + new RecoveredPacket()); + // This "recovered packet" was not recovered using parity packets. recovered_packet_to_insert->was_recovered = false; - // Inserted Media packet is already sent to VCM. + // This media packet has already been passed on. recovered_packet_to_insert->returned = true; recovered_packet_to_insert->seq_num = rx_packet->seq_num; recovered_packet_to_insert->pkt = rx_packet->pkt; recovered_packet_to_insert->pkt->length = rx_packet->pkt->length; + RecoveredPacket* recovered_packet_to_insert_ptr = + recovered_packet_to_insert.get(); // TODO(holmer): Consider replacing this with a binary search for the right // position, and then just insert the new packet. Would get rid of the sort. - recovered_packet_list->push_back(recovered_packet_to_insert); - recovered_packet_list->sort(SortablePacket::LessThan); - UpdateCoveringFecPackets(recovered_packet_to_insert); + recovered_packets->push_back(std::move(recovered_packet_to_insert)); + recovered_packets->sort(SortablePacket::LessThan()); + UpdateCoveringFecPackets(recovered_packet_to_insert_ptr); } void ForwardErrorCorrection::UpdateCoveringFecPackets(RecoveredPacket* packet) { - for (auto* fec_packet : fec_packet_list_) { + for (auto& fec_packet : received_fec_packets_) { // Is this FEC packet protecting the media packet |packet|? auto protected_it = std::lower_bound(fec_packet->protected_pkt_list.begin(), fec_packet->protected_pkt_list.end(), packet, - SortablePacket::LessThan); + SortablePacket::LessThan()); if (protected_it != fec_packet->protected_pkt_list.end() && (*protected_it)->seq_num == packet->seq_num) { // Found an FEC packet which is protecting |packet|. @@ -519,16 +492,16 @@ void ForwardErrorCorrection::UpdateCoveringFecPackets(RecoveredPacket* packet) { void ForwardErrorCorrection::InsertFecPacket( ReceivedPacket* rx_packet, - const RecoveredPacketList* recovered_packet_list) { + const RecoveredPacketList* recovered_packets) { // Check for duplicate. - for (auto* fec_packet : fec_packet_list_) { - if (rx_packet->seq_num == fec_packet->seq_num) { + for (const auto& existing_fec_packet : received_fec_packets_) { + if (rx_packet->seq_num == existing_fec_packet->seq_num) { // Delete duplicate FEC packet data. rx_packet->pkt = nullptr; return; } } - FecPacket* fec_packet = new FecPacket(); + std::unique_ptr fec_packet(new ReceivedFecPacket()); fec_packet->pkt = rx_packet->pkt; fec_packet->seq_num = rx_packet->seq_num; fec_packet->ssrc = rx_packet->ssrc; @@ -543,62 +516,64 @@ void ForwardErrorCorrection::InsertFecPacket( uint8_t packet_mask = fec_packet->pkt->data[12 + byte_idx]; for (uint16_t bit_idx = 0; bit_idx < 8; ++bit_idx) { if (packet_mask & (1 << (7 - bit_idx))) { - ProtectedPacket* protected_packet = new ProtectedPacket(); - fec_packet->protected_pkt_list.push_back(protected_packet); + std::unique_ptr protected_packet( + new ProtectedPacket()); // This wraps naturally with the sequence number. protected_packet->seq_num = static_cast(seq_num_base + (byte_idx << 3) + bit_idx); protected_packet->pkt = nullptr; + fec_packet->protected_pkt_list.push_back(std::move(protected_packet)); } } } if (fec_packet->protected_pkt_list.empty()) { // All-zero packet mask; we can discard this FEC packet. LOG(LS_WARNING) << "FEC packet has an all-zero packet mask."; - delete fec_packet; } else { - AssignRecoveredPackets(fec_packet, recovered_packet_list); + AssignRecoveredPackets(fec_packet.get(), recovered_packets); // TODO(holmer): Consider replacing this with a binary search for the right // position, and then just insert the new packet. Would get rid of the sort. - fec_packet_list_.push_back(fec_packet); - fec_packet_list_.sort(SortablePacket::LessThan); - if (fec_packet_list_.size() > kMaxFecPackets) { - DiscardFecPacket(fec_packet_list_.front()); - fec_packet_list_.pop_front(); + received_fec_packets_.push_back(std::move(fec_packet)); + received_fec_packets_.sort(SortablePacket::LessThan()); + if (received_fec_packets_.size() > kMaxFecPackets) { + received_fec_packets_.pop_front(); } - RTC_DCHECK_LE(fec_packet_list_.size(), kMaxFecPackets); + RTC_DCHECK_LE(received_fec_packets_.size(), kMaxFecPackets); } } void ForwardErrorCorrection::AssignRecoveredPackets( - FecPacket* fec_packet, + ReceivedFecPacket* fec_packet, const RecoveredPacketList* recovered_packets) { - // Search for missing packets which have arrived or have been recovered by - // another FEC packet. - ProtectedPacketList* not_recovered = &fec_packet->protected_pkt_list; - RecoveredPacketList already_recovered; - std::set_intersection( - recovered_packets->cbegin(), recovered_packets->cend(), - not_recovered->cbegin(), not_recovered->cend(), - std::inserter(already_recovered, already_recovered.end()), - SortablePacket::LessThan); - // Set the FEC pointers to all recovered packets so that we don't have to - // search for them when we are doing recovery. - auto not_recovered_it = not_recovered->cbegin(); - for (auto it = already_recovered.cbegin(); - it != already_recovered.end(); ++it) { - // Search for the next recovered packet in |not_recovered|. - while ((*not_recovered_it)->seq_num != (*it)->seq_num) - ++not_recovered_it; - (*not_recovered_it)->pkt = (*it)->pkt; + ProtectedPacketList* protected_packets = &fec_packet->protected_pkt_list; + std::vector recovered_protected_packets; + + // Find intersection between the (sorted) containers |protected_packets| + // and |recovered_packets|, i.e. all protected packets that have already + // been recovered. Update the corresponding protected packets to point to + // the recovered packets. + auto it_p = protected_packets->cbegin(); + auto it_r = recovered_packets->cbegin(); + SortablePacket::LessThan less_than; + while (it_p != protected_packets->end() && it_r != recovered_packets->end()) { + if (less_than(*it_p, *it_r)) { + ++it_p; + } else if (less_than(*it_r, *it_p)) { + ++it_r; + } else { // *it_p == *it_r. + // This protected packet has already been recovered. + (*it_p)->pkt = (*it_r)->pkt; + ++it_p; + ++it_r; + } } } void ForwardErrorCorrection::InsertPackets( - ReceivedPacketList* received_packet_list, - RecoveredPacketList* recovered_packet_list) { - while (!received_packet_list->empty()) { - ReceivedPacket* rx_packet = received_packet_list->front(); + ReceivedPacketList* received_packets, + RecoveredPacketList* recovered_packets) { + while (!received_packets->empty()) { + ReceivedPacket* rx_packet = received_packets->front().get(); // Check for discarding oldest FEC packet, to avoid wrong FEC decoding from // sequence number wrap-around. Detection of old FEC packet is based on @@ -607,32 +582,31 @@ void ForwardErrorCorrection::InsertPackets( // TODO(marpan/holmer): We should be able to improve detection/discarding of // old FEC packets based on timestamp information or better sequence number // thresholding (e.g., to distinguish between wrap-around and reordering). - if (!fec_packet_list_.empty()) { + if (!received_fec_packets_.empty()) { uint16_t seq_num_diff = abs(static_cast(rx_packet->seq_num) - - static_cast(fec_packet_list_.front()->seq_num)); + static_cast(received_fec_packets_.front()->seq_num)); if (seq_num_diff > 0x3fff) { - DiscardFecPacket(fec_packet_list_.front()); - fec_packet_list_.pop_front(); + received_fec_packets_.pop_front(); } } if (rx_packet->is_fec) { - InsertFecPacket(rx_packet, recovered_packet_list); + InsertFecPacket(rx_packet, recovered_packets); } else { // Insert packet at the end of |recoveredPacketList|. - InsertMediaPacket(rx_packet, recovered_packet_list); + InsertMediaPacket(rx_packet, recovered_packets); } - // Delete the received packet "wrapper", but not the packet data. - delete rx_packet; - received_packet_list->pop_front(); + // Delete the received packet "wrapper". + received_packets->pop_front(); } - RTC_DCHECK(received_packet_list->empty()); - DiscardOldPackets(recovered_packet_list); + RTC_DCHECK(received_packets->empty()); + DiscardOldRecoveredPackets(recovered_packets); } -bool ForwardErrorCorrection::StartPacketRecovery(const FecPacket* fec_packet, - RecoveredPacket* recovered) { +bool ForwardErrorCorrection::StartPacketRecovery( + const ReceivedFecPacket* fec_packet, + RecoveredPacket* recovered) { // This is the first packet which we try to recover with. const uint16_t ulp_header_size = fec_packet->pkt->data[0] & 0x40 ? kUlpHeaderSizeLBitSet @@ -715,11 +689,11 @@ void ForwardErrorCorrection::XorPackets(const Packet* src_packet, } bool ForwardErrorCorrection::RecoverPacket( - const FecPacket* fec_packet, + const ReceivedFecPacket* fec_packet, RecoveredPacket* rec_packet_to_insert) { if (!StartPacketRecovery(fec_packet, rec_packet_to_insert)) return false; - for (const auto* protected_packet : fec_packet->protected_pkt_list) { + for (const auto& protected_packet : fec_packet->protected_pkt_list) { if (protected_packet->pkt == nullptr) { // This is the packet we're recovering. rec_packet_to_insert->seq_num = protected_packet->seq_num; @@ -733,56 +707,53 @@ bool ForwardErrorCorrection::RecoverPacket( } void ForwardErrorCorrection::AttemptRecover( - RecoveredPacketList* recovered_packet_list) { - auto fec_packet_list_it = fec_packet_list_.begin(); - while (fec_packet_list_it != fec_packet_list_.end()) { + RecoveredPacketList* recovered_packets) { + auto fec_packet_it = received_fec_packets_.begin(); + while (fec_packet_it != received_fec_packets_.end()) { // Search for each FEC packet's protected media packets. - int packets_missing = NumCoveredPacketsMissing(*fec_packet_list_it); + int packets_missing = NumCoveredPacketsMissing(fec_packet_it->get()); // We can only recover one packet with an FEC packet. if (packets_missing == 1) { // Recovery possible. - RecoveredPacket* packet_to_insert = new RecoveredPacket(); + std::unique_ptr packet_to_insert(new RecoveredPacket()); packet_to_insert->pkt = nullptr; - if (!RecoverPacket(*fec_packet_list_it, packet_to_insert)) { + if (!RecoverPacket(fec_packet_it->get(), packet_to_insert.get())) { // Can't recover using this packet, drop it. - DiscardFecPacket(*fec_packet_list_it); - fec_packet_list_it = fec_packet_list_.erase(fec_packet_list_it); - delete packet_to_insert; + fec_packet_it = received_fec_packets_.erase(fec_packet_it); continue; } + auto packet_to_insert_ptr = packet_to_insert.get(); // Add recovered packet to the list of recovered packets and update any // FEC packets covering this packet with a pointer to the data. // TODO(holmer): Consider replacing this with a binary search for the // right position, and then just insert the new packet. Would get rid of // the sort. - recovered_packet_list->push_back(packet_to_insert); - recovered_packet_list->sort(SortablePacket::LessThan); - UpdateCoveringFecPackets(packet_to_insert); - DiscardOldPackets(recovered_packet_list); - DiscardFecPacket(*fec_packet_list_it); - fec_packet_list_it = fec_packet_list_.erase(fec_packet_list_it); + recovered_packets->push_back(std::move(packet_to_insert)); + recovered_packets->sort(SortablePacket::LessThan()); + UpdateCoveringFecPackets(packet_to_insert_ptr); + DiscardOldRecoveredPackets(recovered_packets); + fec_packet_it = received_fec_packets_.erase(fec_packet_it); // A packet has been recovered. We need to check the FEC list again, as // this may allow additional packets to be recovered. // Restart for first FEC packet. - fec_packet_list_it = fec_packet_list_.begin(); + fec_packet_it = received_fec_packets_.begin(); } else if (packets_missing == 0) { // Either all protected packets arrived or have been recovered. We can // discard this FEC packet. - DiscardFecPacket(*fec_packet_list_it); - fec_packet_list_it = fec_packet_list_.erase(fec_packet_list_it); + fec_packet_it = received_fec_packets_.erase(fec_packet_it); } else { - fec_packet_list_it++; + fec_packet_it++; } } } int ForwardErrorCorrection::NumCoveredPacketsMissing( - const FecPacket* fec_packet) { + const ReceivedFecPacket* fec_packet) { int packets_missing = 0; - for (const auto* protected_packet : fec_packet->protected_pkt_list) { + for (const auto& protected_packet : fec_packet->protected_pkt_list) { if (protected_packet->pkt == nullptr) { ++packets_missing; if (packets_missing > 1) { @@ -793,24 +764,12 @@ int ForwardErrorCorrection::NumCoveredPacketsMissing( return packets_missing; } -void ForwardErrorCorrection::DiscardFecPacket(FecPacket* fec_packet) { - while (!fec_packet->protected_pkt_list.empty()) { - delete fec_packet->protected_pkt_list.front(); - fec_packet->protected_pkt_list.pop_front(); +void ForwardErrorCorrection::DiscardOldRecoveredPackets( + RecoveredPacketList* recovered_packets) { + while (recovered_packets->size() > kMaxMediaPackets) { + recovered_packets->pop_front(); } - RTC_DCHECK(fec_packet->protected_pkt_list.empty()); - delete fec_packet; -} - -void ForwardErrorCorrection::DiscardOldPackets( - RecoveredPacketList* recovered_packet_list) { - while (recovered_packet_list->size() > kMaxMediaPackets) { - ForwardErrorCorrection::RecoveredPacket* packet = - recovered_packet_list->front(); - delete packet; - recovered_packet_list->pop_front(); - } - RTC_DCHECK(recovered_packet_list->size() <= kMaxMediaPackets); + RTC_DCHECK_LE(recovered_packets->size(), kMaxMediaPackets); } uint16_t ForwardErrorCorrection::ParseSequenceNumber(uint8_t* packet) { @@ -818,22 +777,22 @@ uint16_t ForwardErrorCorrection::ParseSequenceNumber(uint8_t* packet) { } int ForwardErrorCorrection::DecodeFec( - ReceivedPacketList* received_packet_list, - RecoveredPacketList* recovered_packet_list) { + ReceivedPacketList* received_packets, + RecoveredPacketList* recovered_packets) { // TODO(marpan/ajm): can we check for multiple ULP headers, and return an // error? - if (recovered_packet_list->size() == kMaxMediaPackets) { + if (recovered_packets->size() == kMaxMediaPackets) { const unsigned int seq_num_diff = - abs(static_cast(received_packet_list->front()->seq_num) - - static_cast(recovered_packet_list->back()->seq_num)); + abs(static_cast(received_packets->front()->seq_num) - + static_cast(recovered_packets->back()->seq_num)); if (seq_num_diff > kMaxMediaPackets) { // A big gap in sequence numbers. The old recovered packets // are now useless, so it's safe to do a reset. - ResetState(recovered_packet_list); + ResetState(recovered_packets); } } - InsertPackets(received_packet_list, recovered_packet_list); - AttemptRecover(recovered_packet_list); + InsertPackets(received_packets, recovered_packets); + AttemptRecover(recovered_packets); return 0; } diff --git a/webrtc/modules/rtp_rtcp/source/forward_error_correction.h b/webrtc/modules/rtp_rtcp/source/forward_error_correction.h index fb35374ac6..ecc8151e89 100644 --- a/webrtc/modules/rtp_rtcp/source/forward_error_correction.h +++ b/webrtc/modules/rtp_rtcp/source/forward_error_correction.h @@ -14,6 +14,7 @@ #include #include +#include #include #include "webrtc/base/refcount.h" @@ -24,9 +25,6 @@ namespace webrtc { -// Forward declaration. -class FecPacket; - // Performs codec-independent forward error correction (FEC), based on RFC 5109. // Option exists to enable unequal protection (UEP) across packets. // This is not to be confused with protection within packets @@ -62,9 +60,12 @@ class ForwardErrorCorrection { // TODO(holmer): Refactor into a proper class. class SortablePacket { public: - // True if first is <= than second. - static bool LessThan(const SortablePacket* first, - const SortablePacket* second); + // Functor which returns true if the sequence number of |first| + // is < the sequence number of |second|. + struct LessThan { + template + bool operator() (const S& first, const T& second); + }; uint16_t seq_num; }; @@ -116,9 +117,9 @@ class ForwardErrorCorrection { rtc::scoped_refptr pkt; // Pointer to the packet storage. }; - typedef std::list PacketList; - typedef std::list ReceivedPacketList; - typedef std::list RecoveredPacketList; + using PacketList = std::list>; + using ReceivedPacketList = std::list>; + using RecoveredPacketList = std::list>; ForwardErrorCorrection(); virtual ~ForwardErrorCorrection(); @@ -159,10 +160,10 @@ class ForwardErrorCorrection { * * \return 0 on success, -1 on failure. */ - int GenerateFec(const PacketList& media_packet_list, + int GenerateFec(const PacketList& media_packets, uint8_t protection_factor, int num_important_packets, bool use_unequal_protection, FecMaskType fec_mask_type, - PacketList* fec_packet_list); + std::list* fec_packets); /** * Decodes a list of media and FEC packets. It will parse the input received @@ -192,8 +193,8 @@ class ForwardErrorCorrection { * * \return 0 on success, -1 on failure. */ - int DecodeFec(ReceivedPacketList* received_packet_list, - RecoveredPacketList* recovered_packet_list); + int DecodeFec(ReceivedPacketList* received_packets, + RecoveredPacketList* recovered_packets); // Get the number of FEC packets, given the number of media packets and the // protection factor. @@ -204,12 +205,32 @@ class ForwardErrorCorrection { // \return Packet overhead in bytes. static size_t PacketOverhead(); - // Reset internal states from last frame and clear the recovered_packet_list. + // Reset internal states from last frame and clears |recovered_packets|. // Frees all memory allocated by this class. - void ResetState(RecoveredPacketList* recovered_packet_list); + void ResetState(RecoveredPacketList* recovered_packets); private: - typedef std::list FecPacketList; + // Used to link media packets to their protecting FEC packets. + // + // TODO(holmer): Refactor into a proper class. + class ProtectedPacket : public ForwardErrorCorrection::SortablePacket { + public: + rtc::scoped_refptr pkt; + }; + + using ProtectedPacketList = std::list>; + + // Used for internal storage of received FEC packets in a list. + // + // TODO(holmer): Refactor into a proper class. + class ReceivedFecPacket : public ForwardErrorCorrection::SortablePacket { + public: + ProtectedPacketList protected_pkt_list; + uint32_t ssrc; // SSRC of the current frame. + rtc::scoped_refptr pkt; + }; + + using ReceivedFecPacketList = std::list>; // Analyzes |media_packets| for holes in the sequence and inserts zero columns // into the |packet_mask| where those holes are found. Zero columns means that @@ -242,21 +263,21 @@ class ForwardErrorCorrection { int num_fec_packets, int new_bit_index, int old_bit_index); - void GenerateFecUlpHeaders(const PacketList& media_packet_list, + void GenerateFecUlpHeaders(const PacketList& media_packets, uint8_t* packet_mask, int num_fec_packets, bool l_bit); - void GenerateFecBitStrings(const PacketList& media_packet_list, + void GenerateFecBitStrings(const PacketList& media_packets, uint8_t* packet_mask, int num_fec_packets, bool l_bit); // Insert received packets into FEC or recovered list. - void InsertPackets(ReceivedPacketList* received_packet_list, - RecoveredPacketList* recovered_packet_list); + void InsertPackets(ReceivedPacketList* received_packets, + RecoveredPacketList* recovered_packets); // Insert media packet into recovered packet list. We delete duplicates. void InsertMediaPacket(ReceivedPacket* rx_packet, - RecoveredPacketList* recovered_packet_list); + RecoveredPacketList* recovered_packets); // Assigns pointers to the recovered packet from all FEC packets which cover // it. @@ -267,21 +288,22 @@ class ForwardErrorCorrection { // Insert packet into FEC list. We delete duplicates. void InsertFecPacket(ReceivedPacket* rx_packet, - const RecoveredPacketList* recovered_packet_list); + const RecoveredPacketList* recovered_packets); // Assigns pointers to already recovered packets covered by this FEC packet. static void AssignRecoveredPackets( - FecPacket* fec_packet, const RecoveredPacketList* recovered_packets); + ReceivedFecPacket* fec_packet, + const RecoveredPacketList* recovered_packets); // Insert into recovered list in correct position. void InsertRecoveredPacket(RecoveredPacket* rec_packet_to_insert, - RecoveredPacketList* recovered_packet_list); + RecoveredPacketList* recovered_packets); // Attempt to recover missing packets. - void AttemptRecover(RecoveredPacketList* recovered_packet_list); + void AttemptRecover(RecoveredPacketList* recovered_packets); // Initializes the packet recovery using the FEC packet. - static bool StartPacketRecovery(const FecPacket* fec_packet, + static bool StartPacketRecovery(const ReceivedFecPacket* fec_packet, RecoveredPacket* recovered); // Performs XOR between |src_packet| and |dst_packet| and stores the result @@ -292,21 +314,21 @@ class ForwardErrorCorrection { static bool FinishPacketRecovery(RecoveredPacket* recovered); // Recover a missing packet. - bool RecoverPacket(const FecPacket* fec_packet, + bool RecoverPacket(const ReceivedFecPacket* fec_packet, RecoveredPacket* rec_packet_to_insert); // Get the number of missing media packets which are covered by this // FEC packet. An FEC packet can recover at most one packet, and if zero // packets are missing the FEC packet can be discarded. // This function returns 2 when two or more packets are missing. - static int NumCoveredPacketsMissing(const FecPacket* fec_packet); + static int NumCoveredPacketsMissing(const ReceivedFecPacket* fec_packet); - static void DiscardFecPacket(FecPacket* fec_packet); - static void DiscardOldPackets(RecoveredPacketList* recovered_packet_list); + static void DiscardOldRecoveredPackets( + RecoveredPacketList* recovered_packets); static uint16_t ParseSequenceNumber(uint8_t* packet); std::vector generated_fec_packets_; - FecPacketList fec_packet_list_; + ReceivedFecPacketList received_fec_packets_; // Arrays used to avoid dynamically allocating memory when generating // the packet masks in the ULPFEC headers. diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.cc b/webrtc/modules/rtp_rtcp/source/producer_fec.cc index 6fe285fed9..c27472bfaf 100644 --- a/webrtc/modules/rtp_rtcp/source/producer_fec.cc +++ b/webrtc/modules/rtp_rtcp/source/producer_fec.cc @@ -10,6 +10,9 @@ #include "webrtc/modules/rtp_rtcp/source/producer_fec.h" +#include +#include + #include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h" #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" @@ -139,11 +142,11 @@ int ProducerFec::AddRtpPacketAndGenerateFec(const uint8_t* data_buffer, const bool marker_bit = (data_buffer[1] & kRtpMarkerBitMask) ? true : false; if (media_packets_fec_.size() < ForwardErrorCorrection::kMaxMediaPackets) { // Generic FEC can only protect up to kMaxMediaPackets packets. - ForwardErrorCorrection::Packet* packet = - new ForwardErrorCorrection::Packet(); + std::unique_ptr packet( + new ForwardErrorCorrection::Packet()); packet->length = payload_length + rtp_header_length; memcpy(packet->data, data_buffer, packet->length); - media_packets_fec_.push_back(packet); + media_packets_fec_.push_back(std::move(packet)); } if (marker_bit) { ++num_frames_; @@ -219,7 +222,7 @@ std::vector ProducerFec::GetFecPackets(int red_pl_type, // media packet. ForwardErrorCorrection::Packet* packet_to_send = fec_packets_.front(); ForwardErrorCorrection::Packet* last_media_packet = - media_packets_fec_.back(); + media_packets_fec_.back().get(); RedPacket* red_packet = new RedPacket( packet_to_send->length + kREDForFECHeaderLength + rtp_header_length); @@ -251,11 +254,7 @@ int ProducerFec::Overhead() const { } void ProducerFec::DeletePackets() { - while (!media_packets_fec_.empty()) { - delete media_packets_fec_.front(); - media_packets_fec_.pop_front(); - } - assert(media_packets_fec_.empty()); + media_packets_fec_.clear(); } } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.h b/webrtc/modules/rtp_rtcp/source/producer_fec.h index 063a45fa95..fc2e8e4a8a 100644 --- a/webrtc/modules/rtp_rtcp/source/producer_fec.h +++ b/webrtc/modules/rtp_rtcp/source/producer_fec.h @@ -11,6 +11,7 @@ #ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_PRODUCER_FEC_H_ #define WEBRTC_MODULES_RTP_RTCP_SOURCE_PRODUCER_FEC_H_ +#include #include #include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h" @@ -72,7 +73,7 @@ class ProducerFec { int Overhead() const; ForwardErrorCorrection* fec_; ForwardErrorCorrection::PacketList media_packets_fec_; - ForwardErrorCorrection::PacketList fec_packets_; + std::list fec_packets_; int num_frames_; int num_first_partition_; int minimum_media_packets_fec_; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc index 198877df26..6046af1001 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc @@ -31,15 +31,6 @@ using PacketList = ForwardErrorCorrection::PacketList; using ReceivedPacketList = ForwardErrorCorrection::ReceivedPacketList; using RecoveredPacketList = ForwardErrorCorrection::RecoveredPacketList; -template void ClearList(std::list* my_list) { - T* packet = NULL; - while (!my_list->empty()) { - packet = my_list->front(); - delete packet; - my_list->pop_front(); - } -} - class RtpFecTest : public ::testing::Test { protected: RtpFecTest() @@ -54,7 +45,7 @@ class RtpFecTest : public ::testing::Test { uint16_t fec_seq_num_; PacketList media_packet_list_; - PacketList fec_packet_list_; + std::list fec_packet_list_; ReceivedPacketList received_packet_list_; RecoveredPacketList recovered_packet_list_; @@ -72,6 +63,9 @@ class RtpFecTest : public ::testing::Test { int ConstructMediaPacketsSeqNum(int num_media_packets, int start_seq_num); int ConstructMediaPackets(int num_media_packets); + // Deep copies |src| to |dst|, but only keeps every Nth packet. + void DeepCopyEveryNthPacket(const PacketList& src, int n, PacketList* dst); + // Construct the received packet list: a subset of the media and FEC packets. void NetworkReceivedPackets(); @@ -79,15 +73,12 @@ class RtpFecTest : public ::testing::Test { // |loss_mask|. // The |packet_list| may be a media packet list (is_fec = false), or a // FEC packet list (is_fec = true). - void ReceivedPackets(const PacketList& packet_list, int* loss_mask, - bool is_fec); + template + void ReceivedPackets(const T& packet_list, int* loss_mask, bool is_fec); // Check for complete recovery after FEC decoding. bool IsRecoveryComplete(); - // Delete the received packets. - void FreeRecoveredPacketList(); - // Delete the media and FEC packets. void TearDown(); }; @@ -145,7 +136,7 @@ TEST_F(RtpFecTest, FecRecoveryWithLoss) { // One packet lost, one FEC packet, expect complete recovery. EXPECT_TRUE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // 2 media packets lost. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -188,7 +179,7 @@ TEST_F(RtpFecTest, FecRecoveryWithSeqNumGapTwoFrames) { ReceivedPackets(fec_packet_list_, fec_loss_mask_, true); // Construct media packets for second frame, with sequence number wrap. - ClearList(&media_packet_list_); + media_packet_list_.clear(); fec_seq_num_ = ConstructMediaPacketsSeqNum(3, 65535); // Expect 3 media packets for this frame. @@ -208,7 +199,6 @@ TEST_F(RtpFecTest, FecRecoveryWithSeqNumGapTwoFrames) { // recovered packets is 2, and not equal to number of media packets (=3). EXPECT_EQ(2, static_cast(recovered_packet_list_.size())); EXPECT_TRUE(recovered_packet_list_.size() != media_packet_list_.size()); - FreeRecoveredPacketList(); } // Verify we can still recovery frame if sequence number wrap occurs within @@ -245,7 +235,6 @@ TEST_F(RtpFecTest, FecRecoveryWithSeqNumGapOneFrameRecovery) { // Wrap-around won't remove FEC packet, as it follows the wrap. EXPECT_EQ(3, static_cast(recovered_packet_list_.size())); EXPECT_TRUE(IsRecoveryComplete()); - FreeRecoveredPacketList(); } // Sequence number wrap occurs within the FEC packets for the frame. @@ -290,7 +279,6 @@ TEST_F(RtpFecTest, FecRecoveryWithSeqNumGapOneFrameNoRecovery) { EXPECT_EQ(2, static_cast(recovered_packet_list_.size())); EXPECT_TRUE(recovered_packet_list_.size() != media_packet_list_.size()); EXPECT_FALSE(IsRecoveryComplete()); - FreeRecoveredPacketList(); } // Verify we can still recovery frame if FEC is received before media packets. @@ -326,7 +314,6 @@ TEST_F(RtpFecTest, FecRecoveryWithFecOutOfOrder) { // Expect 3 media packets in recovered list, and complete recovery. EXPECT_EQ(3, static_cast(recovered_packet_list_.size())); EXPECT_TRUE(IsRecoveryComplete()); - FreeRecoveredPacketList(); } // Test 50% protection with random mask type: Two cases are considered: @@ -371,7 +358,7 @@ TEST_F(RtpFecTest, FecRecoveryWithLoss50percRandomMask) { // With media packet#1 and FEC packets #1, #2, #3, expect complete recovery. EXPECT_TRUE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // 4 consecutive packets lost: media packets 0, 1, 2, 3. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -431,7 +418,7 @@ TEST_F(RtpFecTest, FecRecoveryWithLoss50percBurstyMask) { // Expect complete recovery for consecutive packet loss <= 50%. EXPECT_TRUE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // 4 consecutive packets lost: media packets 1,2, 3, and FEC packet 0. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -447,7 +434,7 @@ TEST_F(RtpFecTest, FecRecoveryWithLoss50percBurstyMask) { // Expect complete recovery for consecutive packet loss <= 50%. EXPECT_TRUE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // 4 packets lost (non-consecutive loss): media packets 0, 3, and FEC# 0, 3. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -518,7 +505,7 @@ TEST_F(RtpFecTest, FecRecoveryWithLossUep) { // One packet lost, one FEC packet, expect complete recovery. EXPECT_TRUE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // 2 media packets lost. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -574,7 +561,7 @@ TEST_F(RtpFecTest, FecRecoveryWithLoss50percUepRandomMask) { // With media packet#3 and FEC packets #0, #1, #3, expect complete recovery. EXPECT_TRUE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // 5 packets lost: 4 media packets and one FEC packet#2 lost. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -604,11 +591,7 @@ TEST_F(RtpFecTest, FecRecoveryNonConsecutivePackets) { // Create a new temporary packet list for generating FEC packets. // This list should have every other packet removed. PacketList protected_media_packets; - int i = 0; - for (auto it = media_packet_list_.begin(); it != media_packet_list_.end(); - ++it, ++i) { - if (i % 2 == 0) protected_media_packets.push_back(*it); - } + DeepCopyEveryNthPacket(media_packet_list_, 2, &protected_media_packets); EXPECT_EQ(0, fec_->GenerateFec(protected_media_packets, kProtectionFactor, kNumImportantPackets, kUseUnequalProtection, @@ -628,7 +611,7 @@ TEST_F(RtpFecTest, FecRecoveryNonConsecutivePackets) { // One packet lost, one FEC packet, expect complete recovery. EXPECT_TRUE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // Unprotected packet lost. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -641,7 +624,7 @@ TEST_F(RtpFecTest, FecRecoveryNonConsecutivePackets) { // Unprotected packet lost. Recovery not possible. EXPECT_FALSE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // 2 media packets lost. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -668,11 +651,7 @@ TEST_F(RtpFecTest, FecRecoveryNonConsecutivePacketsExtension) { // Create a new temporary packet list for generating FEC packets. // This list should have every other packet removed. PacketList protected_media_packets; - int i = 0; - for (auto it = media_packet_list_.begin(); it != media_packet_list_.end(); - ++it, ++i) { - if (i % 2 == 0) protected_media_packets.push_back(*it); - } + DeepCopyEveryNthPacket(media_packet_list_, 2, &protected_media_packets); // Zero column insertion will have to extend the size of the packet // mask since the number of actual packets are 21, while the number @@ -695,7 +674,7 @@ TEST_F(RtpFecTest, FecRecoveryNonConsecutivePacketsExtension) { // One packet lost, one FEC packet, expect complete recovery. EXPECT_TRUE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // Last unprotected packet lost. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -708,7 +687,7 @@ TEST_F(RtpFecTest, FecRecoveryNonConsecutivePacketsExtension) { // Unprotected packet lost. Recovery not possible. EXPECT_FALSE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // 6 media packets lost. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -739,11 +718,7 @@ TEST_F(RtpFecTest, FecRecoveryNonConsecutivePacketsWrap) { // Create a new temporary packet list for generating FEC packets. // This list should have every other packet removed. PacketList protected_media_packets; - int i = 0; - for (auto it = media_packet_list_.begin(); it != media_packet_list_.end(); - ++it, ++i) { - if (i % 2 == 0) protected_media_packets.push_back(*it); - } + DeepCopyEveryNthPacket(media_packet_list_, 2, &protected_media_packets); // Zero column insertion will have to extend the size of the packet // mask since the number of actual packets are 21, while the number @@ -766,7 +741,7 @@ TEST_F(RtpFecTest, FecRecoveryNonConsecutivePacketsWrap) { // One packet lost, one FEC packet, expect complete recovery. EXPECT_TRUE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // Last unprotected packet lost. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -779,7 +754,7 @@ TEST_F(RtpFecTest, FecRecoveryNonConsecutivePacketsWrap) { // Unprotected packet lost. Recovery not possible. EXPECT_FALSE(IsRecoveryComplete()); - FreeRecoveredPacketList(); + recovered_packet_list_.clear(); // 6 media packets lost. memset(media_loss_mask_, 0, sizeof(media_loss_mask_)); @@ -802,15 +777,11 @@ TEST_F(RtpFecTest, FecRecoveryNonConsecutivePacketsWrap) { void RtpFecTest::TearDown() { fec_->ResetState(&recovered_packet_list_); delete fec_; - FreeRecoveredPacketList(); - ClearList(&media_packet_list_); + recovered_packet_list_.clear(); + media_packet_list_.clear(); EXPECT_TRUE(media_packet_list_.empty()); } -void RtpFecTest::FreeRecoveredPacketList() { - ClearList(&recovered_packet_list_); -} - bool RtpFecTest::IsRecoveryComplete() { // We must have equally many recovered packets as original packets. if (recovered_packet_list_.size() != media_packet_list_.size()) { @@ -819,8 +790,11 @@ bool RtpFecTest::IsRecoveryComplete() { // All recovered packets must be identical to the corresponding // original packets. - auto cmp = [](ForwardErrorCorrection::Packet* media_packet, - ForwardErrorCorrection::RecoveredPacket* recovered_packet) { + using PacketPtr = std::unique_ptr; + using RecoveredPacketPtr = + std::unique_ptr; + auto cmp = [](const PacketPtr& media_packet, + const RecoveredPacketPtr& recovered_packet) { if (media_packet->length != recovered_packet->pkt->length) { return false; } @@ -841,16 +815,17 @@ void RtpFecTest::NetworkReceivedPackets() { ReceivedPackets(fec_packet_list_, fec_loss_mask_, kFecPacket); } -void RtpFecTest::ReceivedPackets(const PacketList& packet_list, int* loss_mask, +template +void RtpFecTest::ReceivedPackets(const T& packet_list, int* loss_mask, bool is_fec) { int seq_num = fec_seq_num_; int packet_idx = 0; - for (const auto* packet : packet_list) { + for (const auto& packet : packet_list) { if (loss_mask[packet_idx] == 0) { - auto received_packet = new ForwardErrorCorrection::ReceivedPacket(); + std::unique_ptr received_packet( + new ForwardErrorCorrection::ReceivedPacket()); received_packet->pkt = new ForwardErrorCorrection::Packet(); - received_packet_list_.push_back(received_packet); received_packet->pkt->length = packet->length; memcpy(received_packet->pkt->data, packet->data, packet->length); received_packet->is_fec = is_fec; @@ -869,6 +844,7 @@ void RtpFecTest::ReceivedPackets(const PacketList& packet_list, int* loss_mask, // media packets in ConstructMediaPackets(). received_packet->ssrc = ssrc_; } + received_packet_list_.push_back(std::move(received_packet)); } packet_idx++; // Sequence number of FEC packets are defined as increment by 1 from @@ -880,13 +856,12 @@ void RtpFecTest::ReceivedPackets(const PacketList& packet_list, int* loss_mask, int RtpFecTest::ConstructMediaPacketsSeqNum(int num_media_packets, int start_seq_num) { RTC_DCHECK_GT(num_media_packets, 0); - ForwardErrorCorrection::Packet* media_packet = NULL; int sequence_number = start_seq_num; int time_stamp = random_.Rand(); for (int i = 0; i < num_media_packets; ++i) { - media_packet = new ForwardErrorCorrection::Packet(); - media_packet_list_.push_back(media_packet); + std::unique_ptr media_packet( + new ForwardErrorCorrection::Packet()); const uint32_t kMinPacketSize = kRtpHeaderSize; const uint32_t kMaxPacketSize = IP_PACKET_SIZE - kRtpHeaderSize - kTransportOverhead - @@ -921,8 +896,11 @@ int RtpFecTest::ConstructMediaPacketsSeqNum(int num_media_packets, media_packet->data[j] = random_.Rand(); } sequence_number++; + media_packet_list_.push_back(std::move(media_packet)); } // Last packet, set marker bit. + ForwardErrorCorrection::Packet* media_packet = + media_packet_list_.back().get(); RTC_DCHECK(media_packet); media_packet->data[1] |= 0x80; return sequence_number; @@ -931,3 +909,15 @@ int RtpFecTest::ConstructMediaPacketsSeqNum(int num_media_packets, int RtpFecTest::ConstructMediaPackets(int num_media_packets) { return ConstructMediaPacketsSeqNum(num_media_packets, random_.Rand()); } + +void RtpFecTest::DeepCopyEveryNthPacket(const PacketList& src, int n, + PacketList* dst) { + RTC_DCHECK_GT(n, 0); + int i = 0; + for (const auto& packet : src) { + if (i % n == 0) { + dst->emplace_back(new ForwardErrorCorrection::Packet(*packet)); + } + ++i; + } +} diff --git a/webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc b/webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc index 3e9a5989d6..697df48587 100644 --- a/webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc +++ b/webrtc/modules/rtp_rtcp/test/testFec/test_fec.cc @@ -56,24 +56,25 @@ void ReceivePackets( } random_variable = random->Rand(); } - ForwardErrorCorrection::ReceivedPacket* received_packet = *it; - to_decode_list->push_back(received_packet); + to_decode_list->push_back(std::move(*it)); + received_packet_list->erase(it); // Duplicate packets. + ForwardErrorCorrection::ReceivedPacket* received_packet = + to_decode_list->back().get(); random_variable = random->Rand(); while (random_variable < duplicate_rate) { - ForwardErrorCorrection::ReceivedPacket* duplicate_packet = - new ForwardErrorCorrection::ReceivedPacket(); + std::unique_ptr duplicate_packet( + new ForwardErrorCorrection::ReceivedPacket()); *duplicate_packet = *received_packet; duplicate_packet->pkt = new ForwardErrorCorrection::Packet(); memcpy(duplicate_packet->pkt->data, received_packet->pkt->data, received_packet->pkt->length); duplicate_packet->pkt->length = received_packet->pkt->length; - to_decode_list->push_back(duplicate_packet); + to_decode_list->push_back(std::move(duplicate_packet)); random_variable = random->Rand(); } - received_packet_list->erase(it); } } @@ -108,13 +109,12 @@ TEST(FecTest, MAYBE_FecTest) { ForwardErrorCorrection fec; ForwardErrorCorrection::PacketList media_packet_list; - ForwardErrorCorrection::PacketList fec_packet_list; + std::list fec_packet_list; ForwardErrorCorrection::ReceivedPacketList to_decode_list; ForwardErrorCorrection::ReceivedPacketList received_packet_list; ForwardErrorCorrection::RecoveredPacketList recovered_packet_list; std::list fec_mask_list; - ForwardErrorCorrection::Packet* media_packet = nullptr; // Running over only one loss rate to limit execution time. const float loss_rate[] = {0.5f}; const uint32_t loss_rate_size = sizeof(loss_rate) / sizeof(*loss_rate); @@ -234,8 +234,8 @@ TEST(FecTest, MAYBE_FecTest) { // around is detected. This case is currently not handled below. seq_num = 0; for (uint32_t i = 0; i < num_media_packets; ++i) { - media_packet = new ForwardErrorCorrection::Packet(); - media_packet_list.push_back(media_packet); + std::unique_ptr media_packet( + new ForwardErrorCorrection::Packet()); const uint32_t kMinPacketSize = 12; const uint32_t kMaxPacketSize = static_cast( IP_PACKET_SIZE - 12 - 28 - @@ -273,9 +273,10 @@ TEST(FecTest, MAYBE_FecTest) { for (size_t j = 12; j < media_packet->length; ++j) { media_packet->data[j] = random.Rand(); } + media_packet_list.push_back(std::move(media_packet)); seq_num++; } - media_packet->data[1] |= 0x80; + media_packet_list.back()->data[1] |= 0x80; ASSERT_EQ(0, fec.GenerateFec(media_packet_list, protection_factor, num_imp_packets, kUseUnequalProtection, @@ -288,23 +289,23 @@ TEST(FecTest, MAYBE_FecTest) { memset(media_loss_mask, 0, sizeof(media_loss_mask)); uint32_t media_packet_idx = 0; - for (auto* media_packet : media_packet_list) { + for (const auto& media_packet : media_packet_list) { // We want a value between 0 and 1. const float loss_random_variable = random.Rand(); if (loss_random_variable >= loss_rate[loss_rate_idx]) { media_loss_mask[media_packet_idx] = 1; - auto received_packet = - new ForwardErrorCorrection::ReceivedPacket(); + std::unique_ptr + received_packet( + new ForwardErrorCorrection::ReceivedPacket()); received_packet->pkt = new ForwardErrorCorrection::Packet(); - received_packet_list.push_back(received_packet); - received_packet->pkt->length = media_packet->length; memcpy(received_packet->pkt->data, media_packet->data, media_packet->length); received_packet->seq_num = ByteReader::ReadBigEndian(&media_packet->data[2]); received_packet->is_fec = false; + received_packet_list.push_back(std::move(received_packet)); } media_packet_idx++; } @@ -315,19 +316,17 @@ TEST(FecTest, MAYBE_FecTest) { const float loss_random_variable = random.Rand(); if (loss_random_variable >= loss_rate[loss_rate_idx]) { fec_loss_mask[fec_packet_idx] = 1; - auto received_packet = - new ForwardErrorCorrection::ReceivedPacket(); + std::unique_ptr + received_packet( + new ForwardErrorCorrection::ReceivedPacket()); received_packet->pkt = new ForwardErrorCorrection::Packet(); - - received_packet_list.push_back(received_packet); - received_packet->pkt->length = fec_packet->length; memcpy(received_packet->pkt->data, fec_packet->data, fec_packet->length); - received_packet->seq_num = seq_num; received_packet->is_fec = true; received_packet->ssrc = ssrc; + received_packet_list.push_back(std::move(received_packet)); fec_mask_list.push_back(fec_packet_masks[fec_packet_idx]); } @@ -388,7 +387,7 @@ TEST(FecTest, MAYBE_FecTest) { duplicate_rate, &random); if (fec_packet_received == false) { - for (auto* received_packet : to_decode_list) { + for (const auto& received_packet : to_decode_list) { if (received_packet->is_fec) { fec_packet_received = true; } @@ -401,7 +400,7 @@ TEST(FecTest, MAYBE_FecTest) { << "Received packet list is not empty."; } media_packet_idx = 0; - for (auto* media_packet : media_packet_list) { + for (const auto& media_packet : media_packet_list) { if (media_loss_mask[media_packet_idx] == 1) { // Should have recovered this packet. auto recovered_packet_list_it = recovered_packet_list.cbegin(); @@ -410,7 +409,7 @@ TEST(FecTest, MAYBE_FecTest) { recovered_packet_list.end()) << "Insufficient number of recovered packets."; ForwardErrorCorrection::RecoveredPacket* recovered_packet = - *recovered_packet_list_it; + recovered_packet_list_it->get(); ASSERT_EQ(recovered_packet->pkt->length, media_packet->length) << "Recovered packet length not identical to original " @@ -419,7 +418,6 @@ TEST(FecTest, MAYBE_FecTest) { media_packet->data, media_packet->length)) << "Recovered packet payload not identical to original " << "media packet"; - delete recovered_packet; recovered_packet_list.pop_front(); } ++media_packet_idx; @@ -429,13 +427,7 @@ TEST(FecTest, MAYBE_FecTest) { << "Excessive number of recovered packets.\t size is: " << recovered_packet_list.size(); // -- Teardown -- - auto media_packet_list_it = media_packet_list.begin(); - while (media_packet_list_it != media_packet_list.end()) { - delete *media_packet_list_it; - ++media_packet_list_it; - media_packet_list.pop_front(); - } - RTC_DCHECK(media_packet_list.empty()); + media_packet_list.clear(); // Clear FEC packet list, so we don't pass in a non-empty // list in the next call to DecodeFec(). @@ -443,13 +435,7 @@ TEST(FecTest, MAYBE_FecTest) { // Delete received packets we didn't pass to DecodeFec(), due to // early frame completion. - auto received_packet_it = received_packet_list.cbegin(); - while (received_packet_it != received_packet_list.end()) { - delete *received_packet_it; - ++received_packet_it; - received_packet_list.pop_front(); - } - RTC_DCHECK(received_packet_list.empty()); + received_packet_list.clear(); while (!fec_mask_list.empty()) { fec_mask_list.pop_front(); @@ -461,7 +447,7 @@ TEST(FecTest, MAYBE_FecTest) { } // loop over loss rates } // loop over mask types - // Have DecodeFec free allocated memory. + // Have DecodeFec clear the recovered packet list. fec.ResetState(&recovered_packet_list); ASSERT_TRUE(recovered_packet_list.empty()) << "Recovered packet list is not empty";