Let FlexfecReceiveStreamImpl send RTCP RRs.

webrtc/video/end_to_end_tests.cc

 /*
  *  Copyright (c) 2013 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.
  */
 #include <algorithm>
@@ skipping 686 matching lines @@
     rtc::CriticalSection crit_;
     std::set<uint32_t> protected_sequence_numbers_ GUARDED_BY(crit_);
     // Since several packets can have the same timestamp a multiset is used
     // instead of a set.
     std::multiset<uint32_t> protected_timestamps_ GUARDED_BY(crit_);
   } test;
 
   RunBaseTest(&test);
 }
 
-TEST_P(EndToEndTest, CanReceiveFlexfec) {
-  class FlexfecRenderObserver : public test::EndToEndTest,
-                                public rtc::VideoSinkInterface<VideoFrame> {
-   public:
-    FlexfecRenderObserver()
-        : EndToEndTest(kDefaultTimeoutMs), random_(0xcafef00d1) {}
+class FlexfecRenderObserver : public test::EndToEndTest,
+                              public rtc::VideoSinkInterface<VideoFrame> {
+ public:
+  static constexpr uint32_t kVideoLocalSsrc = 123;
+  static constexpr uint32_t kFlexfecLocalSsrc = 456;
 
-    size_t GetNumFlexfecStreams() const override { return 1; }
+  explicit FlexfecRenderObserver(bool expect_flexfec_rtcp)
+      : test::EndToEndTest(test::CallTest::kDefaultTimeoutMs),
+        expect_flexfec_rtcp_(expect_flexfec_rtcp),
+        received_flexfec_rtcp_(false),
+        random_(0xcafef00d1) {}
 
-   private:
-    Action OnSendRtp(const uint8_t* packet, size_t length) override {
-      rtc::CritScope lock(&crit_);
-      RTPHeader header;
-      EXPECT_TRUE(parser_->Parse(packet, length, &header));
+  size_t GetNumFlexfecStreams() const override { return 1; }
 
-      uint8_t payload_type = header.payloadType;
-      if (payload_type != kFakeVideoSendPayloadType) {
-        EXPECT_EQ(kFlexfecPayloadType, payload_type);
+ private:
+  Action OnSendRtp(const uint8_t* packet, size_t length) override {
+    rtc::CritScope lock(&crit_);
+    RTPHeader header;
+    EXPECT_TRUE(parser_->Parse(packet, length, &header));
+
+    uint8_t payload_type = header.payloadType;
+    if (payload_type != test::CallTest::kFakeVideoSendPayloadType) {
+      EXPECT_EQ(test::CallTest::kFlexfecPayloadType, payload_type);
+    }
+
+    // Is this a retransmitted media packet? From the perspective of FEC, this
+    // packet is then no longer dropped, so remove it from the list of
+    // dropped packets.
+    if (payload_type == test::CallTest::kFakeVideoSendPayloadType) {
+      auto seq_num_it = dropped_sequence_numbers_.find(header.sequenceNumber);
+      if (seq_num_it != dropped_sequence_numbers_.end()) {
+        dropped_sequence_numbers_.erase(seq_num_it);
+        auto ts_it = dropped_timestamps_.find(header.timestamp);
+        EXPECT_NE(ts_it, dropped_timestamps_.end());
+        dropped_timestamps_.erase(ts_it);
+
+        return SEND_PACKET;
+      }
+    }
+
+    // Simulate 5% packet loss. Record what media packets, and corresponding
+    // timestamps, that were dropped.
+    if (random_.Rand(1, 100) <= 5) {
+      if (payload_type == test::CallTest::kFakeVideoSendPayloadType) {
+        dropped_sequence_numbers_.insert(header.sequenceNumber);
+        dropped_timestamps_.insert(header.timestamp);
       }
 
-      // Is this a retransmitted media packet? From the perspective of FEC, this
-      // packet is then no longer dropped, so remove it from the list of
-      // dropped packets.
-      if (payload_type == kFakeVideoSendPayloadType) {
-        auto seq_num_it = dropped_sequence_numbers_.find(header.sequenceNumber);
-        if (seq_num_it != dropped_sequence_numbers_.end()) {
-          dropped_sequence_numbers_.erase(seq_num_it);
-          auto ts_it = dropped_timestamps_.find(header.timestamp);
-          EXPECT_NE(ts_it, dropped_timestamps_.end());
-          dropped_timestamps_.erase(ts_it);
-
-          return SEND_PACKET;
-        }
-      }
-
-      // Simulate 5% packet loss. Record what media packets, and corresponding
-      // timestamps, that were dropped.
-      if (random_.Rand(1, 100) <= 5) {
-        if (payload_type == kFakeVideoSendPayloadType) {
-          dropped_sequence_numbers_.insert(header.sequenceNumber);
-          dropped_timestamps_.insert(header.timestamp);
-        }
-
-        return DROP_PACKET;
-      }
-
-      return SEND_PACKET;
+      return DROP_PACKET;
     }
 
-    void OnFrame(const VideoFrame& video_frame) override {
-      rtc::CritScope lock(&crit_);
-      // Rendering frame with timestamp of packet that was dropped -> FEC
-      // protection worked.
-      auto it = dropped_timestamps_.find(video_frame.timestamp());
-      if (it != dropped_timestamps_.end())
-        observation_complete_.Set();
+    return SEND_PACKET;
+  }
+
+  Action OnReceiveRtcp(const uint8_t* data, size_t length) override {
+    test::RtcpPacketParser parser;
+
+    parser.Parse(data, length);
+    if (parser.sender_ssrc() == kFlexfecLocalSsrc) {
+      received_flexfec_rtcp_ = true;

[danilchap, 2017/01/11 12:13:07]

probably set to true only if (!report_blocks.empty())

[brandtr, 2017/01/11 12:25:36]

Yes, this is better, as it requires the EXPECTs below to always run at least
once.

+
+      EXPECT_EQ(1, parser.receiver_report()->num_packets());
+      const std::vector<rtcp::ReportBlock>& report_blocks =
+          parser.receiver_report()->report_blocks();
+      if (!report_blocks.empty()) {

[brandtr, 2017/01/11 10:47:52]

This check is needed because the newly created RTP module spits out one or two
empty RTCP packets, before sending proper RTCP RRs for FlexFEC.

These empty RTCP packets seem to be sent even if no RTP statistics are provided
to the RTP module. Is this correct/reasonable behaviour of the RTP module?

[danilchap, 2017/01/11 12:13:07]

should be correct, not so sure if reasonable.

quote from rfc3550 section 6.4.2: "An empty RR packet (RC = 0) MUST be put at
the head of a compound RTCP packet when there is no data transmission or
reception to report."

[brandtr, 2017/01/11 12:25:36]

Acknowledged.

+        EXPECT_EQ(1U, report_blocks.size());
+        EXPECT_EQ(test::CallTest::kFlexfecSendSsrc,
+                  report_blocks[0].source_ssrc());
+      }
     }
 
-    void ModifyVideoConfigs(
-        VideoSendStream::Config* send_config,
-        std::vector<VideoReceiveStream::Config>* receive_configs,
-        VideoEncoderConfig* encoder_config) override {
-      (*receive_configs)[0].renderer = this;
+    return SEND_PACKET;
+  }
+
+  void OnFrame(const VideoFrame& video_frame) override {
+    rtc::CritScope lock(&crit_);
+    // Rendering frame with timestamp of packet that was dropped -> FEC
+    // protection worked.
+    auto it = dropped_timestamps_.find(video_frame.timestamp());
+    if (it != dropped_timestamps_.end()) {
+      if (!expect_flexfec_rtcp_ || received_flexfec_rtcp_) {
+        observation_complete_.Set();
+      }
     }
+  }
 
-    void PerformTest() override {
-      EXPECT_TRUE(Wait())
-          << "Timed out waiting for dropped frames to be rendered.";
-    }
+  void ModifyVideoConfigs(
+      VideoSendStream::Config* send_config,
+      std::vector<VideoReceiveStream::Config>* receive_configs,
+      VideoEncoderConfig* encoder_config) override {
+    (*receive_configs)[0].rtp.local_ssrc = kVideoLocalSsrc;
+    (*receive_configs)[0].renderer = this;
+  }
 
-    rtc::CriticalSection crit_;
-    std::set<uint32_t> dropped_sequence_numbers_ GUARDED_BY(crit_);
-    // Since several packets can have the same timestamp a multiset is used
-    // instead of a set.
-    std::multiset<uint32_t> dropped_timestamps_ GUARDED_BY(crit_);
-    Random random_;
-  } test;
+  void ModifyFlexfecConfigs(
+      std::vector<FlexfecReceiveStream::Config>* receive_configs) override {
+    (*receive_configs)[0].local_ssrc = kFlexfecLocalSsrc;
+  }
 
+  void PerformTest() override {
+    EXPECT_TRUE(Wait())
+        << "Timed out waiting for dropped frames to be rendered.";
+  }
+
+  rtc::CriticalSection crit_;
+  std::set<uint32_t> dropped_sequence_numbers_ GUARDED_BY(crit_);
+  // Since several packets can have the same timestamp a multiset is used
+  // instead of a set.
+  std::multiset<uint32_t> dropped_timestamps_ GUARDED_BY(crit_);
+  bool expect_flexfec_rtcp_;
+  bool received_flexfec_rtcp_;
+  Random random_;
+};
+
+TEST_P(EndToEndTest, ReceivesFlexfec) {
+  FlexfecRenderObserver test(false);
   RunBaseTest(&test);
 }
 
+TEST_P(EndToEndTest, ReceivesFlexfecAndSendsCorrespondingRtcp) {
+  FlexfecRenderObserver test(true);
+  RunBaseTest(&test);
+}
+
 TEST_P(EndToEndTest, ReceivedUlpfecPacketsNotNacked) {
   class UlpfecNackObserver : public test::EndToEndTest {
    public:
     UlpfecNackObserver()
         : EndToEndTest(kDefaultTimeoutMs),
           state_(kFirstPacket),
           ulpfec_sequence_number_(0),
           has_last_sequence_number_(false),
           last_sequence_number_(0),
           encoder_(VP8Encoder::Create()),
@@ skipping 3267 matching lines @@
     std::unique_ptr<VideoEncoder> encoder_;
     std::unique_ptr<VideoDecoder> decoder_;
     rtc::CriticalSection crit_;
     int recorded_frames_ GUARDED_BY(crit_);
   } test(this);
 
   RunBaseTest(&test);
 }
 
 }  // namespace webrtc