Add RequestKeyFrame with Fir to RtcpTransceiver

Bug: webrtc:8239
Change-Id: If094d434a7be20cdff5c80447322d68a4a7a4580
Reviewed-on: https://webrtc-review.googlesource.com/22961
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20833}
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver.cc b/modules/rtp_rtcp/source/rtcp_transceiver.cc
index 7c668b9..7feca3f 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver.cc
+++ b/modules/rtp_rtcp/source/rtcp_transceiver.cc
@@ -90,18 +90,32 @@
   });
 }
 
-void RtcpTransceiver::RequestKeyFrame(std::vector<uint32_t> ssrcs) {
+void RtcpTransceiver::SendPictureLossIndication(std::vector<uint32_t> ssrcs) {
   // TODO(danilchap): Replace with lambda with move capture when available.
-  struct RequestKeyFrameClosure {
+  struct Closure {
     void operator()() {
       if (ptr)
-        ptr->RequestKeyFrame(ssrcs);
+        ptr->SendPictureLossIndication(ssrcs);
     }
 
     rtc::WeakPtr<RtcpTransceiverImpl> ptr;
     std::vector<uint32_t> ssrcs;
   };
-  task_queue_->PostTask(RequestKeyFrameClosure{ptr_, std::move(ssrcs)});
+  task_queue_->PostTask(Closure{ptr_, std::move(ssrcs)});
+}
+
+void RtcpTransceiver::SendFullIntraRequest(std::vector<uint32_t> ssrcs) {
+  // TODO(danilchap): Replace with lambda with move capture when available.
+  struct Closure {
+    void operator()() {
+      if (ptr)
+        ptr->SendFullIntraRequest(ssrcs);
+    }
+
+    rtc::WeakPtr<RtcpTransceiverImpl> ptr;
+    std::vector<uint32_t> ssrcs;
+  };
+  task_queue_->PostTask(Closure{ptr_, std::move(ssrcs)});
 }
 
 }  // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver.h b/modules/rtp_rtcp/source/rtcp_transceiver.h
index 4e571ee..9d69545 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver.h
+++ b/modules/rtp_rtcp/source/rtcp_transceiver.h
@@ -44,7 +44,11 @@
   // Stops sending REMB in following compound packets.
   void UnsetRemb();
 
-  void RequestKeyFrame(std::vector<uint32_t> ssrcs);
+  // Request new key frame.
+  // using PLI, https://tools.ietf.org/html/rfc4585#section-6.3.1.1
+  void SendPictureLossIndication(std::vector<uint32_t> ssrcs);
+  // using FIR, https://tools.ietf.org/html/rfc5104#section-4.3.1.2
+  void SendFullIntraRequest(std::vector<uint32_t> ssrcs);
 
  private:
   rtc::TaskQueue* const task_queue_;
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc
index 52bc489..b870dd1 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc
+++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc
@@ -17,6 +17,7 @@
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtcp_packet.h"
 #include "modules/rtp_rtcp/source/rtcp_packet/common_header.h"
+#include "modules/rtp_rtcp/source/rtcp_packet/fir.h"
 #include "modules/rtp_rtcp/source/rtcp_packet/pli.h"
 #include "modules/rtp_rtcp/source/rtcp_packet/receiver_report.h"
 #include "modules/rtp_rtcp/source/rtcp_packet/report_block.h"
@@ -29,6 +30,19 @@
 #include "rtc_base/timeutils.h"
 
 namespace webrtc {
+namespace {
+
+struct SenderReportTimes {
+  int64_t local_received_time_us;
+  NtpTime remote_sent_time;
+};
+
+}  // namespace
+
+struct RtcpTransceiverImpl::RemoteSenderState {
+  uint8_t fir_sequence_number = 0;
+  rtc::Optional<SenderReportTimes> last_received_sender_report;
+};
 
 // Helper to put several RTCP packets into lower layer datagram composing
 // Compound or Reduced-Size RTCP packet, as defined by RFC 5506 section 2.
@@ -116,25 +130,30 @@
   remb_.reset();
 }
 
-void RtcpTransceiverImpl::RequestKeyFrame(
+void RtcpTransceiverImpl::SendPictureLossIndication(
     rtc::ArrayView<const uint32_t> ssrcs) {
   RTC_DCHECK(!ssrcs.empty());
-  const uint32_t sender_ssrc = config_.feedback_ssrc;
-  PacketSender sender(config_.outgoing_transport, config_.max_packet_size);
-  if (config_.rtcp_mode == RtcpMode::kCompound)
-    CreateCompoundPacket(&sender);
+  SendImmediateFeedback([this, ssrcs](PacketSender* sender) {
+    for (uint32_t media_ssrc : ssrcs) {
+      rtcp::Pli pli;
+      pli.SetSenderSsrc(config_.feedback_ssrc);
+      pli.SetMediaSsrc(media_ssrc);
+      sender->AppendPacket(pli);
+    }
+  });
+}
 
-  for (uint32_t media_ssrc : ssrcs) {
-    rtcp::Pli pli;
-    pli.SetSenderSsrc(sender_ssrc);
-    pli.SetMediaSsrc(media_ssrc);
-    sender.AppendPacket(pli);
-  }
-
-  sender.Send();
-
-  if (config_.rtcp_mode == RtcpMode::kCompound)
-    ReschedulePeriodicCompoundPackets();
+void RtcpTransceiverImpl::SendFullIntraRequest(
+    rtc::ArrayView<const uint32_t> ssrcs) {
+  RTC_DCHECK(!ssrcs.empty());
+  SendImmediateFeedback([this, ssrcs](PacketSender* sender) {
+    rtcp::Fir fir;
+    fir.SetSenderSsrc(config_.feedback_ssrc);
+    for (uint32_t media_ssrc : ssrcs)
+      fir.AddRequestTo(media_ssrc,
+                       remote_senders_[media_ssrc].fir_sequence_number++);
+    sender->AppendPacket(fir);
+  });
 }
 
 void RtcpTransceiverImpl::HandleReceivedPacket(
@@ -145,10 +164,12 @@
       rtcp::SenderReport sender_report;
       if (!sender_report.Parse(rtcp_packet_header))
         return;
-      SenderReportTimes& last =
-          last_received_sender_reports_[sender_report.sender_ssrc()];
-      last.local_received_time_us = now_us;
-      last.remote_sent_time = sender_report.ntp();
+      rtc::Optional<SenderReportTimes>& last =
+          remote_senders_[sender_report.sender_ssrc()]
+              .last_received_sender_report;
+      last.emplace();
+      last->local_received_time_us = now_us;
+      last->remote_sent_time = sender_report.ntp();
       break;
     }
   }
@@ -220,6 +241,20 @@
   sender.Send();
 }
 
+void RtcpTransceiverImpl::SendImmediateFeedback(
+    rtc::FunctionView<void(PacketSender*)> append_feedback) {
+  PacketSender sender(config_.outgoing_transport, config_.max_packet_size);
+  if (config_.rtcp_mode == RtcpMode::kCompound)
+    CreateCompoundPacket(&sender);
+
+  append_feedback(&sender);
+
+  sender.Send();
+
+  if (config_.rtcp_mode == RtcpMode::kCompound)
+    ReschedulePeriodicCompoundPackets();
+}
+
 std::vector<rtcp::ReportBlock> RtcpTransceiverImpl::CreateReportBlocks() {
   if (!config_.receive_statistics)
     return {};
@@ -229,10 +264,11 @@
       config_.receive_statistics->RtcpReportBlocks(
           rtcp::ReceiverReport::kMaxNumberOfReportBlocks);
   for (rtcp::ReportBlock& report_block : report_blocks) {
-    auto it = last_received_sender_reports_.find(report_block.source_ssrc());
-    if (it == last_received_sender_reports_.end())
+    auto it = remote_senders_.find(report_block.source_ssrc());
+    if (it == remote_senders_.end() || !it->second.last_received_sender_report)
       continue;
-    const SenderReportTimes& last_sender_report = it->second;
+    const SenderReportTimes& last_sender_report =
+        *it->second.last_received_sender_report;
     report_block.SetLastSr(CompactNtp(last_sender_report.remote_sent_time));
     report_block.SetDelayLastSr(SaturatedUsToCompactNtp(
         rtc::TimeMicros() - last_sender_report.local_received_time_us));
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h
index aed850b..a89f3e1 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h
+++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h
@@ -23,6 +23,7 @@
 #include "modules/rtp_rtcp/source/rtcp_packet/report_block.h"
 #include "modules/rtp_rtcp/source/rtcp_transceiver_config.h"
 #include "rtc_base/constructormagic.h"
+#include "rtc_base/function_view.h"
 #include "rtc_base/weak_ptr.h"
 #include "system_wrappers/include/ntp_time.h"
 
@@ -36,26 +37,19 @@
   explicit RtcpTransceiverImpl(const RtcpTransceiverConfig& config);
   ~RtcpTransceiverImpl();
 
-  // Handles incoming rtcp packets.
   void ReceivePacket(rtc::ArrayView<const uint8_t> packet, int64_t now_us);
 
-  // Sends RTCP packets starting with a sender or receiver report.
   void SendCompoundPacket();
 
-  // (REMB) Receiver Estimated Max Bitrate.
-  // Includes REMB in following compound packets.
   void SetRemb(int bitrate_bps, std::vector<uint32_t> ssrcs);
-  // Stops sending REMB in following compound packets.
   void UnsetRemb();
 
-  void RequestKeyFrame(rtc::ArrayView<const uint32_t> ssrcs);
+  void SendPictureLossIndication(rtc::ArrayView<const uint32_t> ssrcs);
+  void SendFullIntraRequest(rtc::ArrayView<const uint32_t> ssrcs);
 
  private:
   class PacketSender;
-  struct SenderReportTimes {
-    int64_t local_received_time_us;
-    NtpTime remote_sent_time;
-  };
+  struct RemoteSenderState;
 
   void HandleReceivedPacket(const rtcp::CommonHeader& rtcp_packet_header,
                             int64_t now_us);
@@ -67,13 +61,15 @@
   void CreateCompoundPacket(PacketSender* sender);
   // Sends RTCP packets.
   void SendPeriodicCompoundPacket();
+  void SendImmediateFeedback(
+      rtc::FunctionView<void(PacketSender*)> append_feedback);
   // Generate Report Blocks to be send in Sender or Receiver Report.
   std::vector<rtcp::ReportBlock> CreateReportBlocks();
 
   const RtcpTransceiverConfig config_;
 
   rtc::Optional<rtcp::Remb> remb_;
-  std::map<uint32_t, SenderReportTimes> last_received_sender_reports_;
+  std::map<uint32_t, RemoteSenderState> remote_senders_;
   rtc::WeakPtrFactory<RtcpTransceiverImpl> ptr_factory_;
 
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RtcpTransceiverImpl);
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc
index e5f7a65..c7f7482 100644
--- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc
+++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc
@@ -460,7 +460,7 @@
   EXPECT_CALL(outgoing_transport, SendRtcp(_, _))
       .WillOnce(Invoke(&rtcp_parser, &RtcpPacketParser::Parse));
 
-  rtcp_transceiver.RequestKeyFrame(kRemoteSsrcs);
+  rtcp_transceiver.SendPictureLossIndication(kRemoteSsrcs);
 
   // Expect a pli packet per ssrc in the sent single compound packet.
   EXPECT_EQ(rtcp_parser.pli()->num_packets(), 2);
@@ -469,6 +469,59 @@
   EXPECT_EQ(rtcp_parser.pli()->media_ssrc(), kRemoteSsrcs[1]);
 }
 
+TEST(RtcpTransceiverImplTest, RequestKeyFrameWithFullIntraRequest) {
+  const uint32_t kSenderSsrc = 1234;
+  const uint32_t kRemoteSsrcs[] = {4321, 5321};
+  MockTransport outgoing_transport;
+  RtcpTransceiverConfig config;
+  config.feedback_ssrc = kSenderSsrc;
+  config.schedule_periodic_compound_packets = false;
+  config.outgoing_transport = &outgoing_transport;
+  RtcpTransceiverImpl rtcp_transceiver(config);
+  RtcpPacketParser rtcp_parser;
+  EXPECT_CALL(outgoing_transport, SendRtcp(_, _))
+      .WillOnce(Invoke(&rtcp_parser, &RtcpPacketParser::Parse));
+
+  rtcp_transceiver.SendFullIntraRequest(kRemoteSsrcs);
+
+  EXPECT_EQ(rtcp_parser.fir()->num_packets(), 1);
+  EXPECT_EQ(rtcp_parser.fir()->sender_ssrc(), kSenderSsrc);
+  EXPECT_EQ(rtcp_parser.fir()->requests()[0].ssrc, kRemoteSsrcs[0]);
+  EXPECT_EQ(rtcp_parser.fir()->requests()[1].ssrc, kRemoteSsrcs[1]);
+}
+
+TEST(RtcpTransceiverImplTest, RequestKeyFrameWithFirIncreaseSeqNoPerSsrc) {
+  MockTransport outgoing_transport;
+  RtcpTransceiverConfig config;
+  config.schedule_periodic_compound_packets = false;
+  config.outgoing_transport = &outgoing_transport;
+  RtcpTransceiverImpl rtcp_transceiver(config);
+  RtcpPacketParser rtcp_parser;
+  EXPECT_CALL(outgoing_transport, SendRtcp(_, _))
+      .WillRepeatedly(Invoke(&rtcp_parser, &RtcpPacketParser::Parse));
+
+  const uint32_t kBothRemoteSsrcs[] = {4321, 5321};
+  const uint32_t kOneRemoteSsrc[] = {4321};
+
+  rtcp_transceiver.SendFullIntraRequest(kBothRemoteSsrcs);
+  ASSERT_EQ(rtcp_parser.fir()->requests()[0].ssrc, kBothRemoteSsrcs[0]);
+  uint8_t fir_sequence_number0 = rtcp_parser.fir()->requests()[0].seq_nr;
+  ASSERT_EQ(rtcp_parser.fir()->requests()[1].ssrc, kBothRemoteSsrcs[1]);
+  uint8_t fir_sequence_number1 = rtcp_parser.fir()->requests()[1].seq_nr;
+
+  rtcp_transceiver.SendFullIntraRequest(kOneRemoteSsrc);
+  ASSERT_EQ(rtcp_parser.fir()->requests().size(), 1u);
+  ASSERT_EQ(rtcp_parser.fir()->requests()[0].ssrc, kBothRemoteSsrcs[0]);
+  EXPECT_EQ(rtcp_parser.fir()->requests()[0].seq_nr, fir_sequence_number0 + 1);
+
+  rtcp_transceiver.SendFullIntraRequest(kBothRemoteSsrcs);
+  ASSERT_EQ(rtcp_parser.fir()->requests().size(), 2u);
+  ASSERT_EQ(rtcp_parser.fir()->requests()[0].ssrc, kBothRemoteSsrcs[0]);
+  EXPECT_EQ(rtcp_parser.fir()->requests()[0].seq_nr, fir_sequence_number0 + 2);
+  ASSERT_EQ(rtcp_parser.fir()->requests()[1].ssrc, kBothRemoteSsrcs[1]);
+  EXPECT_EQ(rtcp_parser.fir()->requests()[1].seq_nr, fir_sequence_number1 + 1);
+}
+
 TEST(RtcpTransceiverImplTest, KeyFrameRequestCreatesCompoundPacket) {
   const uint32_t kRemoteSsrcs[] = {4321};
   MockTransport outgoing_transport;
@@ -484,7 +537,7 @@
   EXPECT_CALL(outgoing_transport, SendRtcp(_, _))
       .WillOnce(Invoke(&rtcp_parser, &RtcpPacketParser::Parse));
 
-  rtcp_transceiver.RequestKeyFrame(kRemoteSsrcs);
+  rtcp_transceiver.SendFullIntraRequest(kRemoteSsrcs);
 
   // Test sent packet is compound by expecting presense of receiver report.
   EXPECT_EQ(rtcp_parser.receiver_report()->num_packets(), 1);
@@ -506,7 +559,7 @@
   EXPECT_CALL(outgoing_transport, SendRtcp(_, _))
       .WillOnce(Invoke(&rtcp_parser, &RtcpPacketParser::Parse));
 
-  rtcp_transceiver.RequestKeyFrame(kRemoteSsrcs);
+  rtcp_transceiver.SendFullIntraRequest(kRemoteSsrcs);
 
   // Test sent packet is reduced size by expecting absense of receiver report.
   EXPECT_EQ(rtcp_parser.receiver_report()->num_packets(), 0);