Set correct absolute send time on reordered packets
Allow absolute send time to go back in time as long as there has not been a large gap in arival time. Use the first packets arival time as time base.
Bug: b/282153758, webrtc:15230
Change-Id: I8663079ab9c202079bf8db303353918d46ba1d98
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/308142
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40251}
diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
index c417f35f..e100995 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 @@
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 @@
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 @@
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 561410b..b50d2f0 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.h
@@ -105,6 +105,7 @@
// 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 c148ce6..47ad457 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(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 @@
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)));
