From 97f61ea68494c7601313bee84ac7585bb2bf4371 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Wed, 21 Feb 2018 13:01:55 +0100 Subject: [PATCH] Moved bitrate configuration to rtp controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since rtp transport controller send owns the congestion controller it also should own the bitrate configuration logic, this way it can initialize the send side congestion controller with the bitrate configuration. Bug: webrtc:8415 Change-Id: Ifaa16139ca477cb1c80bf4aa24f17652af997553 Reviewed-on: https://webrtc-review.googlesource.com/54303 Commit-Queue: Sebastian Jansson Reviewed-by: Stefan Holmer Reviewed-by: Björn Terelius Cr-Commit-Position: refs/heads/master@{#22127} --- audio/audio_send_stream_unittest.cc | 2 +- call/BUILD.gn | 2 +- call/call.cc | 60 ++++------------ call/rtp_transport_controller_send.cc | 71 +++++++++++++++---- call/rtp_transport_controller_send.h | 23 +++--- .../rtp_transport_controller_send_interface.h | 20 ++++-- ortc/rtptransportcontrolleradapter.cc | 4 +- test/call_test.cc | 2 +- 8 files changed, 101 insertions(+), 83 deletions(-) diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 0c0139352c..c5adca7a23 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -126,7 +126,7 @@ struct ConfigHelper { : stream_config_(nullptr), audio_processing_(new rtc::RefCountedObject()), simulated_clock_(123456), - rtp_transport_(&simulated_clock_, &event_log_), + rtp_transport_(&simulated_clock_, &event_log_, BitrateConstraints()), bitrate_allocator_(&limit_observer_), worker_queue_("ConfigHelper_worker_queue"), audio_encoder_(nullptr) { diff --git a/call/BUILD.gn b/call/BUILD.gn index f647fc7f9e..b943221958 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -92,6 +92,7 @@ rtc_source_set("rtp_sender") { "rtp_transport_controller_send.h", ] deps = [ + ":bitrate_configurator", ":rtp_interfaces", "..:webrtc_common", "../modules/congestion_controller", @@ -147,7 +148,6 @@ rtc_static_library("call") { deps = [ ":bitrate_allocator", - ":bitrate_configurator", ":call_interfaces", ":rtp_interfaces", ":rtp_receiver", diff --git a/call/call.cc b/call/call.cc index fe5f20a8f2..c21c24095f 100644 --- a/call/call.cc +++ b/call/call.cc @@ -24,7 +24,6 @@ #include "call/bitrate_allocator.h" #include "call/call.h" #include "call/flexfec_receive_stream_impl.h" -#include "call/rtp_bitrate_configurator.h" #include "call/rtp_stream_receiver_controller.h" #include "call/rtp_transport_controller_send.h" #include "logging/rtc_event_log/events/rtc_event_audio_receive_stream_config.h" @@ -369,7 +368,6 @@ class Call : public webrtc::Call, // and deleted before any other members. rtc::TaskQueue worker_queue_; - RtpBitrateConfigurator bitrate_configurator_; RTC_DISALLOW_COPY_AND_ASSIGN(Call); }; } // namespace internal @@ -387,9 +385,10 @@ std::string Call::Stats::ToString(int64_t time_ms) const { } Call* Call::Create(const Call::Config& config) { - return new internal::Call(config, - rtc::MakeUnique( - Clock::GetRealTimeClock(), config.event_log)); + return new internal::Call( + config, + rtc::MakeUnique( + Clock::GetRealTimeClock(), config.event_log, config.bitrate_config)); } Call* Call::Create( @@ -435,16 +434,11 @@ Call::Call(const Call::Config& config, receive_side_cc_(clock_, transport_send->packet_router()), video_send_delay_stats_(new SendDelayStats(clock_)), start_ms_(clock_->TimeInMilliseconds()), - worker_queue_("call_worker_queue"), - bitrate_configurator_(config.bitrate_config) { + worker_queue_("call_worker_queue") { RTC_DCHECK(config.event_log != nullptr); transport_send->RegisterNetworkObserver(this); transport_send_ = std::move(transport_send); - transport_send_->OnNetworkAvailability(false); - transport_send_->SetBweBitrates( - bitrate_configurator_.GetConfig().min_bitrate_bps, - bitrate_configurator_.GetConfig().start_bitrate_bps, - bitrate_configurator_.GetConfig().max_bitrate_bps); + call_stats_->RegisterStatsObserver(&receive_side_cc_); call_stats_->RegisterStatsObserver(transport_send_->GetCallStatsObserver()); @@ -941,29 +935,13 @@ Call::Stats Call::GetStats() const { void Call::SetBitrateConfig(const BitrateConstraints& bitrate_config) { TRACE_EVENT0("webrtc", "Call::SetBitrateConfig"); RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_); - rtc::Optional config = - bitrate_configurator_.UpdateWithSdpParameters(bitrate_config); - if (config.has_value()) { - transport_send_->SetBweBitrates(config->min_bitrate_bps, - config->start_bitrate_bps, - config->max_bitrate_bps); - } else { - RTC_LOG(LS_VERBOSE) << "WebRTC.Call.SetBitrateConfig: " - << "nothing to update"; - } + transport_send_->SetSdpBitrateParameters(bitrate_config); } -void Call::SetBitrateConfigMask(const BitrateConstraintsMask& bitrate_mask) { - rtc::Optional config = - bitrate_configurator_.UpdateWithClientPreferences(bitrate_mask); - if (config.has_value()) { - transport_send_->SetBweBitrates(config->min_bitrate_bps, - config->start_bitrate_bps, - config->max_bitrate_bps); - } else { - RTC_LOG(LS_VERBOSE) << "WebRTC.Call.SetBitrateConfigMask: " - << "nothing to update"; - } +void Call::SetBitrateConfigMask(const webrtc::BitrateConstraintsMask& mask) { + TRACE_EVENT0("webrtc", "Call::SetBitrateConfigMask"); + RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_); + transport_send_->SetClientBitratePreferences(mask); } void Call::SetBitrateAllocationStrategy( @@ -1067,21 +1045,7 @@ void Call::OnNetworkRouteChanged(const std::string& transport_name, } if (kv->second != network_route) { kv->second = network_route; - BitrateConstraints bitrate_config = bitrate_configurator_.GetConfig(); - RTC_LOG(LS_INFO) << "Network route changed on transport " << transport_name - << ": new local network id " - << network_route.local_network_id - << " new remote network id " - << network_route.remote_network_id - << " Reset bitrates to min: " - << bitrate_config.min_bitrate_bps - << " bps, start: " << bitrate_config.start_bitrate_bps - << " bps, max: " << bitrate_config.max_bitrate_bps - << " bps."; - RTC_DCHECK_GT(bitrate_config.start_bitrate_bps, 0); - transport_send_->OnNetworkRouteChanged( - network_route, bitrate_config.start_bitrate_bps, - bitrate_config.min_bitrate_bps, bitrate_config.max_bitrate_bps); + transport_send_->OnNetworkRouteChanged(transport_name, network_route); } } diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 70061c1230..a48d84353e 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -9,14 +9,23 @@ */ #include "call/rtp_transport_controller_send.h" +#include "rtc_base/logging.h" namespace webrtc { RtpTransportControllerSend::RtpTransportControllerSend( Clock* clock, - webrtc::RtcEventLog* event_log) + webrtc::RtcEventLog* event_log, + const BitrateConstraints& bitrate_config) : pacer_(clock, &packet_router_, event_log), - send_side_cc_(clock, nullptr /* observer */, event_log, &pacer_) {} + send_side_cc_(clock, nullptr /* observer */, event_log, &pacer_), + bitrate_configurator_(bitrate_config) { + send_side_cc_.SignalNetworkState(kNetworkDown); + send_side_cc_.SetBweBitrates(bitrate_config.min_bitrate_bps, + bitrate_config.start_bitrate_bps, + bitrate_config.max_bitrate_bps); +} +RtpTransportControllerSend::~RtpTransportControllerSend() = default; PacketRouter* RtpTransportControllerSend::packet_router() { return &packet_router_; @@ -76,19 +85,24 @@ void RtpTransportControllerSend::DeRegisterNetworkObserver( NetworkChangedObserver* observer) { send_side_cc_.DeRegisterNetworkObserver(observer); } -void RtpTransportControllerSend::SetBweBitrates(int min_bitrate_bps, - int start_bitrate_bps, - int max_bitrate_bps) { - send_side_cc_.SetBweBitrates(min_bitrate_bps, start_bitrate_bps, - max_bitrate_bps); -} void RtpTransportControllerSend::OnNetworkRouteChanged( - const rtc::NetworkRoute& network_route, - int start_bitrate_bps, - int min_bitrate_bps, - int max_bitrate_bps) { - send_side_cc_.OnNetworkRouteChanged(network_route, start_bitrate_bps, - min_bitrate_bps, max_bitrate_bps); + const std::string& transport_name, + const rtc::NetworkRoute& network_route) { + BitrateConstraints bitrate_config = bitrate_configurator_.GetConfig(); + RTC_LOG(LS_INFO) << "Network route changed on transport " << transport_name + << ": new local network id " + << network_route.local_network_id + << " new remote network id " + << network_route.remote_network_id + << " Reset bitrates to min: " + << bitrate_config.min_bitrate_bps + << " bps, start: " << bitrate_config.start_bitrate_bps + << " bps, max: " << bitrate_config.max_bitrate_bps + << " bps."; + RTC_DCHECK_GT(bitrate_config.start_bitrate_bps, 0); + send_side_cc_.OnNetworkRouteChanged( + network_route, bitrate_config.start_bitrate_bps, + bitrate_config.min_bitrate_bps, bitrate_config.max_bitrate_bps); } void RtpTransportControllerSend::OnNetworkAvailability(bool network_available) { send_side_cc_.SignalNetworkState(network_available ? kNetworkUp @@ -121,4 +135,33 @@ void RtpTransportControllerSend::OnSentPacket( send_side_cc_.OnSentPacket(sent_packet); } +void RtpTransportControllerSend::SetSdpBitrateParameters( + const BitrateConstraints& constraints) { + rtc::Optional updated = + bitrate_configurator_.UpdateWithSdpParameters(constraints); + if (updated.has_value()) { + send_side_cc_.SetBweBitrates(updated->min_bitrate_bps, + updated->start_bitrate_bps, + updated->max_bitrate_bps); + } else { + RTC_LOG(LS_VERBOSE) + << "WebRTC.RtpTransportControllerSend.SetBitrateConfig: " + << "nothing to update"; + } +} + +void RtpTransportControllerSend::SetClientBitratePreferences( + const BitrateConstraintsMask& preferences) { + rtc::Optional updated = + bitrate_configurator_.UpdateWithClientPreferences(preferences); + if (updated.has_value()) { + send_side_cc_.SetBweBitrates(updated->min_bitrate_bps, + updated->start_bitrate_bps, + updated->max_bitrate_bps); + } else { + RTC_LOG(LS_VERBOSE) + << "WebRTC.RtpTransportControllerSend.SetBitrateConfigMask: " + << "nothing to update"; + } +} } // namespace webrtc diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index d0bfa1ca3d..30f059642e 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -11,6 +11,9 @@ #ifndef CALL_RTP_TRANSPORT_CONTROLLER_SEND_H_ #define CALL_RTP_TRANSPORT_CONTROLLER_SEND_H_ +#include + +#include "call/rtp_bitrate_configurator.h" #include "call/rtp_transport_controller_send_interface.h" #include "common_types.h" // NOLINT(build/include) #include "modules/congestion_controller/include/send_side_congestion_controller.h" @@ -26,8 +29,10 @@ class RtcEventLog; // per transport, sharing the same congestion controller. class RtpTransportControllerSend : public RtpTransportControllerSendInterface { public: - RtpTransportControllerSend(Clock* clock, webrtc::RtcEventLog* event_log); - + RtpTransportControllerSend(Clock* clock, + RtcEventLog* event_log, + const BitrateConstraints& bitrate_config); + ~RtpTransportControllerSend() override; // Implements RtpTransportControllerSendInterface PacketRouter* packet_router() override; @@ -50,13 +55,8 @@ class RtpTransportControllerSend : public RtpTransportControllerSendInterface { PacketFeedbackObserver* observer) override; void RegisterNetworkObserver(NetworkChangedObserver* observer) override; void DeRegisterNetworkObserver(NetworkChangedObserver* observer) override; - void SetBweBitrates(int min_bitrate_bps, - int start_bitrate_bps, - int max_bitrate_bps) override; - void OnNetworkRouteChanged(const rtc::NetworkRoute& network_route, - int start_bitrate_bps, - int min_bitrate_bps, - int max_bitrate_bps) override; + void OnNetworkRouteChanged(const std::string& transport_name, + const rtc::NetworkRoute& network_route) override; void OnNetworkAvailability(bool network_available) override; void SetTransportOverhead( size_t transport_overhead_bytes_per_packet) override; @@ -68,11 +68,16 @@ class RtpTransportControllerSend : public RtpTransportControllerSendInterface { void EnablePeriodicAlrProbing(bool enable) override; void OnSentPacket(const rtc::SentPacket& sent_packet) override; + void SetSdpBitrateParameters(const BitrateConstraints& constraints) override; + void SetClientBitratePreferences( + const BitrateConstraintsMask& preferences) override; + private: PacketRouter packet_router_; PacedSender pacer_; SendSideCongestionController send_side_cc_; RtpKeepAliveConfig keepalive_; + RtpBitrateConfigurator bitrate_configurator_; RTC_DISALLOW_COPY_AND_ASSIGN(RtpTransportControllerSend); }; diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h index 284aeec46b..736689e3d7 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -13,6 +13,11 @@ #include #include +#include + +#include "api/optional.h" +#include "call/bitrate_constraints.h" + namespace rtc { struct SentPacket; struct NetworkRoute; @@ -88,13 +93,9 @@ class RtpTransportControllerSendInterface { PacketFeedbackObserver* observer) = 0; virtual void RegisterNetworkObserver(NetworkChangedObserver* observer) = 0; virtual void DeRegisterNetworkObserver(NetworkChangedObserver* observer) = 0; - virtual void SetBweBitrates(int min_bitrate_bps, - int start_bitrate_bps, - int max_bitrate_bps) = 0; - virtual void OnNetworkRouteChanged(const rtc::NetworkRoute& network_route, - int start_bitrate_bps, - int min_bitrate_bps, - int max_bitrate_bps) = 0; + virtual void OnNetworkRouteChanged( + const std::string& transport_name, + const rtc::NetworkRoute& network_route) = 0; virtual void OnNetworkAvailability(bool network_available) = 0; virtual void SetTransportOverhead( size_t transport_overhead_bytes_per_packet) = 0; @@ -105,6 +106,11 @@ class RtpTransportControllerSendInterface { virtual RateLimiter* GetRetransmissionRateLimiter() = 0; virtual void EnablePeriodicAlrProbing(bool enable) = 0; virtual void OnSentPacket(const rtc::SentPacket& sent_packet) = 0; + + virtual void SetSdpBitrateParameters( + const BitrateConstraints& constraints) = 0; + virtual void SetClientBitratePreferences( + const BitrateConstraintsMask& preferences) = 0; }; } // namespace webrtc diff --git a/ortc/rtptransportcontrolleradapter.cc b/ortc/rtptransportcontrolleradapter.cc index 9d07be651b..e278d198b4 100644 --- a/ortc/rtptransportcontrolleradapter.cc +++ b/ortc/rtptransportcontrolleradapter.cc @@ -651,8 +651,8 @@ void RtpTransportControllerAdapter::Init_w() { call_config.bitrate_config.start_bitrate_bps = kStartBandwidthBps; call_config.bitrate_config.max_bitrate_bps = kMaxBandwidthBps; - call_send_rtp_transport_controller_ = - new RtpTransportControllerSend(Clock::GetRealTimeClock(), event_log_); + call_send_rtp_transport_controller_ = new RtpTransportControllerSend( + Clock::GetRealTimeClock(), event_log_, call_config.bitrate_config); call_.reset(webrtc::Call::Create( call_config, std::unique_ptr( call_send_rtp_transport_controller_))); diff --git a/test/call_test.cc b/test/call_test.cc index 148321689f..4d34762589 100644 --- a/test/call_test.cc +++ b/test/call_test.cc @@ -172,7 +172,7 @@ void CallTest::CreateCalls(const Call::Config& sender_config, void CallTest::CreateSenderCall(const Call::Config& config) { sender_call_transport_controller_ = new RtpTransportControllerSend( - Clock::GetRealTimeClock(), config.event_log); + Clock::GetRealTimeClock(), config.event_log, config.bitrate_config); sender_call_.reset( Call::Create(config, std::unique_ptr(