From 9dcbfd861437d11bc30b4a422d611130b3136c44 Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Thu, 14 Jul 2022 09:53:43 +0000 Subject: [PATCH] Revert "In bitrate estimator Improve handing send time of out of order packets" This reverts commit 2295ddbff978b9954688ee6163c9f3f554a7a85e. Reason for revert: Investigation required because it breaks some downstream tests. Original change's description: > 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} Bug: None Change-Id: Ib8ab916b9eedb93aac5fc35c5d291b1f4ed16de0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/268541 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Owners-Override: Mirko Bonadei Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/main@{#37520} --- .../remote_estimator_proxy.cc | 10 +---- .../remote_estimator_proxy_unittest.cc | 45 ------------------- 2 files changed, 2 insertions(+), 53 deletions(-) diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index 15c443722a..0215b64155 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -140,15 +140,9 @@ void RemoteEstimatorProxy::IncomingPacket(Packet packet) { if (network_state_estimator_ && packet.absolute_send_time_24bits) { PacketResult packet_result; packet_result.receive_time = packet.arrival_time; - TimeDelta delta = GetAbsoluteSendTimeDelta( + abs_send_timestamp_ += GetAbsoluteSendTimeDelta( *packet.absolute_send_time_24bits, previous_abs_send_time_); - // `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; - } + 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 6d15f27091..57db595293 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc @@ -631,51 +631,6 @@ 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,