diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index c417f35fb6..e100995b8e 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -18,6 +18,7 @@ #include "absl/types/optional.h" #include "api/units/data_size.h" +#include "api/units/time_delta.h" #include "modules/rtp_rtcp/source/rtcp_packet/remote_estimate.h" #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "modules/rtp_rtcp/source/rtp_header_extensions.h" @@ -42,9 +43,9 @@ TimeDelta GetAbsoluteSendTimeDelta(uint32_t new_sendtime, uint32_t delta = (new_sendtime - previous_sendtime) % kWrapAroundPeriod; if (delta >= kWrapAroundPeriod / 2) { // absolute send time wraps around, thus treat deltas larger than half of - // the wrap around period as negative. Ignore reordering of packets and - // treat them as they have approximately the same send time. - return TimeDelta::Zero(); + // the wrap around period as negative. + delta = (previous_sendtime - new_sendtime) % kWrapAroundPeriod; + return TimeDelta::Micros(int64_t{delta} * -1'000'000 / (1 << 18)); } return TimeDelta::Micros(int64_t{delta} * 1'000'000 / (1 << 18)); } @@ -62,7 +63,8 @@ RemoteEstimatorProxy::RemoteEstimatorProxy( send_interval_(kDefaultInterval), send_periodic_feedback_(true), previous_abs_send_time_(0), - abs_send_timestamp_(Timestamp::Zero()) { + abs_send_timestamp_(Timestamp::Zero()), + last_arrival_time_with_abs_send_time_(Timestamp::MinusInfinity()) { RTC_LOG(LS_INFO) << "Maximum interval between transport feedback RTCP messages: " << kMaxInterval; @@ -138,8 +140,14 @@ void RemoteEstimatorProxy::IncomingPacket(const RtpPacketReceived& packet) { if (network_state_estimator_ && absolute_send_time_24bits.has_value()) { PacketResult packet_result; packet_result.receive_time = packet.arrival_time(); - abs_send_timestamp_ += GetAbsoluteSendTimeDelta(*absolute_send_time_24bits, - previous_abs_send_time_); + if (packet.arrival_time() - last_arrival_time_with_abs_send_time_ < + TimeDelta::Seconds(10)) { + abs_send_timestamp_ += GetAbsoluteSendTimeDelta( + *absolute_send_time_24bits, previous_abs_send_time_); + } else { + abs_send_timestamp_ = packet.arrival_time(); + } + last_arrival_time_with_abs_send_time_ = packet.arrival_time(); previous_abs_send_time_ = *absolute_send_time_24bits; packet_result.sent_packet.send_time = abs_send_timestamp_; packet_result.sent_packet.size = diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/modules/remote_bitrate_estimator/remote_estimator_proxy.h index 561410b0e0..b50d2f0db0 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.h @@ -105,6 +105,7 @@ class RemoteEstimatorProxy { // Unwraps absolute send times. uint32_t previous_abs_send_time_ RTC_GUARDED_BY(&lock_); Timestamp abs_send_timestamp_ RTC_GUARDED_BY(&lock_); + Timestamp last_arrival_time_with_abs_send_time_ RTC_GUARDED_BY(&lock_); }; } // namespace webrtc diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc index c148ce6c62..47ad457be9 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc @@ -572,30 +572,17 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, } TEST_F(RemoteEstimatorProxyTest, ReportsIncomingPacketToNetworkStateEstimator) { - Timestamp first_send_timestamp = Timestamp::Zero(); const DataSize kPacketOverhead = DataSize::Bytes(38); proxy_.SetTransportOverhead(kPacketOverhead); - EXPECT_CALL(network_state_estimator_, OnReceivedPacket(_)) - .WillOnce(Invoke([&](const PacketResult& packet) { + EXPECT_CALL(network_state_estimator_, OnReceivedPacket) + .WillOnce([&](const PacketResult& packet) { EXPECT_EQ(packet.receive_time, kBaseTime); EXPECT_GT(packet.sent_packet.size, kPacketOverhead); - first_send_timestamp = packet.sent_packet.send_time; - })); - // Incoming packet with abs sendtime but without transport sequence number. + // Expect first send time to be equal to the arrival time. + EXPECT_EQ(packet.sent_packet.send_time, kBaseTime); + }); IncomingPacket(kBaseSeq, kBaseTime, AbsoluteSendTime::To24Bits(kBaseTime)); - - // Expect packet with older abs send time to be treated as sent at the same - // time as the previous packet due to reordering. - EXPECT_CALL(network_state_estimator_, OnReceivedPacket(_)) - .WillOnce(Invoke([&first_send_timestamp](const PacketResult& packet) { - EXPECT_EQ(packet.receive_time, kBaseTime); - EXPECT_EQ(packet.sent_packet.send_time, first_send_timestamp); - })); - - IncomingPacket(kBaseSeq + 1, kBaseTime, - /*abs_send_time=*/ - AbsoluteSendTime::To24Bits(kBaseTime - TimeDelta::Millis(12))); } TEST_F(RemoteEstimatorProxyTest, IncomingPacketHandlesWrapInAbsSendTime) { @@ -608,24 +595,67 @@ TEST_F(RemoteEstimatorProxyTest, IncomingPacketHandlesWrapInAbsSendTime) { const TimeDelta kExpectedAbsSendTimeDelta = TimeDelta::Millis(30); Timestamp first_send_timestamp = Timestamp::Zero(); - EXPECT_CALL(network_state_estimator_, OnReceivedPacket(_)) - .WillOnce(Invoke([&first_send_timestamp](const PacketResult& packet) { + EXPECT_CALL(network_state_estimator_, OnReceivedPacket) + .WillOnce([&](const PacketResult& packet) { EXPECT_EQ(packet.receive_time, kBaseTime); first_send_timestamp = packet.sent_packet.send_time; - })); + }); IncomingPacket(kBaseSeq, kBaseTime, kFirstAbsSendTime); - EXPECT_CALL(network_state_estimator_, OnReceivedPacket(_)) - .WillOnce(Invoke([first_send_timestamp, - kExpectedAbsSendTimeDelta](const PacketResult& packet) { + EXPECT_CALL(network_state_estimator_, OnReceivedPacket) + .WillOnce([&](const PacketResult& packet) { EXPECT_EQ(packet.receive_time, kBaseTime + TimeDelta::Millis(123)); EXPECT_EQ(packet.sent_packet.send_time.ms(), (first_send_timestamp + kExpectedAbsSendTimeDelta).ms()); - })); + }); IncomingPacket(kBaseSeq + 1, kBaseTime + TimeDelta::Millis(123), kSecondAbsSendTime); } +TEST_F(RemoteEstimatorProxyTest, IncomingPacketHandlesReorderedPackets) { + const uint32_t kFirstAbsSendTime = + AbsoluteSendTime::To24Bits(Timestamp::Millis((1 << 12))); + Timestamp first_send_timestamp = Timestamp::Zero(); + EXPECT_CALL(network_state_estimator_, OnReceivedPacket) + .WillOnce([&](const PacketResult& packet) { + EXPECT_EQ(packet.receive_time, kBaseTime); + first_send_timestamp = packet.sent_packet.send_time; + }); + IncomingPacket(kBaseSeq + 1, kBaseTime, kFirstAbsSendTime); + + const TimeDelta kExpectedAbsSendTimeDelta = -TimeDelta::Millis(30); + const uint32_t kSecondAbsSendTime = AbsoluteSendTime::To24Bits( + Timestamp::Millis(1 << 12) + kExpectedAbsSendTimeDelta); + EXPECT_CALL(network_state_estimator_, OnReceivedPacket) + .WillOnce([&](const PacketResult& packet) { + EXPECT_EQ(packet.sent_packet.send_time.ms(), + (first_send_timestamp + kExpectedAbsSendTimeDelta).ms()); + }); + IncomingPacket(kBaseSeq, kBaseTime + TimeDelta::Millis(123), + kSecondAbsSendTime); +} + +TEST_F(RemoteEstimatorProxyTest, + IncomingPacketResetSendTimeToArrivalTimeAfterLargeArrivaltimeDelta) { + const uint32_t kFirstAbsSendTime = + AbsoluteSendTime::To24Bits(Timestamp::Millis((1 << 12))); + EXPECT_CALL(network_state_estimator_, OnReceivedPacket) + .WillOnce([&](const PacketResult& packet) { + EXPECT_EQ(packet.receive_time, kBaseTime); + EXPECT_EQ(packet.sent_packet.send_time, kBaseTime); + }); + IncomingPacket(kBaseSeq + 1, kBaseTime, kFirstAbsSendTime); + + EXPECT_CALL(network_state_estimator_, OnReceivedPacket) + .WillOnce([&](const PacketResult& packet) { + EXPECT_EQ(packet.receive_time, kBaseTime + TimeDelta::Seconds(20)); + EXPECT_EQ(packet.sent_packet.send_time, + kBaseTime + TimeDelta::Seconds(20)); + }); + IncomingPacket(kBaseSeq, kBaseTime + TimeDelta::Seconds(20), + kFirstAbsSendTime + 123); +} + TEST_F(RemoteEstimatorProxyTest, SendTransportFeedbackAndNetworkStateUpdate) { IncomingPacket(kBaseSeq, kBaseTime, AbsoluteSendTime::To24Bits(kBaseTime - TimeDelta::Millis(1)));