In RtcpTransceiver implement handling incoming RRTR Bug: webrtc:8239 Change-Id: I4a469b6a0c2e387e35262798f4686fbf310d00cd Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251902 Reviewed-by: Emil Lundmark <lndmrk@webrtc.org> Commit-Queue: Danil Chapovalov <danilchap@webrtc.org> Cr-Commit-Position: refs/heads/main@{#36037}
diff --git a/modules/rtp_rtcp/source/rtcp_packet/dlrr.h b/modules/rtp_rtcp/source/rtcp_packet/dlrr.h index 6fe2099..ad91dfd 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/dlrr.h +++ b/modules/rtp_rtcp/source/rtcp_packet/dlrr.h
@@ -24,11 +24,21 @@ ReceiveTimeInfo() : ssrc(0), last_rr(0), delay_since_last_rr(0) {} ReceiveTimeInfo(uint32_t ssrc, uint32_t last_rr, uint32_t delay) : ssrc(ssrc), last_rr(last_rr), delay_since_last_rr(delay) {} + uint32_t ssrc; uint32_t last_rr; uint32_t delay_since_last_rr; }; +inline bool operator==(const ReceiveTimeInfo& lhs, const ReceiveTimeInfo& rhs) { + return lhs.ssrc == rhs.ssrc && lhs.last_rr == rhs.last_rr && + lhs.delay_since_last_rr == rhs.delay_since_last_rr; +} + +inline bool operator!=(const ReceiveTimeInfo& lhs, const ReceiveTimeInfo& rhs) { + return !(lhs == rhs); +} + // DLRR Report Block: Delay since the Last Receiver Report (RFC 3611). class Dlrr { public:
diff --git a/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc b/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc index 7c50c01..3d9a2a3 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc
@@ -24,19 +24,6 @@ using webrtc::rtcp::Rrtr; namespace webrtc { -// Define comparision operators that shouldn't be needed in production, -// but make testing matches more clear. -namespace rtcp { -bool operator==(const Rrtr& rrtr1, const Rrtr& rrtr2) { - return rrtr1.ntp() == rrtr2.ntp(); -} - -bool operator==(const ReceiveTimeInfo& time1, const ReceiveTimeInfo& time2) { - return time1.ssrc == time2.ssrc && time1.last_rr == time2.last_rr && - time1.delay_since_last_rr == time2.delay_since_last_rr; -} -} // namespace rtcp - namespace { constexpr uint32_t kSenderSsrc = 0x12345678; constexpr uint8_t kEmptyPacket[] = {0x80, 207, 0x00, 0x01,
diff --git a/modules/rtp_rtcp/source/rtcp_packet/rrtr.h b/modules/rtp_rtcp/source/rtcp_packet/rrtr.h index 8eb4ce6..827bd74 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/rrtr.h +++ b/modules/rtp_rtcp/source/rtcp_packet/rrtr.h
@@ -46,6 +46,14 @@ NtpTime ntp_; }; +inline bool operator==(const Rrtr& rrtr1, const Rrtr& rrtr2) { + return rrtr1.ntp() == rrtr2.ntp(); +} + +inline bool operator!=(const Rrtr& rrtr1, const Rrtr& rrtr2) { + return !(rrtr1 == rrtr2); +} + } // namespace rtcp } // namespace webrtc #endif // MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_RRTR_H_
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_config.h b/modules/rtp_rtcp/source/rtcp_transceiver_config.h index 0be633f..2400f9b 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_config.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_config.h
@@ -162,6 +162,13 @@ // https://tools.ietf.org/html/rfc3611#section-4.4 and #section-4.5 bool non_sender_rtt_measurement = false; + // 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 + // turn it off if not needed. + bool reply_to_non_sender_rtt_measurement = false; + // Allows a REMB message to be sent immediately when SetRemb is called without // having to wait for the next compount message to be sent. bool send_remb_on_change = false;
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index c075b2f..5909af9 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc
@@ -370,6 +370,14 @@ if (!extended_reports.Parse(rtcp_packet_header)) return; + if (config_.reply_to_non_sender_rtt_measurement && extended_reports.rrtr()) { + RrtrTimes& rrtr = received_rrtrs_[extended_reports.sender_ssrc()]; + rrtr.received_remote_mid_ntp_time = + CompactNtp(extended_reports.rrtr()->ntp()); + rrtr.local_receive_mid_ntp_time = + CompactNtp(config_.clock->ConvertTimestampToNtpTime(now)); + } + if (extended_reports.dlrr()) HandleDlrr(extended_reports.dlrr(), now); @@ -620,7 +628,29 @@ if (remb_.has_value()) { reserved_bytes += remb_->BlockLength(); } + absl::optional<rtcp::ExtendedReports> xr; + if (!received_rrtrs_.empty()) { + RTC_DCHECK(config_.reply_to_non_sender_rtt_measurement); + xr.emplace(); + uint32_t now_ntp = + CompactNtp(config_.clock->ConvertTimestampToNtpTime(now)); + for (const auto& [ssrc, rrtr_info] : received_rrtrs_) { + rtcp::ReceiveTimeInfo reply; + reply.ssrc = ssrc; + 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); + } + reserved_bytes += xr->BlockLength(); + } if (config_.non_sender_rtt_measurement) { + // It looks like bytes for ExtendedReport header are reserved twice, but in + // practice the same RtcpTransceiver won't both produce RRTR (i.e. it is a + // receiver-only) and reply to RRTR (i.e. remote participant is a receiver + // only). If that happen, then `reserved_bytes` would be slightly larger + // 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); } @@ -635,14 +665,16 @@ sender.AppendPacket(*remb_); } if (!result.has_sender_report && config_.non_sender_rtt_measurement) { - rtcp::ExtendedReports xr; - xr.SetSenderSsrc(result.sender_ssrc); - + if (!xr.has_value()) { + xr.emplace(); + } rtcp::Rrtr rrtr; rrtr.SetNtp(config_.clock->ConvertTimestampToNtpTime(now)); - xr.SetRrtr(rrtr); - - sender.AppendPacket(xr); + xr->SetRrtr(rrtr); + } + if (xr.has_value()) { + xr->SetSenderSsrc(result.sender_ssrc); + sender.AppendPacket(*xr); } }
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h index ce25899..8dc50b8 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h
@@ -82,6 +82,13 @@ class PacketSender; struct RemoteSenderState; struct LocalSenderState; + struct RrtrTimes { + // Received remote NTP timestamp in compact representation. + uint32_t received_remote_mid_ntp_time; + + // Local NTP time when the report was received in compact representation. + uint32_t local_receive_mid_ntp_time; + }; void HandleReceivedPacket(const rtcp::CommonHeader& rtcp_packet_header, Timestamp now, @@ -144,6 +151,7 @@ std::list<LocalSenderState> local_senders_; flat_map<uint32_t, std::list<LocalSenderState>::iterator> local_senders_by_ssrc_; + flat_map<uint32_t, RrtrTimes> received_rrtrs_; RepeatingTaskHandle periodic_task_handle_; };
diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc index 417dcdd..82bfade 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc
@@ -45,6 +45,7 @@ using ::testing::Return; using ::testing::SizeIs; using ::testing::StrictMock; +using ::testing::UnorderedElementsAre; using ::testing::WithArg; using ::webrtc::rtcp::Bye; using ::webrtc::rtcp::CompoundPacket; @@ -1208,6 +1209,43 @@ EXPECT_EQ(rtcp_parser.xr()->rrtr()->ntp(), ntp_time_now); } +TEST(RtcpTransceiverImplTest, RepliesToRrtrWhenEnabled) { + static constexpr uint32_t kSenderSsrc[] = {4321, 9876}; + SimulatedClock clock(0); + RtcpTransceiverConfig config = DefaultTestConfig(); + config.clock = &clock; + config.reply_to_non_sender_rtt_measurement = true; + RtcpPacketParser rtcp_parser; + RtcpParserTransport transport(&rtcp_parser); + config.outgoing_transport = &transport; + RtcpTransceiverImpl rtcp_transceiver(config); + + rtcp::ExtendedReports xr; + rtcp::Rrtr rrtr; + rrtr.SetNtp(NtpTime(uint64_t{0x1111'2222'3333'4444})); + xr.SetRrtr(rrtr); + xr.SetSenderSsrc(kSenderSsrc[0]); + rtcp_transceiver.ReceivePacket(xr.Build(), clock.CurrentTime()); + clock.AdvanceTime(TimeDelta::Millis(1'500)); + + rrtr.SetNtp(NtpTime(uint64_t{0x4444'5555'6666'7777})); + xr.SetRrtr(rrtr); + xr.SetSenderSsrc(kSenderSsrc[1]); + rtcp_transceiver.ReceivePacket(xr.Build(), clock.CurrentTime()); + clock.AdvanceTime(TimeDelta::Millis(500)); + + rtcp_transceiver.SendCompoundPacket(); + + EXPECT_EQ(rtcp_parser.xr()->num_packets(), 1); + static constexpr uint32_t kComactNtpOneSecond = 0x0001'0000; + EXPECT_THAT(rtcp_parser.xr()->dlrr().sub_blocks(), + UnorderedElementsAre( + rtcp::ReceiveTimeInfo(kSenderSsrc[0], 0x2222'3333, + /*delay=*/2 * kComactNtpOneSecond), + rtcp::ReceiveTimeInfo(kSenderSsrc[1], 0x5555'6666, + /*delay=*/kComactNtpOneSecond / 2))); +} + TEST(RtcpTransceiverImplTest, SendsNoXrRrtrWhenDisabled) { SimulatedClock clock(0); RtcpTransceiverConfig config;