diff --git a/modules/congestion_controller/include/receive_side_congestion_controller.h b/modules/congestion_controller/include/receive_side_congestion_controller.h index 77bd4cc66b..fe30865c53 100644 --- a/modules/congestion_controller/include/receive_side_congestion_controller.h +++ b/modules/congestion_controller/include/receive_side_congestion_controller.h @@ -47,6 +47,11 @@ class ReceiveSideCongestionController : public CallStatsObserver { const RTPHeader& header); void SetSendPeriodicFeedback(bool send_periodic_feedback); + // TODO(nisse): Delete these methods, design a more specific interface. + [[deprecated]] virtual RemoteBitrateEstimator* GetRemoteBitrateEstimator( + bool send_side_bwe); + [[deprecated]] virtual const RemoteBitrateEstimator* + GetRemoteBitrateEstimator(bool send_side_bwe) const; // Implements CallStatsObserver. void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; @@ -117,7 +122,6 @@ class ReceiveSideCongestionController : public CallStatsObserver { int min_bitrate_bps_; }; - Clock& clock_; const FieldTrialBasedConfig field_trial_config_; RembThrottler remb_throttler_; WrappingBitrateEstimator remote_bitrate_estimator_; diff --git a/modules/congestion_controller/receive_side_congestion_controller.cc b/modules/congestion_controller/receive_side_congestion_controller.cc index 92ca62b7fc..3f337f84c7 100644 --- a/modules/congestion_controller/receive_side_congestion_controller.cc +++ b/modules/congestion_controller/receive_side_congestion_controller.cc @@ -124,10 +124,10 @@ ReceiveSideCongestionController::ReceiveSideCongestionController( RemoteEstimatorProxy::TransportFeedbackSender feedback_sender, RembThrottler::RembSender remb_sender, NetworkStateEstimator* network_state_estimator) - : clock_(*clock), - remb_throttler_(std::move(remb_sender), clock), + : remb_throttler_(std::move(remb_sender), clock), remote_bitrate_estimator_(&remb_throttler_, clock), - remote_estimator_proxy_(std::move(feedback_sender), + remote_estimator_proxy_(clock, + std::move(feedback_sender), &field_trial_config_, network_state_estimator) {} @@ -148,6 +148,25 @@ void ReceiveSideCongestionController::SetSendPeriodicFeedback( remote_estimator_proxy_.SetSendPeriodicFeedback(send_periodic_feedback); } +RemoteBitrateEstimator* +ReceiveSideCongestionController::GetRemoteBitrateEstimator(bool send_side_bwe) { + if (send_side_bwe) { + return &remote_estimator_proxy_; + } else { + return &remote_bitrate_estimator_; + } +} + +const RemoteBitrateEstimator* +ReceiveSideCongestionController::GetRemoteBitrateEstimator( + bool send_side_bwe) const { + if (send_side_bwe) { + return &remote_estimator_proxy_; + } else { + return &remote_bitrate_estimator_; + } +} + DataRate ReceiveSideCongestionController::LatestReceiveSideEstimate() const { std::vector unused_ssrcs; uint32_t bitrate_bps = 0; @@ -180,16 +199,18 @@ void ReceiveSideCongestionController::Process() { } TimeDelta ReceiveSideCongestionController::MaybeProcess() { - Timestamp now = clock_.CurrentTime(); int64_t time_until_rbe_ms = remote_bitrate_estimator_.TimeUntilNextProcess(); if (time_until_rbe_ms <= 0) { remote_bitrate_estimator_.Process(); time_until_rbe_ms = remote_bitrate_estimator_.TimeUntilNextProcess(); } - TimeDelta time_until_rbe = TimeDelta::Millis(time_until_rbe_ms); - TimeDelta time_until_rep = remote_estimator_proxy_.Process(now); - TimeDelta time_until = std::min(time_until_rbe, time_until_rep); - return std::max(time_until, TimeDelta::Zero()); + int64_t time_until_rep_ms = remote_estimator_proxy_.TimeUntilNextProcess(); + if (time_until_rep_ms <= 0) { + remote_estimator_proxy_.Process(); + time_until_rep_ms = remote_estimator_proxy_.TimeUntilNextProcess(); + } + int64_t time_until_next_ms = std::min(time_until_rbe_ms, time_until_rep_ms); + return TimeDelta::Millis(std::max(time_until_next_ms, 0)); } void ReceiveSideCongestionController::SetMaxDesiredReceiveBitrate( diff --git a/modules/congestion_controller/remb_throttler.h b/modules/congestion_controller/remb_throttler.h index 85292cbc09..2f610c1df9 100644 --- a/modules/congestion_controller/remb_throttler.h +++ b/modules/congestion_controller/remb_throttler.h @@ -16,7 +16,7 @@ #include "api/units/data_rate.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" -#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" +#include "modules/remote_bitrate_estimator/remote_estimator_proxy.h" #include "rtc_base/synchronization/mutex.h" namespace webrtc { diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index 0215b64155..4691fa6283 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -16,7 +16,6 @@ #include #include "api/units/data_size.h" -#include "modules/rtp_rtcp/source/rtcp_packet/remote_estimate.h" #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -47,20 +46,22 @@ TimeDelta GetAbsoluteSendTimeDelta(uint32_t new_sendtime, } // namespace RemoteEstimatorProxy::RemoteEstimatorProxy( + Clock* clock, TransportFeedbackSender feedback_sender, const FieldTrialsView* key_value_config, NetworkStateEstimator* network_state_estimator) - : feedback_sender_(std::move(feedback_sender)), + : clock_(clock), + feedback_sender_(std::move(feedback_sender)), send_config_(key_value_config), - last_process_time_(Timestamp::MinusInfinity()), + last_process_time_ms_(-1), network_state_estimator_(network_state_estimator), media_ssrc_(0), feedback_packet_count_(0), packet_overhead_(DataSize::Zero()), - send_interval_(send_config_.default_interval.Get()), + send_interval_ms_(send_config_.default_interval->ms()), send_periodic_feedback_(true), previous_abs_send_time_(0), - abs_send_timestamp_(Timestamp::Zero()) { + abs_send_timestamp_(clock->CurrentTime()) { RTC_LOG(LS_INFO) << "Maximum interval between transport feedback RTCP messages (ms): " << send_config_.max_interval->ms(); @@ -150,19 +151,32 @@ void RemoteEstimatorProxy::IncomingPacket(Packet packet) { } } -TimeDelta RemoteEstimatorProxy::Process(Timestamp now) { +bool RemoteEstimatorProxy::LatestEstimate(std::vector* ssrcs, + unsigned int* bitrate_bps) const { + return false; +} + +int64_t RemoteEstimatorProxy::TimeUntilNextProcess() { MutexLock lock(&lock_); if (!send_periodic_feedback_) { - return TimeDelta::PlusInfinity(); - } - Timestamp next_process_time = last_process_time_ + send_interval_; - if (now >= next_process_time) { - last_process_time_ = now; - SendPeriodicFeedbacks(); - return send_interval_; + // Wait a day until next process. + return 24 * 60 * 60 * 1000; + } else if (last_process_time_ms_ != -1) { + int64_t now = clock_->TimeInMilliseconds(); + if (now - last_process_time_ms_ < send_interval_ms_) + return last_process_time_ms_ + send_interval_ms_ - now; } + return 0; +} - return next_process_time - now; +void RemoteEstimatorProxy::Process() { + MutexLock lock(&lock_); + if (!send_periodic_feedback_) { + return; + } + last_process_time_ms_ = clock_->TimeInMilliseconds(); + + SendPeriodicFeedbacks(); } void RemoteEstimatorProxy::OnBitrateChanged(int bitrate_bps) { @@ -171,23 +185,18 @@ void RemoteEstimatorProxy::OnBitrateChanged(int bitrate_bps) { // TwccReport size at 50ms interval is 24 byte. // TwccReport size at 250ms interval is 36 byte. // AverageTwccReport = (TwccReport(50ms) + TwccReport(250ms)) / 2 - constexpr DataSize kTwccReportSize = DataSize::Bytes(20 + 8 + 10 + 30); - const DataRate kMinTwccRate = - kTwccReportSize / send_config_.max_interval.Get(); + constexpr int kTwccReportSize = 20 + 8 + 10 + 30; + const double kMinTwccRate = + kTwccReportSize * 8.0 * 1000.0 / send_config_.max_interval->ms(); + const double kMaxTwccRate = + kTwccReportSize * 8.0 * 1000.0 / send_config_.min_interval->ms(); // Let TWCC reports occupy 5% of total bandwidth. - DataRate twcc_bitrate = - DataRate::BitsPerSec(send_config_.bandwidth_fraction * bitrate_bps); - - // Check upper send_interval bound by checking bitrate to avoid overflow when - // dividing by small bitrate, in particular avoid dividing by zero bitrate. - TimeDelta send_interval = twcc_bitrate <= kMinTwccRate - ? send_config_.max_interval.Get() - : std::max(kTwccReportSize / twcc_bitrate, - send_config_.min_interval.Get()); - MutexLock lock(&lock_); - send_interval_ = send_interval; + send_interval_ms_ = static_cast( + 0.5 + kTwccReportSize * 8.0 * 1000.0 / + rtc::SafeClamp(send_config_.bandwidth_fraction * bitrate_bps, + kMinTwccRate, kMaxTwccRate)); } void RemoteEstimatorProxy::SetSendPeriodicFeedback( diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/modules/remote_bitrate_estimator/remote_estimator_proxy.h index 509ad0ba02..6aba92fec0 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.h @@ -18,33 +18,37 @@ #include "absl/types/optional.h" #include "api/field_trials_view.h" -#include "api/rtp_headers.h" #include "api/transport/network_control.h" #include "api/units/data_size.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" +#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "modules/remote_bitrate_estimator/packet_arrival_map.h" -#include "modules/rtp_rtcp/source/rtcp_packet.h" -#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/numerics/sequence_number_util.h" #include "rtc_base/synchronization/mutex.h" namespace webrtc { +class Clock; +namespace rtcp { +class TransportFeedback; +} + // 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 RemoteEstimatorProxy : public RemoteBitrateEstimator { public: // Used for sending transport feedback messages when send side // BWE is used. using TransportFeedbackSender = std::function> packets)>; - RemoteEstimatorProxy(TransportFeedbackSender feedback_sender, + RemoteEstimatorProxy(Clock* clock, + TransportFeedbackSender feedback_sender, const FieldTrialsView* key_value_config, NetworkStateEstimator* network_state_estimator); - ~RemoteEstimatorProxy(); + ~RemoteEstimatorProxy() override; struct Packet { Timestamp arrival_time; @@ -58,12 +62,14 @@ class RemoteEstimatorProxy { void IncomingPacket(int64_t arrival_time_ms, size_t payload_size, - const RTPHeader& header); - - // Sends periodic feedback if it is time to send it. - // Returns time until next call to Process should be made. - TimeDelta Process(Timestamp now); - + const RTPHeader& header) override; + void RemoveStream(uint32_t ssrc) override {} + bool LatestEstimate(std::vector* ssrcs, + unsigned int* bitrate_bps) const override; + void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override {} + void SetMinBitrate(int min_bitrate_bps) override {} + int64_t TimeUntilNextProcess() override; + void Process() override; void OnBitrateChanged(int bitrate); void SetSendPeriodicFeedback(bool send_periodic_feedback); void SetTransportOverhead(DataSize overhead_per_packet); @@ -110,9 +116,10 @@ class RemoteEstimatorProxy { int64_t end_sequence_number_exclusive, bool is_periodic_update) RTC_EXCLUSIVE_LOCKS_REQUIRED(&lock_); + Clock* const clock_; const TransportFeedbackSender feedback_sender_; const TransportWideFeedbackConfig send_config_; - Timestamp last_process_time_; + int64_t last_process_time_ms_; Mutex lock_; // `network_state_estimator_` may be null. @@ -130,7 +137,7 @@ class RemoteEstimatorProxy { // Packet arrival times, by sequence number. PacketArrivalTimeMap packet_arrival_times_ RTC_GUARDED_BY(&lock_); - TimeDelta send_interval_ RTC_GUARDED_BY(&lock_); + int64_t send_interval_ms_ RTC_GUARDED_BY(&lock_); bool send_periodic_feedback_ RTC_GUARDED_BY(&lock_); // Unwraps absolute send times. diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc index 57db595293..c2360c0594 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc @@ -79,7 +79,8 @@ class RemoteEstimatorProxyTest : public ::testing::Test { public: RemoteEstimatorProxyTest() : clock_(0), - proxy_(feedback_sender_.AsStdFunction(), + proxy_(&clock_, + feedback_sender_.AsStdFunction(), &field_trial_config_, &network_state_estimator_) {} @@ -97,7 +98,7 @@ class RemoteEstimatorProxyTest : public ::testing::Test { void Process() { clock_.AdvanceTime(kDefaultSendInterval); - proxy_.Process(clock_.CurrentTime()); + proxy_.Process(); } FieldTrialBasedConfig field_trial_config_; @@ -424,32 +425,41 @@ TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { Process(); } +TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsZeroBeforeFirstProcess) { + EXPECT_EQ(0, proxy_.TimeUntilNextProcess()); +} + TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsDefaultOnUnkownBitrate) { - EXPECT_EQ(proxy_.Process(clock_.CurrentTime()), kDefaultSendInterval); + Process(); + EXPECT_EQ(proxy_.TimeUntilNextProcess(), kDefaultSendInterval.ms()); } TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMinIntervalOn300kbps) { + Process(); proxy_.OnBitrateChanged(300'000); - EXPECT_EQ(proxy_.Process(clock_.CurrentTime()), kMinSendInterval); + EXPECT_EQ(proxy_.TimeUntilNextProcess(), kMinSendInterval.ms()); } TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn0kbps) { + Process(); // 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); + EXPECT_EQ(proxy_.TimeUntilNextProcess(), kMaxSendInterval.ms()); } TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn20kbps) { + Process(); proxy_.OnBitrateChanged(20'000); - EXPECT_EQ(proxy_.Process(clock_.CurrentTime()), kMaxSendInterval); + EXPECT_EQ(proxy_.TimeUntilNextProcess(), kMaxSendInterval.ms()); } TEST_F(RemoteEstimatorProxyTest, TwccReportsUse5PercentOfAvailableBandwidth) { - proxy_.OnBitrateChanged(80'000); + Process(); + proxy_.OnBitrateChanged(80000); // 80kbps * 0.05 = TwccReportSize(68B * 8b/B) * 1000ms / SendInterval(136ms) - EXPECT_EQ(proxy_.Process(clock_.CurrentTime()), TimeDelta::Millis(136)); + EXPECT_EQ(136, proxy_.TimeUntilNextProcess()); } ////////////////////////////////////////////////////////////////////////////// @@ -457,9 +467,9 @@ TEST_F(RemoteEstimatorProxyTest, TwccReportsUse5PercentOfAvailableBandwidth) { // by the sender. ////////////////////////////////////////////////////////////////////////////// typedef RemoteEstimatorProxyTest RemoteEstimatorProxyOnRequestTest; -TEST_F(RemoteEstimatorProxyOnRequestTest, DisablesPeriodicProcess) { +TEST_F(RemoteEstimatorProxyOnRequestTest, TimeUntilNextProcessIsHigh) { proxy_.SetSendPeriodicFeedback(false); - EXPECT_EQ(proxy_.Process(clock_.CurrentTime()), TimeDelta::PlusInfinity()); + EXPECT_GE(proxy_.TimeUntilNextProcess(), 60 * 60 * 1000); } TEST_F(RemoteEstimatorProxyOnRequestTest, ProcessDoesNotSendFeedback) {