From e641a970ef11efd5107d6768486cc740902364c1 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Tue, 23 May 2023 10:39:09 +0200 Subject: [PATCH] In RtcpReceiver remove redundand way to represent RTCP report blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass ReportBlockData instead of RTCPReportBlock from RtcpReceiver to RtpRtcp module Bug: None Change-Id: Ia042bfc626dda532674e070c593db7a04e76254a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/306220 Commit-Queue: Danil Chapovalov Reviewed-by: Åsa Persson Cr-Commit-Position: refs/heads/main@{#40167} --- modules/rtp_rtcp/source/rtcp_receiver.cc | 14 ++------------ modules/rtp_rtcp/source/rtcp_receiver.h | 2 +- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 2 +- modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 12 ++++++------ modules/rtp_rtcp/source/rtp_rtcp_impl.h | 2 +- modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 12 ++++++------ modules/rtp_rtcp/source/rtp_rtcp_impl2.h | 2 +- .../rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc | 16 ++++++++-------- test/fuzzers/rtcp_receiver_fuzzer.cc | 3 ++- 9 files changed, 28 insertions(+), 37 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index 7236875f71..c4371388be 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -127,8 +127,6 @@ struct RTCPReceiver::PacketInformation { uint32_t remote_ssrc = 0; std::vector nack_sequence_numbers; - // TODO(hbos): Remove `report_blocks` in favor of `report_block_datas`. - ReportBlockList report_blocks; std::vector report_block_datas; absl::optional rtt; uint32_t receiver_estimated_max_bitrate_bps = 0; @@ -625,15 +623,6 @@ void RTCPReceiver::HandleReportBlock(const ReportBlock& report_block, packet_information->rtt = rtt; } - packet_information->report_blocks.push_back( - {.sender_ssrc = remote_ssrc, - .source_ssrc = report_block.source_ssrc(), - .fraction_lost = report_block.fraction_lost(), - .packets_lost = report_block.cumulative_lost(), - .extended_highest_sequence_number = report_block.extended_high_seq_num(), - .jitter = report_block.jitter(), - .last_sender_report_timestamp = report_block.last_sr(), - .delay_since_last_sender_report = report_block.delay_since_last_sr()}); packet_information->report_block_datas.push_back(*report_block_data); } @@ -1162,7 +1151,8 @@ void RTCPReceiver::TriggerCallbacksFromRtcpPacket( if ((packet_information.packet_type_flags & kRtcpSr) || (packet_information.packet_type_flags & kRtcpRr)) { - rtp_rtcp_->OnReceivedRtcpReportBlocks(packet_information.report_blocks); + rtp_rtcp_->OnReceivedRtcpReportBlocks( + packet_information.report_block_datas); } if (network_state_estimate_observer_ && diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h index bc0ce566b4..e5e0ab1692 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -55,7 +55,7 @@ class RTCPReceiver final { virtual void OnReceivedNack( const std::vector& nack_sequence_numbers) = 0; virtual void OnReceivedRtcpReportBlocks( - const ReportBlockList& report_blocks) = 0; + rtc::ArrayView report_blocks) = 0; protected: virtual ~ModuleRtpRtcp() = default; diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 02dd0a7da6..159086bf68 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -107,7 +107,7 @@ class MockModuleRtpRtcp : public RTCPReceiver::ModuleRtpRtcp { MOCK_METHOD(void, OnReceivedNack, (const std::vector&), (override)); MOCK_METHOD(void, OnReceivedRtcpReportBlocks, - (const ReportBlockList&), + (rtc::ArrayView), (override)); }; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 6c402640ea..f2cb9db6cc 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -673,7 +673,7 @@ void ModuleRtpRtcpImpl::OnReceivedNack( } void ModuleRtpRtcpImpl::OnReceivedRtcpReportBlocks( - const ReportBlockList& report_blocks) { + rtc::ArrayView report_blocks) { if (rtp_sender_) { uint32_t ssrc = SSRC(); absl::optional rtx_ssrc; @@ -681,13 +681,13 @@ void ModuleRtpRtcpImpl::OnReceivedRtcpReportBlocks( rtx_ssrc = rtp_sender_->packet_generator.RtxSsrc(); } - for (const RTCPReportBlock& report_block : report_blocks) { - if (ssrc == report_block.source_ssrc) { + for (const ReportBlockData& report_block : report_blocks) { + if (ssrc == report_block.source_ssrc()) { rtp_sender_->packet_generator.OnReceivedAckOnSsrc( - report_block.extended_highest_sequence_number); - } else if (rtx_ssrc && *rtx_ssrc == report_block.source_ssrc) { + report_block.extended_highest_sequence_number()); + } else if (rtx_ssrc == report_block.source_ssrc()) { rtp_sender_->packet_generator.OnReceivedAckOnRtxSsrc( - report_block.extended_highest_sequence_number); + report_block.extended_highest_sequence_number()); } } } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index d3a0dd69e1..178c6b00ae 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -228,7 +228,7 @@ class ABSL_DEPRECATED("") ModuleRtpRtcpImpl void OnReceivedNack( const std::vector& nack_sequence_numbers) override; void OnReceivedRtcpReportBlocks( - const ReportBlockList& report_blocks) override; + rtc::ArrayView report_blocks) override; void OnRequestSendReport() override; void SetVideoBitrateAllocation( diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index 546de63eec..0687f32352 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -673,7 +673,7 @@ void ModuleRtpRtcpImpl2::OnReceivedNack( } void ModuleRtpRtcpImpl2::OnReceivedRtcpReportBlocks( - const ReportBlockList& report_blocks) { + rtc::ArrayView report_blocks) { if (rtp_sender_) { uint32_t ssrc = SSRC(); absl::optional rtx_ssrc; @@ -681,13 +681,13 @@ void ModuleRtpRtcpImpl2::OnReceivedRtcpReportBlocks( rtx_ssrc = rtp_sender_->packet_generator.RtxSsrc(); } - for (const RTCPReportBlock& report_block : report_blocks) { - if (ssrc == report_block.source_ssrc) { + for (const ReportBlockData& report_block : report_blocks) { + if (ssrc == report_block.source_ssrc()) { rtp_sender_->packet_generator.OnReceivedAckOnSsrc( - report_block.extended_highest_sequence_number); - } else if (rtx_ssrc && *rtx_ssrc == report_block.source_ssrc) { + report_block.extended_highest_sequence_number()); + } else if (rtx_ssrc == report_block.source_ssrc()) { rtp_sender_->packet_generator.OnReceivedAckOnRtxSsrc( - report_block.extended_highest_sequence_number); + report_block.extended_highest_sequence_number()); } } } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h index c0afe819dd..d6ffd8a9df 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h @@ -238,7 +238,7 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, void OnReceivedNack( const std::vector& nack_sequence_numbers) override; void OnReceivedRtcpReportBlocks( - const ReportBlockList& report_blocks) override; + rtc::ArrayView report_blocks) override; void OnRequestSendReport() override; void SetVideoBitrateAllocation( diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc index 5e3406c96a..6f14b4f3fc 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc @@ -1076,10 +1076,10 @@ TEST_F(RtpRtcpImpl2Test, RtpStateReflectsCurrentState) { EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); // Simulate an RTCP receiver report in order to populate `ssrc_has_acked`. - RTCPReportBlock ack; - ack.source_ssrc = kSenderSsrc; - ack.extended_highest_sequence_number = kSeq; - sender_.impl_->OnReceivedRtcpReportBlocks({ack}); + ReportBlockData ack[1]; + ack[0].set_source_ssrc(kSenderSsrc); + ack[0].set_extended_highest_sequence_number(kSeq); + sender_.impl_->OnReceivedRtcpReportBlocks(ack); RtpState state = sender_.impl_->GetRtpState(); EXPECT_EQ(state.sequence_number, kSeq); @@ -1124,10 +1124,10 @@ TEST_F(RtpRtcpImpl2Test, RtxRtpStateReflectsCurrentState) { EXPECT_EQ(rtx_packet.Ssrc(), kRtxSenderSsrc); // Simulate an RTCP receiver report in order to populate `ssrc_has_acked`. - RTCPReportBlock ack; - ack.source_ssrc = kRtxSenderSsrc; - ack.extended_highest_sequence_number = rtx_packet.SequenceNumber(); - sender_.impl_->OnReceivedRtcpReportBlocks({ack}); + ReportBlockData ack[1]; + ack[0].set_source_ssrc(kRtxSenderSsrc); + ack[0].set_extended_highest_sequence_number(rtx_packet.SequenceNumber()); + sender_.impl_->OnReceivedRtcpReportBlocks(ack); RtpState rtp_state = sender_.impl_->GetRtpState(); RtpState rtx_state = sender_.impl_->GetRtxState(); diff --git a/test/fuzzers/rtcp_receiver_fuzzer.cc b/test/fuzzers/rtcp_receiver_fuzzer.cc index a3e9002532..e61f6c06ac 100644 --- a/test/fuzzers/rtcp_receiver_fuzzer.cc +++ b/test/fuzzers/rtcp_receiver_fuzzer.cc @@ -27,7 +27,8 @@ class NullModuleRtpRtcp : public RTCPReceiver::ModuleRtpRtcp { void SetTmmbn(std::vector) override {} void OnRequestSendReport() override {} void OnReceivedNack(const std::vector&) override {} - void OnReceivedRtcpReportBlocks(const ReportBlockList&) override {} + void OnReceivedRtcpReportBlocks( + rtc::ArrayView report_blocks) override {} }; } // namespace