Cleanup RemoteBitrateEstimatorSingleStream
Store Detectos in a map by value instead of by unccessary pointer
Bug: None
Change-Id: Iab9904aafca02d9f9ae6633c87de860a5bd62ac7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/313621
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40499}
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 077315f..070b2fc 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
@@ -28,6 +28,10 @@
namespace webrtc {
namespace {
+
+constexpr int kTimestampGroupLengthMs = 5;
+constexpr double kTimestampToMs = 1.0 / 90.0;
+
absl::optional<DataRate> OptionalRateFromOptionalBps(
absl::optional<int> bitrate_bps) {
if (bitrate_bps) {
@@ -38,19 +42,8 @@
}
} // namespace
-enum { kTimestampGroupLengthMs = 5 };
-static const double kTimestampToMs = 1.0 / 90.0;
-
-struct RemoteBitrateEstimatorSingleStream::Detector {
- explicit Detector(int64_t last_packet_time_ms)
- : last_packet_time_ms(last_packet_time_ms),
- inter_arrival(90 * kTimestampGroupLengthMs, kTimestampToMs) {}
-
- int64_t last_packet_time_ms;
- InterArrival inter_arrival;
- OveruseEstimator estimator;
- OveruseDetector detector;
-};
+RemoteBitrateEstimatorSingleStream::Detector::Detector()
+ : inter_arrival(90 * kTimestampGroupLengthMs, kTimestampToMs) {}
RemoteBitrateEstimatorSingleStream::RemoteBitrateEstimatorSingleStream(
RemoteBitrateObserver* observer,
@@ -66,13 +59,8 @@
RTC_LOG(LS_INFO) << "RemoteBitrateEstimatorSingleStream: Instantiating.";
}
-RemoteBitrateEstimatorSingleStream::~RemoteBitrateEstimatorSingleStream() {
- while (!overuse_detectors_.empty()) {
- SsrcOveruseEstimatorMap::iterator it = overuse_detectors_.begin();
- delete it->second;
- overuse_detectors_.erase(it);
- }
-}
+RemoteBitrateEstimatorSingleStream::~RemoteBitrateEstimatorSingleStream() =
+ default;
void RemoteBitrateEstimatorSingleStream::IncomingPacket(
const RtpPacketReceived& rtp_packet) {
@@ -89,20 +77,8 @@
uint32_t rtp_timestamp =
rtp_packet.Timestamp() + transmission_time_offset.value_or(0);
int64_t now_ms = clock_->TimeInMilliseconds();
- SsrcOveruseEstimatorMap::iterator it = overuse_detectors_.find(ssrc);
- if (it == overuse_detectors_.end()) {
- // This is a new SSRC. Adding to map.
- // TODO(holmer): If the channel changes SSRC the old SSRC will still be
- // around in this map until the channel is deleted. This is OK since the
- // callback will no longer be called for the old SSRC. This will be
- // automatically cleaned up when we have one RemoteBitrateEstimator per REMB
- // group.
- std::pair<SsrcOveruseEstimatorMap::iterator, bool> insert_result =
- overuse_detectors_.insert(std::make_pair(ssrc, new Detector(now_ms)));
- it = insert_result.first;
- }
- Detector* estimator = it->second;
- estimator->last_packet_time_ms = now_ms;
+ Detector& estimator = overuse_detectors_[ssrc];
+ estimator.last_packet_time_ms = now_ms;
// Check if incoming bitrate estimate is valid, and if it needs to be reset.
absl::optional<uint32_t> incoming_bitrate = incoming_bitrate_.Rate(now_ms);
@@ -118,21 +94,20 @@
size_t payload_size = rtp_packet.payload_size() + rtp_packet.padding_size();
incoming_bitrate_.Update(payload_size, now_ms);
- const BandwidthUsage prior_state = estimator->detector.State();
+ const BandwidthUsage prior_state = estimator.detector.State();
uint32_t timestamp_delta = 0;
int64_t time_delta = 0;
int size_delta = 0;
- if (estimator->inter_arrival.ComputeDeltas(
+ if (estimator.inter_arrival.ComputeDeltas(
rtp_timestamp, rtp_packet.arrival_time().ms(), now_ms, payload_size,
×tamp_delta, &time_delta, &size_delta)) {
double timestamp_delta_ms = timestamp_delta * kTimestampToMs;
- estimator->estimator.Update(time_delta, timestamp_delta_ms, size_delta,
- estimator->detector.State(), now_ms);
- estimator->detector.Detect(estimator->estimator.offset(),
- timestamp_delta_ms,
- estimator->estimator.num_of_deltas(), now_ms);
+ estimator.estimator.Update(time_delta, timestamp_delta_ms, size_delta,
+ estimator.detector.State(), now_ms);
+ estimator.detector.Detect(estimator.estimator.offset(), timestamp_delta_ms,
+ estimator.estimator.num_of_deltas(), now_ms);
}
- if (estimator->detector.State() == BandwidthUsage::kBwOverusing) {
+ if (estimator.detector.State() == BandwidthUsage::kBwOverusing) {
absl::optional<uint32_t> incoming_bitrate_bps =
incoming_bitrate_.Rate(now_ms);
if (incoming_bitrate_bps &&
@@ -162,21 +137,19 @@
void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t now_ms) {
BandwidthUsage bw_state = BandwidthUsage::kBwNormal;
- SsrcOveruseEstimatorMap::iterator it = overuse_detectors_.begin();
+ auto it = overuse_detectors_.begin();
while (it != overuse_detectors_.end()) {
- const int64_t time_of_last_received_packet =
- it->second->last_packet_time_ms;
+ const int64_t time_of_last_received_packet = it->second.last_packet_time_ms;
if (time_of_last_received_packet >= 0 &&
now_ms - time_of_last_received_packet > kStreamTimeOutMs) {
// This over-use detector hasn't received packets for `kStreamTimeOutMs`
// milliseconds and is considered stale.
- delete it->second;
overuse_detectors_.erase(it++);
} else {
// Make sure that we trigger an over-use if any of the over-use detectors
// is detecting over-use.
- if (it->second->detector.State() > bw_state) {
- bw_state = it->second->detector.State();
+ if (it->second.detector.State() > bw_state) {
+ bw_state = it->second.detector.State();
}
++it;
}
@@ -193,10 +166,8 @@
if (remote_rate_.ValidEstimate()) {
process_interval_ms_ = remote_rate_.GetFeedbackInterval().ms();
RTC_DCHECK_GT(process_interval_ms_, 0);
- std::vector<uint32_t> ssrcs;
- GetSsrcs(&ssrcs);
if (observer_)
- observer_->OnReceiveBitrateChanged(ssrcs, target_bitrate);
+ observer_->OnReceiveBitrateChanged(GetSsrcs(), target_bitrate);
}
}
@@ -205,12 +176,8 @@
remote_rate_.SetRtt(TimeDelta::Millis(avg_rtt_ms));
}
-void RemoteBitrateEstimatorSingleStream::RemoveStream(unsigned int ssrc) {
- SsrcOveruseEstimatorMap::iterator it = overuse_detectors_.find(ssrc);
- if (it != overuse_detectors_.end()) {
- delete it->second;
- overuse_detectors_.erase(it);
- }
+void RemoteBitrateEstimatorSingleStream::RemoveStream(uint32_t ssrc) {
+ overuse_detectors_.erase(ssrc);
}
DataRate RemoteBitrateEstimatorSingleStream::LatestEstimate() const {
@@ -220,15 +187,13 @@
return remote_rate_.LatestEstimate();
}
-void RemoteBitrateEstimatorSingleStream::GetSsrcs(
- std::vector<uint32_t>* ssrcs) const {
- RTC_DCHECK(ssrcs);
- ssrcs->resize(overuse_detectors_.size());
- int i = 0;
- for (SsrcOveruseEstimatorMap::const_iterator it = overuse_detectors_.begin();
- it != overuse_detectors_.end(); ++it, ++i) {
- (*ssrcs)[i] = it->first;
+std::vector<uint32_t> RemoteBitrateEstimatorSingleStream::GetSsrcs() const {
+ std::vector<uint32_t> ssrcs;
+ ssrcs.reserve(overuse_detectors_.size());
+ for (const auto& [ssrc, unused] : overuse_detectors_) {
+ ssrcs.push_back(ssrc);
}
+ return ssrcs;
}
} // namespace webrtc
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 05ec03d..d0ca675 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
@@ -23,6 +23,9 @@
#include "api/units/timestamp.h"
#include "modules/remote_bitrate_estimator/aimd_rate_control.h"
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
+#include "modules/remote_bitrate_estimator/inter_arrival.h"
+#include "modules/remote_bitrate_estimator/overuse_detector.h"
+#include "modules/remote_bitrate_estimator/overuse_estimator.h"
#include "rtc_base/rate_statistics.h"
namespace webrtc {
@@ -50,18 +53,23 @@
DataRate LatestEstimate() const override;
private:
- struct Detector;
+ struct Detector {
+ Detector();
- typedef std::map<uint32_t, Detector*> SsrcOveruseEstimatorMap;
+ int64_t last_packet_time_ms;
+ InterArrival inter_arrival;
+ OveruseEstimator estimator;
+ OveruseDetector detector;
+ };
// Triggers a new estimate calculation.
void UpdateEstimate(int64_t time_now);
- void GetSsrcs(std::vector<uint32_t>* ssrcs) const;
+ std::vector<uint32_t> GetSsrcs() const;
Clock* const clock_;
const FieldTrialBasedConfig field_trials_;
- SsrcOveruseEstimatorMap overuse_detectors_;
+ std::map<uint32_t, Detector> overuse_detectors_;
RateStatistics incoming_bitrate_;
uint32_t last_valid_incoming_bitrate_;
AimdRateControl remote_rate_;
