Add functions to send feedback according to RFC 8888 in ReceiveSideCongestionController.

With this cl, sending can be forced with field trial "WebRTC-RFC8888CongestionControlFeedback/force_send:true/"
In the future, ReceiveSideCongestionController::EnablSendCongestionControlFeedbackAccordingToRfc8888 if RFC 8888 has been negotiated.

Bug: webrtc:42225697
Change-Id: Ib09066aa89ca7b3fffc551da541090c69ab8d75f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/352720
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42413}

modules/congestion_controller/BUILD.gn

@@ -19,9 +19,11 @@ rtc_library("congestion_controller") {
 
   deps = [
     "../../api:rtp_parameters",
+    "../../api:sequence_checker",
     "../../api/environment",
     "../../api/transport:network_control",
     "../../api/units:data_rate",
+    "../../api/units:data_size",
     "../../api/units:time_delta",
     "../../api/units:timestamp",
     "../../rtc_base:logging",
@@ -29,6 +31,7 @@ rtc_library("congestion_controller") {
     "../../rtc_base/synchronization:mutex",
     "../pacing",
     "../remote_bitrate_estimator",
+    "../remote_bitrate_estimator:congestion_control_feedback_generator",
     "../remote_bitrate_estimator:transport_sequence_number_feedback_generator",
     "../rtp_rtcp:rtp_rtcp_format",
     "//third_party/abseil-cpp/absl/base:nullability",
@@ -45,6 +48,7 @@ if (rtc_include_tests && !build_with_chromium) {
     ]
     deps = [
       ":congestion_controller",
+      "../../api:rtp_parameters",
       "../../api/environment:environment_factory",
       "../../api/test/network_emulation",
       "../../api/test/network_emulation:create_cross_traffic",
@@ -53,6 +57,7 @@ if (rtc_include_tests && !build_with_chromium) {
       "../../api/units:time_delta",
       "../../api/units:timestamp",
       "../../system_wrappers",
+      "../../test:explicit_key_value_config",
       "../../test:test_support",
       "../../test/scenario",
       "../pacing",

modules/congestion_controller/include/receive_side_congestion_controller.h

@@ -15,10 +15,12 @@
 
 #include "absl/base/nullability.h"
 #include "api/environment/environment.h"
+#include "api/sequence_checker.h"
 #include "api/transport/network_control.h"
 #include "api/units/data_rate.h"
 #include "api/units/time_delta.h"
 #include "modules/congestion_controller/remb_throttler.h"
+#include "modules/remote_bitrate_estimator/congestion_control_feedback_generator.h"
 #include "modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.h"
 #include "modules/rtp_rtcp/source/rtp_packet_received.h"
 #include "rtc_base/synchronization/mutex.h"
@@ -42,6 +44,8 @@ class ReceiveSideCongestionController : public CallStatsObserver {
 
   ~ReceiveSideCongestionController() override = default;
 
+  void EnablSendCongestionControlFeedbackAccordingToRfc8888();
+
   void OnReceivedPacket(const RtpPacketReceived& packet, MediaType media_type);
 
   // Implements CallStatsObserver.
@@ -74,8 +78,18 @@ class ReceiveSideCongestionController : public CallStatsObserver {
 
   const Environment env_;
   RembThrottler remb_throttler_;
+
+  // TODO: bugs.webrtc.org/42224904 - Use sequence checker for all usage of
+  // ReceiveSideCongestionController. At the time of
+  // writing OnReceivedPacket and MaybeProcess can unfortunately be called on an
+  // arbitrary thread by external projects.
+  SequenceChecker sequence_checker_;
+
+  bool send_rfc8888_congestion_feedback_ = false;
   TransportSequenceNumberFeedbackGenenerator
       transport_sequence_number_feedback_generator_;
+  CongestionControlFeedbackGenerator congestion_control_feedback_generator_
+      RTC_GUARDED_BY(sequence_checker_);
 
   mutable Mutex mutex_;
   std::unique_ptr<RemoteBitrateEstimator> rbe_ RTC_GUARDED_BY(mutex_);

modules/congestion_controller/receive_side_congestion_controller.cc

@@ -10,14 +10,18 @@
 
 #include "modules/congestion_controller/include/receive_side_congestion_controller.h"
 
+#include <memory>
+
 #include "absl/base/nullability.h"
 #include "api/environment/environment.h"
 #include "api/media_types.h"
+#include "api/sequence_checker.h"
 #include "api/units/data_rate.h"
-#include "modules/pacing/packet_router.h"
-#include "modules/remote_bitrate_estimator/include/bwe_defines.h"
+#include "modules/remote_bitrate_estimator/congestion_control_feedback_generator.h"
 #include "modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h"
 #include "modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h"
+#include "modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.h"
+#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
 #include "rtc_base/logging.h"
 
 namespace webrtc {
@@ -77,17 +81,37 @@ ReceiveSideCongestionController::ReceiveSideCongestionController(
     absl::Nullable<NetworkStateEstimator*> network_state_estimator)
     : env_(env),
       remb_throttler_(std::move(remb_sender), &env_.clock()),
-      transport_sequence_number_feedback_generator_(std::move(feedback_sender),
+      transport_sequence_number_feedback_generator_(feedback_sender,
                                                     network_state_estimator),
+      congestion_control_feedback_generator_(env, feedback_sender),
       rbe_(std::make_unique<RemoteBitrateEstimatorSingleStream>(
           env_,
           &remb_throttler_)),
       using_absolute_send_time_(false),
-      packets_since_absolute_send_time_(0) {}
+      packets_since_absolute_send_time_(0) {
+  FieldTrialParameter<bool> force_send_rfc8888_feedback("force_send", false);
+  ParseFieldTrial(
+      {&force_send_rfc8888_feedback},
+      env.field_trials().Lookup("WebRTC-RFC8888CongestionControlFeedback"));
+  if (force_send_rfc8888_feedback) {
+    EnablSendCongestionControlFeedbackAccordingToRfc8888();
+  }
+}
+
+void ReceiveSideCongestionController::
+    EnablSendCongestionControlFeedbackAccordingToRfc8888() {
+  RTC_DCHECK_RUN_ON(&sequence_checker_);
+  send_rfc8888_congestion_feedback_ = true;
+}
 
 void ReceiveSideCongestionController::OnReceivedPacket(
     const RtpPacketReceived& packet,
     MediaType media_type) {
+  if (send_rfc8888_congestion_feedback_) {
+    RTC_DCHECK_RUN_ON(&sequence_checker_);
+    congestion_control_feedback_generator_.OnReceivedPacket(packet);
+    return;
+  }
   bool has_transport_sequence_number =
       packet.HasExtension<TransportSequenceNumber>() ||
       packet.HasExtension<TransportSequenceNumberV2>();
@@ -108,12 +132,20 @@ void ReceiveSideCongestionController::OnReceivedPacket(
 }
 
 void ReceiveSideCongestionController::OnBitrateChanged(int bitrate_bps) {
+  RTC_DCHECK_RUN_ON(&sequence_checker_);
+  DataRate send_bandwidth_estimate = DataRate::BitsPerSec(bitrate_bps);
   transport_sequence_number_feedback_generator_.OnSendBandwidthEstimateChanged(
-      DataRate::BitsPerSec(bitrate_bps));
+      send_bandwidth_estimate);
+  congestion_control_feedback_generator_.OnSendBandwidthEstimateChanged(
+      send_bandwidth_estimate);
 }
 
 TimeDelta ReceiveSideCongestionController::MaybeProcess() {
   Timestamp now = env_.clock().CurrentTime();
+  if (send_rfc8888_congestion_feedback_) {
+    RTC_DCHECK_RUN_ON(&sequence_checker_);
+    return congestion_control_feedback_generator_.Process(now);
+  }
   mutex_.Lock();
   TimeDelta time_until_rbe = rbe_->Process();
   mutex_.Unlock();
@@ -130,8 +162,11 @@ void ReceiveSideCongestionController::SetMaxDesiredReceiveBitrate(
 
 void ReceiveSideCongestionController::SetTransportOverhead(
     DataSize overhead_per_packet) {
+  RTC_DCHECK_RUN_ON(&sequence_checker_);
   transport_sequence_number_feedback_generator_.SetTransportOverhead(
       overhead_per_packet);
+  congestion_control_feedback_generator_.SetTransportOverhead(
+      overhead_per_packet);
 }
 
 }  // namespace webrtc

modules/congestion_controller/receive_side_congestion_controller_unittest.cc

@@ -11,17 +11,18 @@
 #include "modules/congestion_controller/include/receive_side_congestion_controller.h"
 
 #include "api/environment/environment_factory.h"
+#include "api/media_types.h"
 #include "api/test/network_emulation/create_cross_traffic.h"
 #include "api/test/network_emulation/cross_traffic.h"
 #include "api/units/data_rate.h"
 #include "api/units/data_size.h"
 #include "api/units/time_delta.h"
 #include "api/units/timestamp.h"
-#include "modules/pacing/packet_router.h"
 #include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
 #include "modules/rtp_rtcp/source/rtp_header_extensions.h"
 #include "modules/rtp_rtcp/source/rtp_packet_received.h"
 #include "system_wrappers/include/clock.h"
+#include "test/explicit_key_value_config.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
 #include "test/scenario/scenario.h"
@@ -34,6 +35,7 @@ using ::testing::_;
 using ::testing::AtLeast;
 using ::testing::ElementsAre;
 using ::testing::MockFunction;
+using ::testing::SizeIs;
 
 constexpr DataRate kInitialBitrate = DataRate::BitsPerSec(60'000);
 
@@ -81,6 +83,76 @@ TEST(ReceiveSideCongestionControllerTest,
   controller.SetMaxDesiredReceiveBitrate(DataRate::BitsPerSec(123));
 }
 
+TEST(ReceiveSideCongestionControllerTest, SendsRfc8888FeedbackIfForced) {
+  test::ExplicitKeyValueConfig field_trials(
+      "WebRTC-RFC8888CongestionControlFeedback/force_send:true/");
+  MockFunction<void(std::vector<std::unique_ptr<rtcp::RtcpPacket>>)>
+      rtcp_sender;
+  MockFunction<void(uint64_t, std::vector<uint32_t>)> remb_sender;
+  SimulatedClock clock(123456);
+  ReceiveSideCongestionController controller(
+      CreateEnvironment(&clock, &field_trials), rtcp_sender.AsStdFunction(),
+      remb_sender.AsStdFunction(), nullptr);
+
+  EXPECT_CALL(rtcp_sender, Call);
+  RtpPacketReceived packet;
+  packet.set_arrival_time(clock.CurrentTime());
+  controller.OnReceivedPacket(packet, MediaType::VIDEO);
+  TimeDelta next_process = controller.MaybeProcess();
+  clock.AdvanceTime(next_process);
+  next_process = controller.MaybeProcess();
+}
+
+TEST(ReceiveSideCongestionControllerTest, SendsRfc8888FeedbackIfEnabled) {
+  MockFunction<void(std::vector<std::unique_ptr<rtcp::RtcpPacket>>)>
+      rtcp_sender;
+  MockFunction<void(uint64_t, std::vector<uint32_t>)> remb_sender;
+  SimulatedClock clock(123456);
+  ReceiveSideCongestionController controller(
+      CreateEnvironment(&clock), rtcp_sender.AsStdFunction(),
+      remb_sender.AsStdFunction(), nullptr);
+  controller.EnablSendCongestionControlFeedbackAccordingToRfc8888();
+
+  EXPECT_CALL(rtcp_sender, Call)
+      .WillOnce(
+          [&](std::vector<std::unique_ptr<rtcp::RtcpPacket>> rtcp_packets) {
+            ASSERT_THAT(rtcp_packets, SizeIs(1));
+            rtc::Buffer buffer = rtcp_packets[0]->Build();
+            rtcp::CommonHeader header;
+            EXPECT_TRUE(header.Parse(buffer.data(), buffer.size()));
+            EXPECT_EQ(header.fmt(),
+                      rtcp::CongestionControlFeedback::kFeedbackMessageType);
+          });
+
+  RtpPacketReceived packet;
+  packet.set_arrival_time(clock.CurrentTime());
+  controller.OnReceivedPacket(packet, MediaType::VIDEO);
+  TimeDelta next_process = controller.MaybeProcess();
+  clock.AdvanceTime(next_process);
+  next_process = controller.MaybeProcess();
+}
+
+TEST(ReceiveSideCongestionControllerTest,
+     SendsNoFeedbackIfNotRfcRfc8888EnabledAndNoTransportFeedback) {
+  MockFunction<void(std::vector<std::unique_ptr<rtcp::RtcpPacket>>)>
+      rtcp_sender;
+  MockFunction<void(uint64_t, std::vector<uint32_t>)> remb_sender;
+  SimulatedClock clock(123456);
+  ReceiveSideCongestionController controller(
+      CreateEnvironment(&clock), rtcp_sender.AsStdFunction(),
+      remb_sender.AsStdFunction(), nullptr);
+
+  // No Transport feedback is sent because received packet does not have
+  // transport sequence number rtp header extension.
+  EXPECT_CALL(rtcp_sender, Call).Times(0);
+  RtpPacketReceived packet;
+  packet.set_arrival_time(clock.CurrentTime());
+  controller.OnReceivedPacket(packet, MediaType::VIDEO);
+  TimeDelta next_process = controller.MaybeProcess();
+  clock.AdvanceTime(next_process);
+  next_process = controller.MaybeProcess();
+}
+
 TEST(ReceiveSideCongestionControllerTest, ConvergesToCapacity) {
   Scenario s("receive_cc_unit/converge");
   NetworkSimulationConfig net_conf;

modules/remote_bitrate_estimator/congestion_control_feedback_generator.h

@@ -44,10 +44,9 @@ namespace webrtc {
 class CongestionControlFeedbackGenerator
     : public RtpTransportFeedbackGenerator {
  public:
-  using RtcpSender = std::function<void(
-      std::vector<std::unique_ptr<rtcp::RtcpPacket>> packets)>;
-  CongestionControlFeedbackGenerator(const Environment& env,
-                                     RtcpSender feedback_sender);
+  CongestionControlFeedbackGenerator(
+      const Environment& env,
+      RtpTransportFeedbackGenerator::RtcpSender feedback_sender);
   ~CongestionControlFeedbackGenerator() = default;
 
   void OnReceivedPacket(const RtpPacketReceived& packet) override;

modules/remote_bitrate_estimator/rtp_transport_feedback_generator.h

@@ -11,6 +11,9 @@
 #ifndef MODULES_REMOTE_BITRATE_ESTIMATOR_RTP_TRANSPORT_FEEDBACK_GENERATOR_H_
 #define MODULES_REMOTE_BITRATE_ESTIMATOR_RTP_TRANSPORT_FEEDBACK_GENERATOR_H_
 
+#include <memory>
+#include <vector>
+
 #include "api/units/data_rate.h"
 #include "api/units/data_size.h"
 #include "api/units/time_delta.h"
@@ -21,6 +24,10 @@ namespace webrtc {
 
 class RtpTransportFeedbackGenerator {
  public:
+  // Function intented to be used for sending RTCP messages generated by an
+  // implementation of this class.
+  using RtcpSender = std::function<void(
+      std::vector<std::unique_ptr<rtcp::RtcpPacket>> packets)>;
   virtual ~RtpTransportFeedbackGenerator() = default;
 
   virtual void OnReceivedPacket(const RtpPacketReceived& packet) = 0;

modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.h

@@ -44,12 +44,8 @@ namespace webrtc {
 class TransportSequenceNumberFeedbackGenenerator
     : public RtpTransportFeedbackGenerator {
  public:
-  // Used for sending transport feedback messages when send side
-  // BWE is used.
-  using RtcpSender = std::function<void(
-      std::vector<std::unique_ptr<rtcp::RtcpPacket>> packets)>;
   TransportSequenceNumberFeedbackGenenerator(
-      RtcpSender feedback_sender,
+      RtpTransportFeedbackGenerator::RtcpSender feedback_sender,
       NetworkStateEstimator* network_state_estimator);
   ~TransportSequenceNumberFeedbackGenenerator();