Use std::unique_ptr in ForwardErrorCorrection.

webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc

Index: webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc
index 67c6a2a345b40e360bf0817faea2eb02d52de3bc..91f4e819686c0a5bca3715011be2f6dd94c2ec58 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc
@@ -30,15 +30,6 @@ using PacketList = ForwardErrorCorrection::PacketList;
 using ReceivedPacketList = ForwardErrorCorrection::ReceivedPacketList;
 using RecoveredPacketList = ForwardErrorCorrection::RecoveredPacketList;
 
-template <typename T> void ClearList(std::list<T*>* 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()
@@ -53,7 +44,7 @@ class RtpFecTest : public ::testing::Test {
   uint16_t fec_seq_num_;
 
   PacketList media_packet_list_;
-  PacketList fec_packet_list_;
+  std::list<ForwardErrorCorrection::Packet*> fec_packet_list_;
   ReceivedPacketList received_packet_list_;
   RecoveredPacketList recovered_packet_list_;
 
@@ -71,6 +62,10 @@ class RtpFecTest : public ::testing::Test {
   int ConstructMediaPacketsSeqNum(int num_media_packets, int start_seq_num);
   int ConstructMediaPackets(int num_media_packets);
 
+  // Creates a deep copy of |src|, removes half the packets, and places
+  // the result in |dst|.
+  void DeepCopyAndRemoveHalf(const PacketList& src, PacketList* dst);
+
   // Construct the received packet list: a subset of the media and FEC packets.
   void NetworkReceivedPackets();
 
@@ -78,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 <typename T>
+  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();
 };
@@ -144,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_));
@@ -187,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.
@@ -207,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<int>(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
@@ -244,7 +235,6 @@ TEST_F(RtpFecTest, FecRecoveryWithSeqNumGapOneFrameRecovery) {
   // Wrap-around won't remove FEC packet, as it follows the wrap.
   EXPECT_EQ(3, static_cast<int>(recovered_packet_list_.size()));
   EXPECT_TRUE(IsRecoveryComplete());
-  FreeRecoveredPacketList();
 }
 
 // Sequence number wrap occurs within the FEC packets for the frame.
@@ -289,7 +279,6 @@ TEST_F(RtpFecTest, FecRecoveryWithSeqNumGapOneFrameNoRecovery) {
   EXPECT_EQ(2, static_cast<int>(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.
@@ -325,7 +314,6 @@ TEST_F(RtpFecTest, FecRecoveryWithFecOutOfOrder) {
   // Expect 3 media packets in recovered list, and complete recovery.
   EXPECT_EQ(3, static_cast<int>(recovered_packet_list_.size()));
   EXPECT_TRUE(IsRecoveryComplete());
-  FreeRecoveredPacketList();
 }
 
 // Test 50% protection with random mask type: Two cases are considered:
@@ -370,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_));
@@ -430,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_));
@@ -446,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_));
@@ -517,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_));
@@ -573,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_));
@@ -603,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);
-  }
+  DeepCopyAndRemoveHalf(media_packet_list_, &protected_media_packets);
 
   EXPECT_EQ(0, fec_->GenerateFEC(protected_media_packets, kProtectionFactor,
                                  kNumImportantPackets, kUseUnequalProtection,
@@ -627,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_));
@@ -640,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_));
@@ -667,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);
-  }
+  DeepCopyAndRemoveHalf(media_packet_list_, &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
@@ -694,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_));
@@ -707,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_));
@@ -738,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);
-  }
+  DeepCopyAndRemoveHalf(media_packet_list_, &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
@@ -765,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_));
@@ -778,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_));
@@ -801,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() {
   // Check that the number of media and recovered packets are equal.
   if (media_packet_list_.size() != recovered_packet_list_.size()) {
@@ -827,8 +799,8 @@ bool RtpFecTest::IsRecoveryComplete() {
     if (recovered_packet_list_it == recovered_packet_list_.end()) {
       return false;
     }
-    media_packet = *media_packet_list_it;
-    recovered_packet = *recovered_packet_list_it;
+    media_packet = media_packet_list_it->get();
+    recovered_packet = recovered_packet_list_it->get();
     if (recovered_packet->pkt->length != media_packet->length) {
       return false;
     }
@@ -848,28 +820,29 @@ void RtpFecTest::NetworkReceivedPackets() {
   ReceivedPackets(fec_packet_list_, fec_loss_mask_, kFecPacket);
 }
 
-void RtpFecTest::ReceivedPackets(const PacketList& packet_list, int* loss_mask,
+template <typename T>
+void RtpFecTest::ReceivedPackets(const T& packet_list, int* loss_mask,
                                  bool is_fec) {
-  ForwardErrorCorrection::Packet* packet;
-  ForwardErrorCorrection::ReceivedPacket* received_packet;
   int seq_num = fec_seq_num_;
   int packet_idx = 0;
 
   auto packet_list_it = packet_list.cbegin();
   while (packet_list_it != packet_list.end()) {
-    packet = *packet_list_it;
     if (loss_mask[packet_idx] == 0) {
-      received_packet = new ForwardErrorCorrection::ReceivedPacket;
+      std::unique_ptr<ForwardErrorCorrection::ReceivedPacket> 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->pkt->length = (*packet_list_it)->length;
+      memcpy(received_packet->pkt->data,
+             (*packet_list_it)->data,
+             (*packet_list_it)->length);
       received_packet->is_fec = is_fec;
       if (!is_fec) {
         // For media packets, the sequence number and marker bit is
         // obtained from RTP header. These were set in ConstructMediaPackets().
         received_packet->seq_num =
-            webrtc::ByteReader<uint16_t>::ReadBigEndian(&packet->data[2]);
+            webrtc::ByteReader<uint16_t>::ReadBigEndian(
+                &(*packet_list_it)->data[2]);
       } else {
         // The sequence number, marker bit, and ssrc number are defined in the
         // RTP header of the FEC packet, which is not constructed in this test.
@@ -880,6 +853,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++;
     packet_list_it++;
@@ -892,13 +866,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<int>();
 
   for (int i = 0; i < num_media_packets; ++i) {
-    media_packet = new ForwardErrorCorrection::Packet;
-    media_packet_list_.push_back(media_packet);
+    std::unique_ptr<ForwardErrorCorrection::Packet> media_packet(
+        new ForwardErrorCorrection::Packet);
     const uint32_t kMinPacketSize = kRtpHeaderSize;
     const uint32_t kMaxPacketSize = IP_PACKET_SIZE - kRtpHeaderSize -
                                     kTransportOverhead -
@@ -933,8 +906,11 @@ int RtpFecTest::ConstructMediaPacketsSeqNum(int num_media_packets,
       media_packet->data[j] = random_.Rand<uint8_t>();
     }
     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;
@@ -943,3 +919,14 @@ int RtpFecTest::ConstructMediaPacketsSeqNum(int num_media_packets,
 int RtpFecTest::ConstructMediaPackets(int num_media_packets) {
   return ConstructMediaPacketsSeqNum(num_media_packets, random_.Rand<int>());
 }
+
+void RtpFecTest::DeepCopyAndRemoveHalf(const PacketList& src,

[stefan-webrtc, 2016/06/29 19:36:18]

DeepCopyEveryOtherPacket() is better I think. Or maybe generalize to
DeepCopyEveryNthPacket()?

[brandtr, 2016/06/30 14:05:55]

Done.

+                                       PacketList* dst) {
+  int i = 0;
+  for (auto& packet : src) {
+    if (i % 2 == 0) {
+      dst->emplace_back(new ForwardErrorCorrection::Packet(*packet));
+    }
+    ++i;
+  }
+}