Wire up FlexfecSender in RTPSender and add unit tests.

webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc

Index: webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index d02d25eb2134166e0ec4a6dc1c362a5982f2c934..2214d103f0c7ccaf457f44f8f2f414374c962f07 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -147,7 +147,7 @@ class RtpSenderTest : public ::testing::Test {
   void SetUpRtpSender(bool pacer) {
     rtp_sender_.reset(new RTPSender(
         false, &fake_clock_, &transport_, pacer ? &mock_paced_sender_ : nullptr,
-        &seq_num_allocator_, nullptr, nullptr, nullptr, nullptr,
+        nullptr, &seq_num_allocator_, nullptr, nullptr, nullptr, nullptr,
         &mock_rtc_event_log_, &send_packet_observer_,
         &retransmission_rate_limiter_));
     rtp_sender_->SetSequenceNumber(kSeqNum);
@@ -239,7 +239,7 @@ class RtpSenderVideoTest : public RtpSenderTest {
     // TODO(pbos): Set up to use pacer.
     SetUpRtpSender(false);
     rtp_sender_video_.reset(
-        new RTPSenderVideo(&fake_clock_, rtp_sender_.get()));
+        new RTPSenderVideo(&fake_clock_, rtp_sender_.get(), nullptr));
   }
   std::unique_ptr<RTPSenderVideo> rtp_sender_video_;
 };
@@ -440,10 +440,9 @@ TEST_F(RtpSenderTestWithoutPacer, AssignSequenceNumberSetPaddingTimestamps) {
 
 TEST_F(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) {
   rtp_sender_.reset(new RTPSender(
-      false, &fake_clock_, &transport_, nullptr,
-      &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, nullptr,
-      &mock_rtc_event_log_, &send_packet_observer_,
-      &retransmission_rate_limiter_));
+      false, &fake_clock_, &transport_, nullptr, nullptr, &seq_num_allocator_,
+      &feedback_observer_, nullptr, nullptr, nullptr, &mock_rtc_event_log_,
+      &send_packet_observer_, &retransmission_rate_limiter_));
   EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
                    kRtpExtensionTransportSequenceNumber,
                    kTransportSequenceNumberExtensionId));
@@ -486,11 +485,11 @@ TEST_F(RtpSenderTestWithoutPacer, OnSendPacketUpdated) {
 }
 
 TEST_F(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) {
-  rtp_sender_.reset(new RTPSender(
-      false, &fake_clock_, &transport_, &mock_paced_sender_,
-      &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, nullptr,
-      &mock_rtc_event_log_, &send_packet_observer_,
-      &retransmission_rate_limiter_));
+  rtp_sender_.reset(
+      new RTPSender(false, &fake_clock_, &transport_, &mock_paced_sender_,
+                    nullptr, &seq_num_allocator_, &feedback_observer_, nullptr,
+                    nullptr, nullptr, &mock_rtc_event_log_,
+                    &send_packet_observer_, &retransmission_rate_limiter_));
   rtp_sender_->SetStorePacketsStatus(true, 10);
   EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
                    kRtpExtensionTransportSequenceNumber,
@@ -768,7 +767,7 @@ TEST_F(RtpSenderTest, OnSendPacketNotUpdatedForRetransmits) {
 
 TEST_F(RtpSenderTest, OnSendPacketNotUpdatedWithoutSeqNumAllocator) {
   rtp_sender_.reset(new RTPSender(
-      false, &fake_clock_, &transport_, &mock_paced_sender_,
+      false, &fake_clock_, &transport_, &mock_paced_sender_, nullptr,
       nullptr /* TransportSequenceNumberAllocator */, nullptr, nullptr, nullptr,
       nullptr, nullptr, &send_packet_observer_, &retransmission_rate_limiter_));
   EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
@@ -791,7 +790,7 @@ TEST_F(RtpSenderTest, SendRedundantPayloads) {
   MockTransport transport;
   rtp_sender_.reset(new RTPSender(
       false, &fake_clock_, &transport, &mock_paced_sender_, nullptr, nullptr,
-      nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr,
+      nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr,
       &retransmission_rate_limiter_));
 
   rtp_sender_->SetSequenceNumber(kSeqNum);
@@ -900,6 +899,80 @@ TEST_F(RtpSenderTestWithoutPacer, SendGenericVideo) {
   EXPECT_EQ(0, memcmp(payload, payload_data, sizeof(payload)));
 }
 
+TEST_F(RtpSenderTest, SendFlexfecPackets) {
+  constexpr int kFlexfecPayloadType = 118;
+  constexpr uint32_t kMediaSsrc = 1234;
+  constexpr uint32_t kFlexfecSsrc = 5678;
+  const std::vector<RtpExtension> kNoRtpExtensions;
+  FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexfecSsrc, kMediaSsrc,
+                               kNoRtpExtensions, &fake_clock_);
+
+  // Reset |rtp_sender_| to use FlexFEC.
+  rtp_sender_.reset(
+      new RTPSender(false, &fake_clock_, &transport_, &mock_paced_sender_,
+                    &flexfec_sender, &seq_num_allocator_, nullptr, nullptr,
+                    nullptr, nullptr, &mock_rtc_event_log_,
+                    &send_packet_observer_, &retransmission_rate_limiter_));
+  rtp_sender_->SetSSRC(kMediaSsrc);
+  rtp_sender_->SetSequenceNumber(kSeqNum);
+  rtp_sender_->SetSendPayloadType(kPayload);
+  rtp_sender_->SetStorePacketsStatus(true, 10);
+
+  // Parameters selected to generate a single FEC packet per media packet.
+  FecProtectionParams params;
+  params.fec_rate = 15;
+  params.max_fec_frames = 1;
+  params.fec_mask_type = kFecMaskRandom;
+  rtp_sender_->SetFecParameters(params, params);
+
+  EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority,
+                                               kMediaSsrc, _, _, _, false));
+  EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kLowPriority,
+                                               kFlexfecSsrc, _, _, _, false));
+  SendGenericPayload();
+  // TODO(brandtr): Update this code when the FlexFEC packets are no longer
+  // lost between PacedSender and RTPSender.
+  constexpr int kNumPacketsExpected = 1;

[danilchap, 2016/11/09 11:41:44]

rather than test for incorrect behavior, make tests less strict (but leave TODO
to make checks stricter):
EXPECT_CALL(...).Times(AtLeast(1));
...
EXPECT_LE(1, transport_.packets_sent());

[brandtr, 2016/11/09 15:16:30]

Good idea. Done.

+  EXPECT_CALL(mock_rtc_event_log_,
+              LogRtpHeader(PacketDirection::kOutgoingPacket, _, _, _))
+      .Times(kNumPacketsExpected);
+  EXPECT_TRUE(rtp_sender_->TimeToSendPacket(
+      kSeqNum, fake_clock_.TimeInMilliseconds(), false, 0));
+  EXPECT_EQ(kNumPacketsExpected, transport_.packets_sent());
+}
+
+TEST_F(RtpSenderTestWithoutPacer, SendFlexfecPackets) {
+  constexpr int kFlexfecPayloadType = 118;
+  constexpr uint32_t kMediaSsrc = 1234;
+  constexpr uint32_t kFlexfecSsrc = 5678;
+  const std::vector<RtpExtension> kNoRtpExtensions;
+  FlexfecSender flexfec_sender(kFlexfecPayloadType, kFlexfecSsrc, kMediaSsrc,
+                               kNoRtpExtensions, &fake_clock_);
+
+  // Reset |rtp_sender_| to use FlexFEC.
+  rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, nullptr,
+                                  &flexfec_sender, &seq_num_allocator_, nullptr,
+                                  nullptr, nullptr, nullptr,
+                                  &mock_rtc_event_log_, &send_packet_observer_,
+                                  &retransmission_rate_limiter_));
+  rtp_sender_->SetSSRC(kMediaSsrc);
+  rtp_sender_->SetSequenceNumber(kSeqNum);
+  rtp_sender_->SetSendPayloadType(kPayload);
+
+  // Parameters selected to generate a single FEC packet per media packet.
+  FecProtectionParams params;
+  params.fec_rate = 15;
+  params.max_fec_frames = 1;
+  params.fec_mask_type = kFecMaskRandom;
+  rtp_sender_->SetFecParameters(params, params);
+
+  EXPECT_CALL(mock_rtc_event_log_,
+              LogRtpHeader(PacketDirection::kOutgoingPacket, _, _, _))
+      .Times(2);
+  SendGenericPayload();
+  EXPECT_EQ(2, transport_.packets_sent());
+}

[danilchap, 2016/11/09 11:41:44]

check that one of those packets is flexfec and another is media (i.e. check
ssrcs and payloadtypes).
If you know media should always go before fec, it is easy.
if order doesn't matter, then still possible, but matcher might be complicated.

[brandtr, 2016/11/09 15:16:30]

Yes, media will always come out before FEC. Adding.

+
 TEST_F(RtpSenderTest, FrameCountCallbacks) {
   class TestCallback : public FrameCountObserver {
    public:
@@ -920,8 +993,8 @@ TEST_F(RtpSenderTest, FrameCountCallbacks) {
 
   rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_,
                                   &mock_paced_sender_, nullptr, nullptr,
-                                  nullptr, &callback, nullptr, nullptr, nullptr,
-                                  &retransmission_rate_limiter_));
+                                  nullptr, nullptr, &callback, nullptr, nullptr,
+                                  nullptr, &retransmission_rate_limiter_));
 
   char payload_name[RTP_PAYLOAD_NAME_SIZE] = "GENERIC";
   const uint8_t payload_type = 127;
@@ -980,9 +1053,10 @@ TEST_F(RtpSenderTest, BitrateCallbacks) {
     uint32_t total_bitrate_;
     uint32_t retransmit_bitrate_;
   } callback;
-  rtp_sender_.reset(new RTPSender(
-      false, &fake_clock_, &transport_, nullptr, nullptr, nullptr, &callback,
-      nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_));
+  rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, nullptr,
+                                  nullptr, nullptr, nullptr, &callback, nullptr,
+                                  nullptr, nullptr, nullptr,
+                                  &retransmission_rate_limiter_));
 
   // Simulate kNumPackets sent with kPacketInterval ms intervals, with the
   // number of packets selected so that we fill (but don't overflow) the one
@@ -1037,9 +1111,10 @@ class RtpSenderAudioTest : public RtpSenderTest {
 
   void SetUp() override {
     payload_ = kAudioPayload;
-    rtp_sender_.reset(new RTPSender(
-        true, &fake_clock_, &transport_, nullptr, nullptr, nullptr, nullptr,
-        nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_));
+    rtp_sender_.reset(new RTPSender(true, &fake_clock_, &transport_, nullptr,
+                                    nullptr, nullptr, nullptr, nullptr, nullptr,
+                                    nullptr, nullptr, nullptr,
+                                    &retransmission_rate_limiter_));
     rtp_sender_->SetSequenceNumber(kSeqNum);
   }
 };