From f66b9f5efe1d10099d8ad1a41fb42476b68dbda6 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 8 May 2023 10:54:14 +0200 Subject: [PATCH] In RtcpTransceiver pass ReportBlockData instead of rtcp::ReportBlock rtcp::ReportBlock class is designed for serialization while ReportBlockData designed for passing report block information across multiple components. This slightly reduce how RtcpTransceiver and RtcpReceiver interact with other webrtc components Bug: webrtc:8239 Change-Id: I582e3d7b32dc6357954b29a1def37e2e72116a74 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304285 Commit-Queue: Danil Chapovalov Reviewed-by: Emil Lundmark Cr-Commit-Position: refs/heads/main@{#40006} --- .../rtp_rtcp/source/rtcp_transceiver_config.h | 14 +++ .../rtp_rtcp/source/rtcp_transceiver_impl.cc | 102 ++++++++++++------ .../rtp_rtcp/source/rtcp_transceiver_impl.h | 19 ++-- .../source/rtcp_transceiver_impl_unittest.cc | 14 +++ 4 files changed, 110 insertions(+), 39 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_config.h b/modules/rtp_rtcp/source/rtcp_transceiver_config.h index 3122ad5c36..c9140f2140 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_config.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_config.h @@ -20,6 +20,7 @@ #include "api/units/time_delta.h" #include "api/units/timestamp.h" #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" @@ -43,9 +44,16 @@ class NetworkLinkRtcpObserver { const rtcp::TransportFeedback& feedback) {} 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, + rtc::ArrayView report_blocks) {} virtual void OnRttUpdate(Timestamp receive_time, TimeDelta rtt) {} }; @@ -102,8 +110,14 @@ class RtpStreamRtcpHandler { rtc::ArrayView sequence_numbers) {} 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) {} }; struct RtcpTransceiverConfig { diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc index d32880e37e..d98ac51433 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.cc @@ -36,6 +36,7 @@ #include "rtc_base/logging.h" #include "rtc_base/numerics/divide_round.h" #include "rtc_base/task_utils/repeating_task.h" +#include "rtc_base/time_utils.h" #include "system_wrappers/include/clock.h" namespace webrtc { @@ -57,6 +58,7 @@ struct RtcpTransceiverImpl::RemoteSenderState { struct RtcpTransceiverImpl::LocalSenderState { uint32_t ssrc; size_t last_num_sent_bytes = 0; + ReportBlockData report_block; // Sequence number of the last FIR message per sender SSRC. flat_map last_fir; RtpStreamRtcpHandler* handler = nullptr; @@ -175,20 +177,21 @@ 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 report_blocks; + std::vector rtcp_report_blocks; + std::vector report_blocks; while (!packet.empty()) { rtcp::CommonHeader rtcp_block; if (!rtcp_block.Parse(packet.data(), packet.size())) break; - HandleReceivedPacket(rtcp_block, now, report_blocks); + HandleReceivedPacket(rtcp_block, now, rtcp_report_blocks, report_blocks); packet = packet.subview(rtcp_block.packet_size()); } if (!report_blocks.empty()) { - ProcessReportBlocks(now, report_blocks); + ProcessReportBlocks(now, rtcp_report_blocks, report_blocks); } } @@ -275,16 +278,19 @@ void RtcpTransceiverImpl::SendFullIntraRequest( void RtcpTransceiverImpl::HandleReceivedPacket( const rtcp::CommonHeader& rtcp_packet_header, Timestamp now, - std::vector& report_blocks) { + 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, report_blocks); + HandleSenderReport(rtcp_packet_header, now, rtcp_report_blocks, + report_blocks); break; case rtcp::ReceiverReport::kPacketType: - HandleReceiverReport(rtcp_packet_header, report_blocks); + HandleReceiverReport(rtcp_packet_header, now, rtcp_report_blocks, + report_blocks); break; case rtcp::ExtendedReports::kPacketType: HandleExtendedReports(rtcp_packet_header, now); @@ -313,7 +319,8 @@ void RtcpTransceiverImpl::HandleBye( void RtcpTransceiverImpl::HandleSenderReport( const rtcp::CommonHeader& rtcp_packet_header, Timestamp now, - std::vector& report_blocks) { + std::vector& rtcp_report_blocks, + std::vector& report_blocks) { rtcp::SenderReport sender_report; if (!sender_report.Parse(rtcp_packet_header)) return; @@ -321,9 +328,11 @@ void RtcpTransceiverImpl::HandleSenderReport( 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(); - CallbackOnReportBlocks(sender_report.sender_ssrc(), received_report_blocks); - report_blocks.insert(report_blocks.end(), received_report_blocks.begin(), - received_report_blocks.end()); + 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()); for (MediaReceiverRtcpObserver* observer : remote_sender.observers) observer->OnSenderReport(sender_report.sender_ssrc(), sender_report.ntp(), @@ -332,27 +341,60 @@ void RtcpTransceiverImpl::HandleSenderReport( void RtcpTransceiverImpl::HandleReceiverReport( const rtcp::CommonHeader& rtcp_packet_header, - std::vector& report_blocks) { + 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(); - CallbackOnReportBlocks(receiver_report.sender_ssrc(), received_report_blocks); - report_blocks.insert(report_blocks.end(), received_report_blocks.begin(), - received_report_blocks.end()); + 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()); } -void RtcpTransceiverImpl::CallbackOnReportBlocks( +void RtcpTransceiverImpl::HandleReportBlocks( uint32_t sender_ssrc, - rtc::ArrayView report_blocks) { - if (local_senders_.empty()) { + Timestamp now, + rtc::ArrayView rtcp_report_blocks, + std::vector& report_blocks) { + if (rtcp_report_blocks.empty()) { return; } - for (const rtcp::ReportBlock& block : report_blocks) { + NtpTime now_ntp = config_.clock->ConvertTimestampToNtpTime(now); + uint32_t receive_time_ntp = CompactNtp(now_ntp); + Timestamp now_utc = + Timestamp::Millis(now_ntp.ToMs() - rtc::kNtpJan1970Millisecs); + + for (const rtcp::ReportBlock& block : rtcp_report_blocks) { + std::optional rtt; + if (block.last_sr() != 0) { + rtt = CompactNtpRttToTimeDelta( + receive_time_ntp - block.delay_since_last_sr() - block.last_sr()); + } + auto sender_it = local_senders_by_ssrc_.find(block.source_ssrc()); if (sender_it != local_senders_by_ssrc_.end()) { - sender_it->second->handler->OnReportBlock(sender_ssrc, block); + LocalSenderState& state = *sender_it->second; + state.report_block.SetReportBlock(sender_ssrc, block, now_utc); + 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 { + // No registered sender for this report block, still report it to the + // network link. + ReportBlockData report_block; + report_block.SetReportBlock(sender_ssrc, block, now_utc); + if (rtt.has_value()) { + report_block.AddRoundTripTimeSample(*rtt); + } + report_blocks.push_back(report_block); } } } @@ -500,7 +542,8 @@ void RtcpTransceiverImpl::HandleDlrr(const rtcp::Dlrr& dlrr, Timestamp now) { void RtcpTransceiverImpl::ProcessReportBlocks( Timestamp now, - rtc::ArrayView report_blocks) { + rtc::ArrayView rtcp_report_blocks, + rtc::ArrayView report_blocks) { RTC_DCHECK(!report_blocks.empty()); if (config_.network_link_observer == nullptr) { return; @@ -510,24 +553,17 @@ void RtcpTransceiverImpl::ProcessReportBlocks( // To avoid too many callbacks, this code accumulate multiple rtts into one. TimeDelta rtt_sum = TimeDelta::Zero(); size_t num_rtts = 0; - uint32_t receive_time_ntp = - CompactNtp(config_.clock->ConvertTimestampToNtpTime(now)); - for (const rtcp::ReportBlock& report_block : report_blocks) { - if (report_block.last_sr() == 0) { - continue; + for (const ReportBlockData& report_block : report_blocks) { + if (report_block.has_rtt()) { + rtt_sum += report_block.last_rtt(); + ++num_rtts; } - - uint32_t rtt_ntp = receive_time_ntp - report_block.delay_since_last_sr() - - report_block.last_sr(); - rtt_sum += CompactNtpRttToTimeDelta(rtt_ntp); - ++num_rtts; } - // For backward compatibility, do not report rtt based on report blocks to the - // `config_.rtt_observer` if (num_rtts > 0) { config_.network_link_observer->OnRttUpdate(now, rtt_sum / num_rtts); } - config_.network_link_observer->OnReportBlocks(now, report_blocks); + config_.network_link_observer->OnReportBlocks(now, rtcp_report_blocks); + config_.network_link_observer->OnReport(now, report_blocks); } void RtcpTransceiverImpl::HandleTargetBitrate( diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h index 5c147b1f34..5620c97c27 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl.h +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl.h @@ -92,17 +92,23 @@ class RtcpTransceiverImpl { void HandleReceivedPacket(const rtcp::CommonHeader& rtcp_packet_header, Timestamp now, - std::vector& report_blocks); + 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& report_blocks); + std::vector& rtcp_report_blocks, + std::vector& report_blocks); void HandleReceiverReport(const rtcp::CommonHeader& rtcp_packet_header, - std::vector& report_blocks); - void CallbackOnReportBlocks( + Timestamp now, + std::vector& rtcp_report_blocks, + std::vector& report_blocks); + void HandleReportBlocks( uint32_t sender_ssrc, - rtc::ArrayView report_blocks); + Timestamp now, + rtc::ArrayView rtcp_report_blocks, + std::vector& report_blocks); void HandlePayloadSpecificFeedback( const rtcp::CommonHeader& rtcp_packet_header, Timestamp now); @@ -122,7 +128,8 @@ class RtcpTransceiverImpl { uint32_t remote_ssrc); void ProcessReportBlocks( Timestamp now, - rtc::ArrayView report_blocks); + rtc::ArrayView rtcp_report_blocks, + 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 ad9561f66b..94b5f5d60f 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc @@ -23,6 +23,7 @@ #include "api/units/timestamp.h" #include "api/video/video_bitrate_allocation.h" #include "modules/rtp_rtcp/include/receive_statistics.h" +#include "modules/rtp_rtcp/include/report_block_data.h" #include "modules/rtp_rtcp/mocks/mock_rtcp_rtt_stats.h" #include "modules/rtp_rtcp/source/rtcp_packet/app.h" #include "modules/rtp_rtcp/source/rtcp_packet/bye.h" @@ -42,6 +43,7 @@ using ::testing::ElementsAre; using ::testing::ElementsAreArray; using ::testing::Ge; using ::testing::NiceMock; +using ::testing::Property; using ::testing::Return; using ::testing::SizeIs; using ::testing::StrictMock; @@ -92,6 +94,7 @@ class MockRtpStreamRtcpHandler : public RtpStreamRtcpHandler { OnReportBlock, (uint32_t, const rtcp::ReportBlock&), (override)); + MOCK_METHOD(void, OnReport, (const ReportBlockData&), (override)); private: int num_calls_ = 0; @@ -116,6 +119,11 @@ class MockNetworkLinkRtcpObserver : public NetworkLinkRtcpObserver { (Timestamp receive_time, rtc::ArrayView report_blocks), (override)); + MOCK_METHOD(void, + OnReport, + (Timestamp receive_time, + rtc::ArrayView report_blocks), + (override)); }; constexpr TimeDelta kReportPeriod = TimeDelta::Seconds(1); @@ -1516,6 +1524,7 @@ TEST_F(RtcpTransceiverImplTest, 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); } @@ -1544,6 +1553,11 @@ TEST_F(RtcpTransceiverImplTest, 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); + EXPECT_CALL(local_stream4, + OnReport(Property(&ReportBlockData::sender_ssrc, kRemoteSsrc))); ASSERT_TRUE(rtcp_transceiver.AddMediaSender(kMediaSsrc1, &local_stream1)); ASSERT_TRUE(rtcp_transceiver.AddMediaSender(kMediaSsrc3, &local_stream3));