in RtcpTransceiver support replying to RRTR per local ssrc

to support clients (e.g. RTCPReceiver) that collect and report RTT per sender ssrc.

Bug: webrtc:8239, webrtc:13853
Change-Id: I907fb35277b0f23bbe9f2cd2ef979ce0fb1f9338
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/257440
Reviewed-by: Emil Lundmark <lndmrk@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36439}

modules/rtp_rtcp/source/rtcp_transceiver_config.h

@@ -172,9 +172,11 @@ struct RtcpTransceiverConfig {
   // Reply to incoming RRTR messages so that remote endpoint may estimate RTT as
   // non-sender as described in https://tools.ietf.org/html/rfc3611#section-4.4
   // and #section-4.5
-  // TODO(danilchap): Make it true by default after users got enough time to
+  bool reply_to_non_sender_rtt_measurement = true;
-  // turn it off if not needed.
+
-  bool reply_to_non_sender_rtt_measurement = false;
+  // Reply to incoming RRTR messages multiple times, one per sender SSRC, to
+  // support clients that calculate and process RTT per sender SSRC.
+  bool reply_to_non_sender_rtt_mesaurments_on_all_ssrcs = true;
 
   // Allows a REMB message to be sent immediately when SetRemb is called without
   // having to wait for the next compount message to be sent.

modules/rtp_rtcp/source/rtcp_transceiver_impl.cc

@@ -571,38 +571,35 @@ void RtcpTransceiverImpl::SchedulePeriodicCompoundPackets(TimeDelta delay) {
       });
 }
 
-RtcpTransceiverImpl::CompoundPacketInfo RtcpTransceiverImpl::FillReports(
+std::vector<uint32_t> RtcpTransceiverImpl::FillReports(
     Timestamp now,
-    size_t reserved_bytes,
+    ReservedBytes reserved,
     PacketSender& rtcp_sender) {
   // Sender/receiver reports should be first in the RTCP packet.
   RTC_DCHECK(rtcp_sender.IsEmpty());
 
   size_t available_bytes = config_.max_packet_size;
-  if (reserved_bytes > available_bytes) {
+  if (reserved.per_packet > available_bytes) {
-    // Because reserved_bytes is unsigned, substracting would underflow and will
+    // Because reserved.per_packet is unsigned, substracting would underflow and
-    // not produce desired result.
+    // will not produce desired result.
     available_bytes = 0;
   } else {
-    available_bytes -= reserved_bytes;
+    available_bytes -= reserved.per_packet;
   }
 
-  CompoundPacketInfo result;
+  const size_t sender_report_size_bytes = 28 + reserved.per_sender;
-  result.sender_ssrc = config_.feedback_ssrc;
+  const size_t full_sender_report_size_bytes =
-  result.has_sender_report = false;
+      sender_report_size_bytes +
-
-  static constexpr size_t kSenderReportSizeBytes = 28;
-  static constexpr size_t kFullSenderReportSizeBytes =
-      kSenderReportSizeBytes +
       rtcp::SenderReport::kMaxNumberOfReportBlocks * rtcp::ReportBlock::kLength;
-  size_t max_full_sender_reports = available_bytes / kFullSenderReportSizeBytes;
+  size_t max_full_sender_reports =
+      available_bytes / full_sender_report_size_bytes;
   size_t max_report_blocks =
       max_full_sender_reports * rtcp::SenderReport::kMaxNumberOfReportBlocks;
   size_t available_bytes_for_last_sender_report =
-      available_bytes - max_full_sender_reports * kFullSenderReportSizeBytes;
+      available_bytes - max_full_sender_reports * full_sender_report_size_bytes;
-  if (available_bytes_for_last_sender_report >= kSenderReportSizeBytes) {
+  if (available_bytes_for_last_sender_report >= sender_report_size_bytes) {
     max_report_blocks +=
-        (available_bytes_for_last_sender_report - kSenderReportSizeBytes) /
+        (available_bytes_for_last_sender_report - sender_report_size_bytes) /
         rtcp::ReportBlock::kLength;
   }
 
@@ -613,13 +610,13 @@ RtcpTransceiverImpl::CompoundPacketInfo RtcpTransceiverImpl::FillReports(
   // is low, more sender reports may fit in.
   size_t max_sender_reports =
       (available_bytes - report_blocks.size() * rtcp::ReportBlock::kLength) /
-      kSenderReportSizeBytes;
+      sender_report_size_bytes;
 
   auto last_handled_sender_it = local_senders_.end();
   auto report_block_it = report_blocks.begin();
-  size_t num_sender_reports = 0;
+  std::vector<uint32_t> sender_ssrcs;
   for (auto it = local_senders_.begin();
-       it != local_senders_.end() && num_sender_reports < max_sender_reports;
+       it != local_senders_.end() && sender_ssrcs.size() < max_sender_reports;
        ++it) {
     LocalSenderState& rtp_sender = *it;
     RtpStreamRtcpHandler::RtpStats stats = rtp_sender.handler->SentStats();
@@ -656,12 +653,7 @@ RtcpTransceiverImpl::CompoundPacketInfo RtcpTransceiverImpl::FillReports(
       report_block_it += num_blocks;
     }
     rtcp_sender.AppendPacket(sender_report);
-    ++num_sender_reports;
+    sender_ssrcs.push_back(rtp_sender.ssrc);
-
-    if (!result.has_sender_report) {
-      result.has_sender_report = true;
-      result.sender_ssrc = rtp_sender.ssrc;
-    }
   }
   if (last_handled_sender_it != local_senders_.end()) {
     // Rotate `local_senders_` so that the 1st unhandled sender become first in
@@ -679,14 +671,16 @@ RtcpTransceiverImpl::CompoundPacketInfo RtcpTransceiverImpl::FillReports(
 
   // In compound mode each RTCP packet has to start with a sender or receiver
   // report.
-  if (config_.rtcp_mode == RtcpMode::kCompound && num_sender_reports == 0 &&
+  if (config_.rtcp_mode == RtcpMode::kCompound && sender_ssrcs.empty() &&
       num_receiver_reports == 0) {
     num_receiver_reports = 1;
   }
 
+  uint32_t sender_ssrc =
+      sender_ssrcs.empty() ? config_.feedback_ssrc : sender_ssrcs.front();
   for (size_t i = 0; i < num_receiver_reports; ++i) {
     rtcp::ReceiverReport receiver_report;
-    receiver_report.SetSenderSsrc(result.sender_ssrc);
+    receiver_report.SetSenderSsrc(sender_ssrc);
     size_t num_blocks =
         std::min<size_t>(rtcp::ReceiverReport::kMaxNumberOfReportBlocks,
                          report_blocks.end() - report_block_it);
@@ -698,28 +692,29 @@ RtcpTransceiverImpl::CompoundPacketInfo RtcpTransceiverImpl::FillReports(
   }
   // All report blocks should be attached at this point.
   RTC_DCHECK_EQ(report_blocks.end() - report_block_it, 0);
-  return result;
+  return sender_ssrcs;
 }
 
 void RtcpTransceiverImpl::CreateCompoundPacket(Timestamp now,
                                                size_t reserved_bytes,
                                                PacketSender& sender) {
   RTC_DCHECK(sender.IsEmpty());
+  ReservedBytes reserved = {.per_packet = reserved_bytes};
   absl::optional<rtcp::Sdes> sdes;
   if (!config_.cname.empty()) {
     sdes.emplace();
     bool added = sdes->AddCName(config_.feedback_ssrc, config_.cname);
     RTC_DCHECK(added) << "Failed to add CNAME " << config_.cname
                       << " to RTCP SDES packet.";
-    reserved_bytes += sdes->BlockLength();
+    reserved.per_packet += sdes->BlockLength();
   }
   if (remb_.has_value()) {
-    reserved_bytes += remb_->BlockLength();
+    reserved.per_packet += remb_->BlockLength();
   }
-  absl::optional<rtcp::ExtendedReports> xr;
+  absl::optional<rtcp::ExtendedReports> xr_with_dlrr;
   if (!received_rrtrs_.empty()) {
     RTC_DCHECK(config_.reply_to_non_sender_rtt_measurement);
-    xr.emplace();
+    xr_with_dlrr.emplace();
     uint32_t now_ntp =
         CompactNtp(config_.clock->ConvertTimestampToNtpTime(now));
     for (const auto& [ssrc, rrtr_info] : received_rrtrs_) {
@@ -728,9 +723,13 @@ void RtcpTransceiverImpl::CreateCompoundPacket(Timestamp now,
       reply.last_rr = rrtr_info.received_remote_mid_ntp_time;
       reply.delay_since_last_rr =
           now_ntp - rrtr_info.local_receive_mid_ntp_time;
-      xr->AddDlrrItem(reply);
+      xr_with_dlrr->AddDlrrItem(reply);
+    }
+    if (config_.reply_to_non_sender_rtt_mesaurments_on_all_ssrcs) {
+      reserved.per_sender += xr_with_dlrr->BlockLength();
+    } else {
+      reserved.per_packet += xr_with_dlrr->BlockLength();
     }
-    reserved_bytes += xr->BlockLength();
   }
   if (config_.non_sender_rtt_measurement) {
     // It looks like bytes for ExtendedReport header are reserved twice, but in
@@ -740,29 +739,40 @@ void RtcpTransceiverImpl::CreateCompoundPacket(Timestamp now,
     // than it should, which is not an issue.
 
     // 4 bytes for common RTCP header + 4 bytes for the ExtenedReports header.
-    reserved_bytes += (4 + 4 + rtcp::Rrtr::kLength);
+    reserved.per_packet += (4 + 4 + rtcp::Rrtr::kLength);
   }
 
-  CompoundPacketInfo result = FillReports(now, reserved_bytes, sender);
+  std::vector<uint32_t> sender_ssrcs = FillReports(now, reserved, sender);
+  bool has_sender_report = !sender_ssrcs.empty();
+  uint32_t sender_ssrc =
+      has_sender_report ? sender_ssrcs.front() : config_.feedback_ssrc;
 
   if (sdes.has_value() && !sender.IsEmpty()) {
     sender.AppendPacket(*sdes);
   }
   if (remb_.has_value()) {
-    remb_->SetSenderSsrc(result.sender_ssrc);
+    remb_->SetSenderSsrc(sender_ssrc);
     sender.AppendPacket(*remb_);
   }
-  if (!result.has_sender_report && config_.non_sender_rtt_measurement) {
+  if (!has_sender_report && config_.non_sender_rtt_measurement) {
-    if (!xr.has_value()) {
+    rtcp::ExtendedReports xr_with_rrtr;
-      xr.emplace();
+    xr_with_rrtr.SetSenderSsrc(config_.feedback_ssrc);
-    }
     rtcp::Rrtr rrtr;
     rrtr.SetNtp(config_.clock->ConvertTimestampToNtpTime(now));
-    xr->SetRrtr(rrtr);
+    xr_with_rrtr.SetRrtr(rrtr);
+    sender.AppendPacket(xr_with_rrtr);
   }
-  if (xr.has_value()) {
+  if (xr_with_dlrr.has_value()) {
-    xr->SetSenderSsrc(result.sender_ssrc);
+    rtc::ArrayView<const uint32_t> ssrcs(&sender_ssrc, 1);
-    sender.AppendPacket(*xr);
+    if (config_.reply_to_non_sender_rtt_mesaurments_on_all_ssrcs &&
+        !sender_ssrcs.empty()) {
+      ssrcs = sender_ssrcs;
+    }
+    RTC_DCHECK(!ssrcs.empty());
+    for (uint32_t ssrc : ssrcs) {
+      xr_with_dlrr->SetSenderSsrc(ssrc);
+      sender.AppendPacket(*xr_with_dlrr);
+    }
   }
 }
 

modules/rtp_rtcp/source/rtcp_transceiver_impl.h

@@ -128,14 +128,15 @@ class RtcpTransceiverImpl {
   void SchedulePeriodicCompoundPackets(TimeDelta delay);
   // Appends RTCP sender and receiver reports to the `sender`.
   // Both sender and receiver reports may have attached report blocks.
-  // Uses up to `config_.max_packet_size - reserved_bytes`
+  // Uses up to `config_.max_packet_size - reserved_bytes.per_packet`
-  struct CompoundPacketInfo {
+  // Returns list of sender ssrc in sender reports.
-    uint32_t sender_ssrc;
+  struct ReservedBytes {
-    bool has_sender_report;
+    size_t per_packet = 0;
+    size_t per_sender = 0;
   };
-  CompoundPacketInfo FillReports(Timestamp now,
+  std::vector<uint32_t> FillReports(Timestamp now,
-                                 size_t reserved_bytes,
+                                    ReservedBytes reserved_bytes,
-                                 PacketSender& rtcp_sender);
+                                    PacketSender& rtcp_sender);
 
   // Creates compound RTCP packet, as defined in
   // https://tools.ietf.org/html/rfc5506#section-2

modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc

@@ -1387,6 +1387,68 @@ TEST(RtcpTransceiverImplTest, RepliesToRrtrWhenEnabled) {
                                         /*delay=*/kComactNtpOneSecond / 2)));
 }
 
+TEST(RtcpTransceiverImplTest, CanReplyToRrtrOnceForAllLocalSsrcs) {
+  static constexpr uint32_t kRemoteSsrc = 4321;
+  static constexpr uint32_t kLocalSsrcs[] = {1234, 5678};
+  SimulatedClock clock(0);
+  RtcpTransceiverConfig config = DefaultTestConfig();
+  config.clock = &clock;
+  config.reply_to_non_sender_rtt_measurement = true;
+  config.reply_to_non_sender_rtt_mesaurments_on_all_ssrcs = false;
+  RtcpPacketParser rtcp_parser;
+  RtcpParserTransport transport(&rtcp_parser);
+  config.outgoing_transport = &transport;
+  RtcpTransceiverImpl rtcp_transceiver(config);
+
+  MockRtpStreamRtcpHandler local_sender0;
+  MockRtpStreamRtcpHandler local_sender1;
+  rtcp_transceiver.AddMediaSender(kLocalSsrcs[0], &local_sender0);
+  rtcp_transceiver.AddMediaSender(kLocalSsrcs[1], &local_sender1);
+
+  rtcp::ExtendedReports xr;
+  rtcp::Rrtr rrtr;
+  rrtr.SetNtp(NtpTime(uint64_t{0x1111'2222'3333'4444}));
+  xr.SetRrtr(rrtr);
+  xr.SetSenderSsrc(kRemoteSsrc);
+  rtcp_transceiver.ReceivePacket(xr.Build(), clock.CurrentTime());
+  clock.AdvanceTime(TimeDelta::Millis(1'500));
+
+  rtcp_transceiver.SendCompoundPacket();
+
+  EXPECT_EQ(rtcp_parser.xr()->num_packets(), 1);
+}
+
+TEST(RtcpTransceiverImplTest, CanReplyToRrtrForEachLocalSsrc) {
+  static constexpr uint32_t kRemoteSsrc = 4321;
+  static constexpr uint32_t kLocalSsrc[] = {1234, 5678};
+  SimulatedClock clock(0);
+  RtcpTransceiverConfig config = DefaultTestConfig();
+  config.clock = &clock;
+  config.reply_to_non_sender_rtt_measurement = true;
+  config.reply_to_non_sender_rtt_mesaurments_on_all_ssrcs = true;
+  RtcpPacketParser rtcp_parser;
+  RtcpParserTransport transport(&rtcp_parser);
+  config.outgoing_transport = &transport;
+  RtcpTransceiverImpl rtcp_transceiver(config);
+
+  MockRtpStreamRtcpHandler local_sender0;
+  MockRtpStreamRtcpHandler local_sender1;
+  rtcp_transceiver.AddMediaSender(kLocalSsrc[0], &local_sender0);
+  rtcp_transceiver.AddMediaSender(kLocalSsrc[1], &local_sender1);
+
+  rtcp::ExtendedReports xr;
+  rtcp::Rrtr rrtr;
+  rrtr.SetNtp(NtpTime(uint64_t{0x1111'2222'3333'4444}));
+  xr.SetRrtr(rrtr);
+  xr.SetSenderSsrc(kRemoteSsrc);
+  rtcp_transceiver.ReceivePacket(xr.Build(), clock.CurrentTime());
+  clock.AdvanceTime(TimeDelta::Millis(1'500));
+
+  rtcp_transceiver.SendCompoundPacket();
+
+  EXPECT_EQ(rtcp_parser.xr()->num_packets(), 2);
+}
+
 TEST(RtcpTransceiverImplTest, SendsNoXrRrtrWhenDisabled) {
   SimulatedClock clock(0);
   RtcpTransceiverConfig config;