Test that caller adapts to link capacity using CCFB

Fix todo to ensure TransportSequence numbers are generated if CCFB according to RFC 8888 is used. Transport sequence numbers are used in BWE algorithms regardless of feedback format.

Bug: webrtc:42225697
Change-Id: I6eab95c0241d590f6e7a90d19c82d13ab8692f2b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370341
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43515}

call/call.cc

@@ -1195,6 +1195,8 @@ Call::Stats Call::GetStats() const {
 
 void Call::EnableSendCongestionControlFeedbackAccordingToRfc8888() {
   receive_side_cc_.EnableSendCongestionControlFeedbackAccordingToRfc8888();
+  transport_send_->packet_router()
+      ->EnableCongestionControlFeedbackAccordingToRfc8888();
 }
 
 int Call::FeedbackAccordingToRfc8888Count() {

modules/pacing/packet_router.cc

@@ -87,6 +87,11 @@ void PacketRouter::RegisterNotifyBweCallback(
   notify_bwe_callback_ = std::move(callback);
 }
 
+void PacketRouter::EnableCongestionControlFeedbackAccordingToRfc8888() {
+  RTC_DCHECK_RUN_ON(&thread_checker_);
+  use_cc_feedback_according_to_rfc8888_ = true;
+}
+
 void PacketRouter::AddSendRtpModuleToMap(RtpRtcpInterface* rtp_module,
                                          uint32_t ssrc) {
   RTC_DCHECK_RUN_ON(&thread_checker_);
@@ -183,9 +188,19 @@ void PacketRouter::SendPacket(std::unique_ptr<RtpPacketToSend> packet,
     RTC_LOG(LS_WARNING) << "Failed to send packet, Not sending media";
     return;
   }
-  // TODO(bugs.webrtc.org/15368): Even if the TransportSequenceNumber extension
-  // is not negotiated, we will need the transport sequence number for BWE.
-  if (packet->HasExtension<TransportSequenceNumber>()) {
+
+  // Transport sequence numbers are used if send side bandwidth estimation is
+  // used. Send side BWE relies on RTCP feedback either using format described
+  // in RFC 8888 or
+  // https://datatracker.ietf.org/doc/html/draft-holmer-rmcat-transport-wide-cc-extensions-01.
+  // If RFC 8888 feedback is used, a transport
+  // sequence number is created for all RTP packets, but not sent in the RTP
+  // packet. Otherwise, the transport sequence number is only created
+  // if the TransportSequenceNumber header extension is negotiated for the
+  // specific media type. Historically, webrtc only used TransportSequenceNumber
+  // on video packets.
+  if (use_cc_feedback_according_to_rfc8888_ ||
+      packet->HasExtension<TransportSequenceNumber>()) {
     packet->set_transport_sequence_number(transport_seq_++);
   }
   rtp_module->AssignSequenceNumber(*packet);

modules/pacing/packet_router.h

@@ -55,6 +55,7 @@ class PacketRouter : public PacingController::PacketSender {
   void RegisterNotifyBweCallback(
       absl::AnyInvocable<void(const RtpPacketToSend& packet,
                               const PacedPacketInfo& pacing_info)> callback);
+  void EnableCongestionControlFeedbackAccordingToRfc8888();
 
   void AddSendRtpModule(RtpRtcpInterface* rtp_module, bool remb_candidate);
   void RemoveSendRtpModule(RtpRtcpInterface* rtp_module);
@@ -116,6 +117,8 @@ class PacketRouter : public PacingController::PacketSender {
       RTC_GUARDED_BY(thread_checker_);
 
   uint64_t transport_seq_ RTC_GUARDED_BY(thread_checker_);
+  bool use_cc_feedback_according_to_rfc8888_ RTC_GUARDED_BY(thread_checker_) =
+      false;
   absl::AnyInvocable<void(RtpPacketToSend& packet,
                           const PacedPacketInfo& pacing_info)>
       notify_bwe_callback_ RTC_GUARDED_BY(thread_checker_) = nullptr;

modules/pacing/packet_router_unittest.cc

@@ -380,6 +380,63 @@ TEST_F(PacketRouterTest, AllocatesTransportSequenceNumbers) {
   packet_router.RemoveSendRtpModule(&rtp_1);
 }
 
+TEST_F(PacketRouterTest,
+       DoesNotAllocateTransportSequenceNumberWithoutExtension) {
+  const uint16_t kSsrc1 = 1234;
+
+  PacketRouter packet_router;
+  testing::MockFunction<void(const RtpPacketToSend& packet,
+                             const PacedPacketInfo& pacing_info)>
+      notify_bwe_callback;
+  NiceMock<MockRtpRtcpInterface> rtp_1;
+  packet_router.RegisterNotifyBweCallback(notify_bwe_callback.AsStdFunction());
+
+  EXPECT_CALL(rtp_1, SSRC()).WillRepeatedly(Return(kSsrc1));
+  EXPECT_CALL(rtp_1, CanSendPacket).WillRepeatedly(Return(true));
+
+  packet_router.AddSendRtpModule(&rtp_1, false);
+
+  auto packet = BuildRtpPacket(kSsrc1);
+  EXPECT_CALL(notify_bwe_callback, Call)
+      .WillOnce([](const RtpPacketToSend& packet,
+                   const PacedPacketInfo& /* pacing_info */) {
+        EXPECT_EQ(packet.transport_sequence_number(), std::nullopt);
+      });
+  packet_router.SendPacket(std::move(packet), PacedPacketInfo());
+
+  packet_router.OnBatchComplete();
+  packet_router.RemoveSendRtpModule(&rtp_1);
+}
+
+TEST_F(PacketRouterTest,
+       AllocateTransportSequenceNumberWithoutExtensionIfRfc8888Enabled) {
+  const uint16_t kSsrc1 = 1234;
+
+  PacketRouter packet_router;
+  testing::MockFunction<void(const RtpPacketToSend& packet,
+                             const PacedPacketInfo& pacing_info)>
+      notify_bwe_callback;
+  NiceMock<MockRtpRtcpInterface> rtp_1;
+  packet_router.RegisterNotifyBweCallback(notify_bwe_callback.AsStdFunction());
+
+  EXPECT_CALL(rtp_1, SSRC()).WillRepeatedly(Return(kSsrc1));
+  EXPECT_CALL(rtp_1, CanSendPacket).WillRepeatedly(Return(true));
+
+  packet_router.AddSendRtpModule(&rtp_1, false);
+  packet_router.EnableCongestionControlFeedbackAccordingToRfc8888();
+
+  auto packet = BuildRtpPacket(kSsrc1);
+  EXPECT_CALL(notify_bwe_callback, Call)
+      .WillOnce([](const RtpPacketToSend& packet,
+                   const PacedPacketInfo& /* pacing_info */) {
+        EXPECT_EQ(packet.transport_sequence_number(), 1);
+      });
+  packet_router.SendPacket(std::move(packet), PacedPacketInfo());
+
+  packet_router.OnBatchComplete();
+  packet_router.RemoveSendRtpModule(&rtp_1);
+}
+
 TEST_F(PacketRouterTest, SendTransportFeedback) {
   NiceMock<MockRtpRtcpInterface> rtp_1;
   NiceMock<MockRtpRtcpInterface> rtp_2;

test/peer_scenario/tests/l4s_test.cc

@@ -10,12 +10,15 @@
 
 #include <atomic>
 
+#include "api/stats/rtcstats_objects.h"
+#include "api/units/data_rate.h"
 #include "api/units/time_delta.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtcp_packet/congestion_control_feedback.h"
 #include "modules/rtp_rtcp/source/rtcp_packet/rtpfb.h"
 #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 #include "modules/rtp_rtcp/source/rtp_util.h"
+#include "pc/test/mock_peer_connection_observers.h"
 #include "test/create_frame_generator_capturer.h"
 #include "test/field_trial.h"
 #include "test/gmock.h"
@@ -62,6 +65,26 @@ class RtcpFeedbackCounter {
   int transport_sequence_number_feedback_ = 0;
 };
 
+rtc::scoped_refptr<const RTCStatsReport> GetStatsAndProcess(
+    PeerScenario& s,
+    PeerScenarioClient* client) {
+  auto stats_collector =
+      rtc::make_ref_counted<webrtc::MockRTCStatsCollectorCallback>();
+  client->pc()->GetStats(stats_collector.get());
+  s.ProcessMessages(TimeDelta::Millis(0));
+  RTC_CHECK(stats_collector->called());
+  return stats_collector->report();
+}
+
+DataRate GetAvailableSendBitrate(
+    const rtc::scoped_refptr<const RTCStatsReport>& report) {
+  auto stats = report->GetStatsOfType<RTCIceCandidatePairStats>();
+  if (stats.empty()) {
+    return DataRate::Zero();
+  }
+  return DataRate::BitsPerSec(*stats[0]->available_outgoing_bitrate);
+}
+
 TEST(L4STest, NegotiateAndUseCcfbIfEnabled) {
   test::ScopedFieldTrials trials(
       "WebRTC-RFC8888CongestionControlFeedback/Enabled/");
@@ -140,5 +163,44 @@ TEST(L4STest, NegotiateAndUseCcfbIfEnabled) {
   EXPECT_EQ(ret_node_feedback_counter.FeedbackAccordingToTransportCc(), 0);
 }
 
+TEST(L4STest, CallerAdaptToLinkCapacityWithoutEcn) {
+  test::ScopedFieldTrials trials(
+      "WebRTC-RFC8888CongestionControlFeedback/Enabled/");
+  PeerScenario s(*test_info_);
+
+  PeerScenarioClient::Config config = PeerScenarioClient::Config();
+  PeerScenarioClient* caller = s.CreateClient(config);
+  PeerScenarioClient* callee = s.CreateClient(config);
+
+  auto caller_to_callee = s.net()
+                              ->NodeBuilder()
+                              .capacity(DataRate::KilobitsPerSec(600))
+                              .Build()
+                              .node;
+  auto callee_to_caller = s.net()->NodeBuilder().Build().node;
+  s.net()->CreateRoute(caller->endpoint(), {caller_to_callee},
+                       callee->endpoint());
+  s.net()->CreateRoute(callee->endpoint(), {callee_to_caller},
+                       caller->endpoint());
+
+  auto signaling = s.ConnectSignaling(caller, callee, {caller_to_callee},
+                                      {callee_to_caller});
+  PeerScenarioClient::VideoSendTrackConfig video_conf;
+  video_conf.generator.squares_video->framerate = 15;
+  caller->CreateVideo("VIDEO_1", video_conf);
+
+  signaling.StartIceSignaling();
+  std::atomic<bool> offer_exchange_done(false);
+  signaling.NegotiateSdp([&](const SessionDescriptionInterface& answer) {
+    offer_exchange_done = true;
+  });
+  s.WaitAndProcess(&offer_exchange_done);
+  s.ProcessMessages(TimeDelta::Seconds(3));
+  DataRate available_bwe =
+      GetAvailableSendBitrate(GetStatsAndProcess(s, caller));
+  EXPECT_GT(available_bwe.kbps(), 500);
+  EXPECT_LT(available_bwe.kbps(), 610);
+}
+
 }  // namespace
 }  // namespace webrtc