From c78f25b7f075d81a2fa17d2b95174be66d51e7cb Mon Sep 17 00:00:00 2001 From: Per K Date: Wed, 22 May 2024 13:13:48 +0000 Subject: [PATCH] Rename RemoteEstimatorProxy to TransportSequenceNumberFeedbackGenenerator This is done to better reflect the responsibility of the class. The implementation implement a new interface FeedbackGeneratorInterface. The purpose of the interface is to allow a new implementation that supports RFC 8888. Bug: webrtc:42225697 Change-Id: Id087dd7422abbcd6016693c076a65f4c4efd5712 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/351280 Reviewed-by: Danil Chapovalov Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#42366} --- .../receive_side_congestion_controller.h | 9 +- .../receive_side_congestion_controller.cc | 17 +-- modules/remote_bitrate_estimator/BUILD.gn | 7 +- .../rtp_transport_feedback_generator.h | 40 +++++++ ...ort_sequence_number_feedback_generator.cc} | 36 +++--- ...port_sequence_number_feedback_generator.h} | 50 ++++---- ...nce_number_feedback_generator_unittest.cc} | 111 +++++++++++------- 7 files changed, 177 insertions(+), 93 deletions(-) create mode 100644 modules/remote_bitrate_estimator/rtp_transport_feedback_generator.h rename modules/remote_bitrate_estimator/{remote_estimator_proxy.cc => transport_sequence_number_feedback_generator.cc} (91%) rename modules/remote_bitrate_estimator/{remote_estimator_proxy.h => transport_sequence_number_feedback_generator.h} (69%) rename modules/remote_bitrate_estimator/{remote_estimator_proxy_unittest.cc => transport_sequence_number_feedback_generator_unittest.cc} (87%) diff --git a/modules/congestion_controller/include/receive_side_congestion_controller.h b/modules/congestion_controller/include/receive_side_congestion_controller.h index 9cbad1187a..deb3a3ebba 100644 --- a/modules/congestion_controller/include/receive_side_congestion_controller.h +++ b/modules/congestion_controller/include/receive_side_congestion_controller.h @@ -12,7 +12,6 @@ #define MODULES_CONGESTION_CONTROLLER_INCLUDE_RECEIVE_SIDE_CONGESTION_CONTROLLER_H_ #include -#include #include "absl/base/nullability.h" #include "api/environment/environment.h" @@ -20,8 +19,7 @@ #include "api/units/data_rate.h" #include "api/units/time_delta.h" #include "modules/congestion_controller/remb_throttler.h" -#include "modules/pacing/packet_router.h" -#include "modules/remote_bitrate_estimator/remote_estimator_proxy.h" +#include "modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/synchronization/mutex.h" #include "rtc_base/thread_annotations.h" @@ -38,7 +36,7 @@ class ReceiveSideCongestionController : public CallStatsObserver { public: ReceiveSideCongestionController( const Environment& env, - RemoteEstimatorProxy::TransportFeedbackSender feedback_sender, + TransportSequenceNumberFeedbackGenenerator::RtcpSender feedback_sender, RembThrottler::RembSender remb_sender, absl::Nullable network_state_estimator); @@ -76,7 +74,8 @@ class ReceiveSideCongestionController : public CallStatsObserver { const Environment env_; RembThrottler remb_throttler_; - RemoteEstimatorProxy remote_estimator_proxy_; + TransportSequenceNumberFeedbackGenenerator + transport_sequence_number_feedback_generator_; mutable Mutex mutex_; std::unique_ptr rbe_ RTC_GUARDED_BY(mutex_); diff --git a/modules/congestion_controller/receive_side_congestion_controller.cc b/modules/congestion_controller/receive_side_congestion_controller.cc index 317858ccdb..c86b277c52 100644 --- a/modules/congestion_controller/receive_side_congestion_controller.cc +++ b/modules/congestion_controller/receive_side_congestion_controller.cc @@ -72,13 +72,13 @@ void ReceiveSideCongestionController::PickEstimator( ReceiveSideCongestionController::ReceiveSideCongestionController( const Environment& env, - RemoteEstimatorProxy::TransportFeedbackSender feedback_sender, + TransportSequenceNumberFeedbackGenenerator::RtcpSender feedback_sender, RembThrottler::RembSender remb_sender, absl::Nullable network_state_estimator) : env_(env), remb_throttler_(std::move(remb_sender), &env_.clock()), - remote_estimator_proxy_(std::move(feedback_sender), - network_state_estimator), + transport_sequence_number_feedback_generator_(std::move(feedback_sender), + network_state_estimator), rbe_(std::make_unique( env_, &remb_throttler_)), @@ -98,7 +98,7 @@ void ReceiveSideCongestionController::OnReceivedPacket( if (has_transport_sequence_number) { // Send-side BWE. - remote_estimator_proxy_.IncomingPacket(packet); + transport_sequence_number_feedback_generator_.OnReceivedPacket(packet); } else { // Receive-side BWE. MutexLock lock(&mutex_); @@ -108,7 +108,8 @@ void ReceiveSideCongestionController::OnReceivedPacket( } void ReceiveSideCongestionController::OnBitrateChanged(int bitrate_bps) { - remote_estimator_proxy_.OnBitrateChanged(bitrate_bps); + transport_sequence_number_feedback_generator_.OnSendBandwidthEstimateChanged( + DataRate::BitsPerSec(bitrate_bps)); } TimeDelta ReceiveSideCongestionController::MaybeProcess() { @@ -116,7 +117,8 @@ TimeDelta ReceiveSideCongestionController::MaybeProcess() { mutex_.Lock(); TimeDelta time_until_rbe = rbe_->Process(); mutex_.Unlock(); - TimeDelta time_until_rep = remote_estimator_proxy_.Process(now); + TimeDelta time_until_rep = + transport_sequence_number_feedback_generator_.Process(now); TimeDelta time_until = std::min(time_until_rbe, time_until_rep); return std::max(time_until, TimeDelta::Zero()); } @@ -128,7 +130,8 @@ void ReceiveSideCongestionController::SetMaxDesiredReceiveBitrate( void ReceiveSideCongestionController::SetTransportOverhead( DataSize overhead_per_packet) { - remote_estimator_proxy_.SetTransportOverhead(overhead_per_packet); + transport_sequence_number_feedback_generator_.SetTransportOverhead( + overhead_per_packet); } } // namespace webrtc diff --git a/modules/remote_bitrate_estimator/BUILD.gn b/modules/remote_bitrate_estimator/BUILD.gn index d691ad7759..4d0728020d 100644 --- a/modules/remote_bitrate_estimator/BUILD.gn +++ b/modules/remote_bitrate_estimator/BUILD.gn @@ -27,9 +27,10 @@ rtc_library("remote_bitrate_estimator") { "remote_bitrate_estimator_abs_send_time.h", "remote_bitrate_estimator_single_stream.cc", "remote_bitrate_estimator_single_stream.h", - "remote_estimator_proxy.cc", - "remote_estimator_proxy.h", + "rtp_transport_feedback_generator.h", "test/bwe_test_logging.h", + "transport_sequence_number_feedback_generator.cc", + "transport_sequence_number_feedback_generator.h", ] deps = [ @@ -115,7 +116,7 @@ if (rtc_include_tests) { "remote_bitrate_estimator_single_stream_unittest.cc", "remote_bitrate_estimator_unittest_helper.cc", "remote_bitrate_estimator_unittest_helper.h", - "remote_estimator_proxy_unittest.cc", + "transport_sequence_number_feedback_generator_unittest.cc", ] deps = [ ":remote_bitrate_estimator", diff --git a/modules/remote_bitrate_estimator/rtp_transport_feedback_generator.h b/modules/remote_bitrate_estimator/rtp_transport_feedback_generator.h new file mode 100644 index 0000000000..724b589a2a --- /dev/null +++ b/modules/remote_bitrate_estimator/rtp_transport_feedback_generator.h @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2024 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef MODULES_REMOTE_BITRATE_ESTIMATOR_RTP_TRANSPORT_FEEDBACK_GENERATOR_H_ +#define MODULES_REMOTE_BITRATE_ESTIMATOR_RTP_TRANSPORT_FEEDBACK_GENERATOR_H_ + +#include "api/units/data_rate.h" +#include "api/units/data_size.h" +#include "api/units/time_delta.h" +#include "api/units/timestamp.h" +#include "modules/rtp_rtcp/source/rtp_packet_received.h" + +namespace webrtc { + +class RtpTransportFeedbackGenerator { + public: + virtual ~RtpTransportFeedbackGenerator() = default; + + virtual void OnReceivedPacket(const RtpPacketReceived& packet) = 0; + + // Sends periodic feedback if it is time to send it. + // Returns time until next call to Process should be made. + virtual TimeDelta Process(Timestamp now) = 0; + + virtual void OnSendBandwidthEstimateChanged(DataRate estimate) = 0; + + // Overhead from transport layers below RTP. Ie, IP, SRTP. + virtual void SetTransportOverhead(DataSize overhead_per_packet) = 0; +}; + +} // namespace webrtc + +#endif // MODULES_REMOTE_BITRATE_ESTIMATOR_RTP_TRANSPORT_FEEDBACK_GENERATOR_H_ diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.cc similarity index 91% rename from modules/remote_bitrate_estimator/remote_estimator_proxy.cc rename to modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.cc index 6953ec8400..b764013285 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.cc @@ -8,7 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "modules/remote_bitrate_estimator/remote_estimator_proxy.h" +#include "modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.h" #include #include @@ -51,9 +51,10 @@ TimeDelta GetAbsoluteSendTimeDelta(uint32_t new_sendtime, } } // namespace -RemoteEstimatorProxy::RemoteEstimatorProxy( - TransportFeedbackSender feedback_sender, - NetworkStateEstimator* network_state_estimator) +TransportSequenceNumberFeedbackGenenerator:: + TransportSequenceNumberFeedbackGenenerator( + RtcpSender feedback_sender, + NetworkStateEstimator* network_state_estimator) : feedback_sender_(std::move(feedback_sender)), last_process_time_(Timestamp::MinusInfinity()), network_state_estimator_(network_state_estimator), @@ -70,10 +71,12 @@ RemoteEstimatorProxy::RemoteEstimatorProxy( << kMaxInterval; } -RemoteEstimatorProxy::~RemoteEstimatorProxy() {} +TransportSequenceNumberFeedbackGenenerator:: + ~TransportSequenceNumberFeedbackGenenerator() {} -void RemoteEstimatorProxy::MaybeCullOldPackets(int64_t sequence_number, - Timestamp arrival_time) { +void TransportSequenceNumberFeedbackGenenerator::MaybeCullOldPackets( + int64_t sequence_number, + Timestamp arrival_time) { if (periodic_window_start_seq_ >= packet_arrival_times_.end_sequence_number() && arrival_time - Timestamp::Zero() >= kBackWindow) { @@ -83,7 +86,8 @@ void RemoteEstimatorProxy::MaybeCullOldPackets(int64_t sequence_number, } } -void RemoteEstimatorProxy::IncomingPacket(const RtpPacketReceived& packet) { +void TransportSequenceNumberFeedbackGenenerator::OnReceivedPacket( + const RtpPacketReceived& packet) { if (packet.arrival_time().IsInfinite()) { RTC_LOG(LS_WARNING) << "Arrival time not set."; return; @@ -157,7 +161,7 @@ void RemoteEstimatorProxy::IncomingPacket(const RtpPacketReceived& packet) { } } -TimeDelta RemoteEstimatorProxy::Process(Timestamp now) { +TimeDelta TransportSequenceNumberFeedbackGenenerator::Process(Timestamp now) { MutexLock lock(&lock_); if (!send_periodic_feedback_) { // If TransportSequenceNumberV2 has been received in one packet, @@ -174,7 +178,8 @@ TimeDelta RemoteEstimatorProxy::Process(Timestamp now) { return next_process_time - now; } -void RemoteEstimatorProxy::OnBitrateChanged(int bitrate_bps) { +void TransportSequenceNumberFeedbackGenenerator::OnSendBandwidthEstimateChanged( + DataRate estimate) { // TwccReportSize = Ipv4(20B) + UDP(8B) + SRTP(10B) + // AverageTwccReport(30B) // TwccReport size at 50ms interval is 24 byte. @@ -184,7 +189,7 @@ void RemoteEstimatorProxy::OnBitrateChanged(int bitrate_bps) { constexpr DataRate kMinTwccRate = kTwccReportSize / kMaxInterval; // Let TWCC reports occupy 5% of total bandwidth. - DataRate twcc_bitrate = DataRate::BitsPerSec(0.05 * bitrate_bps); + DataRate twcc_bitrate = 0.05 * estimate; // Check upper send_interval bound by checking bitrate to avoid overflow when // dividing by small bitrate, in particular avoid dividing by zero bitrate. @@ -197,12 +202,13 @@ void RemoteEstimatorProxy::OnBitrateChanged(int bitrate_bps) { send_interval_ = send_interval; } -void RemoteEstimatorProxy::SetTransportOverhead(DataSize overhead_per_packet) { +void TransportSequenceNumberFeedbackGenenerator::SetTransportOverhead( + DataSize overhead_per_packet) { MutexLock lock(&lock_); packet_overhead_ = overhead_per_packet; } -void RemoteEstimatorProxy::SendPeriodicFeedbacks() { +void TransportSequenceNumberFeedbackGenenerator::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 // a reordering happens and we need to retransmit them. @@ -246,7 +252,7 @@ void RemoteEstimatorProxy::SendPeriodicFeedbacks() { } } -void RemoteEstimatorProxy::SendFeedbackOnRequest( +void TransportSequenceNumberFeedbackGenenerator::SendFeedbackOnRequest( int64_t sequence_number, const FeedbackRequest& feedback_request) { if (feedback_request.sequence_count == 0) { @@ -276,7 +282,7 @@ void RemoteEstimatorProxy::SendFeedbackOnRequest( } std::unique_ptr -RemoteEstimatorProxy::MaybeBuildFeedbackPacket( +TransportSequenceNumberFeedbackGenenerator::MaybeBuildFeedbackPacket( bool include_timestamps, int64_t begin_sequence_number_inclusive, int64_t end_sequence_number_exclusive, diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.h similarity index 69% rename from modules/remote_bitrate_estimator/remote_estimator_proxy.h rename to modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.h index b50d2f0db0..2291031f35 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h +++ b/modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.h @@ -8,8 +8,8 @@ * be found in the AUTHORS file in the root of the source tree. */ -#ifndef MODULES_REMOTE_BITRATE_ESTIMATOR_REMOTE_ESTIMATOR_PROXY_H_ -#define MODULES_REMOTE_BITRATE_ESTIMATOR_REMOTE_ESTIMATOR_PROXY_H_ +#ifndef MODULES_REMOTE_BITRATE_ESTIMATOR_TRANSPORT_SEQUENCE_NUMBER_FEEDBACK_GENERATOR_H_ +#define MODULES_REMOTE_BITRATE_ESTIMATOR_TRANSPORT_SEQUENCE_NUMBER_FEEDBACK_GENERATOR_H_ #include #include @@ -20,10 +20,12 @@ #include "api/field_trials_view.h" #include "api/rtp_headers.h" #include "api/transport/network_control.h" +#include "api/units/data_rate.h" #include "api/units/data_size.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "modules/remote_bitrate_estimator/packet_arrival_map.h" +#include "modules/remote_bitrate_estimator/rtp_transport_feedback_generator.h" #include "modules/rtp_rtcp/source/rtcp_packet.h" #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" @@ -32,27 +34,31 @@ namespace webrtc { -// Class used when send-side BWE is enabled: This proxy is instantiated on the -// receive side. It buffers a number of receive timestamps and then sends -// transport feedback messages back too the send side. -class RemoteEstimatorProxy { +// Class used when send-side BWE is enabled. +// The class is responsible for generating RTCP feedback packets based on +// incoming media packets. Incoming packets must have a transport sequence +// number, Ie. either the extension +// http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01 or +// http://www.webrtc.org/experiments/rtp-hdrext/transport-wide-cc-02 must be +// used. +class TransportSequenceNumberFeedbackGenenerator + : public RtpTransportFeedbackGenerator { public: // Used for sending transport feedback messages when send side // BWE is used. - using TransportFeedbackSender = std::function> packets)>; - RemoteEstimatorProxy(TransportFeedbackSender feedback_sender, - NetworkStateEstimator* network_state_estimator); - ~RemoteEstimatorProxy(); + TransportSequenceNumberFeedbackGenenerator( + RtcpSender feedback_sender, + NetworkStateEstimator* network_state_estimator); + ~TransportSequenceNumberFeedbackGenenerator(); - void IncomingPacket(const RtpPacketReceived& packet); + void OnReceivedPacket(const RtpPacketReceived& packet) override; + void OnSendBandwidthEstimateChanged(DataRate estimate) override; - // Sends periodic feedback if it is time to send it. - // Returns time until next call to Process should be made. - TimeDelta Process(Timestamp now); + TimeDelta Process(Timestamp now) override; - void OnBitrateChanged(int bitrate); - void SetTransportOverhead(DataSize overhead_per_packet); + void SetTransportOverhead(DataSize overhead_per_packet) override; private: void MaybeCullOldPackets(int64_t sequence_number, Timestamp arrival_time) @@ -62,12 +68,12 @@ class RemoteEstimatorProxy { const FeedbackRequest& feedback_request) RTC_EXCLUSIVE_LOCKS_REQUIRED(&lock_); - // Returns a Transport Feedback packet with information about as many packets - // that has been received between [`begin_sequence_number_incl`, + // Returns a Transport Feedback packet with information about as many + // packets that has been received between [`begin_sequence_number_incl`, // `end_sequence_number_excl`) that can fit in it. If `is_periodic_update`, // this represents sending a periodic feedback message, which will make it - // update the `periodic_window_start_seq_` variable with the first packet that - // was not included in the feedback packet, so that the next update can + // update the `periodic_window_start_seq_` variable with the first packet + // that was not included in the feedback packet, so that the next update can // continue from that sequence number. // // If no incoming packets were added, nullptr is returned. @@ -80,7 +86,7 @@ class RemoteEstimatorProxy { int64_t end_sequence_number_exclusive, bool is_periodic_update) RTC_EXCLUSIVE_LOCKS_REQUIRED(&lock_); - const TransportFeedbackSender feedback_sender_; + const RtcpSender feedback_sender_; Timestamp last_process_time_; Mutex lock_; @@ -110,4 +116,4 @@ class RemoteEstimatorProxy { } // namespace webrtc -#endif // MODULES_REMOTE_BITRATE_ESTIMATOR_REMOTE_ESTIMATOR_PROXY_H_ +#endif // MODULES_REMOTE_BITRATE_ESTIMATOR_TRANSPORT_SEQUENCE_NUMBER_FEEDBACK_GENERATOR_H_ diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator_unittest.cc similarity index 87% rename from modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc rename to modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator_unittest.cc index 47ad457be9..bc1ce81edc 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc +++ b/modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator_unittest.cc @@ -8,15 +8,15 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "modules/remote_bitrate_estimator/remote_estimator_proxy.h" +#include "modules/remote_bitrate_estimator/transport_sequence_number_feedback_generator.h" #include #include #include #include "absl/types/optional.h" -#include "api/transport/network_types.h" #include "api/transport/test/mock_network_control.h" +#include "api/units/data_rate.h" #include "api/units/data_size.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" @@ -76,11 +76,12 @@ std::vector Timestamps( return timestamps; } -class RemoteEstimatorProxyTest : public ::testing::Test { +class TransportSequenceNumberFeedbackGeneneratorTest : public ::testing::Test { public: - RemoteEstimatorProxyTest() + TransportSequenceNumberFeedbackGeneneratorTest() : clock_(0), - proxy_(feedback_sender_.AsStdFunction(), &network_state_estimator_) {} + feedback_generator_(feedback_sender_.AsStdFunction(), + &network_state_estimator_) {} protected: void IncomingPacket(uint16_t seq, @@ -95,7 +96,7 @@ class RemoteEstimatorProxyTest : public ::testing::Test { if (abs_send_time) { packet.SetExtension(*abs_send_time); } - proxy_.IncomingPacket(packet); + feedback_generator_.OnReceivedPacket(packet); } void IncomingPacketV2( @@ -108,22 +109,23 @@ class RemoteEstimatorProxyTest : public ::testing::Test { packet.SetSsrc(kMediaSsrc); packet.SetExtension(seq, feedback_request); - proxy_.IncomingPacket(packet); + feedback_generator_.OnReceivedPacket(packet); } void Process() { clock_.AdvanceTime(kDefaultSendInterval); - proxy_.Process(clock_.CurrentTime()); + feedback_generator_.Process(clock_.CurrentTime()); } SimulatedClock clock_; MockFunction>)> feedback_sender_; ::testing::NiceMock network_state_estimator_; - RemoteEstimatorProxy proxy_; + TransportSequenceNumberFeedbackGenenerator feedback_generator_; }; -TEST_F(RemoteEstimatorProxyTest, SendsSinglePacketFeedback) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + SendsSinglePacketFeedback) { IncomingPacket(kBaseSeq, kBaseTime); EXPECT_CALL(feedback_sender_, Call) @@ -143,7 +145,7 @@ TEST_F(RemoteEstimatorProxyTest, SendsSinglePacketFeedback) { Process(); } -TEST_F(RemoteEstimatorProxyTest, DuplicatedPackets) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, DuplicatedPackets) { IncomingPacket(kBaseSeq, kBaseTime); IncomingPacket(kBaseSeq, kBaseTime + TimeDelta::Seconds(1)); @@ -165,7 +167,8 @@ TEST_F(RemoteEstimatorProxyTest, DuplicatedPackets) { Process(); } -TEST_F(RemoteEstimatorProxyTest, FeedbackWithMissingStart) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + FeedbackWithMissingStart) { // First feedback. IncomingPacket(kBaseSeq, kBaseTime); IncomingPacket(kBaseSeq + 1, kBaseTime + TimeDelta::Seconds(1)); @@ -193,7 +196,8 @@ TEST_F(RemoteEstimatorProxyTest, FeedbackWithMissingStart) { Process(); } -TEST_F(RemoteEstimatorProxyTest, SendsFeedbackWithVaryingDeltas) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + SendsFeedbackWithVaryingDeltas) { IncomingPacket(kBaseSeq, kBaseTime); IncomingPacket(kBaseSeq + 1, kBaseTime + kMaxSmallDelta); IncomingPacket(kBaseSeq + 2, @@ -219,7 +223,8 @@ TEST_F(RemoteEstimatorProxyTest, SendsFeedbackWithVaryingDeltas) { Process(); } -TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + SendsFragmentedFeedback) { static constexpr TimeDelta kTooLargeDelta = rtcp::TransportFeedback::kDeltaTick * (1 << 16); @@ -256,7 +261,8 @@ TEST_F(RemoteEstimatorProxyTest, SendsFragmentedFeedback) { Process(); } -TEST_F(RemoteEstimatorProxyTest, HandlesReorderingAndWrap) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + HandlesReorderingAndWrap) { const TimeDelta kDelta = TimeDelta::Seconds(1); const uint16_t kLargeSeq = 62762; IncomingPacket(kBaseSeq, kBaseTime); @@ -278,7 +284,8 @@ TEST_F(RemoteEstimatorProxyTest, HandlesReorderingAndWrap) { Process(); } -TEST_F(RemoteEstimatorProxyTest, HandlesMalformedSequenceNumbers) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + HandlesMalformedSequenceNumbers) { // This test generates incoming packets with large jumps in sequence numbers. // When unwrapped, the sequeunce numbers of these 30 incoming packets, will // span a range of roughly 650k packets. Test that we only send feedback for @@ -308,7 +315,8 @@ TEST_F(RemoteEstimatorProxyTest, HandlesMalformedSequenceNumbers) { Process(); } -TEST_F(RemoteEstimatorProxyTest, HandlesBackwardsWrappingSequenceNumbers) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + HandlesBackwardsWrappingSequenceNumbers) { // This test is like HandlesMalformedSequenceNumbers but for negative wrap // arounds. Test that we only send feedback for the packets with highest // sequence numbers. Test for regression found in chromium:949020. @@ -337,7 +345,8 @@ TEST_F(RemoteEstimatorProxyTest, HandlesBackwardsWrappingSequenceNumbers) { Process(); } -TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + ResendsTimestampsOnReordering) { IncomingPacket(kBaseSeq, kBaseTime); IncomingPacket(kBaseSeq + 2, kBaseTime + TimeDelta::Millis(2)); @@ -380,7 +389,8 @@ TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { Process(); } -TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + RemovesTimestampsOutOfScope) { const Timestamp kTimeoutTime = kBaseTime + kBackWindow; IncomingPacket(kBaseSeq + 2, kBaseTime); @@ -438,42 +448,57 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { Process(); } -TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsDefaultOnUnkownBitrate) { - EXPECT_EQ(proxy_.Process(clock_.CurrentTime()), kDefaultSendInterval); +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + TimeUntilNextProcessIsDefaultOnUnkownBitrate) { + EXPECT_EQ(feedback_generator_.Process(clock_.CurrentTime()), + kDefaultSendInterval); } -TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMinIntervalOn300kbps) { - proxy_.OnBitrateChanged(300'000); - EXPECT_EQ(proxy_.Process(clock_.CurrentTime()), kMinSendInterval); +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + TimeUntilNextProcessIsMinIntervalOn300kbps) { + feedback_generator_.OnSendBandwidthEstimateChanged( + DataRate::BitsPerSec(300'000)); + EXPECT_EQ(feedback_generator_.Process(clock_.CurrentTime()), + kMinSendInterval); } -TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn0kbps) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + TimeUntilNextProcessIsMaxIntervalOn0kbps) { // TimeUntilNextProcess should be limited by `kMaxSendIntervalMs` when // bitrate is small. We choose 0 bps as a special case, which also tests // erroneous behaviors like division-by-zero. - proxy_.OnBitrateChanged(0); - EXPECT_EQ(proxy_.Process(clock_.CurrentTime()), kMaxSendInterval); + feedback_generator_.OnSendBandwidthEstimateChanged(DataRate::Zero()); + EXPECT_EQ(feedback_generator_.Process(clock_.CurrentTime()), + kMaxSendInterval); } -TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn20kbps) { - proxy_.OnBitrateChanged(20'000); - EXPECT_EQ(proxy_.Process(clock_.CurrentTime()), kMaxSendInterval); +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + TimeUntilNextProcessIsMaxIntervalOn20kbps) { + feedback_generator_.OnSendBandwidthEstimateChanged( + DataRate::BitsPerSec(20'000)); + EXPECT_EQ(feedback_generator_.Process(clock_.CurrentTime()), + kMaxSendInterval); } -TEST_F(RemoteEstimatorProxyTest, TwccReportsUse5PercentOfAvailableBandwidth) { - proxy_.OnBitrateChanged(80'000); +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + TwccReportsUse5PercentOfAvailableBandwidth) { + feedback_generator_.OnSendBandwidthEstimateChanged( + DataRate::BitsPerSec(80'000)); // 80kbps * 0.05 = TwccReportSize(68B * 8b/B) * 1000ms / SendInterval(136ms) - EXPECT_EQ(proxy_.Process(clock_.CurrentTime()), TimeDelta::Millis(136)); + EXPECT_EQ(feedback_generator_.Process(clock_.CurrentTime()), + TimeDelta::Millis(136)); } ////////////////////////////////////////////////////////////////////////////// // Tests for the extended protocol where the feedback is explicitly requested // by the sender. ////////////////////////////////////////////////////////////////////////////// -typedef RemoteEstimatorProxyTest RemoteEstimatorProxyOnRequestTest; +typedef TransportSequenceNumberFeedbackGeneneratorTest + RemoteEstimatorProxyOnRequestTest; TEST_F(RemoteEstimatorProxyOnRequestTest, DisablesPeriodicProcess) { IncomingPacketV2(kBaseSeq, kBaseTime); - EXPECT_EQ(proxy_.Process(clock_.CurrentTime()), TimeDelta::PlusInfinity()); + EXPECT_EQ(feedback_generator_.Process(clock_.CurrentTime()), + TimeDelta::PlusInfinity()); } TEST_F(RemoteEstimatorProxyOnRequestTest, ProcessDoesNotSendFeedback) { @@ -571,9 +596,10 @@ TEST_F(RemoteEstimatorProxyOnRequestTest, kFivePacketsFeedbackRequest); } -TEST_F(RemoteEstimatorProxyTest, ReportsIncomingPacketToNetworkStateEstimator) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + ReportsIncomingPacketToNetworkStateEstimator) { const DataSize kPacketOverhead = DataSize::Bytes(38); - proxy_.SetTransportOverhead(kPacketOverhead); + feedback_generator_.SetTransportOverhead(kPacketOverhead); EXPECT_CALL(network_state_estimator_, OnReceivedPacket) .WillOnce([&](const PacketResult& packet) { @@ -585,7 +611,8 @@ TEST_F(RemoteEstimatorProxyTest, ReportsIncomingPacketToNetworkStateEstimator) { IncomingPacket(kBaseSeq, kBaseTime, AbsoluteSendTime::To24Bits(kBaseTime)); } -TEST_F(RemoteEstimatorProxyTest, IncomingPacketHandlesWrapInAbsSendTime) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + IncomingPacketHandlesWrapInAbsSendTime) { // abs send time use 24bit precision. const uint32_t kFirstAbsSendTime = AbsoluteSendTime::To24Bits(Timestamp::Millis((1 << 24) - 30)); @@ -612,7 +639,8 @@ TEST_F(RemoteEstimatorProxyTest, IncomingPacketHandlesWrapInAbsSendTime) { kSecondAbsSendTime); } -TEST_F(RemoteEstimatorProxyTest, IncomingPacketHandlesReorderedPackets) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + IncomingPacketHandlesReorderedPackets) { const uint32_t kFirstAbsSendTime = AbsoluteSendTime::To24Bits(Timestamp::Millis((1 << 12))); Timestamp first_send_timestamp = Timestamp::Zero(); @@ -635,7 +663,7 @@ TEST_F(RemoteEstimatorProxyTest, IncomingPacketHandlesReorderedPackets) { kSecondAbsSendTime); } -TEST_F(RemoteEstimatorProxyTest, +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, IncomingPacketResetSendTimeToArrivalTimeAfterLargeArrivaltimeDelta) { const uint32_t kFirstAbsSendTime = AbsoluteSendTime::To24Bits(Timestamp::Millis((1 << 12))); @@ -656,7 +684,8 @@ TEST_F(RemoteEstimatorProxyTest, kFirstAbsSendTime + 123); } -TEST_F(RemoteEstimatorProxyTest, SendTransportFeedbackAndNetworkStateUpdate) { +TEST_F(TransportSequenceNumberFeedbackGeneneratorTest, + SendTransportFeedbackAndNetworkStateUpdate) { IncomingPacket(kBaseSeq, kBaseTime, AbsoluteSendTime::To24Bits(kBaseTime - TimeDelta::Millis(1)));