Reorder member functions in FecReceiver unit test file.

- Place all member function definitions between test fixture
  declaration and unit tests.
- Rename GenerateFrame -> PacketizeFrame.

No functional changes are intended by this CL.

BUG=webrtc:5654

Review-Url: https://codereview.webrtc.org/2275303003
Cr-Commit-Position: refs/heads/master@{#14488}

webrtc/modules/rtp_rtcp/include/fec_receiver.h

@@ -35,6 +35,9 @@ class FecReceiver {
 
   // Takes a RED packet, strips the RED header, and adds the resulting
   // "virtual" RTP packet(s) into the internal buffer.
+  //
+  // TODO(brandtr): Set |ulpfec_payload_type| during constructor call,
+  // rather than as a parameter here.
   virtual int32_t AddReceivedRedPacket(const RTPHeader& rtp_header,
                                        const uint8_t* incoming_rtp_packet,
                                        size_t packet_length,

webrtc/modules/rtp_rtcp/source/fec_receiver_impl.h

@@ -46,6 +46,7 @@ class FecReceiverImpl : public FecReceiver {
   ForwardErrorCorrection::RecoveredPacketList recovered_packets_;
   FecPacketCounter packet_counter_;
 };
+
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_RTP_RTCP_SOURCE_FEC_RECEIVER_IMPL_H_

webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc

@@ -45,56 +45,32 @@ class ReceiverFecTest : public ::testing::Test {
         receiver_fec_(FecReceiver::Create(&rtp_data_callback_)),
         packet_generator_(kMediaSsrc) {}
 
+  // Generates |num_fec_packets| FEC packets, given |media_packets|.
   void EncodeFec(const ForwardErrorCorrection::PacketList& media_packets,
                  size_t num_fec_packets,
-                 std::list<ForwardErrorCorrection::Packet*>* fec_packets) {
+                 std::list<ForwardErrorCorrection::Packet*>* fec_packets);
-    uint8_t protection_factor = num_fec_packets * 255 / media_packets.size();
-    EXPECT_EQ(0, fec_->EncodeFec(media_packets, protection_factor, 0, false,
-                                 kFecMaskBursty, fec_packets));
-    ASSERT_EQ(num_fec_packets, fec_packets->size());
-  }
 
-  void GenerateFrame(size_t num_media_packets,
+  // Generates |num_media_packets| corresponding to a single frame.
-                     size_t frame_offset,
+  void PacketizeFrame(size_t num_media_packets,
-                     std::list<AugmentedPacket*>* augmented_packets,
+                      size_t frame_offset,
-                     ForwardErrorCorrection::PacketList* media_packets) {
+                      std::list<AugmentedPacket*>* augmented_packets,
-    packet_generator_.NewFrame(num_media_packets);
+                      ForwardErrorCorrection::PacketList* packets);
-    for (size_t i = 0; i < num_media_packets; ++i) {
-      std::unique_ptr<AugmentedPacket> next_packet(
-          packet_generator_.NextPacket(frame_offset + i, kRtpHeaderSize + 10));
-      augmented_packets->push_back(next_packet.get());
-      media_packets->push_back(std::move(next_packet));
-    }
-  }
 
+  // Build a media packet using |packet_generator_| and add it
+  // to the receiver.
+  void BuildAndAddRedMediaPacket(AugmentedPacket* packet);
+
+  // Build a FEC packet using |packet_generator_| and add it
+  // to the receiver.
+  void BuildAndAddRedFecPacket(Packet* packet);
+
+  // Ensure that |rtp_data_callback_| will be called correctly
+  // and that the recovered packet will be identical to the lost packet.
   void VerifyReconstructedMediaPacket(const AugmentedPacket& packet,
-                                      size_t times) {
+                                      size_t times);
-    // Verify that the content of the reconstructed packet is equal to the
-    // content of |packet|, and that the same content is received |times| number
-    // of times in a row.
-    EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, packet.length))
-        .With(Args<0, 1>(ElementsAreArray(packet.data, packet.length)))
-        .Times(times)
-        .WillRepeatedly(Return(true));
-  }
-
-  void BuildAndAddRedMediaPacket(AugmentedPacket* packet) {
-    std::unique_ptr<AugmentedPacket> red_packet(
-        UlpfecPacketGenerator::BuildMediaRedPacket(*packet));
-    EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket(
-                     red_packet->header.header, red_packet->data,
-                     red_packet->length, kFecPayloadType));
-  }
-
-  void BuildAndAddRedFecPacket(Packet* packet) {
-    std::unique_ptr<AugmentedPacket> red_packet(
-        packet_generator_.BuildUlpfecRedPacket(*packet));
-    EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket(
-                     red_packet->header.header, red_packet->data,
-                     red_packet->length, kFecPayloadType));
-  }
 
   void InjectGarbagePacketLength(size_t fec_garbage_offset);
+
   static void SurvivesMaliciousPacket(const uint8_t* data,
                                       size_t length,
                                       uint8_t ulpfec_payload_type);
@@ -105,30 +81,63 @@ class ReceiverFecTest : public ::testing::Test {
   UlpfecPacketGenerator packet_generator_;
 };
 
-TEST_F(ReceiverFecTest, TwoMediaOneFec) {
+void ReceiverFecTest::EncodeFec(
-  const size_t kNumFecPackets = 1;
+    const ForwardErrorCorrection::PacketList& media_packets,
-  std::list<AugmentedPacket*> augmented_media_packets;
+    size_t num_fec_packets,
-  ForwardErrorCorrection::PacketList media_packets;
+    std::list<ForwardErrorCorrection::Packet*>* fec_packets) {
-  GenerateFrame(2, 0, &augmented_media_packets, &media_packets);
+  const uint8_t protection_factor =
-  std::list<ForwardErrorCorrection::Packet*> fec_packets;
+      num_fec_packets * 255 / media_packets.size();
-  EncodeFec(media_packets, kNumFecPackets, &fec_packets);
+  // Unequal protection is turned off, and the number of important
+  // packets is thus irrelevant.
+  constexpr int kNumImportantPackets = 0;
+  constexpr bool kUseUnequalProtection = false;
+  constexpr FecMaskType kFecMaskType = kFecMaskBursty;
+  EXPECT_EQ(
+      0, fec_->EncodeFec(media_packets, protection_factor, kNumImportantPackets,
+                         kUseUnequalProtection, kFecMaskType, fec_packets));
+  ASSERT_EQ(num_fec_packets, fec_packets->size());
+}
 
-  // Recovery
+void ReceiverFecTest::PacketizeFrame(
-  auto it = augmented_media_packets.begin();
+    size_t num_media_packets,
-  BuildAndAddRedMediaPacket(*it);
+    size_t frame_offset,
-  VerifyReconstructedMediaPacket(**it, 1);
+    std::list<AugmentedPacket*>* augmented_packets,
-  EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
+    ForwardErrorCorrection::PacketList* packets) {
-  // Drop one media packet.
+  packet_generator_.NewFrame(num_media_packets);
-  auto fec_it = fec_packets.begin();
+  for (size_t i = 0; i < num_media_packets; ++i) {
-  BuildAndAddRedFecPacket(*fec_it);
+    std::unique_ptr<AugmentedPacket> next_packet(
-  ++it;
+        packet_generator_.NextPacket(frame_offset + i, kRtpHeaderSize + 10));
-  VerifyReconstructedMediaPacket(**it, 1);
+    augmented_packets->push_back(next_packet.get());
-  EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
+    packets->push_back(std::move(next_packet));
+  }
+}
 
-  FecPacketCounter counter = receiver_fec_->GetPacketCounter();
+void ReceiverFecTest::BuildAndAddRedMediaPacket(AugmentedPacket* packet) {
-  EXPECT_EQ(2U, counter.num_packets);
+  std::unique_ptr<AugmentedPacket> red_packet(
-  EXPECT_EQ(1U, counter.num_fec_packets);
+      packet_generator_.BuildMediaRedPacket(*packet));
-  EXPECT_EQ(1U, counter.num_recovered_packets);
+  EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket(
+                   red_packet->header.header, red_packet->data,
+                   red_packet->length, kFecPayloadType));
+}
+
+void ReceiverFecTest::BuildAndAddRedFecPacket(Packet* packet) {
+  std::unique_ptr<AugmentedPacket> red_packet(
+      packet_generator_.BuildUlpfecRedPacket(*packet));
+  EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket(
+                   red_packet->header.header, red_packet->data,
+                   red_packet->length, kFecPayloadType));
+}
+
+void ReceiverFecTest::VerifyReconstructedMediaPacket(
+    const AugmentedPacket& packet,
+    size_t times) {
+  // Verify that the content of the reconstructed packet is equal to the
+  // content of |packet|, and that the same content is received |times| number
+  // of times in a row.
+  EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, packet.length))
+      .With(Args<0, 1>(ElementsAreArray(packet.data, packet.length)))
+      .Times(times)
+      .WillRepeatedly(Return(true));
 }
 
 void ReceiverFecTest::InjectGarbagePacketLength(size_t fec_garbage_offset) {
@@ -138,7 +147,7 @@ void ReceiverFecTest::InjectGarbagePacketLength(size_t fec_garbage_offset) {
   const size_t kNumFecPackets = 1;
   std::list<AugmentedPacket*> augmented_media_packets;
   ForwardErrorCorrection::PacketList media_packets;
-  GenerateFrame(2, 0, &augmented_media_packets, &media_packets);
+  PacketizeFrame(2, 0, &augmented_media_packets, &media_packets);
   std::list<ForwardErrorCorrection::Packet*> fec_packets;
   EncodeFec(media_packets, kNumFecPackets, &fec_packets);
   ByteWriter<uint16_t>::WriteBigEndian(
@@ -156,6 +165,47 @@ void ReceiverFecTest::InjectGarbagePacketLength(size_t fec_garbage_offset) {
   EXPECT_EQ(0U, counter.num_recovered_packets);
 }
 
+void ReceiverFecTest::SurvivesMaliciousPacket(const uint8_t* data,
+                                              size_t length,
+                                              uint8_t ulpfec_payload_type) {
+  webrtc::RTPHeader header;
+  std::unique_ptr<webrtc::RtpHeaderParser> parser(
+      webrtc::RtpHeaderParser::Create());
+  ASSERT_TRUE(parser->Parse(data, length, &header));
+
+  webrtc::NullRtpData null_callback;
+  std::unique_ptr<webrtc::FecReceiver> receiver_fec(
+      webrtc::FecReceiver::Create(&null_callback));
+
+  receiver_fec->AddReceivedRedPacket(header, data, length, ulpfec_payload_type);
+}
+
+TEST_F(ReceiverFecTest, TwoMediaOneFec) {
+  constexpr size_t kNumFecPackets = 1u;
+  std::list<AugmentedPacket*> augmented_media_packets;
+  ForwardErrorCorrection::PacketList media_packets;
+  PacketizeFrame(2, 0, &augmented_media_packets, &media_packets);
+  std::list<ForwardErrorCorrection::Packet*> fec_packets;
+  EncodeFec(media_packets, kNumFecPackets, &fec_packets);
+
+  // Recovery
+  auto it = augmented_media_packets.begin();
+  BuildAndAddRedMediaPacket(*it);
+  VerifyReconstructedMediaPacket(**it, 1);
+  EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
+  // Drop one media packet.
+  auto fec_it = fec_packets.begin();
+  BuildAndAddRedFecPacket(*fec_it);
+  ++it;
+  VerifyReconstructedMediaPacket(**it, 1);
+  EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
+
+  FecPacketCounter counter = receiver_fec_->GetPacketCounter();
+  EXPECT_EQ(2u, counter.num_packets);
+  EXPECT_EQ(1u, counter.num_fec_packets);
+  EXPECT_EQ(1u, counter.num_recovered_packets);
+}
+
 TEST_F(ReceiverFecTest, InjectGarbageFecHeaderLengthRecovery) {
   // Byte offset 8 is the 'length recovery' field of the FEC header.
   InjectGarbagePacketLength(8);
@@ -171,7 +221,7 @@ TEST_F(ReceiverFecTest, TwoMediaTwoFec) {
   const size_t kNumFecPackets = 2;
   std::list<AugmentedPacket*> augmented_media_packets;
   ForwardErrorCorrection::PacketList media_packets;
-  GenerateFrame(2, 0, &augmented_media_packets, &media_packets);
+  PacketizeFrame(2, 0, &augmented_media_packets, &media_packets);
   std::list<ForwardErrorCorrection::Packet*> fec_packets;
   EncodeFec(media_packets, kNumFecPackets, &fec_packets);
 
@@ -193,8 +243,8 @@ TEST_F(ReceiverFecTest, TwoFramesOneFec) {
   const size_t kNumFecPackets = 1;
   std::list<AugmentedPacket*> augmented_media_packets;
   ForwardErrorCorrection::PacketList media_packets;
-  GenerateFrame(1, 0, &augmented_media_packets, &media_packets);
+  PacketizeFrame(1, 0, &augmented_media_packets, &media_packets);
-  GenerateFrame(1, 1, &augmented_media_packets, &media_packets);
+  PacketizeFrame(1, 1, &augmented_media_packets, &media_packets);
   std::list<ForwardErrorCorrection::Packet*> fec_packets;
   EncodeFec(media_packets, kNumFecPackets, &fec_packets);
 
@@ -214,8 +264,8 @@ TEST_F(ReceiverFecTest, OneCompleteOneUnrecoverableFrame) {
   const size_t kNumFecPackets = 1;
   std::list<AugmentedPacket*> augmented_media_packets;
   ForwardErrorCorrection::PacketList media_packets;
-  GenerateFrame(1, 0, &augmented_media_packets, &media_packets);
+  PacketizeFrame(1, 0, &augmented_media_packets, &media_packets);
-  GenerateFrame(2, 1, &augmented_media_packets, &media_packets);
+  PacketizeFrame(2, 1, &augmented_media_packets, &media_packets);
 
   std::list<ForwardErrorCorrection::Packet*> fec_packets;
   EncodeFec(media_packets, kNumFecPackets, &fec_packets);
@@ -237,7 +287,7 @@ TEST_F(ReceiverFecTest, MaxFramesOneFec) {
   std::list<AugmentedPacket*> augmented_media_packets;
   ForwardErrorCorrection::PacketList media_packets;
   for (size_t i = 0; i < kNumMediaPackets; ++i) {
-    GenerateFrame(1, i, &augmented_media_packets, &media_packets);
+    PacketizeFrame(1, i, &augmented_media_packets, &media_packets);
   }
   std::list<ForwardErrorCorrection::Packet*> fec_packets;
   EncodeFec(media_packets, kNumFecPackets, &fec_packets);
@@ -262,7 +312,7 @@ TEST_F(ReceiverFecTest, TooManyFrames) {
   std::list<AugmentedPacket*> augmented_media_packets;
   ForwardErrorCorrection::PacketList media_packets;
   for (size_t i = 0; i < kNumMediaPackets; ++i) {
-    GenerateFrame(1, i, &augmented_media_packets, &media_packets);
+    PacketizeFrame(1, i, &augmented_media_packets, &media_packets);
   }
   std::list<ForwardErrorCorrection::Packet*> fec_packets;
   EXPECT_EQ(-1, fec_->EncodeFec(media_packets,
@@ -278,8 +328,8 @@ TEST_F(ReceiverFecTest, PacketNotDroppedTooEarly) {
   const size_t kNumMediaPacketsBatch1 = 2;
   std::list<AugmentedPacket*> augmented_media_packets_batch1;
   ForwardErrorCorrection::PacketList media_packets_batch1;
-  GenerateFrame(kNumMediaPacketsBatch1, 0, &augmented_media_packets_batch1,
+  PacketizeFrame(kNumMediaPacketsBatch1, 0, &augmented_media_packets_batch1,
-                &media_packets_batch1);
+                 &media_packets_batch1);
   std::list<ForwardErrorCorrection::Packet*> fec_packets;
   EncodeFec(media_packets_batch1, kNumFecPacketsBatch1, &fec_packets);
 
@@ -294,7 +344,8 @@ TEST_F(ReceiverFecTest, PacketNotDroppedTooEarly) {
   std::list<AugmentedPacket*> augmented_media_packets_batch2;
   ForwardErrorCorrection::PacketList media_packets_batch2;
   for (size_t i = 0; i < kNumMediaPacketsBatch2; ++i) {
-    GenerateFrame(1, i, &augmented_media_packets_batch2, &media_packets_batch2);
+    PacketizeFrame(1, i, &augmented_media_packets_batch2,
+                   &media_packets_batch2);
   }
   for (auto it = augmented_media_packets_batch2.begin();
        it != augmented_media_packets_batch2.end(); ++it) {
@@ -319,8 +370,8 @@ TEST_F(ReceiverFecTest, PacketDroppedWhenTooOld) {
   const size_t kNumMediaPacketsBatch1 = 2;
   std::list<AugmentedPacket*> augmented_media_packets_batch1;
   ForwardErrorCorrection::PacketList media_packets_batch1;
-  GenerateFrame(kNumMediaPacketsBatch1, 0, &augmented_media_packets_batch1,
+  PacketizeFrame(kNumMediaPacketsBatch1, 0, &augmented_media_packets_batch1,
-                &media_packets_batch1);
+                 &media_packets_batch1);
   std::list<ForwardErrorCorrection::Packet*> fec_packets;
   EncodeFec(media_packets_batch1, kNumFecPacketsBatch1, &fec_packets);
 
@@ -335,7 +386,8 @@ TEST_F(ReceiverFecTest, PacketDroppedWhenTooOld) {
   std::list<AugmentedPacket*> augmented_media_packets_batch2;
   ForwardErrorCorrection::PacketList media_packets_batch2;
   for (size_t i = 0; i < kNumMediaPacketsBatch2; ++i) {
-    GenerateFrame(1, i, &augmented_media_packets_batch2, &media_packets_batch2);
+    PacketizeFrame(1, i, &augmented_media_packets_batch2,
+                   &media_packets_batch2);
   }
   for (auto it = augmented_media_packets_batch2.begin();
        it != augmented_media_packets_batch2.end(); ++it) {
@@ -363,7 +415,7 @@ TEST_F(ReceiverFecTest, OldFecPacketDropped) {
     std::list<AugmentedPacket*> frame_augmented_media_packets;
     ForwardErrorCorrection::PacketList frame_media_packets;
     std::list<ForwardErrorCorrection::Packet*> fec_packets;
-    GenerateFrame(2, 0, &frame_augmented_media_packets, &frame_media_packets);
+    PacketizeFrame(2, 0, &frame_augmented_media_packets, &frame_media_packets);
     EncodeFec(frame_media_packets, 1, &fec_packets);
     for (auto it = fec_packets.begin(); it != fec_packets.end(); ++it) {
       // Only FEC packets inserted. No packets recoverable at this time.
@@ -389,21 +441,6 @@ TEST_F(ReceiverFecTest, OldFecPacketDropped) {
   EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec());
 }
 
-void ReceiverFecTest::SurvivesMaliciousPacket(const uint8_t* data,
-                                              size_t length,
-                                              uint8_t ulpfec_payload_type) {
-  webrtc::RTPHeader header;
-  std::unique_ptr<webrtc::RtpHeaderParser> parser(
-      webrtc::RtpHeaderParser::Create());
-  ASSERT_TRUE(parser->Parse(data, length, &header));
-
-  webrtc::NullRtpData null_callback;
-  std::unique_ptr<webrtc::FecReceiver> receiver_fec(
-      webrtc::FecReceiver::Create(&null_callback));
-
-  receiver_fec->AddReceivedRedPacket(header, data, length, ulpfec_payload_type);
-}
-
 TEST_F(ReceiverFecTest, TruncatedPacketWithFBitSet) {
   const uint8_t kTruncatedPacket[] = {0x80,
                                       0x2a,