Don't round the computed time deltas to nearest ms.

Resolving https://bugs.chromium.org/p/webrtc/issues/detail?id=14023

At the moment, in DelayBasedBwe the time deltas are rounded to the
nearest millisecond. This change makes sure the numbers are passed as
doubles as expected by the TrendlineEstimator.

Change-Id: I68882547fb19af0e67e7b5d8de4159083a54d7eb
Bug: webrtc:14023
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261320
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36806}

AUTHORS

@@ -44,6 +44,7 @@ Dirk-Jan C. Binnema <djcb@djcbsoftware.nl>
 Dmitry Lizin <sdkdimon@gmail.com>
 Eike Rathke <erathke@redhat.com>
 Eric Rescorla, RTFM Inc. <ekr@rtfm.com>
+Filip Hlasek <filip@orcamobility.ai>
 Frederik Riedel, Frogg GmbH <frederik.riedel@frogg.io>
 Giji Gangadharan <giji.g@samsung.com>
 Graham Yoakum <gyoakum@skobalt.com>

modules/congestion_controller/goog_cc/delay_based_bwe.cc

@@ -193,7 +193,8 @@ void DelayBasedBwe::IncomingPacketFeedback(const PacketResult& packet_feedback,
       packet_feedback.sent_packet.send_time, packet_feedback.receive_time,
       at_time, packet_size.bytes(), &send_delta, &recv_delta, &size_delta);
 
-  delay_detector_for_packet->Update(recv_delta.ms(), send_delta.ms(),
+  delay_detector_for_packet->Update(recv_delta.ms<double>(),
+                                    send_delta.ms<double>(),
                                     packet_feedback.sent_packet.send_time.ms(),
                                     packet_feedback.receive_time.ms(),
                                     packet_size.bytes(), calculated_deltas);

modules/congestion_controller/goog_cc/delay_based_bwe_unittest.cc

@@ -214,6 +214,41 @@ TEST_F(DelayBasedBweTest, TestInitialOveruse) {
               15000);
 }
 
+TEST_F(DelayBasedBweTest, TestTimestampPrecisionHandling) {
+  // This test does some basic checks to make sure that timestamps with higher
+  // than millisecond precision are handled properly and do not cause any
+  // problems in the estimator. Specifically, previously reported in
+  // webrtc:14023 and described in more details there, the rounding to the
+  // nearest milliseconds caused discrepancy in the accumulated delay. This lead
+  // to false-positive overuse detection.
+  // Technical details of the test:
+  // Send times(ms): 0.000,  9.725, 20.000, 29.725, 40.000, 49.725, ...
+  // Recv times(ms): 0.500, 10.000, 20.500, 30.000, 40.500, 50.000, ...
+  // Send deltas(ms):   9.750,  10.250,  9.750, 10.250,  9.750, ...
+  // Recv deltas(ms):   9.500,  10.500,  9.500, 10.500,  9.500, ...
+  // There is no delay building up between the send times and the receive times,
+  // therefore this case should never lead to an overuse detection. However, if
+  // the time deltas were accidentally rounded to the nearest milliseconds, then
+  // all the send deltas would be equal to 10ms while some recv deltas would
+  // round up to 11ms which would lead in a false illusion of delay build up.
+  uint32_t last_bitrate = bitrate_observer_.latest_bitrate();
+  for (int i = 0; i < 1000; ++i) {
+    clock_.AdvanceTimeMicroseconds(500);
+    IncomingFeedback(clock_.CurrentTime(),
+                     clock_.CurrentTime() - TimeDelta::Micros(500), 1000,
+                     PacedPacketInfo());
+    clock_.AdvanceTimeMicroseconds(9500);
+    IncomingFeedback(clock_.CurrentTime(),
+                     clock_.CurrentTime() - TimeDelta::Micros(250), 1000,
+                     PacedPacketInfo());
+    clock_.AdvanceTimeMicroseconds(10000);
+
+    // The bitrate should never decrease in this test.
+    EXPECT_LE(last_bitrate, bitrate_observer_.latest_bitrate());
+    last_bitrate = bitrate_observer_.latest_bitrate();
+  }
+}
+
 class DelayBasedBweTestWithBackoffTimeoutExperiment : public DelayBasedBweTest {
  public:
   DelayBasedBweTestWithBackoffTimeoutExperiment()

modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.cc

@@ -179,10 +179,17 @@ void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms,
                                          size_t payload_size,
                                          const PacedPacketInfo& pacing_info) {
   RTC_CHECK_GE(arrival_time_ms + arrival_time_offset_ms_, 0);
+  IncomingFeedback(Timestamp::Millis(arrival_time_ms + arrival_time_offset_ms_),
+                   Timestamp::Millis(send_time_ms), payload_size, pacing_info);
+}
+
+void DelayBasedBweTest::IncomingFeedback(Timestamp receive_time,
+                                         Timestamp send_time,
+                                         size_t payload_size,
+                                         const PacedPacketInfo& pacing_info) {
   PacketResult packet;
-  packet.receive_time =
+  packet.receive_time = receive_time;
-      Timestamp::Millis(arrival_time_ms + arrival_time_offset_ms_);
+  packet.sent_packet.send_time = send_time;
-  packet.sent_packet.send_time = Timestamp::Millis(send_time_ms);
   packet.sent_packet.size = DataSize::Bytes(payload_size);
   packet.sent_packet.pacing_info = pacing_info;
   if (packet.sent_packet.pacing_info.probe_cluster_id !=

modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.h

@@ -131,6 +131,10 @@ class DelayBasedBweTest : public ::testing::Test {
                         int64_t send_time_ms,
                         size_t payload_size,
                         const PacedPacketInfo& pacing_info);
+  void IncomingFeedback(Timestamp receive_time,
+                        Timestamp send_time,
+                        size_t payload_size,
+                        const PacedPacketInfo& pacing_info);
 
   // Generates a frame of packets belonging to a stream at a given bitrate and
   // with a given ssrc. The stream is pushed through a very simple simulated