From fab4992f3d38ec43e7a09e1a1ee7fa2667d1f736 Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Thu, 19 Dec 2024 12:24:38 +0000 Subject: [PATCH] Only notify NetworkLinkObserver if SSRC match known SSRC There is one RTCP receiver per receive stream. Therefore, only handle a received CongestionControlFeedback in the RTCP receiver corresponding to the first SSRC in the report. Bug: webrtc:42225697 Change-Id: I9bc0009cb6840cddeaca25f39c597bc2c13a3604 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/372280 Reviewed-by: Danil Chapovalov Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#43613} --- modules/rtp_rtcp/source/rtcp_receiver.cc | 9 ++++- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 39 ++++++++++++++++--- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index 672ee98d29..21b6c9552f 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -1085,10 +1085,15 @@ bool RTCPReceiver::HandleCongestionControlFeedback( const CommonHeader& rtcp_block, PacketInformation* packet_information) { rtcp::CongestionControlFeedback feedback; - if (!feedback.Parse(rtcp_block)) { + if (!feedback.Parse(rtcp_block) || feedback.packets().empty()) { return false; } - packet_information->congestion_control_feedback.emplace(std::move(feedback)); + uint32_t first_media_source_ssrc = feedback.packets()[0].ssrc; + if (first_media_source_ssrc == local_media_ssrc() || + registered_ssrcs_.contains(first_media_source_ssrc)) { + packet_information->congestion_control_feedback.emplace( + std::move(feedback)); + } return true; } diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index f8789edcc9..75d268d0e7 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -1747,13 +1747,13 @@ TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnCongestionControlFeedback) { ReceiverMocks mocks; mocks.field_trials = "WebRTC-RFC8888CongestionControlFeedback/Enabled/"; RTCPReceiver receiver = Create(mocks); - receiver.SetRemoteSSRC(kSenderSsrc); - rtcp::CongestionControlFeedback packet({{ - .ssrc = 123, - .sequence_number = 1, - }}, - /*report_timestamp_compact_ntp=*/324); + rtcp::CongestionControlFeedback packet( + {{ + .ssrc = mocks.config.local_media_ssrc, + .sequence_number = 1, + }}, + /*report_timestamp_compact_ntp=*/324); packet.SetSenderSsrc(kSenderSsrc); EXPECT_CALL( @@ -1764,6 +1764,33 @@ TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnCongestionControlFeedback) { receiver.IncomingPacket(packet.Build()); } +TEST(RtcpReceiverTest, FiltersCongestionControlFeedbackOnFirstSsrc) { + ReceiverMocks mocks_1; + mocks_1.field_trials = "WebRTC-RFC8888CongestionControlFeedback/Enabled/"; + RTCPReceiver receiver_1 = Create(mocks_1); + + ReceiverMocks mocks_2; + mocks_2.field_trials = "WebRTC-RFC8888CongestionControlFeedback/Enabled/"; + mocks_2.config.local_media_ssrc = 789; + mocks_2.config.rtx_send_ssrc = 345; + RTCPReceiver receiver_2 = Create(mocks_2); + + rtcp::CongestionControlFeedback packet( + {{ + .ssrc = mocks_2.config.local_media_ssrc, + .sequence_number = 1, + }}, + /*report_timestamp_compact_ntp=*/324); + packet.SetSenderSsrc(kSenderSsrc); + + EXPECT_CALL(mocks_1.network_link_rtcp_observer, OnCongestionControlFeedback) + .Times(0); + EXPECT_CALL(mocks_2.network_link_rtcp_observer, OnCongestionControlFeedback) + .Times(1); + receiver_1.IncomingPacket(packet.Build()); + receiver_2.IncomingPacket(packet.Build()); +} + TEST(RtcpReceiverTest, NotifiesNetworkStateEstimateObserverOnRemoteNetworkEstimate) { ReceiverMocks mocks;