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;
