From 2295ddbff978b9954688ee6163c9f3f554a7a85e Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 6 Jul 2022 22:06:19 +0200 Subject: [PATCH] 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 Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#37494} --- .../remote_estimator_proxy.cc | 10 ++++- .../remote_estimator_proxy_unittest.cc | 45 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index 0215b64155..15c443722a 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -140,9 +140,15 @@ void RemoteEstimatorProxy::IncomingPacket(Packet packet) { 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 57db595293..6d15f27091 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 @@ TEST_F(RemoteEstimatorProxyTest, IncomingPacketHandlesWrapInAbsSendTime) { .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,