Callback to NetworkStateEstimateObserver before NetworkLinkRtcpObserver
If RTCP compound message is received with both these messages,
NetworkStateEstimator should be invoked before NetworkLinkRtcpObserver
since remote network state estimate may set limits on the BWE
calculated from the transport feedback.
Bug: webrtc:42220808
Change-Id: Ieac9c1d7d9c28e690351bcf1d8125c9e0099f962
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365583
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43239}
diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn
index c0318eb..494caaa 100644
--- a/modules/rtp_rtcp/BUILD.gn
+++ b/modules/rtp_rtcp/BUILD.gn
@@ -502,6 +502,7 @@
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 0000000..8c6ea45
--- /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 8732b37..10f9d87 100644
--- a/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -1163,6 +1163,13 @@
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 @@
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 31e5d34..b01c23c 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 @@
StrictMock<MockVideoBitrateAllocationObserver> bitrate_allocation_observer;
StrictMock<MockModuleRtpRtcp> rtp_rtcp_impl;
NiceMock<MockNetworkLinkRtcpObserver> network_link_rtcp_observer;
+ NiceMock<MockNetworkStateEstimateObserver> 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 @@
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<rtcp::RemoteEstimate> remote_estimate =
+ std::make_unique<rtcp::RemoteEstimate>();
+ remote_estimate->SetEstimate(estimate);
+ std::unique_ptr<rtcp::TransportFeedback> feedback_packet =
+ std::make_unique<rtcp::TransportFeedback>();
+ 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/";
