In bitrate estimator Improve handing send time of out of order packets
Bug: None
Change-Id: I74da3b616fb9419de8f7d9d28326354cee1c178d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268061
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37494}
diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
index 0215b64..15c4437 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
@@ -140,9 +140,15 @@
if (network_state_estimator_ && packet.absolute_send_time_24bits) {
PacketResult packet_result;
packet_result.receive_time = packet.arrival_time;
- abs_send_timestamp_ += GetAbsoluteSendTimeDelta(
+ TimeDelta delta = GetAbsoluteSendTimeDelta(
*packet.absolute_send_time_24bits, previous_abs_send_time_);
- previous_abs_send_time_ = *packet.absolute_send_time_24bits;
+ // `GetAbsoluteSendTimeDelta` returns zero when packet arrived out of order.
+ // Don't adjust previous_abs_send_time, otherwise abs_send_timestamp_ would
+ // accumulate error.
+ if (delta > TimeDelta::Zero()) {
+ abs_send_timestamp_ += delta;
+ previous_abs_send_time_ = *packet.absolute_send_time_24bits;
+ }
packet_result.sent_packet.send_time = abs_send_timestamp_;
packet_result.sent_packet.size = packet.size + packet_overhead_;
packet_result.sent_packet.sequence_number = seq;
diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
index 57db595..6d15f27 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
@@ -631,6 +631,51 @@
.transport_sequence_number = kBaseSeq + 1});
}
+TEST_F(RemoteEstimatorProxyTest, OutOfOrderPacketReportsWithSameSendTime) {
+ // abs send time use 24bit precision.
+ const uint32_t kFirstAbsSendTime =
+ AbsoluteSendTime::To24Bits(Timestamp::Millis(15));
+ // Second abs send time has wrapped.
+ const uint32_t kSecondAbsSendTime =
+ AbsoluteSendTime::To24Bits(Timestamp::Millis(30));
+ const uint32_t kThirdAbsSendTime =
+ AbsoluteSendTime::To24Bits(Timestamp::Millis(45));
+
+ Timestamp first_send_timestamp = Timestamp::Zero();
+ EXPECT_CALL(network_state_estimator_, OnReceivedPacket)
+ .WillOnce([&](const PacketResult& packet) {
+ first_send_timestamp = packet.sent_packet.send_time;
+ })
+ .WillOnce([&](const PacketResult& packet) {
+ // Expect out of order packet repeats previous timestamp.
+ EXPECT_EQ(packet.sent_packet.send_time, first_send_timestamp);
+ })
+ .WillOnce([&](const PacketResult& packet) {
+ // Expect 2nd arrived packet doesn't affect difference in absolute send
+ // time between 3rd and 1st packets.
+ // Round to milliseconds precision to compensate for precision loss due
+ // to convertion to/from 6x18 format of the absolute send time.
+ EXPECT_EQ((packet.sent_packet.send_time - first_send_timestamp).ms(),
+ 15);
+ });
+
+ proxy_.IncomingPacket({.arrival_time = kBaseTime,
+ .size = kDefaultPacketSize,
+ .ssrc = kMediaSsrc,
+ .absolute_send_time_24bits = kSecondAbsSendTime,
+ .transport_sequence_number = kBaseSeq + 2});
+ proxy_.IncomingPacket({.arrival_time = kBaseTime + TimeDelta::Millis(10),
+ .size = kDefaultPacketSize,
+ .ssrc = kMediaSsrc,
+ .absolute_send_time_24bits = kFirstAbsSendTime,
+ .transport_sequence_number = kBaseSeq + 1});
+ proxy_.IncomingPacket({.arrival_time = kBaseTime + TimeDelta::Millis(20),
+ .size = kDefaultPacketSize,
+ .ssrc = kMediaSsrc,
+ .absolute_send_time_24bits = kThirdAbsSendTime,
+ .transport_sequence_number = kBaseSeq + 3});
+}
+
TEST_F(RemoteEstimatorProxyTest, SendTransportFeedbackAndNetworkStateUpdate) {
proxy_.IncomingPacket(
{.arrival_time = kBaseTime,
