From 45087cd23ff5df9122ebdacf9e4c50661adcf3cf Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Thu, 1 Mar 2018 15:56:57 +0100 Subject: [PATCH] Moved retransmission rate limiter to Call class. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ownership of the retransmission rate limiter for video is moved from send side congestion controller to Call. This is to reduce the interface on the rtp transport controller send. Bug: webrtc:8415 Change-Id: Ie9c7317400a9eb61a3c8325b9e527844ffc13769 Reviewed-on: https://webrtc-review.googlesource.com/58745 Commit-Queue: Sebastian Jansson Reviewed-by: Stefan Holmer Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/master@{#22254} --- call/BUILD.gn | 1 + call/call.cc | 11 ++++++++++- call/rtp_transport_controller_send.cc | 3 --- call/rtp_transport_controller_send.h | 1 - call/rtp_transport_controller_send_interface.h | 1 - call/test/mock_rtp_transport_controller_send.h | 1 - .../include/send_side_congestion_controller.h | 2 +- .../send_side_congestion_controller_interface.h | 1 - .../include/send_side_congestion_controller.h | 1 - .../rtp/send_side_congestion_controller.cc | 16 ---------------- 10 files changed, 12 insertions(+), 26 deletions(-) diff --git a/call/BUILD.gn b/call/BUILD.gn index 9b9296ebd5..44abad6b80 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -173,6 +173,7 @@ rtc_static_library("call") { "../modules/utility", "../modules/video_coding:video_coding", "../rtc_base:checks", + "../rtc_base:rate_limiter", "../rtc_base:rtc_base_approved", "../rtc_base:rtc_task_queue", "../rtc_base:sequenced_task_checker", diff --git a/call/call.cc b/call/call.cc index 973b9db1e2..9801a6f9e8 100644 --- a/call/call.cc +++ b/call/call.cc @@ -50,6 +50,7 @@ #include "rtc_base/location.h" #include "rtc_base/logging.h" #include "rtc_base/ptr_util.h" +#include "rtc_base/rate_limiter.h" #include "rtc_base/sequenced_task_checker.h" #include "rtc_base/task_queue.h" #include "rtc_base/thread_annotations.h" @@ -67,6 +68,7 @@ namespace webrtc { namespace { +static const int64_t kRetransmitWindowSizeMs = 500; // TODO(nisse): This really begs for a shared context struct. bool UseSendSideBwe(const std::vector& extensions, @@ -349,6 +351,7 @@ class Call : public webrtc::Call, RTC_GUARDED_BY(&bitrate_crit_); AvgCounter pacer_bitrate_kbps_counter_ RTC_GUARDED_BY(&bitrate_crit_); + RateLimiter retransmission_rate_limiter_; std::unique_ptr transport_send_; ReceiveSideCongestionController receive_side_cc_; const std::unique_ptr video_send_delay_stats_; @@ -421,6 +424,7 @@ Call::Call(const Call::Config& config, configured_max_padding_bitrate_bps_(0), estimated_send_bitrate_kbps_counter_(clock_, nullptr, true), pacer_bitrate_kbps_counter_(clock_, nullptr, true), + retransmission_rate_limiter_(clock_, kRetransmitWindowSizeMs), receive_side_cc_(clock_, transport_send->packet_router()), video_send_delay_stats_(new SendDelayStats(clock_)), start_ms_(clock_->TimeInMilliseconds()), @@ -704,7 +708,7 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream( video_send_delay_stats_.get(), event_log_, std::move(config), std::move(encoder_config), suspended_video_send_ssrcs_, suspended_video_payload_states_, std::move(fec_controller), - transport_send_->GetRetransmissionRateLimiter()); + &retransmission_rate_limiter_); { WriteLockScoped write_lock(*send_crit_); @@ -1042,6 +1046,11 @@ void Call::OnNetworkChanged(uint32_t target_bitrate_bps, return; } RTC_DCHECK_RUN_ON(&worker_queue_); + // TODO(srte): communicate bandwidth in the OnNetworkChanged event, or + // evaluate the feasability of using target bitrate _bps instead. + uint32_t bandwidth_bps; + if (transport_send_->AvailableBandwidth(&bandwidth_bps)) + retransmission_rate_limiter_.SetMaxRate(bandwidth_bps); // For controlling the rate of feedback messages. receive_side_cc_.OnBitrateChanged(target_bitrate_bps); bitrate_allocator_->OnNetworkChanged(target_bitrate_bps, fraction_loss, diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 5c6b6b9417..bcfc131f7a 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -149,9 +149,6 @@ int64_t RtpTransportControllerSend::GetPacerQueuingDelayMs() const { int64_t RtpTransportControllerSend::GetFirstPacketTimeMs() const { return send_side_cc_->GetFirstPacketTimeMs(); } -RateLimiter* RtpTransportControllerSend::GetRetransmissionRateLimiter() { - return send_side_cc_->GetRetransmissionRateLimiter(); -} void RtpTransportControllerSend::EnablePeriodicAlrProbing(bool enable) { send_side_cc_->EnablePeriodicAlrProbing(enable); } diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index 49bd8b8f7f..a7c291c544 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -64,7 +64,6 @@ class RtpTransportControllerSend : public RtpTransportControllerSendInterface { bool AvailableBandwidth(uint32_t* bandwidth) const override; int64_t GetPacerQueuingDelayMs() const override; int64_t GetFirstPacketTimeMs() const override; - RateLimiter* GetRetransmissionRateLimiter() override; void EnablePeriodicAlrProbing(bool enable) override; void OnSentPacket(const rtc::SentPacket& sent_packet) override; diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h index 56cac12214..07c7f17098 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -99,7 +99,6 @@ class RtpTransportControllerSendInterface { virtual bool AvailableBandwidth(uint32_t* bandwidth) const = 0; virtual int64_t GetPacerQueuingDelayMs() const = 0; virtual int64_t GetFirstPacketTimeMs() const = 0; - virtual RateLimiter* GetRetransmissionRateLimiter() = 0; virtual void EnablePeriodicAlrProbing(bool enable) = 0; virtual void OnSentPacket(const rtc::SentPacket& sent_packet) = 0; diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h index 3ecb7a99ee..1cbbb77ce8 100644 --- a/call/test/mock_rtp_transport_controller_send.h +++ b/call/test/mock_rtp_transport_controller_send.h @@ -48,7 +48,6 @@ class MockRtpTransportControllerSend MOCK_CONST_METHOD1(AvailableBandwidth, bool(uint32_t*)); MOCK_CONST_METHOD0(GetPacerQueuingDelayMs, int64_t()); MOCK_CONST_METHOD0(GetFirstPacketTimeMs, int64_t()); - MOCK_METHOD0(GetRetransmissionRateLimiter, RateLimiter*()); MOCK_METHOD1(EnablePeriodicAlrProbing, void(bool)); MOCK_METHOD1(OnSentPacket, void(const rtc::SentPacket&)); MOCK_METHOD1(SetSdpBitrateParameters, void(const BitrateConstraints&)); diff --git a/modules/congestion_controller/include/send_side_congestion_controller.h b/modules/congestion_controller/include/send_side_congestion_controller.h index 9f2d0a6986..30bc0a991a 100644 --- a/modules/congestion_controller/include/send_side_congestion_controller.h +++ b/modules/congestion_controller/include/send_side_congestion_controller.h @@ -90,7 +90,7 @@ class SendSideCongestionController TransportFeedbackObserver* GetTransportFeedbackObserver() override; - RateLimiter* GetRetransmissionRateLimiter() override; + RTC_DEPRECATED virtual RateLimiter* GetRetransmissionRateLimiter(); void EnablePeriodicAlrProbing(bool enable) override; void OnSentPacket(const rtc::SentPacket& sent_packet) override; diff --git a/modules/congestion_controller/include/send_side_congestion_controller_interface.h b/modules/congestion_controller/include/send_side_congestion_controller_interface.h index 4d64424fa7..f03ffff8c8 100644 --- a/modules/congestion_controller/include/send_side_congestion_controller_interface.h +++ b/modules/congestion_controller/include/send_side_congestion_controller_interface.h @@ -61,7 +61,6 @@ class SendSideCongestionControllerInterface : public CallStatsObserver, virtual int64_t GetPacerQueuingDelayMs() const = 0; virtual int64_t GetFirstPacketTimeMs() const = 0; virtual TransportFeedbackObserver* GetTransportFeedbackObserver() = 0; - virtual RateLimiter* GetRetransmissionRateLimiter() = 0; virtual void EnablePeriodicAlrProbing(bool enable) = 0; virtual void OnSentPacket(const rtc::SentPacket& sent_packet) = 0; RTC_DISALLOW_COPY_AND_ASSIGN(SendSideCongestionControllerInterface); diff --git a/modules/congestion_controller/rtp/include/send_side_congestion_controller.h b/modules/congestion_controller/rtp/include/send_side_congestion_controller.h index 220800dcd2..6266f09867 100644 --- a/modules/congestion_controller/rtp/include/send_side_congestion_controller.h +++ b/modules/congestion_controller/rtp/include/send_side_congestion_controller.h @@ -97,7 +97,6 @@ class SendSideCongestionController TransportFeedbackObserver* GetTransportFeedbackObserver() override; - RateLimiter* GetRetransmissionRateLimiter() override; void EnablePeriodicAlrProbing(bool enable) override; void OnSentPacket(const rtc::SentPacket& sent_packet) override; diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller.cc b/modules/congestion_controller/rtp/send_side_congestion_controller.cc index d2be85468e..95cbd98fe8 100644 --- a/modules/congestion_controller/rtp/send_side_congestion_controller.cc +++ b/modules/congestion_controller/rtp/send_side_congestion_controller.cc @@ -37,8 +37,6 @@ namespace webrtc { namespace webrtc_cc { namespace { -static const int64_t kRetransmitWindowSizeMs = 500; - const char kPacerPushbackExperiment[] = "WebRTC-PacerPushbackExperiment"; bool IsPacerPushbackExperimentEnabled() { @@ -120,7 +118,6 @@ class ControlHandler : public NetworkControllerObserver { rtc::Optional last_transfer_rate(); bool pacer_configured(); - RateLimiter* retransmission_rate_limiter(); private: void OnNetworkInvalidation(); @@ -132,7 +129,6 @@ class ControlHandler : public NetworkControllerObserver { uint8_t fraction_loss, int64_t rtt); PacerController* pacer_controller_; - RateLimiter retransmission_rate_limiter_; rtc::CriticalSection state_lock_; rtc::Optional last_target_rate_ @@ -156,7 +152,6 @@ class ControlHandler : public NetworkControllerObserver { ControlHandler::ControlHandler(PacerController* pacer_controller, const Clock* clock) : pacer_controller_(pacer_controller), - retransmission_rate_limiter_(clock, kRetransmitWindowSizeMs), pacer_pushback_experiment_(IsPacerPushbackExperimentEnabled()) { sequenced_checker_.Detach(); } @@ -180,9 +175,6 @@ void ControlHandler::OnProbeClusterConfig(ProbeClusterConfig config) { void ControlHandler::OnTargetTransferRate(TargetTransferRate target_rate) { RTC_DCHECK_CALLED_SEQUENTIALLY(&sequenced_checker_); - retransmission_rate_limiter_.SetMaxRate( - target_rate.network_estimate.bandwidth.bps()); - current_target_rate_msg_ = target_rate; OnNetworkInvalidation(); rtc::CritScope cs(&state_lock_); @@ -282,10 +274,6 @@ bool ControlHandler::pacer_configured() { rtc::CritScope cs(&state_lock_); return pacer_configured_; } - -RateLimiter* ControlHandler::retransmission_rate_limiter() { - return &retransmission_rate_limiter_; -} } // namespace send_side_cc_internal SendSideCongestionController::SendSideCongestionController( @@ -388,10 +376,6 @@ RtcpBandwidthObserver* SendSideCongestionController::GetBandwidthObserver() { return this; } -RateLimiter* SendSideCongestionController::GetRetransmissionRateLimiter() { - return control_handler->retransmission_rate_limiter(); -} - void SendSideCongestionController::EnablePeriodicAlrProbing(bool enable) { WaitOnTask([this, enable]() { streams_config_.requests_alr_probing = enable;