Add FlexfecReceiver unit test for infinite recovery loop.

webrtc/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 283 matching lines @@
   auto fec_it = fec_packets.begin();
   std::unique_ptr<Packet> packet_with_rtp_header =
       packet_generator_.BuildFlexfecPacket(**fec_it);
   media_it++;
   EXPECT_CALL(recovered_packet_receiver_,
               OnRecoveredPacket(_, (*media_it)->length))
       .With(
           Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)));
   receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
 
-  // Receive FEC packet again.
+  // Receive the FEC packet again, but do not call back.
   receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
 
-  // Do not call back again.
+  // Receive the first media packet again, but do not call back.
+  media_it = media_packets.begin();
+  receiver_.OnRtpPacket(ParsePacket(**media_it));
+
+  // Receive the second media packet again (the one recovered above),
+  // but do not call back again.
+  media_it++;

[danilchap, 2017/05/23 13:07:15]

nit: ++media_it;

[brandtr, 2017/05/23 13:19:17]

The style of this file is the other way :/

+  receiver_.OnRtpPacket(ParsePacket(**media_it));
 }
 
 // Here we are implicitly assuming packet masks that are suitable for
 // this type of 50% correlated loss. If we are changing our precomputed
 // packet masks, this test might need to be updated.
 TEST_F(FlexfecReceiverTest, RecoversFrom50PercentLoss) {
   const size_t kNumFecPackets = 5;
   const size_t kNumFrames = 2 * kNumFecPackets;
   const size_t kNumMediaPacketsPerFrame = 1;
 
@@ skipping 139 matching lines @@
   // Add FEC packets.
   auto fec_it = fec_packets.begin();
   std::unique_ptr<Packet> packet_with_rtp_header;
   while (fec_it != fec_packets.end()) {
     packet_with_rtp_header = packet_generator_.BuildFlexfecPacket(**fec_it);
     receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
     ++fec_it;
   }
 }
 
+// Recovered media packets may be fed back into the FlexfecReceiver by the
+// callback. This test ensures the idempotency of such a situation.
+TEST_F(FlexfecReceiverTest, RecoveryCallbackDoesNotLoopInfinitely) {
+  class LoopbackRecoveredPacketReceiver : public RecoveredPacketReceiver {
+   public:
+    const int kMaxRecursionDepth = 10;
+
+    LoopbackRecoveredPacketReceiver()
+        : receiver_(nullptr),
+          did_receive_call_back_(false),
+          recursion_depth_(0),
+          deep_recursion_(false) {}
+
+    void SetReceiver(FlexfecReceiver* receiver) { receiver_ = receiver; }
+    bool DidReceiveCallback() const { return did_receive_call_back_; }
+    bool DeepRecursion() const { return deep_recursion_; }
+
+    // Implements RecoveredPacketReceiver.
+    void OnRecoveredPacket(const uint8_t* packet, size_t length) {
+      RtpPacketReceived parsed_packet;
+      EXPECT_TRUE(parsed_packet.Parse(packet, length));
+
+      did_receive_call_back_ = true;
+
+      if (recursion_depth_ > kMaxRecursionDepth) {

[danilchap, 2017/05/23 13:07:14]

alternative approach:
ASSERT_LT(recursion_depth_, kMaxRecursionDepth);
that will return too prevening stack overflow, but will point to this line and
save from extra member and accessor

[brandtr, 2017/05/23 13:19:17]

Good idea! Done.

[nisse-webrtc, 2017/05/23 14:05:25]

ASSERT_* does some magic which works as intended only when used in the top-level
test function.

It might work, but in a unexpected way, because (1) ASSERT_* is more or less is
the same as EXPECT_* + a return statement on failure, and (2) the return type of
this function happens to be void. I'm not sure it's a good idea to rely on that.
If you really want to, add a comment to not mislead readers to think that
ASSERT_* outside of top-level works in general; there's already too much
confusion around this.

[brandtr, 2017/05/23 15:02:47]

Better avoid confusion and use the old approach then.

Do ASSERTs only work at the top-level because the tests have return type void?

+        deep_recursion_ = true;
+        return;
+      }
+      ++recursion_depth_;
+      RTC_DCHECK(receiver_);
+      receiver_->OnRtpPacket(parsed_packet);
+      --recursion_depth_;
+    }
+
+   private:
+    FlexfecReceiver* receiver_;
+    bool did_receive_call_back_;
+    int recursion_depth_;
+    bool deep_recursion_;
+  } loopback_recovered_packet_receiver;
+
+  // Feed recovered packets back into |receiver|.
+  FlexfecReceiver receiver(kFlexfecSsrc, kMediaSsrc,
+                           &loopback_recovered_packet_receiver);
+  loopback_recovered_packet_receiver.SetReceiver(&receiver);
+
+  const size_t kNumMediaPackets = 2;
+  const size_t kNumFecPackets = 1;
+
+  PacketList media_packets;
+  PacketizeFrame(kNumMediaPackets, 0, &media_packets);
+  std::list<Packet*> fec_packets = EncodeFec(media_packets, kNumFecPackets);
+
+  // Receive first media packet but drop second.
+  auto media_it = media_packets.begin();
+  receiver.OnRtpPacket(ParsePacket(**media_it));
+
+  // Receive FEC packet and verify that a packet was recovered.
+  auto fec_it = fec_packets.begin();
+  std::unique_ptr<Packet> packet_with_rtp_header =
+      packet_generator_.BuildFlexfecPacket(**fec_it);
+  receiver.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
+  EXPECT_TRUE(loopback_recovered_packet_receiver.DidReceiveCallback());
+  EXPECT_FALSE(loopback_recovered_packet_receiver.DeepRecursion());
+}
+
 TEST_F(FlexfecReceiverTest, CalculatesNumberOfPackets) {
   const size_t kNumMediaPackets = 2;
   const size_t kNumFecPackets = 1;
 
   PacketList media_packets;
   PacketizeFrame(kNumMediaPackets, 0, &media_packets);
   std::list<Packet*> fec_packets = EncodeFec(media_packets, kNumFecPackets);
 
   // Receive first media packet but drop second.
   auto media_it = media_packets.begin();
@@ skipping 11 matching lines @@
   receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
 
   // Check stats calculations.
   FecPacketCounter packet_counter = receiver_.GetPacketCounter();
   EXPECT_EQ(2U, packet_counter.num_packets);
   EXPECT_EQ(1U, packet_counter.num_fec_packets);
   EXPECT_EQ(1U, packet_counter.num_recovered_packets);
 }
 
 }  // namespace webrtc