From 434deda6faaae2da5a036c351ee70a5704b752ef Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 22 May 2023 12:56:56 +0200 Subject: [PATCH] Cleanup RtcpReceiver from using RtcpBandwidthObser callback interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All known users were updated to NetworkLinkRtcpObserver interface instead Bug: webrtc:13757 Change-Id: I1f2a7be0c9192890b38a811a739ddd666b0985f2 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/306161 Reviewed-by: Åsa Persson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#40113} --- modules/rtp_rtcp/BUILD.gn | 1 - modules/rtp_rtcp/include/rtp_rtcp_defines.h | 15 -- .../mocks/mock_rtcp_bandwidth_observer.h | 28 ---- modules/rtp_rtcp/source/rtcp_receiver.cc | 29 +--- modules/rtp_rtcp/source/rtcp_receiver.h | 1 - .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 152 +++++------------- modules/rtp_rtcp/source/rtp_rtcp_interface.h | 6 - 7 files changed, 44 insertions(+), 188 deletions(-) delete mode 100644 modules/rtp_rtcp/mocks/mock_rtcp_bandwidth_observer.h diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index da585820f9..2048cdb1d8 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -482,7 +482,6 @@ rtc_library("mock_rtp_rtcp") { public = [ "mocks/mock_network_link_rtcp_observer.h", "mocks/mock_recovered_packet_receiver.h", - "mocks/mock_rtcp_bandwidth_observer.h", "mocks/mock_rtcp_rtt_stats.h", "mocks/mock_rtp_rtcp.h", ] diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 4bdad6e86d..652ea25224 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -167,21 +167,6 @@ class RtcpLossNotificationObserver { bool decodability_flag) = 0; }; -// TODO(bugs.webrtc.org/13757): Remove this interface in favor of the -// NetworkLinkRtcpObserver that uses more descriptive types. -class RtcpBandwidthObserver { - public: - // REMB or TMMBR - virtual void OnReceivedEstimatedBitrate(uint32_t bitrate) = 0; - - virtual void OnReceivedRtcpReceiverReport( - const ReportBlockList& report_blocks, - int64_t rtt, - int64_t now_ms) = 0; - - virtual ~RtcpBandwidthObserver() {} -}; - // Interface to watch incoming rtcp packets related to the link in general. // All message handlers have default empty implementation. This way users only // need to implement the ones they are interested in. diff --git a/modules/rtp_rtcp/mocks/mock_rtcp_bandwidth_observer.h b/modules/rtp_rtcp/mocks/mock_rtcp_bandwidth_observer.h deleted file mode 100644 index 12f143ae8b..0000000000 --- a/modules/rtp_rtcp/mocks/mock_rtcp_bandwidth_observer.h +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright (c) 2018 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef MODULES_RTP_RTCP_MOCKS_MOCK_RTCP_BANDWIDTH_OBSERVER_H_ -#define MODULES_RTP_RTCP_MOCKS_MOCK_RTCP_BANDWIDTH_OBSERVER_H_ - -#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "test/gmock.h" - -namespace webrtc { - -class MockRtcpBandwidthObserver : public RtcpBandwidthObserver { - public: - MOCK_METHOD(void, OnReceivedEstimatedBitrate, (uint32_t), (override)); - MOCK_METHOD(void, - OnReceivedRtcpReceiverReport, - (const ReportBlockList&, int64_t, int64_t), - (override)); -}; -} // namespace webrtc -#endif // MODULES_RTP_RTCP_MOCKS_MOCK_RTCP_BANDWIDTH_OBSERVER_H_ diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index bf63828ff7..7236875f71 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -144,7 +144,6 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config, receiver_only_(config.receiver_only), rtp_rtcp_(owner), registered_ssrcs_(false, config), - deprecated_rtcp_bandwidth_observer_(config.bandwidth_callback), network_link_rtcp_observer_(config.network_link_rtcp_observer), rtcp_intra_frame_observer_(config.intra_frame_callback), rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer), @@ -173,7 +172,6 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config, receiver_only_(config.receiver_only), rtp_rtcp_(owner), registered_ssrcs_(true, config), - deprecated_rtcp_bandwidth_observer_(config.bandwidth_callback), network_link_rtcp_observer_(config.network_link_rtcp_observer), rtcp_intra_frame_observer_(config.intra_frame_callback), rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer), @@ -1076,16 +1074,10 @@ void RTCPReceiver::NotifyTmmbrUpdated() { std::vector bounding = TMMBRHelp::FindBoundingSet(TmmbrReceived()); - if (!bounding.empty()) { + if (!bounding.empty() && network_link_rtcp_observer_) { // We have a new bandwidth estimate on this channel. uint64_t bitrate_bps = TMMBRHelp::CalcMinBitrateBps(bounding); - if (deprecated_rtcp_bandwidth_observer_ && - bitrate_bps <= std::numeric_limits::max()) { - deprecated_rtcp_bandwidth_observer_->OnReceivedEstimatedBitrate( - bitrate_bps); - } - if (network_link_rtcp_observer_ && - bitrate_bps < std::numeric_limits::max()) { + if (bitrate_bps < std::numeric_limits::max()) { network_link_rtcp_observer_->OnReceiverEstimatedMaxBitrate( clock_->CurrentTime(), DataRate::BitsPerSec(bitrate_bps)); } @@ -1147,23 +1139,6 @@ void RTCPReceiver::TriggerCallbacksFromRtcpPacket( loss_notification->decodability_flag()); } } - if (deprecated_rtcp_bandwidth_observer_) { - RTC_DCHECK(!receiver_only_); - if (packet_information.packet_type_flags & kRtcpRemb) { - RTC_LOG(LS_VERBOSE) - << "Incoming REMB: " - << packet_information.receiver_estimated_max_bitrate_bps; - deprecated_rtcp_bandwidth_observer_->OnReceivedEstimatedBitrate( - packet_information.receiver_estimated_max_bitrate_bps); - } - if ((packet_information.packet_type_flags & kRtcpSr) || - (packet_information.packet_type_flags & kRtcpRr)) { - deprecated_rtcp_bandwidth_observer_->OnReceivedRtcpReceiverReport( - packet_information.report_blocks, - packet_information.rtt.value_or(TimeDelta::Zero()).ms(), - clock_->TimeInMilliseconds()); - } - } if (network_link_rtcp_observer_) { Timestamp now = clock_->CurrentTime(); diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h index 4453c5724c..bc0ce566b4 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -355,7 +355,6 @@ class RTCPReceiver final { // The set of registered local SSRCs. RegisteredSsrcs registered_ssrcs_; - RtcpBandwidthObserver* const deprecated_rtcp_bandwidth_observer_; NetworkLinkRtcpObserver* const network_link_rtcp_observer_; RtcpIntraFrameObserver* const rtcp_intra_frame_observer_; RtcpLossNotificationObserver* const rtcp_loss_notification_observer_; diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 78abe07139..02dd0a7da6 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -21,7 +21,6 @@ #include "api/video/video_bitrate_allocator.h" #include "modules/rtp_rtcp/include/report_block_data.h" #include "modules/rtp_rtcp/mocks/mock_network_link_rtcp_observer.h" -#include "modules/rtp_rtcp/mocks/mock_rtcp_bandwidth_observer.h" #include "modules/rtp_rtcp/source/byte_io.h" #include "modules/rtp_rtcp/source/rtcp_packet.h" #include "modules/rtp_rtcp/source/rtcp_packet/app.h" @@ -52,6 +51,7 @@ namespace { using rtcp::ReceiveTimeInfo; using ::testing::_; using ::testing::AllOf; +using ::testing::ElementsAre; using ::testing::ElementsAreArray; using ::testing::Eq; using ::testing::Field; @@ -145,7 +145,6 @@ struct ReceiverMocks { // Callbacks to packet_type_counter_observer are frequent but most of the time // are not interesting. NiceMock packet_type_counter_observer; - StrictMock bandwidth_observer; StrictMock intra_frame_observer; StrictMock rtcp_loss_notification_observer; StrictMock bitrate_allocation_observer; @@ -159,7 +158,7 @@ RtpRtcpInterface::Configuration DefaultConfiguration(ReceiverMocks* mocks) { config.receiver_only = false; config.rtcp_packet_type_counter_observer = &mocks->packet_type_counter_observer; - config.bandwidth_callback = &mocks->bandwidth_observer; + config.network_link_rtcp_observer = &mocks->network_link_rtcp_observer; config.intra_frame_callback = &mocks->intra_frame_observer; config.rtcp_loss_notification_observer = &mocks->rtcp_loss_notification_observer; @@ -199,13 +198,10 @@ TEST(RtcpReceiverTest, InjectSrPacket) { EXPECT_FALSE(receiver.GetSenderReportStats()); - int64_t now = mocks.clock.TimeInMilliseconds(); rtcp::SenderReport sr; sr.SetSenderSsrc(kSenderSsrc); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks(IsEmpty())); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(IsEmpty(), _, now)); receiver.IncomingPacket(sr.Build()); EXPECT_TRUE(receiver.GetSenderReportStats()); @@ -216,15 +212,12 @@ TEST(RtcpReceiverTest, InjectSrPacketFromUnknownSender) { RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); - int64_t now = mocks.clock.TimeInMilliseconds(); rtcp::SenderReport sr; sr.SetSenderSsrc(kUnknownSenderSsrc); // The parser will handle report blocks in Sender Report from other than their // expected peer. EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(_, _, now)); receiver.IncomingPacket(sr.Build()); // But will not flag that he's gotten sender information. @@ -254,7 +247,7 @@ TEST(RtcpReceiverTest, InjectSrPacketCalculatesRTT) { sr.AddReportBlock(block); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); + EXPECT_CALL(mocks.network_link_rtcp_observer, OnRttUpdate); receiver.IncomingPacket(sr.Build()); EXPECT_THAT(receiver.LastRtt(), Near(kRtt, TimeDelta::Millis(1))); @@ -283,20 +276,19 @@ TEST(RtcpReceiverTest, InjectSrPacketCalculatesNegativeRTTAsOneMs) { sr.AddReportBlock(block); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks(SizeIs(1))); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(SizeIs(1), _, _)); + EXPECT_CALL(mocks.network_link_rtcp_observer, + OnRttUpdate(_, TimeDelta::Millis(1))); receiver.IncomingPacket(sr.Build()); EXPECT_EQ(receiver.LastRtt(), TimeDelta::Millis(1)); } -TEST(RtcpReceiverTest, - TwoReportBlocksWithLastOneWithoutLastSrCalculatesRttForBandwidthObserver) { +TEST(RtcpReceiverTest, TwoReportBlocksWithLastOneWithoutLastSrCalculatesRtt) { ReceiverMocks mocks; RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); - const TimeDelta kRtt = TimeDelta::Millis(120); + const TimeDelta kRtt = TimeDelta::Millis(125); const uint32_t kDelayNtp = 123000; const TimeDelta kDelay = CompactNtpRttToTimeDelta(kDelayNtp); @@ -315,8 +307,7 @@ TEST(RtcpReceiverTest, sr.AddReportBlock(block); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks(SizeIs(2))); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(SizeIs(2), kRtt.ms(), _)); + EXPECT_CALL(mocks.network_link_rtcp_observer, OnRttUpdate(_, kRtt)); receiver.IncomingPacket(sr.Build()); } @@ -325,13 +316,10 @@ TEST(RtcpReceiverTest, InjectRrPacket) { RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); - int64_t now = mocks.clock.TimeInMilliseconds(); rtcp::ReceiverReport rr; rr.SetSenderSsrc(kSenderSsrc); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks(IsEmpty())); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(IsEmpty(), _, now)); receiver.IncomingPacket(rr.Build()); EXPECT_THAT(receiver.GetLatestReportBlockData(), IsEmpty()); @@ -342,7 +330,6 @@ TEST(RtcpReceiverTest, InjectRrPacketWithReportBlockNotToUsIgnored) { RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); - int64_t now = mocks.clock.TimeInMilliseconds(); rtcp::ReportBlock rb; rb.SetMediaSsrc(kNotToUsSsrc); rtcp::ReceiverReport rr; @@ -350,8 +337,7 @@ TEST(RtcpReceiverTest, InjectRrPacketWithReportBlockNotToUsIgnored) { rr.AddReportBlock(rb); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks(IsEmpty())); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(IsEmpty(), _, now)); + EXPECT_CALL(mocks.network_link_rtcp_observer, OnReport).Times(0); receiver.IncomingPacket(rr.Build()); EXPECT_EQ(0, receiver.LastReceivedReportBlockMs()); @@ -363,7 +349,7 @@ TEST(RtcpReceiverTest, InjectRrPacketWithOneReportBlock) { RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); - int64_t now = mocks.clock.TimeInMilliseconds(); + Timestamp now = mocks.clock.CurrentTime(); rtcp::ReportBlock rb; rb.SetMediaSsrc(kReceiverMainSsrc); @@ -372,11 +358,10 @@ TEST(RtcpReceiverTest, InjectRrPacketWithOneReportBlock) { rr.AddReportBlock(rb); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks(SizeIs(1))); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(SizeIs(1), _, now)); + EXPECT_CALL(mocks.network_link_rtcp_observer, OnReport(now, SizeIs(1))); receiver.IncomingPacket(rr.Build()); - EXPECT_EQ(now, receiver.LastReceivedReportBlockMs()); + EXPECT_EQ(receiver.LastReceivedReportBlockMs(), now.ms()); EXPECT_THAT(receiver.GetLatestReportBlockData(), SizeIs(1)); } @@ -385,7 +370,7 @@ TEST(RtcpReceiverTest, InjectSrPacketWithOneReportBlock) { RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); - int64_t now = mocks.clock.TimeInMilliseconds(); + Timestamp now = mocks.clock.CurrentTime(); rtcp::ReportBlock rb; rb.SetMediaSsrc(kReceiverMainSsrc); @@ -394,11 +379,10 @@ TEST(RtcpReceiverTest, InjectSrPacketWithOneReportBlock) { sr.AddReportBlock(rb); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks(SizeIs(1))); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(SizeIs(1), _, now)); + EXPECT_CALL(mocks.network_link_rtcp_observer, OnReport(now, SizeIs(1))); receiver.IncomingPacket(sr.Build()); - EXPECT_EQ(now, receiver.LastReceivedReportBlockMs()); + EXPECT_EQ(receiver.LastReceivedReportBlockMs(), now.ms()); EXPECT_THAT(receiver.GetLatestReportBlockData(), SizeIs(1)); } @@ -410,7 +394,7 @@ TEST(RtcpReceiverTest, InjectRrPacketWithTwoReportBlocks) { RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); - int64_t now = mocks.clock.TimeInMilliseconds(); + Timestamp now = mocks.clock.CurrentTime(); rtcp::ReportBlock rb1; rb1.SetMediaSsrc(kReceiverMainSsrc); @@ -428,11 +412,10 @@ TEST(RtcpReceiverTest, InjectRrPacketWithTwoReportBlocks) { rr1.AddReportBlock(rb2); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks(SizeIs(2))); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(SizeIs(2), _, now)); + EXPECT_CALL(mocks.network_link_rtcp_observer, OnReport(now, SizeIs(2))); receiver.IncomingPacket(rr1.Build()); - EXPECT_EQ(now, receiver.LastReceivedReportBlockMs()); + EXPECT_EQ(receiver.LastReceivedReportBlockMs(), now.ms()); EXPECT_THAT( receiver.GetLatestReportBlockData(), UnorderedElementsAre(Property(&ReportBlockData::fraction_lost_raw, 0), @@ -458,11 +441,10 @@ TEST(RtcpReceiverTest, InjectRrPacketWithTwoReportBlocks) { // Advance time to make 1st sent time and 2nd sent time different. mocks.clock.AdvanceTimeMilliseconds(500); - now = mocks.clock.TimeInMilliseconds(); + now = mocks.clock.CurrentTime(); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks(SizeIs(2))); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(SizeIs(2), _, now)); + EXPECT_CALL(mocks.network_link_rtcp_observer, OnReport(now, SizeIs(2))); receiver.IncomingPacket(rr2.Build()); EXPECT_THAT( @@ -499,14 +481,12 @@ TEST(RtcpReceiverTest, rr1.SetSenderSsrc(kSenderSsrc); rr1.AddReportBlock(rb1); - int64_t now = mocks.clock.TimeInMilliseconds(); + Timestamp now = mocks.clock.CurrentTime(); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks(SizeIs(1))); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(SizeIs(1), _, now)); receiver.IncomingPacket(rr1.Build()); - EXPECT_EQ(now, receiver.LastReceivedReportBlockMs()); + EXPECT_EQ(receiver.LastReceivedReportBlockMs(), now.ms()); EXPECT_THAT(receiver.GetLatestReportBlockData(), ElementsAre(AllOf( @@ -527,8 +507,6 @@ TEST(RtcpReceiverTest, rr2.AddReportBlock(rb2); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks(SizeIs(1))); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedRtcpReceiverReport(SizeIs(1), _, now)); receiver.IncomingPacket(rr2.Build()); EXPECT_THAT(receiver.GetLatestReportBlockData(), @@ -543,10 +521,7 @@ TEST(RtcpReceiverTest, TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnReportBlocks) { ReceiverMocks mocks; - RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks); - config.bandwidth_callback = nullptr; - config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer; - RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); + RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); rtcp::ReportBlock rb1; @@ -600,7 +575,6 @@ TEST(RtcpReceiverTest, GetRtt) { Timestamp now = mocks.clock.CurrentTime(); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); EXPECT_CALL(mocks.network_link_rtcp_observer, OnRttUpdate(now, Gt(TimeDelta::Zero()))); receiver.IncomingPacket(rr.Build()); @@ -641,7 +615,7 @@ TEST(RtcpReceiverTest, InjectSdesWithOneChunk) { receiver.IncomingPacket(sdes.Build()); } -TEST(RtcpReceiverTest, InjectByePacket_RemovesReportBlocks) { +TEST(RtcpReceiverTest, InjectByePacketRemovesReportBlocks) { ReceiverMocks mocks; RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); @@ -656,7 +630,6 @@ TEST(RtcpReceiverTest, InjectByePacket_RemovesReportBlocks) { rr.AddReportBlock(rb2); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rr.Build()); EXPECT_THAT(receiver.GetLatestReportBlockData(), SizeIs(2)); @@ -671,7 +644,6 @@ TEST(RtcpReceiverTest, InjectByePacket_RemovesReportBlocks) { // Inject packet again. EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rr.Build()); EXPECT_THAT(receiver.GetLatestReportBlockData(), SizeIs(2)); @@ -1169,7 +1141,6 @@ TEST(RtcpReceiverTest, ReceiverRttResetOnSrWithoutXr) { sr.SetSenderSsrc(kSenderSsrc); sr.AddReportBlock(rb); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(sr.Build()); @@ -1392,7 +1363,6 @@ TEST(RtcpReceiverTest, ReceiveReportTimeout) { rr1.AddReportBlock(rb1); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rr1.Build()); mocks.clock.AdvanceTimeMilliseconds(3 * kRtcpIntervalMs - 1); @@ -1402,7 +1372,6 @@ TEST(RtcpReceiverTest, ReceiveReportTimeout) { // Add a RR with the same extended max as the previous RR to trigger a // sequence number timeout, but not a RR timeout. EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rr1.Build()); mocks.clock.AdvanceTimeMilliseconds(2); @@ -1427,7 +1396,6 @@ TEST(RtcpReceiverTest, ReceiveReportTimeout) { rr2.AddReportBlock(rb2); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rr2.Build()); EXPECT_FALSE(receiver.RtcpRrTimeout()); @@ -1436,7 +1404,6 @@ TEST(RtcpReceiverTest, ReceiveReportTimeout) { // Verify we can get a timeout again once we've received new RR. mocks.clock.AdvanceTimeMilliseconds(2 * kRtcpIntervalMs); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rr2.Build()); mocks.clock.AdvanceTimeMilliseconds(kRtcpIntervalMs + 1); @@ -1460,10 +1427,10 @@ TEST(RtcpReceiverTest, TmmbrPacketAccepted) { RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); - const uint32_t kBitrateBps = 30000; + const DataRate kBitrate = DataRate::BitsPerSec(30'000); auto tmmbr = std::make_unique(); tmmbr->SetSenderSsrc(kSenderSsrc); - tmmbr->AddTmmbr(rtcp::TmmbItem(kReceiverMainSsrc, kBitrateBps, 0)); + tmmbr->AddTmmbr(rtcp::TmmbItem(kReceiverMainSsrc, kBitrate.bps(), 0)); auto sr = std::make_unique(); sr->SetSenderSsrc(kSenderSsrc); rtcp::CompoundPacket compound; @@ -1472,15 +1439,15 @@ TEST(RtcpReceiverTest, TmmbrPacketAccepted) { EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); EXPECT_CALL(mocks.rtp_rtcp_impl, SetTmmbn(SizeIs(1))); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedEstimatedBitrate(kBitrateBps)); + EXPECT_CALL(mocks.network_link_rtcp_observer, + OnReceiverEstimatedMaxBitrate(_, kBitrate)); receiver.IncomingPacket(compound.Build()); - std::vector tmmbr_received = receiver.TmmbrReceived(); - ASSERT_EQ(1u, tmmbr_received.size()); - EXPECT_EQ(kBitrateBps, tmmbr_received[0].bitrate_bps()); - EXPECT_EQ(kSenderSsrc, tmmbr_received[0].ssrc()); + EXPECT_THAT( + receiver.TmmbrReceived(), + ElementsAre(AllOf( + Property(&rtcp::TmmbItem::bitrate_bps, Eq(kBitrate.bps())), + Property(&rtcp::TmmbItem::ssrc, Eq(kSenderSsrc))))); } TEST(RtcpReceiverTest, TmmbrPacketNotForUsIgnored) { @@ -1500,8 +1467,8 @@ TEST(RtcpReceiverTest, TmmbrPacketNotForUsIgnored) { compound.Append(std::move(tmmbr)); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedEstimatedBitrate).Times(0); + EXPECT_CALL(mocks.network_link_rtcp_observer, OnReceiverEstimatedMaxBitrate) + .Times(0); receiver.IncomingPacket(compound.Build()); EXPECT_EQ(0u, receiver.TmmbrReceived().size()); @@ -1522,8 +1489,8 @@ TEST(RtcpReceiverTest, TmmbrPacketZeroRateIgnored) { compound.Append(std::move(tmmbr)); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedEstimatedBitrate).Times(0); + EXPECT_CALL(mocks.network_link_rtcp_observer, OnReceiverEstimatedMaxBitrate) + .Times(0); receiver.IncomingPacket(compound.Build()); EXPECT_EQ(0u, receiver.TmmbrReceived().size()); @@ -1548,8 +1515,8 @@ TEST(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) { EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); EXPECT_CALL(mocks.rtp_rtcp_impl, SetTmmbn); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedEstimatedBitrate); + EXPECT_CALL(mocks.network_link_rtcp_observer, + OnReceiverEstimatedMaxBitrate); receiver.IncomingPacket(compound.Build()); // 5 seconds between each packet. @@ -1610,7 +1577,6 @@ TEST(RtcpReceiverTest, EXPECT_EQ(0u, report_block.num_rtts()); }); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rtcp_report.Build()); } @@ -1645,7 +1611,6 @@ TEST(RtcpReceiverTest, VerifyRttObtainedFromReportBlockDataObserver) { sr.AddReportBlock(block); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); InSequence sequence; EXPECT_CALL(observer, OnReportBlockDataUpdated) .WillOnce([&](ReportBlockData report_block_data) { @@ -1677,7 +1642,6 @@ TEST(RtcpReceiverTest, GetReportBlockDataAfterOneReportBlock) { rtcp_report.SetSenderSsrc(kSenderSsrc); rtcp_report.AddReportBlock(rtcp_block); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rtcp_report.Build()); auto report_block_datas = receiver.GetLatestReportBlockData(); @@ -1703,7 +1667,6 @@ TEST(RtcpReceiverTest, GetReportBlockDataAfterTwoReportBlocksOfSameSsrc) { rtcp_report1.SetSenderSsrc(kSenderSsrc); rtcp_report1.AddReportBlock(rtcp_block1); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rtcp_report1.Build()); // Inject a report block with an increased the sequence number for the same @@ -1716,7 +1679,6 @@ TEST(RtcpReceiverTest, GetReportBlockDataAfterTwoReportBlocksOfSameSsrc) { rtcp_report2.SetSenderSsrc(kSenderSsrc); rtcp_report2.AddReportBlock(rtcp_block2); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rtcp_report2.Build()); // Only the latest block should be returned. @@ -1743,7 +1705,6 @@ TEST(RtcpReceiverTest, GetReportBlockDataAfterTwoReportBlocksOfDifferentSsrcs) { rtcp_report1.SetSenderSsrc(kSenderSsrc); rtcp_report1.AddReportBlock(rtcp_block1); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rtcp_report1.Build()); // Inject a report block for a different source SSRC. @@ -1755,7 +1716,6 @@ TEST(RtcpReceiverTest, GetReportBlockDataAfterTwoReportBlocksOfDifferentSsrcs) { rtcp_report2.SetSenderSsrc(kSenderSsrc); rtcp_report2.AddReportBlock(rtcp_block2); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); - EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); receiver.IncomingPacket(rtcp_report2.Build()); // Both report blocks should be returned. @@ -1772,8 +1732,6 @@ TEST(RtcpReceiverTest, GetReportBlockDataAfterTwoReportBlocksOfDifferentSsrcs) { TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnTransportFeedback) { ReceiverMocks mocks; RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks); - config.bandwidth_callback = nullptr; - config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer; RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); @@ -1798,8 +1756,6 @@ TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnTransportFeedbackOnRtxSsrc) { ReceiverMocks mocks; RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks); - config.bandwidth_callback = nullptr; - config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer; RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); @@ -1816,10 +1772,7 @@ TEST(RtcpReceiverTest, TEST(RtcpReceiverTest, DoesNotNotifyNetworkLinkObserverOnTransportFeedbackForUnregistedSsrc) { ReceiverMocks mocks; - RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks); - config.bandwidth_callback = nullptr; - config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer; - RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); + RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); rtcp::TransportFeedback packet; @@ -1832,27 +1785,9 @@ TEST(RtcpReceiverTest, receiver.IncomingPacket(packet.Build()); } -TEST(RtcpReceiverTest, ReceivesRemb) { - ReceiverMocks mocks; - RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); - receiver.SetRemoteSSRC(kSenderSsrc); - - const uint32_t kBitrateBps = 500000; - rtcp::Remb remb; - remb.SetSenderSsrc(kSenderSsrc); - remb.SetBitrateBps(kBitrateBps); - - EXPECT_CALL(mocks.bandwidth_observer, - OnReceivedEstimatedBitrate(kBitrateBps)); - receiver.IncomingPacket(remb.Build()); -} - TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnRemb) { ReceiverMocks mocks; - RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks); - config.bandwidth_callback = nullptr; - config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer; - RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); + RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); rtcp::Remb remb; @@ -1867,10 +1802,7 @@ TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnRemb) { TEST(RtcpReceiverTest, HandlesInvalidTransportFeedback) { ReceiverMocks mocks; - RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks); - config.bandwidth_callback = nullptr; - config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer; - RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl); + RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); // Send a compound packet with a TransportFeedback followed by something else. diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index 6e62bf8d7f..73c6c4329f 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h @@ -69,12 +69,6 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { // Called when the receiver sends a loss notification. RtcpLossNotificationObserver* rtcp_loss_notification_observer = nullptr; - // Called when we receive a changed estimate from the receiver of out - // stream. - // TODO(bugs.webrtc.org/13757): Deprecated and Remove in favor of - // `network_link_rtcp_observer` - RtcpBandwidthObserver* bandwidth_callback = nullptr; - // Called when receive an RTCP message related to the link in general, e.g. // bandwidth estimation related message. NetworkLinkRtcpObserver* network_link_rtcp_observer = nullptr;