Add method ReceiveSideCongestionController::SetTransportOverhead
The method can be used to ensure packets reported to NetworkStateEstimator include transport overhead.
Change-Id: I30f0271aac82633893660c61ea59e3b7c2cf9f31
Bug: webrtc:10742
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265405
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37179}
diff --git a/modules/congestion_controller/include/receive_side_congestion_controller.h b/modules/congestion_controller/include/receive_side_congestion_controller.h
index fdef7f9..1dcb468 100644
--- a/modules/congestion_controller/include/receive_side_congestion_controller.h
+++ b/modules/congestion_controller/include/receive_side_congestion_controller.h
@@ -63,6 +63,8 @@
// `bitrate` using RTCP REMB.
void SetMaxDesiredReceiveBitrate(DataRate bitrate);
+ void SetTransportOverhead(DataSize overhead_per_packet);
+
// Implements Module.
int64_t TimeUntilNextProcess() override;
void Process() override;
diff --git a/modules/congestion_controller/receive_side_congestion_controller.cc b/modules/congestion_controller/receive_side_congestion_controller.cc
index 61a126f..57be0d2 100644
--- a/modules/congestion_controller/receive_side_congestion_controller.cc
+++ b/modules/congestion_controller/receive_side_congestion_controller.cc
@@ -189,4 +189,9 @@
remb_throttler_.SetMaxDesiredReceiveBitrate(bitrate);
}
+void ReceiveSideCongestionController::SetTransportOverhead(
+ DataSize overhead_per_packet) {
+ remote_estimator_proxy_.SetTransportOverhead(overhead_per_packet);
+}
+
} // namespace webrtc
diff --git a/modules/remote_bitrate_estimator/BUILD.gn b/modules/remote_bitrate_estimator/BUILD.gn
index 3583e81..1cc69ae 100644
--- a/modules/remote_bitrate_estimator/BUILD.gn
+++ b/modules/remote_bitrate_estimator/BUILD.gn
@@ -127,6 +127,7 @@
"../../api/transport:mock_network_control",
"../../api/transport:network_control",
"../../api/units:data_rate",
+ "../../api/units:data_size",
"../../rtc_base",
"../../rtc_base:checks",
"../../rtc_base:random",
diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
index fd5f629..a570d5e 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc
@@ -15,6 +15,7 @@
#include <memory>
#include <utility>
+#include "api/units/data_size.h"
#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
@@ -40,6 +41,7 @@
network_state_estimator_(network_state_estimator),
media_ssrc_(0),
feedback_packet_count_(0),
+ packet_overhead_(DataSize::Zero()),
send_interval_ms_(send_config_.default_interval->ms()),
send_periodic_feedback_(true),
previous_abs_send_time_(0),
@@ -116,9 +118,8 @@
TimeDelta::Millis(0));
previous_abs_send_time_ = header.extension.absoluteSendTime;
packet_result.sent_packet.send_time = abs_send_timestamp_;
- // TODO(webrtc:10742): Take IP header and transport overhead into account.
packet_result.sent_packet.size =
- DataSize::Bytes(header.headerLength + payload_size);
+ DataSize::Bytes(header.headerLength + payload_size) + packet_overhead_;
packet_result.sent_packet.sequence_number = seq;
network_state_estimator_->OnReceivedPacket(packet_result);
}
@@ -178,6 +179,11 @@
send_periodic_feedback_ = send_periodic_feedback;
}
+void RemoteEstimatorProxy::SetTransportOverhead(DataSize overhead_per_packet) {
+ MutexLock lock(&lock_);
+ packet_overhead_ = overhead_per_packet;
+}
+
void RemoteEstimatorProxy::SendPeriodicFeedbacks() {
// `periodic_window_start_seq_` is the first sequence number to include in
// the current feedback packet. Some older may still be in the map, in case
diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/modules/remote_bitrate_estimator/remote_estimator_proxy.h
index 438aa94..b4ec396 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.h
@@ -18,6 +18,7 @@
#include "api/field_trials_view.h"
#include "api/transport/network_control.h"
+#include "api/units/data_size.h"
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "modules/remote_bitrate_estimator/packet_arrival_map.h"
#include "rtc_base/experiments/field_trial_parser.h"
@@ -58,6 +59,7 @@
void Process() override;
void OnBitrateChanged(int bitrate);
void SetSendPeriodicFeedback(bool send_periodic_feedback);
+ void SetTransportOverhead(DataSize overhead_per_packet);
private:
struct TransportWideFeedbackConfig {
@@ -113,6 +115,7 @@
uint32_t media_ssrc_ RTC_GUARDED_BY(&lock_);
uint8_t feedback_packet_count_ RTC_GUARDED_BY(&lock_);
SeqNumUnwrapper<uint16_t> unwrapper_ RTC_GUARDED_BY(&lock_);
+ DataSize packet_overhead_ RTC_GUARDED_BY(&lock_);
// The next sequence number that should be the start sequence number during
// periodic reporting. Will be absl::nullopt before the first seen packet.
diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
index 58f5816..7ebfc8d 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc
@@ -16,6 +16,7 @@
#include "api/transport/field_trial_based_config.h"
#include "api/transport/network_types.h"
#include "api/transport/test/mock_network_control.h"
+#include "api/units/data_size.h"
#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "system_wrappers/include/clock.h"
@@ -575,16 +576,22 @@
TEST_F(RemoteEstimatorProxyTest, ReportsIncomingPacketToNetworkStateEstimator) {
Timestamp first_send_timestamp = Timestamp::Millis(0);
+ const DataSize kPacketOverhead = DataSize::Bytes(38);
+ webrtc::RTPHeader first_header = CreateHeader(
+ absl::nullopt, absl::nullopt, AbsoluteSendTime::MsTo24Bits(kBaseTimeMs));
+ proxy_.SetTransportOverhead(kPacketOverhead);
+
EXPECT_CALL(network_state_estimator_, OnReceivedPacket(_))
- .WillOnce(Invoke([&first_send_timestamp](const PacketResult& packet) {
+ .WillOnce(Invoke([&](const PacketResult& packet) {
EXPECT_EQ(packet.receive_time, Timestamp::Millis(kBaseTimeMs));
+ EXPECT_EQ(
+ packet.sent_packet.size,
+ DataSize::Bytes(kDefaultPacketSize + first_header.headerLength) +
+ kPacketOverhead);
first_send_timestamp = packet.sent_packet.send_time;
}));
// Incoming packet with abs sendtime but without transport sequence number.
- proxy_.IncomingPacket(
- kBaseTimeMs, kDefaultPacketSize,
- CreateHeader(absl::nullopt, absl::nullopt,
- AbsoluteSendTime::MsTo24Bits(kBaseTimeMs)));
+ proxy_.IncomingPacket(kBaseTimeMs, kDefaultPacketSize, first_header);
// Expect packet with older abs send time to be treated as sent at the same
// time as the previous packet due to reordering.
@@ -593,6 +600,7 @@
EXPECT_EQ(packet.receive_time, Timestamp::Millis(kBaseTimeMs));
EXPECT_EQ(packet.sent_packet.send_time, first_send_timestamp);
}));
+
proxy_.IncomingPacket(
kBaseTimeMs, kDefaultPacketSize,
CreateHeader(absl::nullopt, absl::nullopt,
