diff --git a/call/BUILD.gn b/call/BUILD.gn index 697ab196cc..6351a8ac90 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -96,6 +96,7 @@ rtc_source_set("rtp_sender") { ":rtp_interfaces", "..:webrtc_common", "../modules/congestion_controller", + "../modules/congestion_controller/network_control", "../modules/pacing", "../modules/utility", "../rtc_base:rtc_base", @@ -168,6 +169,7 @@ rtc_static_library("call") { "../logging:rtc_stream_config", "../modules/bitrate_controller", "../modules/congestion_controller", + "../modules/congestion_controller/network_control", "../modules/pacing", "../modules/rtp_rtcp", "../modules/rtp_rtcp:rtp_rtcp_format", @@ -177,6 +179,7 @@ rtc_static_library("call") { "../rtc_base:rate_limiter", "../rtc_base:rtc_base_approved", "../rtc_base:rtc_task_queue", + "../rtc_base:safe_minmax", "../rtc_base:sequenced_task_checker", "../system_wrappers", "../system_wrappers:metrics_api", diff --git a/call/call.cc b/call/call.cc index a9e81c4bdc..daca232777 100644 --- a/call/call.cc +++ b/call/call.cc @@ -35,8 +35,8 @@ #include "logging/rtc_event_log/rtc_event_log.h" #include "logging/rtc_event_log/rtc_stream_config.h" #include "modules/bitrate_controller/include/bitrate_controller.h" -#include "modules/congestion_controller/include/network_changed_observer.h" #include "modules/congestion_controller/include/receive_side_congestion_controller.h" +#include "modules/congestion_controller/network_control/include/network_control.h" #include "modules/rtp_rtcp/include/flexfec_receiver.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/include/rtp_header_parser.h" @@ -49,6 +49,7 @@ #include "rtc_base/constructormagic.h" #include "rtc_base/location.h" #include "rtc_base/logging.h" +#include "rtc_base/numerics/safe_minmax.h" #include "rtc_base/ptr_util.h" #include "rtc_base/rate_limiter.h" #include "rtc_base/sequenced_task_checker.h" @@ -166,7 +167,7 @@ namespace internal { class Call : public webrtc::Call, public PacketReceiver, public RecoveredPacketReceiver, - public NetworkChangedObserver, + public TargetTransferRateObserver, public BitrateAllocator::LimitObserver { public: Call(const Call::Config& config, @@ -227,11 +228,8 @@ class Call : public webrtc::Call, void OnSentPacket(const rtc::SentPacket& sent_packet) override; - // Implements BitrateObserver. - void OnNetworkChanged(uint32_t bitrate_bps, - uint8_t fraction_loss, - int64_t rtt_ms, - int64_t probing_interval_ms) override; + // Implements TargetTransferRateObserver, + void OnTargetTransferRate(TargetTransferRate msg) override; // Implements BitrateAllocator::LimitObserver. void OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps, @@ -344,6 +342,8 @@ class Call : public webrtc::Call, rtc::Optional last_received_rtp_video_ms_; TimeInterval sent_rtp_audio_timer_ms_; + rtc::CriticalSection last_bandwidth_bps_crit_; + uint32_t last_bandwidth_bps_ RTC_GUARDED_BY(&last_bandwidth_bps_crit_); // TODO(holmer): Remove this lock once BitrateController no longer calls // OnNetworkChanged from multiple threads. rtc::CriticalSection bitrate_crit_; @@ -423,6 +423,7 @@ Call::Call(const Call::Config& config, received_audio_bytes_per_second_counter_(clock_, nullptr, true), received_video_bytes_per_second_counter_(clock_, nullptr, true), received_rtcp_bytes_per_second_counter_(clock_, nullptr, true), + last_bandwidth_bps_(0), min_allocated_send_bitrate_bps_(0), configured_max_padding_bitrate_bps_(0), estimated_send_bitrate_kbps_counter_(clock_, nullptr, true), @@ -433,7 +434,7 @@ Call::Call(const Call::Config& config, start_ms_(clock_->TimeInMilliseconds()), worker_queue_("call_worker_queue") { RTC_DCHECK(config.event_log != nullptr); - transport_send->RegisterNetworkObserver(this); + transport_send->RegisterTargetTransferRateObserver(this); transport_send_ = std::move(transport_send); call_stats_->RegisterStatsObserver(&receive_side_cc_); @@ -904,13 +905,15 @@ Call::Stats Call::GetStats() const { // RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_); Stats stats; // Fetch available send/receive bitrates. - uint32_t send_bandwidth = 0; - transport_send_->AvailableBandwidth(&send_bandwidth); std::vector ssrcs; uint32_t recv_bandwidth = 0; receive_side_cc_.GetRemoteBitrateEstimator(false)->LatestEstimate( &ssrcs, &recv_bandwidth); - stats.send_bandwidth_bps = send_bandwidth; + + { + rtc::CritScope cs(&last_bandwidth_bps_crit_); + stats.send_bandwidth_bps = last_bandwidth_bps_; + } stats.recv_bandwidth_bps = recv_bandwidth; // TODO(srte): It is unclear if we only want to report queues if network is // available. @@ -1045,26 +1048,26 @@ void Call::OnSentPacket(const rtc::SentPacket& sent_packet) { transport_send_->OnSentPacket(sent_packet); } -void Call::OnNetworkChanged(uint32_t target_bitrate_bps, - uint8_t fraction_loss, - int64_t rtt_ms, - int64_t probing_interval_ms) { +void Call::OnTargetTransferRate(TargetTransferRate msg) { // TODO(perkj): Consider making sure CongestionController operates on // |worker_queue_|. if (!worker_queue_.IsCurrent()) { - worker_queue_.PostTask( - [this, target_bitrate_bps, fraction_loss, rtt_ms, probing_interval_ms] { - OnNetworkChanged(target_bitrate_bps, fraction_loss, rtt_ms, - probing_interval_ms); - }); + worker_queue_.PostTask([this, msg] { OnTargetTransferRate(msg); }); 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); + uint32_t target_bitrate_bps = msg.target_rate.bps(); + int loss_ratio_255 = msg.network_estimate.loss_rate_ratio * 255; + uint8_t fraction_loss = + rtc::dchecked_cast(rtc::SafeClamp(loss_ratio_255, 0, 255)); + int64_t rtt_ms = msg.network_estimate.round_trip_time.ms(); + int64_t probing_interval_ms = msg.network_estimate.bwe_period.ms(); + uint32_t bandwidth_bps = msg.network_estimate.bandwidth.bps(); + { + rtc::CritScope cs(&last_bandwidth_bps_crit_); + last_bandwidth_bps_ = 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 80df7654ea..044497d3bf 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -11,6 +11,7 @@ #include "call/rtp_transport_controller_send.h" #include "modules/congestion_controller/include/send_side_congestion_controller.h" +#include "modules/congestion_controller/network_control/include/network_control.h" #include "rtc_base/location.h" #include "rtc_base/logging.h" #include "rtc_base/ptr_util.h" @@ -21,14 +22,16 @@ RtpTransportControllerSend::RtpTransportControllerSend( Clock* clock, webrtc::RtcEventLog* event_log, const BitrateConstraints& bitrate_config) - : pacer_(clock, &packet_router_, event_log), + : clock_(clock), + pacer_(clock, &packet_router_, event_log), send_side_cc_( rtc::MakeUnique(clock, nullptr /* observer */, event_log, &pacer_)), bitrate_configurator_(bitrate_config), - process_thread_(ProcessThread::Create("SendControllerThread")) { + process_thread_(ProcessThread::Create("SendControllerThread")), + observer_(nullptr) { send_side_cc_->SignalNetworkState(kNetworkDown); send_side_cc_->SetBweBitrates(bitrate_config.min_bitrate_bps, bitrate_config.start_bitrate_bps, @@ -45,6 +48,28 @@ RtpTransportControllerSend::~RtpTransportControllerSend() { process_thread_->DeRegisterModule(&pacer_); } +void RtpTransportControllerSend::OnNetworkChanged(uint32_t bitrate_bps, + uint8_t fraction_loss, + int64_t rtt_ms, + int64_t probing_interval_ms) { + // TODO(srte): Skip this step when old SendSideCongestionController is + // deprecated. + TargetTransferRate msg; + msg.at_time = Timestamp::ms(clock_->TimeInMilliseconds()); + msg.target_rate = DataRate::bps(bitrate_bps); + msg.network_estimate.at_time = msg.at_time; + msg.network_estimate.bwe_period = TimeDelta::ms(probing_interval_ms); + uint32_t bandwidth_bps; + if (send_side_cc_->AvailableBandwidth(&bandwidth_bps)) + msg.network_estimate.bandwidth = DataRate::bps(bandwidth_bps); + msg.network_estimate.loss_rate_ratio = fraction_loss / 255.0; + msg.network_estimate.round_trip_time = TimeDelta::ms(rtt_ms); + rtc::CritScope cs(&observer_crit_); + // We wont register as observer until we have an observer. + RTC_DCHECK(observer_ != nullptr); + observer_->OnTargetTransferRate(msg); +} + PacketRouter* RtpTransportControllerSend::packet_router() { return &packet_router_; } @@ -91,9 +116,15 @@ void RtpTransportControllerSend::DeRegisterPacketFeedbackObserver( PacketFeedbackObserver* observer) { send_side_cc_->DeRegisterPacketFeedbackObserver(observer); } -void RtpTransportControllerSend::RegisterNetworkObserver( - NetworkChangedObserver* observer) { - send_side_cc_->RegisterNetworkObserver(observer); + +void RtpTransportControllerSend::RegisterTargetTransferRateObserver( + TargetTransferRateObserver* observer) { + { + rtc::CritScope cs(&observer_crit_); + RTC_DCHECK(observer_ == nullptr); + observer_ = observer; + } + send_side_cc_->RegisterNetworkObserver(this); } void RtpTransportControllerSend::OnNetworkRouteChanged( const std::string& transport_name, @@ -141,9 +172,6 @@ void RtpTransportControllerSend::OnNetworkAvailability(bool network_available) { RtcpBandwidthObserver* RtpTransportControllerSend::GetBandwidthObserver() { return send_side_cc_->GetBandwidthObserver(); } -bool RtpTransportControllerSend::AvailableBandwidth(uint32_t* bandwidth) const { - return send_side_cc_->AvailableBandwidth(bandwidth); -} int64_t RtpTransportControllerSend::GetPacerQueuingDelayMs() const { return pacer_.QueueInMs(); } diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index d786edbc9e..84ad424fc2 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -31,12 +31,20 @@ class RtcEventLog; // TODO(nisse): When we get the underlying transports here, we should // have one object implementing RtpTransportControllerSendInterface // per transport, sharing the same congestion controller. -class RtpTransportControllerSend : public RtpTransportControllerSendInterface { +class RtpTransportControllerSend : public RtpTransportControllerSendInterface, + public NetworkChangedObserver { public: RtpTransportControllerSend(Clock* clock, RtcEventLog* event_log, const BitrateConstraints& bitrate_config); ~RtpTransportControllerSend() override; + + // Implements NetworkChangedObserver interface. + void OnNetworkChanged(uint32_t bitrate_bps, + uint8_t fraction_loss, + int64_t rtt_ms, + int64_t probing_interval_ms) override; + // Implements RtpTransportControllerSendInterface PacketRouter* packet_router() override; @@ -56,12 +64,12 @@ class RtpTransportControllerSend : public RtpTransportControllerSendInterface { PacketFeedbackObserver* observer) override; void DeRegisterPacketFeedbackObserver( PacketFeedbackObserver* observer) override; - void RegisterNetworkObserver(NetworkChangedObserver* observer) override; + void RegisterTargetTransferRateObserver( + TargetTransferRateObserver* observer) override; void OnNetworkRouteChanged(const std::string& transport_name, const rtc::NetworkRoute& network_route) override; void OnNetworkAvailability(bool network_available) override; RtcpBandwidthObserver* GetBandwidthObserver() override; - bool AvailableBandwidth(uint32_t* bandwidth) const override; int64_t GetPacerQueuingDelayMs() const override; int64_t GetFirstPacketTimeMs() const override; void EnablePeriodicAlrProbing(bool enable) override; @@ -72,6 +80,7 @@ class RtpTransportControllerSend : public RtpTransportControllerSendInterface { const BitrateConstraintsMask& preferences) override; private: + const Clock* const clock_; PacketRouter packet_router_; PacedSender pacer_; const std::unique_ptr send_side_cc_; @@ -79,6 +88,8 @@ class RtpTransportControllerSend : public RtpTransportControllerSendInterface { RtpBitrateConfigurator bitrate_configurator_; std::map network_routes_; const std::unique_ptr process_thread_; + rtc::CriticalSection observer_crit_; + TargetTransferRateObserver* observer_ RTC_GUARDED_BY(observer_crit_); 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 07c7f17098..50f80d5e42 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -25,7 +25,7 @@ struct NetworkRoute; namespace webrtc { class CallStatsObserver; -class NetworkChangedObserver; +class TargetTransferRateObserver; class Module; class PacedSender; class PacketFeedbackObserver; @@ -90,13 +90,13 @@ class RtpTransportControllerSendInterface { PacketFeedbackObserver* observer) = 0; virtual void DeRegisterPacketFeedbackObserver( PacketFeedbackObserver* observer) = 0; - virtual void RegisterNetworkObserver(NetworkChangedObserver* observer) = 0; + virtual void RegisterTargetTransferRateObserver( + TargetTransferRateObserver* observer) = 0; virtual void OnNetworkRouteChanged( const std::string& transport_name, const rtc::NetworkRoute& network_route) = 0; virtual void OnNetworkAvailability(bool network_available) = 0; virtual RtcpBandwidthObserver* GetBandwidthObserver() = 0; - virtual bool AvailableBandwidth(uint32_t* bandwidth) const = 0; virtual int64_t GetPacerQueuingDelayMs() const = 0; virtual int64_t GetFirstPacketTimeMs() const = 0; virtual void EnablePeriodicAlrProbing(bool enable) = 0; diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h index 1cbbb77ce8..25a1252b4d 100644 --- a/call/test/mock_rtp_transport_controller_send.h +++ b/call/test/mock_rtp_transport_controller_send.h @@ -39,7 +39,8 @@ class MockRtpTransportControllerSend MOCK_METHOD0(GetCallStatsObserver, CallStatsObserver*()); MOCK_METHOD1(RegisterPacketFeedbackObserver, void(PacketFeedbackObserver*)); MOCK_METHOD1(DeRegisterPacketFeedbackObserver, void(PacketFeedbackObserver*)); - MOCK_METHOD1(RegisterNetworkObserver, void(NetworkChangedObserver*)); + MOCK_METHOD1(RegisterTargetTransferRateObserver, + void(TargetTransferRateObserver*)); MOCK_METHOD2(OnNetworkRouteChanged, void(const std::string&, const rtc::NetworkRoute&)); MOCK_METHOD1(OnNetworkAvailability, void(bool)); diff --git a/modules/congestion_controller/include/network_changed_observer.h b/modules/congestion_controller/include/network_changed_observer.h index bb95a3d703..34ac32aed1 100644 --- a/modules/congestion_controller/include/network_changed_observer.h +++ b/modules/congestion_controller/include/network_changed_observer.h @@ -15,11 +15,18 @@ namespace webrtc { +// Note: This interface will be deprecated in favor of the +// TargetTransferRateObserver interface. The new interface provides more +// information about the network connection and uses structs to make it easier +// to add fields. + // Observer class for bitrate changes announced due to change in bandwidth // estimate or due to that the send pacer is full. Fraction loss and rtt is // also part of this callback to allow the observer to optimize its settings // for different types of network environments. The bitrate does not include // packet headers and is measured in bits per second. +// TODO(srte): Deprecate and remove this class when SendSideCongestionController +// is no longer using this as part of our public API. class NetworkChangedObserver { public: virtual void OnNetworkChanged(uint32_t bitrate_bps, diff --git a/modules/congestion_controller/network_control/include/network_control.h b/modules/congestion_controller/network_control/include/network_control.h index d87acbf649..fedd794a0b 100644 --- a/modules/congestion_controller/network_control/include/network_control.h +++ b/modules/congestion_controller/network_control/include/network_control.h @@ -18,10 +18,20 @@ namespace webrtc { +class TargetTransferRateObserver { + public: + // Called to indicate target transfer rate as well as giving information about + // the current estimate of network parameters. + virtual void OnTargetTransferRate(TargetTransferRate) = 0; + + protected: + virtual ~TargetTransferRateObserver() = default; +}; + // NetworkControllerObserver is an interface implemented by observers of network // controllers. It contains declarations of the possible configuration messages // that can be sent from a network controller implementation. -class NetworkControllerObserver { +class NetworkControllerObserver : public TargetTransferRateObserver { public: // Called when congestion window configutation is changed. virtual void OnCongestionWindow(CongestionWindow) = 0; @@ -29,12 +39,6 @@ class NetworkControllerObserver { virtual void OnPacerConfig(PacerConfig) = 0; // Called to indicate that a new probe should be sent. virtual void OnProbeClusterConfig(ProbeClusterConfig) = 0; - // Called to indicate target transfer rate as well as giving information about - // the current estimate of network parameters. - virtual void OnTargetTransferRate(TargetTransferRate) = 0; - - protected: - virtual ~NetworkControllerObserver() = default; }; // NetworkControllerInterface is implemented by network controllers. A network