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}

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",

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_

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(

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<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 @@ 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<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/";