From 4f4e989436a2fef419e269bc218264b8e88003df Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 26 Jul 2023 15:51:49 +0200 Subject: [PATCH] In remote bitrate estimator pass packet using RtpPacketReceived class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:15054 Change-Id: I23c9008e1979a56bba9421a00e4f0f8ff937d122 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/313261 Reviewed-by: Erik SprÃ¥ng Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#40485} --- .../receive_side_congestion_controller.cc | 5 +--- .../include/remote_bitrate_estimator.h | 14 +++-------- .../remote_bitrate_estimator_abs_send_time.cc | 25 ++++++++----------- .../remote_bitrate_estimator_abs_send_time.h | 9 +------ .../remote_bitrate_estimator_single_stream.cc | 21 +++++++++------- .../remote_bitrate_estimator_single_stream.h | 4 +-- ...emote_bitrate_estimator_unittest_helper.cc | 23 ++++++++++------- 7 files changed, 43 insertions(+), 58 deletions(-) diff --git a/modules/congestion_controller/receive_side_congestion_controller.cc b/modules/congestion_controller/receive_side_congestion_controller.cc index d4c745b546..e042678897 100644 --- a/modules/congestion_controller/receive_side_congestion_controller.cc +++ b/modules/congestion_controller/receive_side_congestion_controller.cc @@ -98,11 +98,8 @@ void ReceiveSideCongestionController::OnReceivedPacket( } else { // Receive-side BWE. MutexLock lock(&mutex_); - RTPHeader header; - packet.GetHeader(&header); PickEstimator(packet.HasExtension()); - rbe_->IncomingPacket(packet.arrival_time().ms(), - packet.payload_size() + packet.padding_size(), header); + rbe_->IncomingPacket(packet); } } diff --git a/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h b/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h index 0d4e15e9e1..93a0554059 100644 --- a/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h +++ b/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h @@ -13,15 +13,13 @@ #ifndef MODULES_REMOTE_BITRATE_ESTIMATOR_INCLUDE_REMOTE_BITRATE_ESTIMATOR_H_ #define MODULES_REMOTE_BITRATE_ESTIMATOR_INCLUDE_REMOTE_BITRATE_ESTIMATOR_H_ -#include -#include +#include #include #include "api/units/data_rate.h" #include "api/units/time_delta.h" #include "modules/include/module_common_types.h" -#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" -#include "modules/rtp_rtcp/source/rtcp_packet.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" namespace webrtc { @@ -45,12 +43,8 @@ class RemoteBitrateEstimator : public CallStatsObserver { // Called for each incoming packet. Updates the incoming payload bitrate // estimate and the over-use detector. If an over-use is detected the - // remote bitrate estimate will be updated. Note that `payload_size` is the - // packet size excluding headers. - // Note that `arrival_time_ms` can be of an arbitrary time base. - virtual void IncomingPacket(int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header) = 0; + // remote bitrate estimate will be updated. + virtual void IncomingPacket(const RtpPacketReceived& rtp_packet) = 0; // Removes all data for `ssrc`. virtual void RemoveStream(uint32_t ssrc) = 0; diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc index 135f5f3424..b4df1d7a77 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc @@ -23,6 +23,8 @@ #include "api/units/timestamp.h" #include "modules/remote_bitrate_estimator/include/bwe_defines.h" #include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" +#include "modules/rtp_rtcp/source/rtp_header_extensions.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "system_wrappers/include/metrics.h" @@ -208,26 +210,19 @@ bool RemoteBitrateEstimatorAbsSendTime::IsBitrateImproving( } void RemoteBitrateEstimatorAbsSendTime::IncomingPacket( - int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header) { - if (!header.extension.hasAbsoluteSendTime) { + const RtpPacketReceived& rtp_packet) { + uint32_t send_time_24bits; + if (!rtp_packet.GetExtension(&send_time_24bits)) { RTC_LOG(LS_WARNING) << "RemoteBitrateEstimatorAbsSendTimeImpl: Incoming packet " "is missing absolute send time extension!"; return; } - IncomingPacketInfo(Timestamp::Millis(arrival_time_ms), - header.extension.absoluteSendTime, - DataSize::Bytes(payload_size), header.ssrc); -} -void RemoteBitrateEstimatorAbsSendTime::IncomingPacketInfo( - Timestamp arrival_time, - uint32_t send_time_24bits, - DataSize payload_size, - uint32_t ssrc) { - RTC_CHECK(send_time_24bits < (1ul << 24)); + Timestamp arrival_time = rtp_packet.arrival_time(); + DataSize payload_size = + DataSize::Bytes(rtp_packet.payload_size() + rtp_packet.padding_size()); + if (!uma_recorded_) { RTC_HISTOGRAM_ENUMERATION(kBweTypeHistogram, BweNames::kReceiverAbsSendTime, BweNames::kBweNamesMax); @@ -270,7 +265,7 @@ void RemoteBitrateEstimatorAbsSendTime::IncomingPacketInfo( TimeoutStreams(now); RTC_DCHECK(inter_arrival_); RTC_DCHECK(estimator_); - ssrcs_.insert_or_assign(ssrc, now); + ssrcs_.insert_or_assign(rtp_packet.Ssrc(), now); // For now only try to detect probes while we don't have a valid estimate. // We currently assume that only packets larger than 200 bytes are paced by diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h index 7db2aea67d..5924f2be81 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h @@ -49,9 +49,7 @@ class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator { ~RemoteBitrateEstimatorAbsSendTime() override; - void IncomingPacket(int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header) override; + void IncomingPacket(const RtpPacketReceived& rtp_packet) override; TimeDelta Process() override; void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; void RemoveStream(uint32_t ssrc) override; @@ -89,11 +87,6 @@ class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator { static void MaybeAddCluster(const Cluster& cluster_aggregate, std::list& clusters); - void IncomingPacketInfo(Timestamp arrival_time, - uint32_t send_time_24bits, - DataSize payload_size, - uint32_t ssrc); - std::list ComputeClusters() const; const Cluster* FindBestProbe(const std::list& clusters) const; diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc index 20326286fa..077315fb3e 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc @@ -19,6 +19,8 @@ #include "modules/remote_bitrate_estimator/inter_arrival.h" #include "modules/remote_bitrate_estimator/overuse_detector.h" #include "modules/remote_bitrate_estimator/overuse_estimator.h" +#include "modules/rtp_rtcp/source/rtp_header_extensions.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "system_wrappers/include/clock.h" @@ -73,19 +75,19 @@ RemoteBitrateEstimatorSingleStream::~RemoteBitrateEstimatorSingleStream() { } void RemoteBitrateEstimatorSingleStream::IncomingPacket( - int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header) { + const RtpPacketReceived& rtp_packet) { + absl::optional transmission_time_offset = + rtp_packet.GetExtension(); if (!uma_recorded_) { - BweNames type = BweNames::kReceiverTOffset; - if (!header.extension.hasTransmissionTimeOffset) - type = BweNames::kReceiverNoExtension; + BweNames type = transmission_time_offset.has_value() + ? BweNames::kReceiverTOffset + : BweNames::kReceiverNoExtension; RTC_HISTOGRAM_ENUMERATION(kBweTypeHistogram, type, BweNames::kBweNamesMax); uma_recorded_ = true; } - uint32_t ssrc = header.ssrc; + uint32_t ssrc = rtp_packet.Ssrc(); uint32_t rtp_timestamp = - header.timestamp + header.extension.transmissionTimeOffset; + rtp_packet.Timestamp() + transmission_time_offset.value_or(0); int64_t now_ms = clock_->TimeInMilliseconds(); SsrcOveruseEstimatorMap::iterator it = overuse_detectors_.find(ssrc); if (it == overuse_detectors_.end()) { @@ -113,6 +115,7 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket( incoming_bitrate_.Reset(); last_valid_incoming_bitrate_ = 0; } + size_t payload_size = rtp_packet.payload_size() + rtp_packet.padding_size(); incoming_bitrate_.Update(payload_size, now_ms); const BandwidthUsage prior_state = estimator->detector.State(); @@ -120,7 +123,7 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket( int64_t time_delta = 0; int size_delta = 0; if (estimator->inter_arrival.ComputeDeltas( - rtp_timestamp, arrival_time_ms, now_ms, payload_size, + rtp_timestamp, rtp_packet.arrival_time().ms(), now_ms, payload_size, ×tamp_delta, &time_delta, &size_delta)) { double timestamp_delta_ms = timestamp_delta * kTimestampToMs; estimator->estimator.Update(time_delta, timestamp_delta_ms, size_delta, diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h index 5e2961d901..05ec03d5a1 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h @@ -43,9 +43,7 @@ class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator { ~RemoteBitrateEstimatorSingleStream() override; - void IncomingPacket(int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header) override; + void IncomingPacket(const RtpPacketReceived& rtp_packet) override; TimeDelta Process() override; void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; void RemoveStream(uint32_t ssrc) override; diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc index 899037f5a7..ee9644530a 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc @@ -13,6 +13,9 @@ #include #include +#include "modules/rtp_rtcp/include/rtp_header_extension_map.h" +#include "modules/rtp_rtcp/source/rtp_header_extensions.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/checks.h" namespace webrtc { @@ -229,15 +232,17 @@ void RemoteBitrateEstimatorTest::IncomingPacket(uint32_t ssrc, int64_t arrival_time, uint32_t rtp_timestamp, uint32_t absolute_send_time) { - RTPHeader header; - memset(&header, 0, sizeof(header)); - header.ssrc = ssrc; - header.timestamp = rtp_timestamp; - header.extension.hasAbsoluteSendTime = true; - header.extension.absoluteSendTime = absolute_send_time; - RTC_CHECK_GE(arrival_time + arrival_time_offset_ms_, 0); - bitrate_estimator_->IncomingPacket(arrival_time + arrival_time_offset_ms_, - payload_size, header); + RtpHeaderExtensionMap extensions; + extensions.Register(1); + RtpPacketReceived rtp_packet(&extensions); + rtp_packet.SetSsrc(ssrc); + rtp_packet.SetTimestamp(rtp_timestamp); + rtp_packet.SetExtension(absolute_send_time); + rtp_packet.SetPayloadSize(payload_size); + rtp_packet.set_arrival_time( + Timestamp::Millis(arrival_time + arrival_time_offset_ms_)); + + bitrate_estimator_->IncomingPacket(rtp_packet); } // Generates a frame of packets belonging to a stream at a given bitrate and