Add support for early loss detection using transport feedback.

Bug: webrtc:10676
Change-Id: Ifdef133e123a0c54204397fb323f4c671c40a464
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/135881
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28106}

call/BUILD.gn

@@ -156,6 +156,7 @@ rtc_source_set("rtp_sender") {
     "../rtc_base:rtc_task_queue",
     "../rtc_base/task_utils:repeating_task",
     "../system_wrappers:field_trial",
+    "//third_party/abseil-cpp/absl/algorithm:container",
     "//third_party/abseil-cpp/absl/container:inlined_vector",
     "//third_party/abseil-cpp/absl/memory",
     "//third_party/abseil-cpp/absl/types:optional",

call/rtp_video_sender.cc

@@ -15,6 +15,7 @@
 #include <string>
 #include <utility>
 
+#include "absl/algorithm/container.h"
 #include "absl/memory/memory.h"
 #include "api/array_view.h"
 #include "api/transport/field_trial_based_config.h"
@@ -222,6 +223,8 @@ RtpVideoSender::RtpVideoSender(
           webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")),
       account_for_packetization_overhead_(!webrtc::field_trial::IsDisabled(
           "WebRTC-SubtractPacketizationOverhead")),
+      use_early_loss_detection_(
+          !webrtc::field_trial::IsDisabled("WebRTC-UseEarlyLossDetection")),
       active_(false),
       module_process_thread_(nullptr),
       suspended_ssrcs_(std::move(suspended_ssrcs)),
@@ -563,7 +566,7 @@ void RtpVideoSender::DeliverRtcp(const uint8_t* packet, size_t length) {
 
 void RtpVideoSender::ConfigureSsrcs() {
   // Configure regular SSRCs.
-  RTC_CHECK(ssrc_to_acknowledged_packets_observers_.empty());
+  RTC_CHECK(ssrc_to_rtp_sender_.empty());
   for (size_t i = 0; i < rtp_config_.ssrcs.size(); ++i) {
     uint32_t ssrc = rtp_config_.ssrcs[i];
     RtpRtcp* const rtp_rtcp = rtp_streams_[i].rtp_rtcp.get();
@@ -574,10 +577,9 @@ void RtpVideoSender::ConfigureSsrcs() {
     if (it != suspended_ssrcs_.end())
       rtp_rtcp->SetRtpState(it->second);
 
-    AcknowledgedPacketsObserver* receive_observer =
-        rtp_rtcp->GetAcknowledgedPacketsObserver();
-    RTC_DCHECK(receive_observer != nullptr);
-    ssrc_to_acknowledged_packets_observers_[ssrc] = receive_observer;
+    RTPSender* rtp_sender = rtp_rtcp->RtpSender();
+    RTC_DCHECK(rtp_sender != nullptr);
+    ssrc_to_rtp_sender_[ssrc] = rtp_sender;
   }
 
   // Set up RTX if available.
@@ -785,8 +787,8 @@ void RtpVideoSender::OnPacketFeedbackVector(
     rtc::CritScope cs(&crit_);
     for (const PacketFeedback& packet : packet_feedback_vector) {
       if (packet.send_time_ms == PacketFeedback::kNoSendTime || !packet.ssrc ||
-          std::find(rtp_config_.ssrcs.begin(), rtp_config_.ssrcs.end(),
-                    *packet.ssrc) == rtp_config_.ssrcs.end()) {
+          absl::c_find(rtp_config_.ssrcs, *packet.ssrc) ==
+              rtp_config_.ssrcs.end()) {
         // If packet send time is missing, the feedback for this packet has
         // probably already been processed, so ignore it.
         // If packet does not belong to a registered media ssrc, we are also
@@ -807,17 +809,54 @@ void RtpVideoSender::OnPacketFeedbackVector(
     }
   }
 
+  if (use_early_loss_detection_) {
+    // Map from SSRC to vector of RTP sequence numbers that are indicated as
+    // lost by feedback, without being trailed by any received packets.
+    std::map<uint32_t, std::vector<uint16_t>> early_loss_detected_per_ssrc;
+
+    for (const PacketFeedback& packet : packet_feedback_vector) {
+      if (packet.send_time_ms == PacketFeedback::kNoSendTime || !packet.ssrc ||
+          absl::c_find(rtp_config_.ssrcs, *packet.ssrc) ==
+              rtp_config_.ssrcs.end()) {
+        // If packet send time is missing, the feedback for this packet has
+        // probably already been processed, so ignore it.
+        // If packet does not belong to a registered media ssrc, we are also
+        // not interested in it.
+        continue;
+      }
+
+      if (packet.arrival_time_ms == PacketFeedback::kNotReceived) {
+        // Last known lost packet, might not be detectable as lost by remote
+        // jitter buffer.
+        early_loss_detected_per_ssrc[*packet.ssrc].push_back(
+            packet.rtp_sequence_number);
+      } else {
+        // Packet received, so any loss prior to this is already detectable.
+        early_loss_detected_per_ssrc.erase(*packet.ssrc);
+      }
+    }
+
+    for (const auto& kv : early_loss_detected_per_ssrc) {
+      const uint32_t ssrc = kv.first;
+      auto it = ssrc_to_rtp_sender_.find(ssrc);
+      RTC_DCHECK(it != ssrc_to_rtp_sender_.end());
+      RTPSender* rtp_sender = it->second;
+      for (uint16_t sequence_number : kv.second) {
+        rtp_sender->ReSendPacket(sequence_number);
+      }
+    }
+  }
+
   for (const auto& kv : acked_packets_per_ssrc) {
     const uint32_t ssrc = kv.first;
-    if (ssrc_to_acknowledged_packets_observers_.find(ssrc) ==
-        ssrc_to_acknowledged_packets_observers_.end()) {
+    auto it = ssrc_to_rtp_sender_.find(ssrc);
+    if (it == ssrc_to_rtp_sender_.end()) {
       // Packets not for a media SSRC, so likely RTX or FEC. If so, ignore
       // since there's no RTP history to clean up anyway.
       continue;
     }
     rtc::ArrayView<const uint16_t> rtp_sequence_numbers(kv.second);
-    ssrc_to_acknowledged_packets_observers_[ssrc]->OnPacketsAcknowledged(
-        rtp_sequence_numbers);
+    it->second->OnPacketsAcknowledged(rtp_sequence_numbers);
   }
 }
 

call/rtp_video_sender.h

@@ -27,6 +27,7 @@
 #include "call/rtp_video_sender_interface.h"
 #include "logging/rtc_event_log/rtc_event_log.h"
 #include "modules/rtp_rtcp/include/flexfec_sender.h"
+#include "modules/rtp_rtcp/source/rtp_sender.h"
 #include "modules/rtp_rtcp/source/rtp_sender_video.h"
 #include "modules/rtp_rtcp/source/rtp_sequence_number_map.h"
 #include "modules/rtp_rtcp/source/rtp_video_header.h"
@@ -161,6 +162,7 @@ class RtpVideoSender : public RtpVideoSenderInterface,
 
   const bool send_side_bwe_with_overhead_;
   const bool account_for_packetization_overhead_;
+  const bool use_early_loss_detection_;
 
   // TODO(holmer): Remove crit_ once RtpVideoSender runs on the
   // transport task queue.
@@ -196,11 +198,10 @@ class RtpVideoSender : public RtpVideoSenderInterface,
   std::vector<FrameCounts> frame_counts_ RTC_GUARDED_BY(crit_);
   FrameCountObserver* const frame_count_observer_;
 
-  // Effectively const map from ssrc to AcknowledgedPacketsObserver. This
-  // map is set at construction time and never changed, but it's
+  // Effectively const map from ssrc to RTPSender, for all media ssrcs.
+  // This map is set at construction time and never changed, but it's
   // non-trivial to make it properly const.
-  std::map<uint32_t, AcknowledgedPacketsObserver*>
-      ssrc_to_acknowledged_packets_observers_;
+  std::map<uint32_t, RTPSender*> ssrc_to_rtp_sender_;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(RtpVideoSender);
 };

call/rtp_video_sender_unittest.cc

@@ -49,6 +49,7 @@ const int16_t kInitialPictureId2 = 44;
 const int16_t kInitialTl0PicIdx1 = 99;
 const int16_t kInitialTl0PicIdx2 = 199;
 const int64_t kRetransmitWindowSizeMs = 500;
+const int kTransportsSequenceExtensionId = 7;
 
 class MockRtcpIntraFrameObserver : public RtcpIntraFrameObserver {
  public:
@@ -100,6 +101,8 @@ VideoSendStream::Config CreateVideoSendStreamConfig(
   config.rtp.payload_type = payload_type;
   config.rtp.rtx.payload_type = payload_type + 1;
   config.rtp.nack.rtp_history_ms = 1000;
+  config.rtp.extensions.emplace_back(RtpExtension::kTransportSequenceNumberUri,
+                                     kTransportsSequenceExtensionId);
   return config;
 }
 
@@ -391,7 +394,7 @@ TEST(RtpVideoSenderTest, FrameCountCallbacks) {
 }
 
 // Integration test verifying that ack of packet via TransportFeedback means
-// that the packet is removed from RtpPacketHistory and won't be retranmistted
+// that the packet is removed from RtpPacketHistory and won't be retransmitted
 // again.
 TEST(RtpVideoSenderTest, DoesNotRetrasmitAckedPackets) {
   const int64_t kTimeoutMs = 500;
@@ -513,4 +516,119 @@ TEST(RtpVideoSenderTest, DoesNotRetrasmitAckedPackets) {
   test.router()->DeliverRtcp(nack_buffer.data(), nack_buffer.size());
   ASSERT_TRUE(event.Wait(kTimeoutMs));
 }
+
+// Integration test verifying that retransmissions are sent for packets which
+// can be detected as lost early, using transport wide feedback.
+TEST(RtpVideoSenderTest, EarlyRetransmits) {
+  const int64_t kTimeoutMs = 500;
+
+  RtpVideoSenderTestFixture test({kSsrc1, kSsrc2}, {kRtxSsrc1, kRtxSsrc2},
+                                 kPayloadType, {});
+  test.router()->SetActive(true);
+
+  constexpr uint8_t kPayload = 'a';
+  EncodedImage encoded_image;
+  encoded_image.SetTimestamp(1);
+  encoded_image.capture_time_ms_ = 2;
+  encoded_image._frameType = VideoFrameType::kVideoFrameKey;
+  encoded_image.Allocate(1);
+  encoded_image.data()[0] = kPayload;
+  encoded_image.set_size(1);
+  encoded_image.SetSpatialIndex(0);
+
+  CodecSpecificInfo codec_specific;
+  codec_specific.codecType = VideoCodecType::kVideoCodecGeneric;
+
+  // Send two tiny images, mapping to single RTP packets. Capture sequence
+  // numbers.
+  rtc::Event event;
+  uint16_t frame1_rtp_sequence_number = 0;
+  uint16_t frame1_transport_sequence_number = 0;
+  EXPECT_CALL(test.transport(), SendRtp)
+      .WillOnce([&event, &frame1_rtp_sequence_number,
+                 &frame1_transport_sequence_number](
+                    const uint8_t* packet, size_t length,
+                    const PacketOptions& options) {
+        RtpPacket rtp_packet;
+        EXPECT_TRUE(rtp_packet.Parse(packet, length));
+        frame1_rtp_sequence_number = rtp_packet.SequenceNumber();
+        frame1_transport_sequence_number = options.packet_id;
+        EXPECT_EQ(rtp_packet.Ssrc(), kSsrc1);
+        event.Set();
+        return true;
+      });
+  EXPECT_EQ(test.router()
+                ->OnEncodedImage(encoded_image, &codec_specific, nullptr)
+                .error,
+            EncodedImageCallback::Result::OK);
+  const int64_t send_time_ms = test.clock().TimeInMilliseconds();
+
+  test.clock().AdvanceTimeMilliseconds(33);
+  ASSERT_TRUE(event.Wait(kTimeoutMs));
+
+  uint16_t frame2_rtp_sequence_number = 0;
+  uint16_t frame2_transport_sequence_number = 0;
+  encoded_image.SetSpatialIndex(1);
+  EXPECT_CALL(test.transport(), SendRtp)
+      .WillOnce([&event, &frame2_rtp_sequence_number,
+                 &frame2_transport_sequence_number](
+                    const uint8_t* packet, size_t length,
+                    const PacketOptions& options) {
+        RtpPacket rtp_packet;
+        EXPECT_TRUE(rtp_packet.Parse(packet, length));
+        frame2_rtp_sequence_number = rtp_packet.SequenceNumber();
+        frame2_transport_sequence_number = options.packet_id;
+        EXPECT_EQ(rtp_packet.Ssrc(), kSsrc2);
+        event.Set();
+        return true;
+      });
+  EXPECT_EQ(test.router()
+                ->OnEncodedImage(encoded_image, &codec_specific, nullptr)
+                .error,
+            EncodedImageCallback::Result::OK);
+  test.clock().AdvanceTimeMilliseconds(33);
+  ASSERT_TRUE(event.Wait(kTimeoutMs));
+
+  EXPECT_NE(frame1_transport_sequence_number, frame2_transport_sequence_number);
+
+  // Inject a transport feedback where the packet for the first frame is lost,
+  // expect a retransmission for it.
+  EXPECT_CALL(test.transport(), SendRtp)
+      .WillOnce([&event, &frame1_rtp_sequence_number](
+                    const uint8_t* packet, size_t length,
+                    const PacketOptions& options) {
+        RtpPacket rtp_packet;
+        EXPECT_TRUE(rtp_packet.Parse(packet, length));
+        EXPECT_EQ(rtp_packet.Ssrc(), kRtxSsrc2);
+
+        // Retransmitted sequence number from the RTX header should match
+        // the lost packet.
+        rtc::ArrayView<const uint8_t> payload = rtp_packet.payload();
+        EXPECT_EQ(ByteReader<uint16_t>::ReadBigEndian(payload.data()),
+                  frame1_rtp_sequence_number);
+        event.Set();
+        return true;
+      });
+
+  PacketFeedback first_packet_feedback(PacketFeedback::kNotReceived,
+                                       frame1_transport_sequence_number);
+  first_packet_feedback.rtp_sequence_number = frame1_rtp_sequence_number;
+  first_packet_feedback.ssrc = kSsrc1;
+  first_packet_feedback.send_time_ms = send_time_ms;
+
+  PacketFeedback second_packet_feedback(test.clock().TimeInMilliseconds(),
+                                        frame2_transport_sequence_number);
+  first_packet_feedback.rtp_sequence_number = frame2_rtp_sequence_number;
+  first_packet_feedback.ssrc = kSsrc2;
+  first_packet_feedback.send_time_ms = send_time_ms + 33;
+
+  std::vector<PacketFeedback> feedback_vector = {first_packet_feedback,
+                                                 second_packet_feedback};
+
+  test.router()->OnPacketFeedbackVector(feedback_vector);
+
+  // Wait for pacer to run and send the RTX packet.
+  test.clock().AdvanceTimeMilliseconds(33);
+  ASSERT_TRUE(event.Wait(kTimeoutMs));
+}
 }  // namespace webrtc

modules/rtp_rtcp/include/rtp_rtcp.h

@@ -277,11 +277,6 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface {
   virtual StreamDataCountersCallback* GetSendChannelRtpStatisticsCallback()
       const = 0;
 
-  // Returns a pointer to an observer that handles information about packets
-  // that have been received by the remote end, or nullptr if not applicable.
-  virtual AcknowledgedPacketsObserver* GetAcknowledgedPacketsObserver()
-      const = 0;
-
   // **************************************************************************
   // RTCP
   // **************************************************************************

modules/rtp_rtcp/include/rtp_rtcp_defines.h

@@ -315,17 +315,6 @@ class TransportFeedbackObserver {
   virtual void OnTransportFeedback(const rtcp::TransportFeedback& feedback) = 0;
 };
 
-class AcknowledgedPacketsObserver {
- public:
-  AcknowledgedPacketsObserver() = default;
-  virtual ~AcknowledgedPacketsObserver() = default;
-
-  // Indicates RTP sequence numbers for packets that have been acknowledged as
-  // received by the remote end.
-  virtual void OnPacketsAcknowledged(
-      rtc::ArrayView<const uint16_t> sequence_numbers) = 0;
-};
-
 // Interface for PacketRouter to send rtcp feedback on behalf of
 // congestion controller.
 // TODO(bugs.webrtc.org/8239): Remove and use RtcpTransceiver directly

modules/rtp_rtcp/mocks/mock_rtp_rtcp.h

@@ -160,8 +160,6 @@ class MockRtpRtcp : public RtpRtcp {
                void(StreamDataCountersCallback*));
   MOCK_CONST_METHOD0(GetSendChannelRtpStatisticsCallback,
                      StreamDataCountersCallback*());
-  MOCK_CONST_METHOD0(GetAcknowledgedPacketsObserver,
-                     AcknowledgedPacketsObserver*());
   MOCK_METHOD1(SetVideoBitrateAllocation, void(const VideoBitrateAllocation&));
   MOCK_METHOD0(RtpSender, RTPSender*());
   MOCK_CONST_METHOD0(RtpSender, const RTPSender*());

modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -871,11 +871,6 @@ ModuleRtpRtcpImpl::GetSendChannelRtpStatisticsCallback() const {
   return rtp_sender_->GetRtpStatisticsCallback();
 }
 
-AcknowledgedPacketsObserver* ModuleRtpRtcpImpl::GetAcknowledgedPacketsObserver()
-    const {
-  return rtp_sender_.get();
-}
-
 void ModuleRtpRtcpImpl::SetVideoBitrateAllocation(
     const VideoBitrateAllocation& bitrate) {
   rtcp_sender_.SetVideoBitrateAllocation(bitrate);

modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -280,7 +280,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp {
       StreamDataCountersCallback* callback) override;
   StreamDataCountersCallback* GetSendChannelRtpStatisticsCallback()
       const override;
-  AcknowledgedPacketsObserver* GetAcknowledgedPacketsObserver() const override;
 
   void OnReceivedNack(
       const std::vector<uint16_t>& nack_sequence_numbers) override;

modules/rtp_rtcp/source/rtp_sender.h

@@ -42,7 +42,7 @@ class RateLimiter;
 class RtcEventLog;
 class RtpPacketToSend;
 
-class RTPSender : public AcknowledgedPacketsObserver {
+class RTPSender {
  public:
   RTPSender(bool audio,
             Clock* clock,
@@ -175,8 +175,7 @@ class RTPSender : public AcknowledgedPacketsObserver {
 
   void SetRtt(int64_t rtt_ms);
 
-  void OnPacketsAcknowledged(
-      rtc::ArrayView<const uint16_t> sequence_numbers) override;
+  void OnPacketsAcknowledged(rtc::ArrayView<const uint16_t> sequence_numbers);
 
  private:
   // Maps capture time in milliseconds to send-side delay in milliseconds.