From 53817b8c18ea30c6f3285dd7300cf10e5be887ee Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 10 May 2023 14:58:04 +0200 Subject: [PATCH] Delete legacy RtpRtcpInterface::RTT Bug: webrtc:13757 Change-Id: Ibc39814e4a3b01425753fbcde61006a15430a6ee Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304820 Commit-Queue: Danil Chapovalov Reviewed-by: Philip Eliasson Cr-Commit-Position: refs/heads/main@{#40042} --- modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 8 ----- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 14 -------- modules/rtp_rtcp/source/rtp_rtcp_impl.h | 5 --- modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 17 --------- modules/rtp_rtcp/source/rtp_rtcp_impl2.h | 5 --- .../rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 35 ++++++++----------- modules/rtp_rtcp/source/rtp_rtcp_interface.h | 8 ----- 7 files changed, 14 insertions(+), 78 deletions(-) diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index a6c5cec705..5af821e028 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -119,14 +119,6 @@ class MockRtpRtcpInterface : public RtpRtcpInterface { MOCK_METHOD(RtcpMode, RTCP, (), (const, override)); MOCK_METHOD(void, SetRTCPStatus, (RtcpMode method), (override)); MOCK_METHOD(int32_t, SetCNAME, (absl::string_view cname), (override)); - MOCK_METHOD(int32_t, - RTT, - (uint32_t remote_ssrc, - int64_t* rtt, - int64_t* avg_rtt, - int64_t* min_rtt, - int64_t* max_rtt), - (const, override)); MOCK_METHOD(absl::optional, LastRtt, (), (const, override)); MOCK_METHOD(int64_t, ExpectedRetransmissionTimeMs, (), (const, override)); MOCK_METHOD(int32_t, SendRTCP, (RTCPPacketType packet_type), (override)); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 466503b4a0..689123bbb3 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -464,20 +464,6 @@ int32_t ModuleRtpRtcpImpl::SetCNAME(absl::string_view c_name) { return rtcp_sender_.SetCNAME(c_name); } -// Get RoundTripTime. -int32_t ModuleRtpRtcpImpl::RTT(const uint32_t remote_ssrc, - int64_t* rtt, - int64_t* avg_rtt, - int64_t* min_rtt, - int64_t* max_rtt) const { - int32_t ret = rtcp_receiver_.RTT(remote_ssrc, rtt, avg_rtt, min_rtt, max_rtt); - if (rtt && *rtt == 0) { - // Try to get RTT from RtcpRttStats class. - *rtt = rtt_ms(); - } - return ret; -} - absl::optional ModuleRtpRtcpImpl::LastRtt() const { absl::optional rtt = rtcp_receiver_.LastRtt(); if (!rtt.has_value()) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 491fd2fafb..d3a0dd69e1 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -170,11 +170,6 @@ class ABSL_DEPRECATED("") ModuleRtpRtcpImpl int32_t SetCNAME(absl::string_view c_name) override; // Get RoundTripTime. - [[deprecated]] int32_t RTT(uint32_t remote_ssrc, - int64_t* rtt, - int64_t* avg_rtt, - int64_t* min_rtt, - int64_t* max_rtt) const override; absl::optional LastRtt() const override; int64_t ExpectedRetransmissionTimeMs() const override; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index 0f64e5463d..f92be76c65 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -446,23 +446,6 @@ int32_t ModuleRtpRtcpImpl2::SetCNAME(absl::string_view c_name) { return rtcp_sender_.SetCNAME(c_name); } -// TODO(tommi): Check if `avg_rtt_ms`, `min_rtt_ms`, `max_rtt_ms` params are -// actually used in practice (some callers ask for it but don't use it). It -// could be that only `rtt` is needed and if so, then the fast path could be to -// just call rtt_ms() and rely on the calculation being done periodically. -int32_t ModuleRtpRtcpImpl2::RTT(const uint32_t remote_ssrc, - int64_t* rtt, - int64_t* avg_rtt, - int64_t* min_rtt, - int64_t* max_rtt) const { - int32_t ret = rtcp_receiver_.RTT(remote_ssrc, rtt, avg_rtt, min_rtt, max_rtt); - if (rtt && *rtt == 0) { - // Try to get RTT from RtcpRttStats class. - *rtt = rtt_ms(); - } - return ret; -} - absl::optional ModuleRtpRtcpImpl2::LastRtt() const { absl::optional rtt = rtcp_receiver_.LastRtt(); if (!rtt.has_value()) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h index 033b65d30f..c0afe819dd 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h @@ -181,11 +181,6 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, int32_t SetCNAME(absl::string_view c_name) override; // Get RoundTripTime. - [[deprecated]] int32_t RTT(uint32_t remote_ssrc, - int64_t* rtt, - int64_t* avg_rtt, - int64_t* min_rtt, - int64_t* max_rtt) const override; absl::optional LastRtt() const override; int64_t ExpectedRetransmissionTimeMs() const override; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 057935e221..2c05827cb5 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -14,6 +14,7 @@ #include #include +#include "api/units/time_delta.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtcp_packet.h" #include "modules/rtp_rtcp/source/rtcp_packet/nack.h" @@ -36,7 +37,7 @@ namespace webrtc { namespace { const uint32_t kSenderSsrc = 0x12345; const uint32_t kReceiverSsrc = 0x23456; -const int64_t kOneWayNetworkDelayMs = 100; +constexpr TimeDelta kOneWayNetworkDelay = TimeDelta::Millis(100); const uint8_t kBaseLayerTid = 0; const uint8_t kHigherLayerTid = 1; const uint16_t kSequenceNumber = 100; @@ -44,6 +45,10 @@ const uint8_t kPayloadType = 100; const int kWidth = 320; const int kHeight = 100; +MATCHER_P2(Near, value, margin, "") { + return value - margin <= arg && arg <= value + margin; +} + #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" @@ -110,7 +115,7 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver { receive_statistics_(ReceiveStatistics::Create(clock)), clock_(clock) { CreateModuleImpl(); - transport_.SimulateNetworkDelay(kOneWayNetworkDelayMs, clock); + transport_.SimulateNetworkDelay(kOneWayNetworkDelay.ms(), clock); } const bool is_sender_; @@ -291,28 +296,16 @@ TEST_F(RtpRtcpImplTest, Rtt) { EXPECT_EQ(0, receiver_.impl_->SendRTCP(kRtcpReport)); // Verify RTT. - int64_t rtt; - int64_t avg_rtt; - int64_t min_rtt; - int64_t max_rtt; - EXPECT_EQ( - 0, sender_.impl_->RTT(kReceiverSsrc, &rtt, &avg_rtt, &min_rtt, &max_rtt)); - EXPECT_NEAR(2 * kOneWayNetworkDelayMs, rtt, 1); - EXPECT_NEAR(2 * kOneWayNetworkDelayMs, avg_rtt, 1); - EXPECT_NEAR(2 * kOneWayNetworkDelayMs, min_rtt, 1); - EXPECT_NEAR(2 * kOneWayNetworkDelayMs, max_rtt, 1); - - // No RTT from other ssrc. - EXPECT_EQ(-1, sender_.impl_->RTT(kReceiverSsrc + 1, &rtt, &avg_rtt, &min_rtt, - &max_rtt)); + EXPECT_THAT(sender_.impl_->LastRtt(), + Near(2 * kOneWayNetworkDelay, TimeDelta::Millis(1))); // Verify RTT from rtt_stats config. EXPECT_EQ(0, sender_.rtt_stats_.LastProcessedRtt()); EXPECT_EQ(0, sender_.impl_->rtt_ms()); sender_.impl_->Process(); - EXPECT_NEAR(2 * kOneWayNetworkDelayMs, sender_.rtt_stats_.LastProcessedRtt(), - 1); - EXPECT_NEAR(2 * kOneWayNetworkDelayMs, sender_.impl_->rtt_ms(), 1); + EXPECT_NEAR(2 * kOneWayNetworkDelay.ms(), + sender_.rtt_stats_.LastProcessedRtt(), 1); + EXPECT_NEAR(2 * kOneWayNetworkDelay.ms(), sender_.impl_->rtt_ms(), 1); } TEST_F(RtpRtcpImplTest, RttForReceiverOnly) { @@ -329,9 +322,9 @@ TEST_F(RtpRtcpImplTest, RttForReceiverOnly) { EXPECT_EQ(0, receiver_.rtt_stats_.LastProcessedRtt()); EXPECT_EQ(0, receiver_.impl_->rtt_ms()); receiver_.impl_->Process(); - EXPECT_NEAR(2 * kOneWayNetworkDelayMs, + EXPECT_NEAR(2 * kOneWayNetworkDelay.ms(), receiver_.rtt_stats_.LastProcessedRtt(), 1); - EXPECT_NEAR(2 * kOneWayNetworkDelayMs, receiver_.impl_->rtt_ms(), 1); + EXPECT_NEAR(2 * kOneWayNetworkDelay.ms(), receiver_.impl_->rtt_ms(), 1); } TEST_F(RtpRtcpImplTest, NoSrBeforeMedia) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index a5c5adfc16..ebd16eeeb3 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h @@ -378,14 +378,6 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { // Returns -1 on failure else 0. virtual int32_t SetCNAME(absl::string_view cname) = 0; - // Returns current RTT (round-trip time) estimate. - // Returns -1 on failure else 0. - [[deprecated]] virtual int32_t RTT(uint32_t remote_ssrc, - int64_t* rtt, - int64_t* avg_rtt, - int64_t* min_rtt, - int64_t* max_rtt) const = 0; - // Returns current RTT (round-trip time) estimate. virtual absl::optional LastRtt() const = 0;