From 1f206b841eb98d63a0a9e47b1f76ea59933d25b5 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Wed, 1 Feb 2023 11:12:46 +0000 Subject: [PATCH] Use ArrayView in the IncomingRtcpPacket function. The lowest level and some of the highest levels of this function are already using ArrayView. Make this consistent throughout. Use deprecation for the old API rather than deleting it, since upstream may be using it. Bug: webrtc:14870 Change-Id: If5e1a6e9802ecf7e8e3ec27befb5167ca9985517 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291706 Commit-Queue: Harald Alvestrand Reviewed-by: Evan Shrubsole Cr-Commit-Position: refs/heads/main@{#39241} --- audio/channel_receive.cc | 2 +- audio/channel_send.cc | 2 +- audio/voip/audio_ingress.cc | 2 +- call/rtp_video_sender.cc | 2 +- modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 4 ++++ modules/rtp_rtcp/source/nack_rtx_unittest.cc | 2 +- modules/rtp_rtcp/source/rtcp_receiver.h | 4 +++- modules/rtp_rtcp/source/rtp_rtcp_impl.h | 3 +++ modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 6 +++--- modules/rtp_rtcp/source/rtp_rtcp_impl2.h | 10 ++++++++-- modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc | 8 ++++---- modules/rtp_rtcp/source/rtp_rtcp_interface.h | 7 +++++-- test/fuzzers/rtcp_receiver_fuzzer.cc | 2 +- video/rtp_video_stream_receiver2.cc | 3 ++- 14 files changed, 38 insertions(+), 19 deletions(-) diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index b95d98c20c..04ccc9886a 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -739,7 +739,7 @@ void ChannelReceive::ReceivedRTCPPacket(const uint8_t* data, size_t length) { UpdatePlayoutTimestamp(true, rtc::TimeMillis()); // Deliver RTCP packet to RTP/RTCP module for parsing - rtp_rtcp_->IncomingRtcpPacket(data, length); + rtp_rtcp_->IncomingRtcpPacket(rtc::MakeArrayView(data, length)); int64_t rtt = 0; rtp_rtcp_->RTT(remote_ssrc_, &rtt, /*avg_rtt=*/nullptr, /*min_rtt=*/nullptr, diff --git a/audio/channel_send.cc b/audio/channel_send.cc index 26b5bf452d..d00688f849 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc @@ -628,7 +628,7 @@ void ChannelSend::ReceivedRTCPPacket(const uint8_t* data, size_t length) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); // Deliver RTCP packet to RTP/RTCP module for parsing - rtp_rtcp_->IncomingRtcpPacket(data, length); + rtp_rtcp_->IncomingRtcpPacket(rtc::MakeArrayView(data, length)); int64_t rtt = GetRTT(); if (rtt == 0) { diff --git a/audio/voip/audio_ingress.cc b/audio/voip/audio_ingress.cc index 9492a51a21..0f9c9ccdab 100644 --- a/audio/voip/audio_ingress.cc +++ b/audio/voip/audio_ingress.cc @@ -210,7 +210,7 @@ void AudioIngress::ReceivedRTCPPacket( } // Deliver RTCP packet to RTP/RTCP module for parsing and processing. - rtp_rtcp_->IncomingRtcpPacket(rtcp_packet.data(), rtcp_packet.size()); + rtp_rtcp_->IncomingRtcpPacket(rtcp_packet); int64_t rtt = 0; if (rtp_rtcp_->RTT(remote_ssrc_, &rtt, nullptr, nullptr, nullptr) != 0) { diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index de19b97c66..526e33c229 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -715,7 +715,7 @@ uint32_t RtpVideoSender::GetPacketizationOverheadRate() const { void RtpVideoSender::DeliverRtcp(const uint8_t* packet, size_t length) { // Runs on a network thread. for (const RtpStreamSender& stream : rtp_streams_) - stream.rtp_rtcp->IncomingRtcpPacket(packet, length); + stream.rtp_rtcp->IncomingRtcpPacket(rtc::MakeArrayView(packet, length)); } void RtpVideoSender::ConfigureSsrcs( diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 75c30742df..44a64841d0 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -33,6 +33,10 @@ class MockRtpRtcpInterface : public RtpRtcpInterface { IncomingRtcpPacket, (const uint8_t* incoming_packet, size_t packet_length), (override)); + MOCK_METHOD(void, + IncomingRtcpPacket, + (rtc::ArrayView packet), + (override)); MOCK_METHOD(void, SetRemoteSSRC, (uint32_t ssrc), (override)); MOCK_METHOD(void, SetLocalSsrc, (uint32_t ssrc), (override)); MOCK_METHOD(void, SetMaxRtpPacketSize, (size_t size), (override)); diff --git a/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/modules/rtp_rtcp/source/nack_rtx_unittest.cc index 87c6e661dc..869e1cc5bf 100644 --- a/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -103,7 +103,7 @@ class RtxLoopBackTransport : public webrtc::Transport { } bool SendRtcp(const uint8_t* data, size_t len) override { - module_->IncomingRtcpPacket((const uint8_t*)data, len); + module_->IncomingRtcpPacket(rtc::MakeArrayView((const uint8_t*)data, len)); return true; } int count_; diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h index cdf4cbadf8..696e0fbd7e 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -98,7 +98,9 @@ class RTCPReceiver final { ~RTCPReceiver(); - void IncomingPacket(const uint8_t* packet, size_t packet_size) { + [[deprecated("Use ArrayView verwsion")]] void IncomingPacket( + const uint8_t* packet, + size_t packet_size) { IncomingPacket(rtc::MakeArrayView(packet, packet_size)); } void IncomingPacket(rtc::ArrayView packet); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index f164195168..38a9bf0305 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -62,6 +62,9 @@ class ABSL_DEPRECATED("") ModuleRtpRtcpImpl // Called when we receive an RTCP packet. void IncomingRtcpPacket(const uint8_t* incoming_packet, size_t incoming_packet_length) override; + void IncomingRtcpPacket(rtc::ArrayView packet) override { + IncomingRtcpPacket(packet.data(), packet.size()); + } void SetRemoteSSRC(uint32_t ssrc) override; void SetLocalSsrc(uint32_t ssrc) override; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index 31dd1499d5..565e1ea785 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -143,10 +143,10 @@ absl::optional ModuleRtpRtcpImpl2::FlexfecSsrc() const { return absl::nullopt; } -void ModuleRtpRtcpImpl2::IncomingRtcpPacket(const uint8_t* rtcp_packet, - const size_t length) { +void ModuleRtpRtcpImpl2::IncomingRtcpPacket( + rtc::ArrayView rtcp_packet) { RTC_DCHECK_RUN_ON(&rtcp_thread_checker_); - rtcp_receiver_.IncomingPacket(rtcp_packet, length); + rtcp_receiver_.IncomingPacket(rtcp_packet); } void ModuleRtpRtcpImpl2::RegisterSendPayloadFrequency(int payload_type, diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h index e7a3ac03e8..b6071d96ee 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h @@ -66,8 +66,14 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, // Receiver part. // Called when we receive an RTCP packet. - void IncomingRtcpPacket(const uint8_t* incoming_packet, - size_t incoming_packet_length) override; + [[deprecated("Use ArrayView version")]] void IncomingRtcpPacket( + const uint8_t* incoming_packet, + size_t incoming_packet_length) override { + IncomingRtcpPacket( + rtc::MakeArrayView(incoming_packet, incoming_packet_length)); + } + void IncomingRtcpPacket( + rtc::ArrayView incoming_packet) override; void SetRemoteSSRC(uint32_t ssrc) override; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc index b793ba82a5..ceddc5fdc7 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc @@ -130,7 +130,7 @@ class SendTransport : public Transport, Packet packet = std::move(rtcp_packets_.front()); rtcp_packets_.pop_front(); EXPECT_TRUE(receiver_); - receiver_->IncomingRtcpPacket(packet.data.data(), packet.data.size()); + receiver_->IncomingRtcpPacket(packet.data); } } TaskQueueBase* GetAsTaskQueue() override { @@ -372,7 +372,7 @@ class RtpRtcpImpl2Test : public ::testing::Test { nack.SetMediaSsrc(sender ? kSenderSsrc : kReceiverSsrc); nack.SetPacketIds(list, kListLength); rtc::Buffer packet = nack.Build(); - module->impl_->IncomingRtcpPacket(packet.data(), packet.size()); + module->impl_->IncomingRtcpPacket(packet); } }; @@ -798,7 +798,7 @@ TEST_F(RtpRtcpImpl2Test, SenderReportStatsNotUpdatedWithUnexpectedSsrc) { sr.SetPacketCount(123u); sr.SetOctetCount(456u); auto raw_packet = sr.Build(); - receiver_.impl_->IncomingRtcpPacket(raw_packet.data(), raw_packet.size()); + receiver_.impl_->IncomingRtcpPacket(raw_packet); EXPECT_THAT(receiver_.impl_->GetSenderReportStats(), Eq(absl::nullopt)); } @@ -816,7 +816,7 @@ TEST_F(RtpRtcpImpl2Test, SenderReportStatsCheckStatsFromLastReport) { sr.SetPacketCount(kPacketCount); sr.SetOctetCount(kOctetCount); auto raw_packet = sr.Build(); - receiver_.impl_->IncomingRtcpPacket(raw_packet.data(), raw_packet.size()); + receiver_.impl_->IncomingRtcpPacket(raw_packet); EXPECT_THAT( receiver_.impl_->GetSenderReportStats(), diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index 2024b308dd..4bcae06fbf 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h @@ -186,8 +186,11 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { // Receiver functions // ************************************************************************** - virtual void IncomingRtcpPacket(const uint8_t* incoming_packet, - size_t incoming_packet_length) = 0; + [[deprecated("Use ArrayView version")]] virtual void IncomingRtcpPacket( + const uint8_t* incoming_packet, + size_t incoming_packet_length) = 0; + virtual void IncomingRtcpPacket( + rtc::ArrayView incoming_packet) = 0; virtual void SetRemoteSSRC(uint32_t ssrc) = 0; diff --git a/test/fuzzers/rtcp_receiver_fuzzer.cc b/test/fuzzers/rtcp_receiver_fuzzer.cc index 8bad9e456a..a3e9002532 100644 --- a/test/fuzzers/rtcp_receiver_fuzzer.cc +++ b/test/fuzzers/rtcp_receiver_fuzzer.cc @@ -47,6 +47,6 @@ void FuzzOneInput(const uint8_t* data, size_t size) { RTCPReceiver receiver(config, &rtp_rtcp_module); - receiver.IncomingPacket(data, size); + receiver.IncomingPacket(rtc::MakeArrayView(data, size)); } } // namespace webrtc diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc index 6b1e8ff05c..3e305a5444 100644 --- a/video/rtp_video_stream_receiver2.cc +++ b/video/rtp_video_stream_receiver2.cc @@ -1131,7 +1131,8 @@ bool RtpVideoStreamReceiver2::DeliverRtcp(const uint8_t* rtcp_packet, return false; } - rtp_rtcp_->IncomingRtcpPacket(rtcp_packet, rtcp_packet_length); + rtp_rtcp_->IncomingRtcpPacket( + rtc::MakeArrayView(rtcp_packet, rtcp_packet_length)); int64_t rtt = 0; rtp_rtcp_->RTT(config_.rtp.remote_ssrc, &rtt, nullptr, nullptr, nullptr);