In Scream, ensure start bitrate is set correct if first feedback delayed

This fixes an issue where the time from when a packet was received until the feedback is sent is not accounted for when calculating the initial reference window.

Bug: webrtc:447037083
Change-Id: I97fa2a9e386425ad8426ae777233eb6714b14dd5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/462080
Reviewed-by: Jakob Ivarsson‎ <jakobi@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#47350}

modules/congestion_controller/scream/scream_network_controller_unittest.cc

diff --git a/modules/congestion_controller/scream/scream_network_controller_unittest.cc b/modules/congestion_controller/scream/scream_network_controller_unittest.cc
index c5046b3..9d6af1c 100644
--- a/modules/congestion_controller/scream/scream_network_controller_unittest.cc
+++ b/modules/congestion_controller/scream/scream_network_controller_unittest.cc
@@ -454,7 +454,7 @@
   PaddingTestResult result = ProcessUntilPaddingStartAndStop(
       clock, scream_controller, feedback_generator);
 
-  EXPECT_LT(result.target_rate, DataRate::KilobitsPerSec(720));
+  EXPECT_LT(result.target_rate, DataRate::KilobitsPerSec(750));
   // Padding should stop when congestion is detected.
   EXPECT_LT(result.padding_stop - result.padding_start, TimeDelta::Seconds(1));
 }

modules/congestion_controller/scream/scream_v2.cc

diff --git a/modules/congestion_controller/scream/scream_v2.cc b/modules/congestion_controller/scream/scream_v2.cc
index 786ee7e..db74e55 100644
--- a/modules/congestion_controller/scream/scream_v2.cc
+++ b/modules/congestion_controller/scream/scream_v2.cc
@@ -96,16 +96,21 @@
 
   delay_based_congestion_control_.OnTransportPacketsFeedback(msg);
 
+  UpdateFeedbackHoldTime(msg);
+
   if (!first_feedback_processed_) {
-    RTC_LOG(LS_INFO) << "Initial RTT: "
-                     << delay_based_congestion_control_.rtt().ms()
-                     << "ms, Start Bitrate: " << target_rate_.kbps() << "kbps";
     ref_window_ =
         std::max(params_.min_ref_window.Get(),
-                 target_rate_ * delay_based_congestion_control_.rtt());
+                 target_rate_ * (delay_based_congestion_control_.rtt() +
+                                 feedback_hold_time_));
+    RTC_LOG(LS_INFO) << "Initial RTT: "
+                     << delay_based_congestion_control_.rtt().ms()
+                     << " feedback_hold_time: " << feedback_hold_time_.ms()
+                     << "ms, Start Bitrate: " << target_rate_.kbps() << "kbps"
+                     << " ref_window_=" << ref_window_.bytes();
     first_feedback_processed_ = true;
   }
-  UpdateFeedbackHoldTime(msg);
+
   UpdateL4SAlpha(msg);
   UpdateRefWindow(msg);
   UpdateTargetRate(msg);

modules/congestion_controller/scream/test/cc_feedback_generator.cc

diff --git a/modules/congestion_controller/scream/test/cc_feedback_generator.cc b/modules/congestion_controller/scream/test/cc_feedback_generator.cc
index 1da5c34..f57d635 100644
--- a/modules/congestion_controller/scream/test/cc_feedback_generator.cc
+++ b/modules/congestion_controller/scream/test/cc_feedback_generator.cc
@@ -127,8 +127,15 @@
 
 std::optional<TransportPacketsFeedback>
 CcFeedbackGenerator::MaybeSendFeedback(Timestamp time) {
-  if (last_feedback_time_.IsFinite() &&
-      time - last_feedback_time_ < time_between_feedback_) {
+  if (packets_received_.empty()) {
+    return std::nullopt;
+  }
+  if (last_feedback_time_.IsInfinite()) {
+    last_feedback_time_ =
+        Timestamp::Micros(packets_received_.front().receive_time_us) +
+        one_way_delay_;
+  }
+  if (time - last_feedback_time_ < time_between_feedback_) {
     return std::nullopt;
   }
   // Time to deliver feedback if there are packets to deliver.

modules/congestion_controller/scream/test/cc_feedback_generator_unittest.cc

diff --git a/modules/congestion_controller/scream/test/cc_feedback_generator_unittest.cc b/modules/congestion_controller/scream/test/cc_feedback_generator_unittest.cc
index f579a83..c20bfb6 100644
--- a/modules/congestion_controller/scream/test/cc_feedback_generator_unittest.cc
+++ b/modules/congestion_controller/scream/test/cc_feedback_generator_unittest.cc
@@ -48,9 +48,9 @@
 
   EXPECT_EQ(feedback_1.feedback_time, clock.CurrentTime());
   EXPECT_EQ(feedback_1.data_in_flight, 3 * kPacketSize);
-  ASSERT_THAT(feedback_1.packet_feedbacks, SizeIs(1));
+  ASSERT_THAT(feedback_1.packet_feedbacks, SizeIs(4));
   EXPECT_EQ(feedback_1.packet_feedbacks[0].arrival_time_offset,
-            TimeDelta::Zero());
+            TimeDelta::Millis(50));
   for (const PacketResult& packet : feedback_1.packet_feedbacks) {
     EXPECT_EQ((packet.receive_time - packet.sent_packet.send_time),
               TimeDelta::Millis(25 + 8));
@@ -64,6 +64,22 @@
   EXPECT_EQ((feedback_2.feedback_time - feedback_1.feedback_time).ms(), 50);
 }
 
+TEST(CcFeedbackGeneratorTest, SendsFirstFeedbackAfterTimeBetweenFeedback) {
+  SimulatedClock clock(Timestamp::Seconds(1234));
+  CcFeedbackGenerator feedback_generator(
+      {.network_config = {.queue_delay_ms = 25,
+                          .link_capacity = DataRate::KilobitsPerSec(1000)},
+       .time_between_feedback = TimeDelta::Millis(50)});
+
+  TransportPacketsFeedback feedback =
+      feedback_generator.ProcessUntilNextFeedback(
+          /*send_rate=*/DataRate::KilobitsPerSec(500), clock, nullptr);
+
+  ASSERT_FALSE(feedback.packet_feedbacks.empty());
+  EXPECT_EQ(feedback.packet_feedbacks[0].arrival_time_offset,
+            TimeDelta::Millis(50));
+}
+
 TEST(CcFeedbackGeneratorTest, CeMarksPacketsIfSendRateIsTooHigh) {
   SimulatedClock clock(Timestamp::Seconds(1234));
   CcFeedbackGenerator feedback_generator(