From 2198f95118035afcadc00c907404387b4e99501b Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Tue, 9 May 2023 10:11:21 +0200 Subject: [PATCH] In RtcpTransceiver remove callback that pass rtcp::ReportBlock Same information is already passed using ReporBlockData class Bug: webrtc:8239 Change-Id: Iaae0edd34941c45527414a20923b5238e7a822fe Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304641 Reviewed-by: Emil Lundmark Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#40021} --- .../rtp_rtcp/source/rtcp_transceiver_config.h | 10 ------ .../rtp_rtcp/source/rtcp_transceiver_impl.cc | 36 ++++++------------- .../rtp_rtcp/source/rtcp_transceiver_impl.h | 9 ++--- .../source/rtcp_transceiver_impl_unittest.cc | 13 ------- 4 files changed, 12 insertions(+), 56 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_config.h b/modules/rtp_rtcp/source/rtcp_transceiver_config.h index c9140f2140..fbaa917ae7 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_config.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_config.h @@ -22,7 +22,6 @@ #include "api/video/video_bitrate_allocation.h" #include "modules/rtp_rtcp/include/report_block_data.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "modules/rtp_rtcp/source/rtcp_packet/report_block.h" #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "system_wrappers/include/clock.h" #include "system_wrappers/include/ntp_time.h" @@ -45,11 +44,6 @@ class NetworkLinkRtcpObserver { virtual void OnReceiverEstimatedMaxBitrate(Timestamp receive_time, DataRate bitrate) {} - // TODO(bugs.webrtc.org/8239): Remove this callback in favor of the `OnReport` - virtual void OnReportBlocks( - Timestamp receive_time, - rtc::ArrayView report_blocks) {} - // Called on an RTCP packet with sender or receiver reports with non zero // report blocks. Report blocks are combined from all reports into one array. virtual void OnReport(Timestamp receive_time, @@ -111,10 +105,6 @@ class RtpStreamRtcpHandler { virtual void OnFir(uint32_t sender_ssrc) {} virtual void OnPli(uint32_t sender_ssrc) {} - // TODO(bugs.webrtc.org/8239): Remove this callback in favor of the `OnReport` - virtual void OnReportBlock(uint32_t sender_ssrc, - const rtcp::ReportBlock& report_block) {} - // Called on an RTCP packet with sender or receiver reports with a report // block for the handled RTP stream. virtual void OnReport(const ReportBlockData& report_block) {} diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index d98ac51433..bc54d9a407 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc @@ -177,7 +177,6 @@ void RtcpTransceiverImpl::SetReadyToSend(bool ready) { void RtcpTransceiverImpl::ReceivePacket(rtc::ArrayView packet, Timestamp now) { // Report blocks may be spread across multiple sender and receiver reports. - std::vector rtcp_report_blocks; std::vector report_blocks; while (!packet.empty()) { @@ -185,13 +184,13 @@ void RtcpTransceiverImpl::ReceivePacket(rtc::ArrayView packet, if (!rtcp_block.Parse(packet.data(), packet.size())) break; - HandleReceivedPacket(rtcp_block, now, rtcp_report_blocks, report_blocks); + HandleReceivedPacket(rtcp_block, now, report_blocks); packet = packet.subview(rtcp_block.packet_size()); } if (!report_blocks.empty()) { - ProcessReportBlocks(now, rtcp_report_blocks, report_blocks); + ProcessReportBlocks(now, report_blocks); } } @@ -278,19 +277,16 @@ void RtcpTransceiverImpl::SendFullIntraRequest( void RtcpTransceiverImpl::HandleReceivedPacket( const rtcp::CommonHeader& rtcp_packet_header, Timestamp now, - std::vector& rtcp_report_blocks, std::vector& report_blocks) { switch (rtcp_packet_header.type()) { case rtcp::Bye::kPacketType: HandleBye(rtcp_packet_header); break; case rtcp::SenderReport::kPacketType: - HandleSenderReport(rtcp_packet_header, now, rtcp_report_blocks, - report_blocks); + HandleSenderReport(rtcp_packet_header, now, report_blocks); break; case rtcp::ReceiverReport::kPacketType: - HandleReceiverReport(rtcp_packet_header, now, rtcp_report_blocks, - report_blocks); + HandleReceiverReport(rtcp_packet_header, now, report_blocks); break; case rtcp::ExtendedReports::kPacketType: HandleExtendedReports(rtcp_packet_header, now); @@ -319,7 +315,6 @@ void RtcpTransceiverImpl::HandleBye( void RtcpTransceiverImpl::HandleSenderReport( const rtcp::CommonHeader& rtcp_packet_header, Timestamp now, - std::vector& rtcp_report_blocks, std::vector& report_blocks) { rtcp::SenderReport sender_report; if (!sender_report.Parse(rtcp_packet_header)) @@ -327,33 +322,25 @@ void RtcpTransceiverImpl::HandleSenderReport( RemoteSenderState& remote_sender = remote_senders_[sender_report.sender_ssrc()]; remote_sender.last_received_sender_report = {{now, sender_report.ntp()}}; - const auto& received_report_blocks = sender_report.report_blocks(); - HandleReportBlocks(sender_report.sender_ssrc(), now, received_report_blocks, - report_blocks); - rtcp_report_blocks.insert(rtcp_report_blocks.end(), - received_report_blocks.begin(), - received_report_blocks.end()); + HandleReportBlocks(sender_report.sender_ssrc(), now, + sender_report.report_blocks(), report_blocks); - for (MediaReceiverRtcpObserver* observer : remote_sender.observers) + for (MediaReceiverRtcpObserver* observer : remote_sender.observers) { observer->OnSenderReport(sender_report.sender_ssrc(), sender_report.ntp(), sender_report.rtp_timestamp()); + } } void RtcpTransceiverImpl::HandleReceiverReport( const rtcp::CommonHeader& rtcp_packet_header, Timestamp now, - std::vector& rtcp_report_blocks, std::vector& report_blocks) { rtcp::ReceiverReport receiver_report; if (!receiver_report.Parse(rtcp_packet_header)) { return; } - const auto& received_report_blocks = receiver_report.report_blocks(); - HandleReportBlocks(receiver_report.sender_ssrc(), now, received_report_blocks, - report_blocks); - rtcp_report_blocks.insert(rtcp_report_blocks.end(), - received_report_blocks.begin(), - received_report_blocks.end()); + HandleReportBlocks(receiver_report.sender_ssrc(), now, + receiver_report.report_blocks(), report_blocks); } void RtcpTransceiverImpl::HandleReportBlocks( @@ -383,7 +370,6 @@ void RtcpTransceiverImpl::HandleReportBlocks( if (rtt.has_value()) { state.report_block.AddRoundTripTimeSample(*rtt); } - state.handler->OnReportBlock(sender_ssrc, block); state.handler->OnReport(state.report_block); report_blocks.push_back(state.report_block); } else { @@ -542,7 +528,6 @@ void RtcpTransceiverImpl::HandleDlrr(const rtcp::Dlrr& dlrr, Timestamp now) { void RtcpTransceiverImpl::ProcessReportBlocks( Timestamp now, - rtc::ArrayView rtcp_report_blocks, rtc::ArrayView report_blocks) { RTC_DCHECK(!report_blocks.empty()); if (config_.network_link_observer == nullptr) { @@ -562,7 +547,6 @@ void RtcpTransceiverImpl::ProcessReportBlocks( if (num_rtts > 0) { config_.network_link_observer->OnRttUpdate(now, rtt_sum / num_rtts); } - config_.network_link_observer->OnReportBlocks(now, rtcp_report_blocks); config_.network_link_observer->OnReport(now, report_blocks); } diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h index 5620c97c27..955d03f6e4 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h @@ -92,17 +92,14 @@ class RtcpTransceiverImpl { void HandleReceivedPacket(const rtcp::CommonHeader& rtcp_packet_header, Timestamp now, - std::vector& rtcp_report_blocks, std::vector& report_blocks); // Individual rtcp packet handlers. void HandleBye(const rtcp::CommonHeader& rtcp_packet_header); void HandleSenderReport(const rtcp::CommonHeader& rtcp_packet_header, Timestamp now, - std::vector& rtcp_report_blocks, std::vector& report_blocks); void HandleReceiverReport(const rtcp::CommonHeader& rtcp_packet_header, Timestamp now, - std::vector& rtcp_report_blocks, std::vector& report_blocks); void HandleReportBlocks( uint32_t sender_ssrc, @@ -126,10 +123,8 @@ class RtcpTransceiverImpl { void HandleDlrr(const rtcp::Dlrr& dlrr, Timestamp now); void HandleTargetBitrate(const rtcp::TargetBitrate& target_bitrate, uint32_t remote_ssrc); - void ProcessReportBlocks( - Timestamp now, - rtc::ArrayView rtcp_report_blocks, - rtc::ArrayView report_blocks); + void ProcessReportBlocks(Timestamp now, + rtc::ArrayView report_blocks); void ReschedulePeriodicCompoundPackets(); void SchedulePeriodicCompoundPackets(TimeDelta delay); diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc index 94b5f5d60f..533e2d60d7 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc @@ -90,10 +90,6 @@ class MockRtpStreamRtcpHandler : public RtpStreamRtcpHandler { (override)); MOCK_METHOD(void, OnFir, (uint32_t), (override)); MOCK_METHOD(void, OnPli, (uint32_t), (override)); - MOCK_METHOD(void, - OnReportBlock, - (uint32_t, const rtcp::ReportBlock&), - (override)); MOCK_METHOD(void, OnReport, (const ReportBlockData&), (override)); private: @@ -114,11 +110,6 @@ class MockNetworkLinkRtcpObserver : public NetworkLinkRtcpObserver { OnReceiverEstimatedMaxBitrate, (Timestamp receive_time, DataRate bitrate), (override)); - MOCK_METHOD(void, - OnReportBlocks, - (Timestamp receive_time, - rtc::ArrayView report_blocks), - (override)); MOCK_METHOD(void, OnReport, (Timestamp receive_time, @@ -1523,7 +1514,6 @@ TEST_F(RtcpTransceiverImplTest, rr2->SetReportBlocks(std::vector(2)); packet.Append(std::move(rr2)); - EXPECT_CALL(link_observer, OnReportBlocks(receive_time, SizeIs(64))); EXPECT_CALL(link_observer, OnReport(receive_time, SizeIs(64))); rtcp_transceiver.ReceivePacket(packet.Build(), receive_time); @@ -1550,9 +1540,6 @@ TEST_F(RtcpTransceiverImplTest, MockRtpStreamRtcpHandler local_stream1; MockRtpStreamRtcpHandler local_stream3; MockRtpStreamRtcpHandler local_stream4; - EXPECT_CALL(local_stream1, OnReportBlock(kRemoteSsrc, _)); - EXPECT_CALL(local_stream3, OnReportBlock).Times(0); - EXPECT_CALL(local_stream4, OnReportBlock(kRemoteSsrc, _)); EXPECT_CALL(local_stream1, OnReport(Property(&ReportBlockData::sender_ssrc, kRemoteSsrc))); EXPECT_CALL(local_stream3, OnReport).Times(0);