Delete deprecated ReceiveSideCongestionController constructor This makes Environment and thus field trials a required parameter, thus RemoteBitrateEstimators member no longer need to fallback to the global field trial string. Bug: webrtc:42220378 Change-Id: Ieb6ff442d5fde5fa9715573c758a7e078f0ceea4 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349922 Commit-Queue: Danil Chapovalov <danilchap@webrtc.org> Reviewed-by: Per Kjellander <perkj@webrtc.org> Cr-Commit-Position: refs/heads/main@{#42314}
diff --git a/modules/congestion_controller/include/receive_side_congestion_controller.h b/modules/congestion_controller/include/receive_side_congestion_controller.h index b4cbfb6..9cbad11 100644 --- a/modules/congestion_controller/include/receive_side_congestion_controller.h +++ b/modules/congestion_controller/include/receive_side_congestion_controller.h
@@ -42,12 +42,6 @@ RembThrottler::RembSender remb_sender, absl::Nullable<NetworkStateEstimator*> network_state_estimator); - [[deprecated]] ReceiveSideCongestionController( - Clock* clock, - RemoteEstimatorProxy::TransportFeedbackSender feedback_sender, - RembThrottler::RembSender remb_sender, - NetworkStateEstimator* network_state_estimator); - ~ReceiveSideCongestionController() override = default; void OnReceivedPacket(const RtpPacketReceived& packet, MediaType media_type); @@ -80,7 +74,7 @@ void PickEstimator(bool has_absolute_send_time) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_); - Clock& clock_; + const Environment env_; RembThrottler remb_throttler_; RemoteEstimatorProxy remote_estimator_proxy_;
diff --git a/modules/congestion_controller/receive_side_congestion_controller.cc b/modules/congestion_controller/receive_side_congestion_controller.cc index 3bcc82f..317858c 100644 --- a/modules/congestion_controller/receive_side_congestion_controller.cc +++ b/modules/congestion_controller/receive_side_congestion_controller.cc
@@ -51,7 +51,7 @@ << "WrappingBitrateEstimator: Switching to absolute send time RBE."; using_absolute_send_time_ = true; rbe_ = std::make_unique<RemoteBitrateEstimatorAbsSendTime>( - &remb_throttler_, &clock_); + env_, &remb_throttler_); } packets_since_absolute_send_time_ = 0; } else { @@ -64,7 +64,7 @@ "time offset RBE."; using_absolute_send_time_ = false; rbe_ = std::make_unique<RemoteBitrateEstimatorSingleStream>( - &remb_throttler_, &clock_); + env_, &remb_throttler_); } } } @@ -75,26 +75,13 @@ RemoteEstimatorProxy::TransportFeedbackSender feedback_sender, RembThrottler::RembSender remb_sender, absl::Nullable<NetworkStateEstimator*> network_state_estimator) - : clock_(env.clock()), - remb_throttler_(std::move(remb_sender), &clock_), + : env_(env), + remb_throttler_(std::move(remb_sender), &env_.clock()), remote_estimator_proxy_(std::move(feedback_sender), network_state_estimator), - rbe_( - std::make_unique<RemoteBitrateEstimatorSingleStream>(&remb_throttler_, - &clock_)), - using_absolute_send_time_(false), - packets_since_absolute_send_time_(0) {} - -ReceiveSideCongestionController::ReceiveSideCongestionController( - Clock* clock, - RemoteEstimatorProxy::TransportFeedbackSender feedback_sender, - RembThrottler::RembSender remb_sender, - NetworkStateEstimator* network_state_estimator) - : clock_(*clock), - remb_throttler_(std::move(remb_sender), clock), - remote_estimator_proxy_(std::move(feedback_sender), - network_state_estimator), - rbe_(new RemoteBitrateEstimatorSingleStream(&remb_throttler_, clock)), + rbe_(std::make_unique<RemoteBitrateEstimatorSingleStream>( + env_, + &remb_throttler_)), using_absolute_send_time_(false), packets_since_absolute_send_time_(0) {} @@ -125,7 +112,7 @@ } TimeDelta ReceiveSideCongestionController::MaybeProcess() { - Timestamp now = clock_.CurrentTime(); + Timestamp now = env_.clock().CurrentTime(); mutex_.Lock(); TimeDelta time_until_rbe = rbe_->Process(); mutex_.Unlock();
diff --git a/modules/remote_bitrate_estimator/BUILD.gn b/modules/remote_bitrate_estimator/BUILD.gn index b9346ae..d691ad7 100644 --- a/modules/remote_bitrate_estimator/BUILD.gn +++ b/modules/remote_bitrate_estimator/BUILD.gn
@@ -36,7 +36,7 @@ "../../api:field_trials_view", "../../api:network_state_predictor_api", "../../api:rtp_headers", - "../../api/transport:field_trial_based_config", + "../../api/environment", "../../api/transport:network_control", "../../api/units:data_rate", "../../api/units:data_size", @@ -55,10 +55,10 @@ "../../rtc_base/experiments:field_trial_parser", "../../rtc_base/synchronization:mutex", "../../system_wrappers", - "../../system_wrappers:field_trial", "../../system_wrappers:metrics", ] absl_deps = [ + "//third_party/abseil-cpp/absl/base:nullability", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:optional", ] @@ -120,6 +120,7 @@ deps = [ ":remote_bitrate_estimator", "..:module_api_public", + "../../api/environment:environment_factory", "../../api/transport:mock_network_control", "../../api/transport:network_control", "../../api/units:data_rate",
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc index fcfbb2e..278ad9d 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
@@ -16,7 +16,8 @@ #include <memory> #include <utility> -#include "api/transport/field_trial_based_config.h" +#include "absl/base/nullability.h" +#include "api/environment/environment.h" #include "api/units/data_rate.h" #include "api/units/data_size.h" #include "api/units/time_delta.h" @@ -88,11 +89,9 @@ } RemoteBitrateEstimatorAbsSendTime::RemoteBitrateEstimatorAbsSendTime( - RemoteBitrateObserver* observer, - Clock* clock) - : clock_(clock), observer_(observer), remote_rate_(field_trials_) { - RTC_DCHECK(clock_); - RTC_DCHECK(observer_); + const Environment& env, + absl::Nonnull<RemoteBitrateObserver*> observer) + : env_(env), observer_(observer), remote_rate_(env_.field_trials()) { RTC_LOG(LS_INFO) << "RemoteBitrateEstimatorAbsSendTime: Instantiating."; } @@ -225,7 +224,7 @@ Timestamp send_time = Timestamp::Millis(static_cast<int64_t>(timestamp) * kTimestampToMs); - Timestamp now = clock_->CurrentTime(); + Timestamp now = env_.clock().CurrentTime(); // TODO(holmer): SSRCs are only needed for REMB, should be broken out from // here.
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h index 9bcdfb8..e48f3ab 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
@@ -19,8 +19,9 @@ #include <memory> #include <vector> +#include "absl/base/nullability.h" +#include "api/environment/environment.h" #include "api/rtp_headers.h" -#include "api/transport/field_trial_based_config.h" #include "api/units/data_rate.h" #include "api/units/data_size.h" #include "api/units/time_delta.h" @@ -32,14 +33,14 @@ #include "modules/remote_bitrate_estimator/overuse_estimator.h" #include "rtc_base/bitrate_tracker.h" #include "rtc_base/checks.h" -#include "system_wrappers/include/clock.h" namespace webrtc { class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator { public: - RemoteBitrateEstimatorAbsSendTime(RemoteBitrateObserver* observer, - Clock* clock); + RemoteBitrateEstimatorAbsSendTime( + const Environment& env, + absl::Nonnull<RemoteBitrateObserver*> observer); RemoteBitrateEstimatorAbsSendTime() = delete; RemoteBitrateEstimatorAbsSendTime(const RemoteBitrateEstimatorAbsSendTime&) = @@ -98,9 +99,8 @@ void TimeoutStreams(Timestamp now); - Clock* const clock_; - const FieldTrialBasedConfig field_trials_; - RemoteBitrateObserver* const observer_; + const Environment env_; + const absl::Nonnull<RemoteBitrateObserver*> observer_; std::unique_ptr<InterArrival> inter_arrival_; std::unique_ptr<OveruseEstimator> estimator_; OveruseDetector detector_;
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc index d8ef23c..6cdf8a6 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc
@@ -10,6 +10,9 @@ #include "modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h" +#include <memory> + +#include "api/environment/environment_factory.h" #include "modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h" #include "test/gtest.h" @@ -25,12 +28,10 @@ RemoteBitrateEstimatorAbsSendTimeTest& operator=( const RemoteBitrateEstimatorAbsSendTimeTest&) = delete; - virtual void SetUp() { - bitrate_estimator_.reset(new RemoteBitrateEstimatorAbsSendTime( - bitrate_observer_.get(), &clock_)); + void SetUp() override { + bitrate_estimator_ = std::make_unique<RemoteBitrateEstimatorAbsSendTime>( + CreateEnvironment(&clock_), bitrate_observer_.get()); } - - protected: }; TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, InitialBehavior) {
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc index 1605008..9db92ac 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
@@ -13,7 +13,9 @@ #include <cstdint> #include <utility> +#include "absl/base/nullability.h" #include "absl/types/optional.h" +#include "api/environment/environment.h" #include "modules/remote_bitrate_estimator/aimd_rate_control.h" #include "modules/remote_bitrate_estimator/include/bwe_defines.h" #include "modules/remote_bitrate_estimator/inter_arrival.h" @@ -39,13 +41,13 @@ inter_arrival(90 * kTimestampGroupLengthMs, kTimestampToMs) {} RemoteBitrateEstimatorSingleStream::RemoteBitrateEstimatorSingleStream( - RemoteBitrateObserver* observer, - Clock* clock) - : clock_(clock), + const Environment& env, + absl::Nonnull<RemoteBitrateObserver*> observer) + : env_(env), + observer_(observer), incoming_bitrate_(kBitrateWindow), last_valid_incoming_bitrate_(DataRate::Zero()), - remote_rate_(field_trials_), - observer_(observer), + remote_rate_(env_.field_trials()), process_interval_(kProcessInterval), uma_recorded_(false) { RTC_LOG(LS_INFO) << "RemoteBitrateEstimatorSingleStream: Instantiating."; @@ -68,7 +70,7 @@ uint32_t ssrc = rtp_packet.Ssrc(); uint32_t rtp_timestamp = rtp_packet.Timestamp() + transmission_time_offset.value_or(0); - Timestamp now = clock_->CurrentTime(); + Timestamp now = env_.clock().CurrentTime(); Detector& estimator = overuse_detectors_[ssrc]; estimator.last_packet_time = now; @@ -114,7 +116,7 @@ } TimeDelta RemoteBitrateEstimatorSingleStream::Process() { - Timestamp now = clock_->CurrentTime(); + Timestamp now = env_.clock().CurrentTime(); Timestamp next_process_time = last_process_time_.has_value() ? *last_process_time_ + process_interval_ : now;
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h index 44976ca..16a0097 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
@@ -17,8 +17,9 @@ #include <map> #include <vector> +#include "absl/base/nullability.h" #include "absl/types/optional.h" -#include "api/transport/field_trial_based_config.h" +#include "api/environment/environment.h" #include "api/units/data_rate.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" @@ -31,13 +32,11 @@ namespace webrtc { -class Clock; -struct RTPHeader; - class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator { public: - RemoteBitrateEstimatorSingleStream(RemoteBitrateObserver* observer, - Clock* clock); + RemoteBitrateEstimatorSingleStream( + const Environment& env, + absl::Nonnull<RemoteBitrateObserver*> observer); RemoteBitrateEstimatorSingleStream() = delete; RemoteBitrateEstimatorSingleStream( @@ -68,13 +67,12 @@ std::vector<uint32_t> GetSsrcs() const; - Clock* const clock_; - const FieldTrialBasedConfig field_trials_; + const Environment env_; + const absl::Nonnull<RemoteBitrateObserver*> observer_; std::map<uint32_t, Detector> overuse_detectors_; BitrateTracker incoming_bitrate_; DataRate last_valid_incoming_bitrate_; AimdRateControl remote_rate_; - RemoteBitrateObserver* const observer_; absl::optional<Timestamp> last_process_time_; TimeDelta process_interval_; bool uma_recorded_;
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc index 3d72807..15696dc 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc
@@ -10,6 +10,9 @@ #include "modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h" +#include <memory> + +#include "api/environment/environment_factory.h" #include "modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.h" #include "test/gtest.h" @@ -24,12 +27,10 @@ RemoteBitrateEstimatorSingleTest& operator=( const RemoteBitrateEstimatorSingleTest&) = delete; - virtual void SetUp() { - bitrate_estimator_.reset(new RemoteBitrateEstimatorSingleStream( - bitrate_observer_.get(), &clock_)); + void SetUp() override { + bitrate_estimator_ = std::make_unique<RemoteBitrateEstimatorSingleStream>( + CreateEnvironment(&clock_), bitrate_observer_.get()); } - - protected: }; TEST_F(RemoteBitrateEstimatorSingleTest, InitialBehavior) {