From 39ac25d6ecc9bd035bb5191d46186915cd1bf445 Mon Sep 17 00:00:00 2001 From: Per K Date: Wed, 7 Feb 2024 14:16:20 +0100 Subject: [PATCH] Add PeerConnectionInterface::ReconfigureBandwidthEstimation Using the Api, BWE components are recreated and new settings can be applied. Initially, the only configuration available is allowing BWE probes without media". Note that BWE components are created when transport first becomes writable. So calling this method before a PeerConnection is connected is cheap and only changes configuration. Integration test in https://webrtc-review.googlesource.com/c/src/+/337322 Bug: webrtc:14928 Change-Id: If2c848489bf94a1f7a5ebf90d2886d90c202c7c3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/336240 Reviewed-by: Harald Alvestrand Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#41687} --- api/BUILD.gn | 1 + api/peer_connection_interface.h | 8 +++ api/test/mock_peerconnectioninterface.h | 4 ++ api/transport/BUILD.gn | 6 +++ api/transport/bandwidth_estimation_settings.h | 27 ++++++++++ call/BUILD.gn | 1 + call/rtp_transport_config.h | 4 -- call/rtp_transport_controller_send.cc | 50 ++++++++++++++----- call/rtp_transport_controller_send.h | 5 +- .../rtp_transport_controller_send_interface.h | 4 ++ .../test/mock_rtp_transport_controller_send.h | 4 ++ pc/BUILD.gn | 1 + pc/peer_connection.cc | 9 ++++ pc/peer_connection.h | 2 + pc/peer_connection_proxy.h | 4 ++ pc/test/fake_peer_connection_base.h | 3 ++ pc/test/mock_peer_connection_internal.h | 4 ++ 17 files changed, 120 insertions(+), 17 deletions(-) create mode 100644 api/transport/bandwidth_estimation_settings.h diff --git a/api/BUILD.gn b/api/BUILD.gn index 1f03468d65..61c000d3f0 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -355,6 +355,7 @@ rtc_library("libjingle_peerconnection_api") { "neteq:neteq_api", "rtc_event_log", "task_queue", + "transport:bandwidth_estimation_settings", "transport:bitrate_settings", "transport:enums", "transport:network_control", diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 7699f33438..38699ec98a 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -112,6 +112,7 @@ #include "api/set_remote_description_observer_interface.h" #include "api/stats/rtc_stats_collector_callback.h" #include "api/task_queue/task_queue_factory.h" +#include "api/transport/bandwidth_estimation_settings.h" #include "api/transport/bitrate_settings.h" #include "api/transport/enums.h" #include "api/transport/network_control.h" @@ -1133,6 +1134,13 @@ class RTC_EXPORT PeerConnectionInterface : public webrtc::RefCountInterface { // to the provided value. virtual RTCError SetBitrate(const BitrateSettings& bitrate) = 0; + // Allows an application to reconfigure bandwidth estimation. + // The method can be called both before and after estimation has started. + // Estimation starts when the first RTP packet is sent. + // Estimation will be restarted if already started. + virtual void ReconfigureBandwidthEstimation( + const BandwidthEstimationSettings& settings) {} + // Enable/disable playout of received audio streams. Enabled by default. Note // that even if playout is enabled, streams will only be played out if the // appropriate SDP is also applied. Setting `playout` to false will stop diff --git a/api/test/mock_peerconnectioninterface.h b/api/test/mock_peerconnectioninterface.h index ccc6ce46b1..22a77d7dfe 100644 --- a/api/test/mock_peerconnectioninterface.h +++ b/api/test/mock_peerconnectioninterface.h @@ -177,6 +177,10 @@ class MockPeerConnectionInterface : public webrtc::PeerConnectionInterface { (const std::vector&), (override)); MOCK_METHOD(RTCError, SetBitrate, (const BitrateSettings&), (override)); + MOCK_METHOD(void, + ReconfigureBandwidthEstimation, + (const BandwidthEstimationSettings&), + (override)); MOCK_METHOD(void, SetAudioPlayout, (bool), (override)); MOCK_METHOD(void, SetAudioRecording, (bool), (override)); MOCK_METHOD(rtc::scoped_refptr, diff --git a/api/transport/BUILD.gn b/api/transport/BUILD.gn index 84a0968ee9..f7a27c5899 100644 --- a/api/transport/BUILD.gn +++ b/api/transport/BUILD.gn @@ -18,6 +18,12 @@ rtc_library("bitrate_settings") { absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] } +rtc_library("bandwidth_estimation_settings") { + visibility = [ "*" ] + sources = [ "bandwidth_estimation_settings.h" ] + deps = [ "../../rtc_base/system:rtc_export" ] +} + rtc_source_set("enums") { visibility = [ "*" ] sources = [ "enums.h" ] diff --git a/api/transport/bandwidth_estimation_settings.h b/api/transport/bandwidth_estimation_settings.h new file mode 100644 index 0000000000..7ae8cc9ef8 --- /dev/null +++ b/api/transport/bandwidth_estimation_settings.h @@ -0,0 +1,27 @@ +/* + * 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 API_TRANSPORT_BANDWIDTH_ESTIMATION_SETTINGS_H_ +#define API_TRANSPORT_BANDWIDTH_ESTIMATION_SETTINGS_H_ + +#include "rtc_base/system/rtc_export.h" +namespace webrtc { +// Configuration settings affecting bandwidth estimation. +// These settings can be set and changed by an application. +struct RTC_EXPORT BandwidthEstimationSettings { + // A bandwith estimation probe may be sent using a RtpTransceiver with + // direction SendOnly or SendRecv that supports RTX. The probe can be sent + // without first sending media packets in which case Rtp padding packets are + // used. + bool allow_probe_without_media = false; +}; + +} // namespace webrtc +#endif // API_TRANSPORT_BANDWIDTH_ESTIMATION_SETTINGS_H_ diff --git a/call/BUILD.gn b/call/BUILD.gn index 8af3eac754..613b2b94d2 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -117,6 +117,7 @@ rtc_library("rtp_interfaces") { "../api/crypto:options", "../api/environment", "../api/rtc_event_log", + "../api/transport:bandwidth_estimation_settings", "../api/transport:bitrate_settings", "../api/transport:network_control", "../api/units:time_delta", diff --git a/call/rtp_transport_config.h b/call/rtp_transport_config.h index 412ff78e08..cce5214fc8 100644 --- a/call/rtp_transport_config.h +++ b/call/rtp_transport_config.h @@ -38,10 +38,6 @@ struct RtpTransportConfig { // The burst interval of the pacer, see TaskQueuePacedSender constructor. absl::optional pacer_burst_interval; - - // A bandwith estimation probe may be sent on a writable Rtp stream that have - // RTX configured. It can be sent without first sending media packets. - bool allow_bandwidth_estimation_probe_without_media = false; }; } // namespace webrtc diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index d439a2f898..32c4addfb9 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -70,8 +70,6 @@ bool IsRelayed(const rtc::NetworkRoute& route) { RtpTransportControllerSend::RtpTransportControllerSend( const RtpTransportConfig& config) : env_(config.env), - allow_bandwidth_estimation_probe_without_media_( - config.allow_bandwidth_estimation_probe_without_media), task_queue_(TaskQueueBase::Current()), bitrate_configurator_(config.bitrate_config), pacer_started_(false), @@ -168,7 +166,7 @@ void RtpTransportControllerSend::RegisterSendingRtpStream( packet_router_.AddSendRtpModule(&rtp_module, /*remb_candidate=*/true); pacer_.SetAllowProbeWithoutMediaPacket( - allow_bandwidth_estimation_probe_without_media_ && + bwe_settings_.allow_probe_without_media && packet_router_.SupportsRtxPayloadPadding()); } @@ -188,7 +186,7 @@ void RtpTransportControllerSend::DeRegisterSendingRtpStream( pacer_.RemovePacketsForSsrc(*rtp_module.FlexfecSsrc()); } pacer_.SetAllowProbeWithoutMediaPacket( - allow_bandwidth_estimation_probe_without_media_ && + bwe_settings_.allow_probe_without_media && packet_router_.SupportsRtxPayloadPadding()); } @@ -257,6 +255,29 @@ RtpTransportControllerSend::GetStreamFeedbackProvider() { return &feedback_demuxer_; } +void RtpTransportControllerSend::ReconfigureBandwidthEstimation( + const BandwidthEstimationSettings& settings) { + RTC_DCHECK_RUN_ON(&sequence_checker_); + bwe_settings_ = settings; + + if (controller_) { + // Recreate the controller and handler. + control_handler_ = nullptr; + controller_ = nullptr; + // The BWE controller is created when/if the network is available. + MaybeCreateControllers(); + if (controller_) { + BitrateConstraints constraints = bitrate_configurator_.GetConfig(); + UpdateBitrateConstraints(constraints); + UpdateStreamsConfig(); + UpdateNetworkAvailability(); + } + } + pacer_.SetAllowProbeWithoutMediaPacket( + bwe_settings_.allow_probe_without_media && + packet_router_.SupportsRtxPayloadPadding()); +} + void RtpTransportControllerSend::RegisterTargetTransferRateObserver( TargetTransferRateObserver* observer) { RTC_DCHECK_RUN_ON(&sequence_checker_); @@ -358,9 +379,6 @@ void RtpTransportControllerSend::OnNetworkAvailability(bool network_available) { RTC_DCHECK_RUN_ON(&sequence_checker_); RTC_LOG(LS_VERBOSE) << "SignalNetworkState " << (network_available ? "Up" : "Down"); - NetworkAvailability msg; - msg.at_time = Timestamp::Millis(env_.clock().TimeInMilliseconds()); - msg.network_available = network_available; network_available_ = network_available; if (network_available) { pacer_.Resume(); @@ -373,11 +391,7 @@ void RtpTransportControllerSend::OnNetworkAvailability(bool network_available) { if (!controller_) { MaybeCreateControllers(); } - if (controller_) { - control_handler_->SetNetworkAvailability(network_available); - PostUpdates(controller_->OnNetworkAvailability(msg)); - UpdateControlState(); - } + UpdateNetworkAvailability(); for (auto& rtp_sender : video_rtp_senders_) { rtp_sender->OnNetworkAvailability(network_available); } @@ -620,6 +634,18 @@ void RtpTransportControllerSend::MaybeCreateControllers() { StartProcessPeriodicTasks(); } +void RtpTransportControllerSend::UpdateNetworkAvailability() { + if (!controller_) { + return; + } + NetworkAvailability msg; + msg.at_time = Timestamp::Millis(env_.clock().TimeInMilliseconds()); + msg.network_available = network_available_; + control_handler_->SetNetworkAvailability(network_available_); + PostUpdates(controller_->OnNetworkAvailability(msg)); + UpdateControlState(); +} + void RtpTransportControllerSend::UpdateInitialConstraints( TargetRateConstraints new_contraints) { if (!new_contraints.starting_rate) diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index 3c84beb65f..4428336672 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -84,6 +84,8 @@ class RtpTransportControllerSend final RtpPacketSender* packet_sender() override; void SetAllocatedSendBitrateLimits(BitrateAllocationLimits limits) override; + void ReconfigureBandwidthEstimation( + const BandwidthEstimationSettings& settings) override; void SetPacingFactor(float pacing_factor) override; void SetQueueTimeLimit(int limit_ms) override; @@ -127,6 +129,7 @@ class RtpTransportControllerSend final private: void MaybeCreateControllers() RTC_RUN_ON(sequence_checker_); + void UpdateNetworkAvailability() RTC_RUN_ON(sequence_checker_); void UpdateInitialConstraints(TargetRateConstraints new_contraints) RTC_RUN_ON(sequence_checker_); @@ -150,7 +153,6 @@ class RtpTransportControllerSend final const Environment env_; SequenceChecker sequence_checker_; - const bool allow_bandwidth_estimation_probe_without_media_; TaskQueueBase* task_queue_; PacketRouter packet_router_; std::vector> video_rtp_senders_ @@ -158,6 +160,7 @@ class RtpTransportControllerSend final RtpBitrateConfigurator bitrate_configurator_; std::map network_routes_ RTC_GUARDED_BY(sequence_checker_); + BandwidthEstimationSettings bwe_settings_ RTC_GUARDED_BY(sequence_checker_); bool pacer_started_ RTC_GUARDED_BY(sequence_checker_); TaskQueuePacedSender pacer_; diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h index 1938d86c57..c4b1536bad 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -24,6 +24,7 @@ #include "api/fec_controller.h" #include "api/frame_transformer_interface.h" #include "api/rtc_event_log/rtc_event_log.h" +#include "api/transport/bandwidth_estimation_settings.h" #include "api/transport/bitrate_settings.h" #include "api/units/timestamp.h" #include "call/rtp_config.h" @@ -125,6 +126,9 @@ class RtpTransportControllerSendInterface { virtual void SetAllocatedSendBitrateLimits( BitrateAllocationLimits limits) = 0; + virtual void ReconfigureBandwidthEstimation( + const BandwidthEstimationSettings& settings) = 0; + virtual void SetPacingFactor(float pacing_factor) = 0; virtual void SetQueueTimeLimit(int limit_ms) = 0; diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h index 2a0bc233ff..63f686eb3c 100644 --- a/call/test/mock_rtp_transport_controller_send.h +++ b/call/test/mock_rtp_transport_controller_send.h @@ -69,6 +69,10 @@ class MockRtpTransportControllerSend SetAllocatedSendBitrateLimits, (BitrateAllocationLimits), (override)); + MOCK_METHOD(void, + ReconfigureBandwidthEstimation, + (const BandwidthEstimationSettings&), + (override)); MOCK_METHOD(void, SetPacingFactor, (float), (override)); MOCK_METHOD(void, SetQueueTimeLimit, (int), (override)); MOCK_METHOD(StreamFeedbackProvider*, diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 1734ab551c..14971b8af1 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -426,6 +426,7 @@ rtc_source_set("peer_connection_proxy") { deps = [ ":proxy", "../api:libjingle_peerconnection_api", + "../api/transport:bandwidth_estimation_settings", ] } diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 3ab2c52130..e90d1e83f9 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1681,6 +1681,15 @@ RTCError PeerConnection::SetBitrate(const BitrateSettings& bitrate) { return RTCError::OK(); } +void PeerConnection::ReconfigureBandwidthEstimation( + const BandwidthEstimationSettings& settings) { + worker_thread()->PostTask(SafeTask(worker_thread_safety_, [this, settings]() { + RTC_DCHECK_RUN_ON(worker_thread()); + call_->GetTransportControllerSend()->ReconfigureBandwidthEstimation( + settings); + })); +} + void PeerConnection::SetAudioPlayout(bool playout) { if (!worker_thread()->IsCurrent()) { worker_thread()->BlockingCall( diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 406bc1cef2..3c923139dc 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -236,6 +236,8 @@ class PeerConnection : public PeerConnectionInternal, const std::vector& candidates) override; RTCError SetBitrate(const BitrateSettings& bitrate) override; + void ReconfigureBandwidthEstimation( + const BandwidthEstimationSettings& settings) override; void SetAudioPlayout(bool playout) override; void SetAudioRecording(bool recording) override; diff --git a/pc/peer_connection_proxy.h b/pc/peer_connection_proxy.h index 6db27f2dd5..aaccfe45d0 100644 --- a/pc/peer_connection_proxy.h +++ b/pc/peer_connection_proxy.h @@ -16,6 +16,7 @@ #include #include "api/peer_connection_interface.h" +#include "api/transport/bandwidth_estimation_settings.h" #include "pc/proxy.h" namespace webrtc { @@ -137,6 +138,9 @@ PROXY_METHOD2(void, std::function) PROXY_METHOD1(bool, RemoveIceCandidates, const std::vector&) PROXY_METHOD1(RTCError, SetBitrate, const BitrateSettings&) +PROXY_METHOD1(void, + ReconfigureBandwidthEstimation, + const BandwidthEstimationSettings&) PROXY_METHOD1(void, SetAudioPlayout, bool) PROXY_METHOD1(void, SetAudioRecording, bool) // This method will be invoked on the network thread. See diff --git a/pc/test/fake_peer_connection_base.h b/pc/test/fake_peer_connection_base.h index 1615088e99..9e4ed6d175 100644 --- a/pc/test/fake_peer_connection_base.h +++ b/pc/test/fake_peer_connection_base.h @@ -197,6 +197,9 @@ class FakePeerConnectionBase : public PeerConnectionInternal { return RTCError(RTCErrorType::UNSUPPORTED_OPERATION, "Not implemented"); } + void ReconfigureBandwidthEstimation( + const BandwidthEstimationSettings& settings) override {} + void SetAudioPlayout(bool playout) override {} void SetAudioRecording(bool recording) override {} diff --git a/pc/test/mock_peer_connection_internal.h b/pc/test/mock_peer_connection_internal.h index 5fd7a50b4f..b5f47cc46a 100644 --- a/pc/test/mock_peer_connection_internal.h +++ b/pc/test/mock_peer_connection_internal.h @@ -170,6 +170,10 @@ class MockPeerConnectionInternal : public PeerConnectionInternal { (const std::vector&), (override)); MOCK_METHOD(RTCError, SetBitrate, (const BitrateSettings&), (override)); + MOCK_METHOD(void, + ReconfigureBandwidthEstimation, + (const BandwidthEstimationSettings&), + (override)); MOCK_METHOD(void, SetAudioPlayout, (bool), (override)); MOCK_METHOD(void, SetAudioRecording, (bool), (override)); MOCK_METHOD(rtc::scoped_refptr,