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) {
