diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index c0318eb650..494caaa811 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -502,6 +502,7 @@ rtc_library("mock_rtp_rtcp") { testonly = true public = [ "mocks/mock_network_link_rtcp_observer.h", + "mocks/mock_network_state_estimator_observer.h", "mocks/mock_recovered_packet_receiver.h", "mocks/mock_rtcp_rtt_stats.h", "mocks/mock_rtp_rtcp.h", diff --git a/modules/rtp_rtcp/mocks/mock_network_state_estimator_observer.h b/modules/rtp_rtcp/mocks/mock_network_state_estimator_observer.h new file mode 100644 index 0000000000..8c6ea45185 --- /dev/null +++ b/modules/rtp_rtcp/mocks/mock_network_state_estimator_observer.h @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2024 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ +#ifndef MODULES_RTP_RTCP_MOCKS_MOCK_NETWORK_STATE_ESTIMATOR_OBSERVER_H_ +#define MODULES_RTP_RTCP_MOCKS_MOCK_NETWORK_STATE_ESTIMATOR_OBSERVER_H_ + +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "test/gmock.h" + +namespace webrtc { + +class MockNetworkStateEstimateObserver : public NetworkStateEstimateObserver { + public: + MOCK_METHOD(void, + OnRemoteNetworkEstimate, + (NetworkStateEstimate estimate), + (override)); +}; + +} // namespace webrtc + +#endif // MODULES_RTP_RTCP_MOCKS_MOCK_NETWORK_STATE_ESTIMATOR_OBSERVER_H_ diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index 8732b3772c..10f9d87730 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -1163,6 +1163,13 @@ void RTCPReceiver::TriggerCallbacksFromRtcpPacket( loss_notification->decodability_flag()); } } + // Network state estimate should be applied before other feedback since it may + // affect how other feedback is handled. + if (network_state_estimate_observer_ && + packet_information.network_state_estimate) { + network_state_estimate_observer_->OnRemoteNetworkEstimate( + *packet_information.network_state_estimate); + } if (network_link_rtcp_observer_) { Timestamp now = env_.clock().CurrentTime(); @@ -1194,12 +1201,6 @@ void RTCPReceiver::TriggerCallbacksFromRtcpPacket( packet_information.report_block_datas); } - if (network_state_estimate_observer_ && - packet_information.network_state_estimate) { - network_state_estimate_observer_->OnRemoteNetworkEstimate( - *packet_information.network_state_estimate); - } - if (bitrate_allocation_observer_ && packet_information.target_bitrate_allocation) { bitrate_allocation_observer_->OnBitrateAllocationUpdated( diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 31e5d343ff..b01c23c310 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -22,6 +22,7 @@ #include "absl/strings/string_view.h" #include "api/array_view.h" #include "api/environment/environment_factory.h" +#include "api/transport/network_types.h" #include "api/units/data_rate.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" @@ -32,6 +33,7 @@ #include "modules/rtp_rtcp/include/rtcp_statistics.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/mocks/mock_network_link_rtcp_observer.h" +#include "modules/rtp_rtcp/mocks/mock_network_state_estimator_observer.h" #include "modules/rtp_rtcp/source/byte_io.h" #include "modules/rtp_rtcp/source/ntp_time_util.h" #include "modules/rtp_rtcp/source/rtcp_packet/app.h" @@ -46,6 +48,7 @@ #include "modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request.h" #include "modules/rtp_rtcp/source/rtcp_packet/receiver_report.h" #include "modules/rtp_rtcp/source/rtcp_packet/remb.h" +#include "modules/rtp_rtcp/source/rtcp_packet/remote_estimate.h" #include "modules/rtp_rtcp/source/rtcp_packet/rtpfb.h" #include "modules/rtp_rtcp/source/rtcp_packet/sdes.h" #include "modules/rtp_rtcp/source/rtcp_packet/sender_report.h" @@ -168,12 +171,14 @@ struct ReceiverMocks { StrictMock bitrate_allocation_observer; StrictMock rtp_rtcp_impl; NiceMock network_link_rtcp_observer; + NiceMock network_state_estimate_observer; RtpRtcpInterface::Configuration config = { .receiver_only = false, .intra_frame_callback = &intra_frame_observer, .rtcp_loss_notification_observer = &rtcp_loss_notification_observer, .network_link_rtcp_observer = &network_link_rtcp_observer, + .network_state_estimate_observer = &network_state_estimate_observer, .bitrate_allocation_observer = &bitrate_allocation_observer, .rtcp_packet_type_counter_observer = &packet_type_counter_observer, .rtcp_report_interval_ms = kRtcpIntervalMs, @@ -1761,6 +1766,56 @@ TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnCongestionControlFeedback) { receiver.IncomingPacket(packet.Build()); } +TEST(RtcpReceiverTest, + NotifiesNetworkStateEstimateObserverOnRemoteNetworkEstimate) { + ReceiverMocks mocks; + RTCPReceiver receiver = Create(mocks); + receiver.SetRemoteSSRC(kSenderSsrc); + + NetworkStateEstimate estimate; + estimate.link_capacity_lower = DataRate::BitsPerSec(1000); + estimate.link_capacity_upper = DataRate::BitsPerSec(10000); + rtcp::RemoteEstimate remote_estimate; + remote_estimate.SetEstimate(estimate); + + EXPECT_CALL(mocks.network_state_estimate_observer, + OnRemoteNetworkEstimate( + AllOf(Field(&NetworkStateEstimate::link_capacity_lower, + DataRate::BitsPerSec(1000)), + Field(&NetworkStateEstimate::link_capacity_upper, + DataRate::BitsPerSec(10000))))); + + receiver.IncomingPacket(remote_estimate.Build()); +} + +TEST(RtcpReceiverTest, + NotifiesNetworkStateEstimateObserverBeforeNetworkLinkObserver) { + ReceiverMocks mocks; + RTCPReceiver receiver = Create(mocks); + receiver.SetRemoteSSRC(kSenderSsrc); + + NetworkStateEstimate estimate; + estimate.link_capacity_lower = DataRate::BitsPerSec(1000); + estimate.link_capacity_upper = DataRate::BitsPerSec(10000); + std::unique_ptr remote_estimate = + std::make_unique(); + remote_estimate->SetEstimate(estimate); + std::unique_ptr feedback_packet = + std::make_unique(); + feedback_packet->SetMediaSsrc(mocks.config.local_media_ssrc); + feedback_packet->SetSenderSsrc(kSenderSsrc); + feedback_packet->SetBase(123, Timestamp::Millis(1)); + feedback_packet->AddReceivedPacket(123, Timestamp::Millis(1)); + rtcp::CompoundPacket compound; + compound.Append(std::move(remote_estimate)); + compound.Append(std::move(feedback_packet)); + + InSequence s; + EXPECT_CALL(mocks.network_state_estimate_observer, OnRemoteNetworkEstimate); + EXPECT_CALL(mocks.network_link_rtcp_observer, OnTransportFeedback); + receiver.IncomingPacket(compound.Build()); +} + TEST(RtcpReceiverTest, HandlesInvalidCongestionControlFeedback) { ReceiverMocks mocks; mocks.field_trials = "WebRTC-RFC8888CongestionControlFeedback/Enabled/";