From f6934c3f7d04eabeeacc5013efa7c87a4e9380ff Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Thu, 9 Jun 2022 12:45:36 +0200 Subject: [PATCH] Add method ReceiveSideCongestionController::SetTransportOverhead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Björn Terelius Cr-Commit-Position: refs/heads/main@{#37179} --- .../receive_side_congestion_controller.h | 2 ++ .../receive_side_congestion_controller.cc | 5 +++++ modules/remote_bitrate_estimator/BUILD.gn | 1 + .../remote_estimator_proxy.cc | 10 ++++++++-- .../remote_estimator_proxy.h | 3 +++ .../remote_estimator_proxy_unittest.cc | 18 +++++++++++++----- 6 files changed, 32 insertions(+), 7 deletions(-) diff --git a/modules/congestion_controller/include/receive_side_congestion_controller.h b/modules/congestion_controller/include/receive_side_congestion_controller.h index fdef7f95c8..1dcb468569 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 @@ class ReceiveSideCongestionController : public CallStatsObserver, // `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 61a126fbe3..57be0d22fd 100644 --- a/modules/congestion_controller/receive_side_congestion_controller.cc +++ b/modules/congestion_controller/receive_side_congestion_controller.cc @@ -189,4 +189,9 @@ void ReceiveSideCongestionController::SetMaxDesiredReceiveBitrate( 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 3583e81c66..1cc69ae05b 100644 --- a/modules/remote_bitrate_estimator/BUILD.gn +++ b/modules/remote_bitrate_estimator/BUILD.gn @@ -127,6 +127,7 @@ if (rtc_include_tests) { "../../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 fd5f629235..a570d5ebca 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -15,6 +15,7 @@ #include #include +#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 @@ RemoteEstimatorProxy::RemoteEstimatorProxy( 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 @@ void RemoteEstimatorProxy::IncomingPacket(int64_t arrival_time_ms, 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 @@ void RemoteEstimatorProxy::SetSendPeriodicFeedback( 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 438aa94dc5..b4ec39687d 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 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { 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 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { uint32_t media_ssrc_ RTC_GUARDED_BY(&lock_); uint8_t feedback_packet_count_ RTC_GUARDED_BY(&lock_); SeqNumUnwrapper 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 58f58168d1..7ebfc8d3e4 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(RemoteEstimatorProxyOnRequestTest, 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 @@ TEST_F(RemoteEstimatorProxyTest, ReportsIncomingPacketToNetworkStateEstimator) { 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,