Cleanup RemoteBitrateEstimate::LatestEstimate function
Return the bitrate estimate as DataRate type
Remove list of affected ssrcs as unused
Bug: None
Change-Id: Ie31dce591d861624736d834194f90eb6c93f70f6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267280
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37397}
diff --git a/modules/congestion_controller/include/receive_side_congestion_controller.h b/modules/congestion_controller/include/receive_side_congestion_controller.h
index 8fc88d2..09b8245 100644
--- a/modules/congestion_controller/include/receive_side_congestion_controller.h
+++ b/modules/congestion_controller/include/receive_side_congestion_controller.h
@@ -94,8 +94,7 @@
void RemoveStream(unsigned int ssrc);
- bool LatestEstimate(std::vector<unsigned int>* ssrcs,
- unsigned int* bitrate_bps) const;
+ DataRate LatestEstimate() const;
void SetMinBitrate(int min_bitrate_bps);
diff --git a/modules/congestion_controller/receive_side_congestion_controller.cc b/modules/congestion_controller/receive_side_congestion_controller.cc
index f157635..38717a1 100644
--- a/modules/congestion_controller/receive_side_congestion_controller.cc
+++ b/modules/congestion_controller/receive_side_congestion_controller.cc
@@ -62,11 +62,11 @@
rbe_->RemoveStream(ssrc);
}
-bool ReceiveSideCongestionController::WrappingBitrateEstimator::LatestEstimate(
- std::vector<unsigned int>* ssrcs,
- unsigned int* bitrate_bps) const {
+DataRate
+ReceiveSideCongestionController::WrappingBitrateEstimator::LatestEstimate()
+ const {
MutexLock lock(&mutex_);
- return rbe_->LatestEstimate(ssrcs, bitrate_bps);
+ return rbe_->LatestEstimate();
}
void ReceiveSideCongestionController::WrappingBitrateEstimator::SetMinBitrate(
@@ -143,13 +143,7 @@
}
DataRate ReceiveSideCongestionController::LatestReceiveSideEstimate() const {
- std::vector<uint32_t> unused_ssrcs;
- uint32_t bitrate_bps = 0;
- if (remote_bitrate_estimator_.LatestEstimate(&unused_ssrcs, &bitrate_bps)) {
- return DataRate::BitsPerSec(bitrate_bps);
- } else {
- return DataRate::Zero();
- }
+ return remote_bitrate_estimator_.LatestEstimate();
}
void ReceiveSideCongestionController::RemoveStream(uint32_t ssrc) {
diff --git a/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h b/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
index cc94fb9..d45f870 100644
--- a/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
+++ b/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h
@@ -17,6 +17,7 @@
#include <memory>
#include <vector>
+#include "api/units/data_rate.h"
#include "api/units/time_delta.h"
#include "modules/include/module_common_types.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
@@ -54,11 +55,8 @@
// Removes all data for `ssrc`.
virtual void RemoveStream(uint32_t ssrc) = 0;
- // Returns true if a valid estimate exists and sets `bitrate_bps` to the
- // estimated payload bitrate in bits per second. `ssrcs` is the list of ssrcs
- // currently being received and of which the bitrate estimate is based upon.
- virtual bool LatestEstimate(std::vector<uint32_t>* ssrcs,
- uint32_t* bitrate_bps) const = 0;
+ // Returns latest estimate or DataRate::Zero() if estimation is unavailable.
+ virtual DataRate LatestEstimate() const = 0;
virtual void SetMinBitrate(int min_bitrate_bps) = 0;
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 cedd4d1..934190c 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
@@ -387,26 +387,13 @@
ssrcs_.erase(ssrc);
}
-bool RemoteBitrateEstimatorAbsSendTime::LatestEstimate(
- std::vector<uint32_t>* ssrcs,
- uint32_t* bitrate_bps) const {
- // Currently accessed from both the process thread (see
- // ModuleRtpRtcpImpl::Process()) and the configuration thread (see
- // Call::GetStats()). Should in the future only be accessed from a single
- // thread.
- RTC_DCHECK(ssrcs);
- RTC_DCHECK(bitrate_bps);
+DataRate RemoteBitrateEstimatorAbsSendTime::LatestEstimate() const {
+ // Currently accessed only from the worker thread (see Call::GetStats()).
MutexLock lock(&mutex_);
- if (!remote_rate_.ValidEstimate()) {
- return false;
+ if (!remote_rate_.ValidEstimate() || ssrcs_.empty()) {
+ return DataRate::Zero();
}
- *ssrcs = Keys(ssrcs_);
- if (ssrcs_.empty()) {
- *bitrate_bps = 0;
- } else {
- *bitrate_bps = remote_rate_.LatestEstimate().bps<uint32_t>();
- }
- return true;
+ return remote_rate_.LatestEstimate();
}
void RemoteBitrateEstimatorAbsSendTime::SetMinBitrate(int min_bitrate_bps) {
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 440fbe8..c5ac10f 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
@@ -58,8 +58,7 @@
TimeDelta Process() override;
void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override;
void RemoveStream(uint32_t ssrc) override;
- bool LatestEstimate(std::vector<uint32_t>* ssrcs,
- uint32_t* bitrate_bps) const override;
+ DataRate LatestEstimate() const override;
void SetMinBitrate(int min_bitrate_bps) override;
private:
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 3869d78..3146b9e 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
@@ -224,20 +224,12 @@
}
}
-bool RemoteBitrateEstimatorSingleStream::LatestEstimate(
- std::vector<uint32_t>* ssrcs,
- uint32_t* bitrate_bps) const {
+DataRate RemoteBitrateEstimatorSingleStream::LatestEstimate() const {
MutexLock lock(&mutex_);
- RTC_DCHECK(bitrate_bps);
- if (!remote_rate_->ValidEstimate()) {
- return false;
+ if (!remote_rate_->ValidEstimate() || overuse_detectors_.empty()) {
+ return DataRate::Zero();
}
- GetSsrcs(ssrcs);
- if (ssrcs->empty())
- *bitrate_bps = 0;
- else
- *bitrate_bps = remote_rate_->LatestEstimate().bps<uint32_t>();
- return true;
+ return remote_rate_->LatestEstimate();
}
void RemoteBitrateEstimatorSingleStream::GetSsrcs(
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 033a189..4efeb3b 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
@@ -19,6 +19,7 @@
#include <vector>
#include "api/transport/field_trial_based_config.h"
+#include "api/units/data_rate.h"
#include "api/units/time_delta.h"
#include "api/units/timestamp.h"
#include "modules/remote_bitrate_estimator/aimd_rate_control.h"
@@ -51,8 +52,7 @@
TimeDelta Process() override;
void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override;
void RemoveStream(uint32_t ssrc) override;
- bool LatestEstimate(std::vector<uint32_t>* ssrcs,
- uint32_t* bitrate_bps) const override;
+ DataRate LatestEstimate() const override;
void SetMinBitrate(int min_bitrate_bps) override;
private:
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc
index 4aca22b..899037f 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc
@@ -313,15 +313,12 @@
const int kFramerate = 50; // 50 fps to avoid rounding errors.
const int kFrameIntervalMs = 1000 / kFramerate;
const uint32_t kFrameIntervalAbsSendTime = AbsSendTime(1, kFramerate);
- uint32_t bitrate_bps = 0;
uint32_t timestamp = 0;
uint32_t absolute_send_time = 0;
- std::vector<uint32_t> ssrcs;
- EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps));
- EXPECT_EQ(0u, ssrcs.size());
+ EXPECT_EQ(bitrate_estimator_->LatestEstimate(), DataRate::Zero());
clock_.AdvanceTimeMilliseconds(1000);
bitrate_estimator_->Process();
- EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps));
+ EXPECT_EQ(bitrate_estimator_->LatestEstimate(), DataRate::Zero());
EXPECT_FALSE(bitrate_observer_->updated());
bitrate_observer_->Reset();
clock_.AdvanceTimeMilliseconds(1000);
@@ -329,8 +326,7 @@
for (int i = 0; i < 5 * kFramerate + 1 + kNumInitialPackets; ++i) {
if (i == kNumInitialPackets) {
bitrate_estimator_->Process();
- EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps));
- EXPECT_EQ(0u, ssrcs.size());
+ EXPECT_EQ(bitrate_estimator_->LatestEstimate(), DataRate::Zero());
EXPECT_FALSE(bitrate_observer_->updated());
bitrate_observer_->Reset();
}
@@ -343,17 +339,13 @@
AddAbsSendTime(absolute_send_time, kFrameIntervalAbsSendTime);
}
bitrate_estimator_->Process();
- EXPECT_TRUE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps));
- ASSERT_EQ(1u, ssrcs.size());
- EXPECT_EQ(kDefaultSsrc, ssrcs.front());
+ uint32_t bitrate_bps = bitrate_estimator_->LatestEstimate().bps<uint32_t>();
EXPECT_NEAR(expected_converge_bitrate, bitrate_bps, kAcceptedBitrateErrorBps);
EXPECT_TRUE(bitrate_observer_->updated());
bitrate_observer_->Reset();
EXPECT_EQ(bitrate_observer_->latest_bitrate(), bitrate_bps);
bitrate_estimator_->RemoveStream(kDefaultSsrc);
- EXPECT_TRUE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps));
- ASSERT_EQ(0u, ssrcs.size());
- EXPECT_EQ(0u, bitrate_bps);
+ EXPECT_EQ(bitrate_estimator_->LatestEstimate(), DataRate::Zero());
}
void RemoteBitrateEstimatorTest::RateIncreaseReorderingTestHelper(
@@ -504,20 +496,11 @@
bitrate_drop_time - overuse_start_time, 33);
// Remove stream one by one.
- uint32_t latest_bps = 0;
- std::vector<uint32_t> ssrcs;
for (int i = 0; i < number_of_streams; i++) {
- EXPECT_TRUE(bitrate_estimator_->LatestEstimate(&ssrcs, &latest_bps));
- EXPECT_EQ(number_of_streams - i, static_cast<int>(ssrcs.size()));
- EXPECT_EQ(bitrate_bps, latest_bps);
- for (int j = i; j < number_of_streams; j++) {
- EXPECT_EQ(kDefaultSsrc + j, ssrcs[j - i]);
- }
+ EXPECT_EQ(bitrate_estimator_->LatestEstimate().bps(), bitrate_bps);
bitrate_estimator_->RemoveStream(kDefaultSsrc + i);
}
- EXPECT_TRUE(bitrate_estimator_->LatestEstimate(&ssrcs, &latest_bps));
- EXPECT_EQ(0u, ssrcs.size());
- EXPECT_EQ(0u, latest_bps);
+ EXPECT_EQ(bitrate_estimator_->LatestEstimate(), DataRate::Zero());
}
void RemoteBitrateEstimatorTest::TestTimestampGroupingTestHelper() {
@@ -590,9 +573,7 @@
AddAbsSendTime(absolute_send_time, kFrameIntervalAbsSendTime);
bitrate_estimator_->Process();
}
- uint32_t bitrate_before = 0;
- std::vector<uint32_t> ssrcs;
- bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_before);
+ DataRate bitrate_before = bitrate_estimator_->LatestEstimate();
clock_.AdvanceTimeMilliseconds(silence_time_s * 1000);
absolute_send_time =
@@ -607,8 +588,7 @@
AddAbsSendTime(absolute_send_time, kFrameIntervalAbsSendTime);
bitrate_estimator_->Process();
}
- uint32_t bitrate_after = 0;
- bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_after);
+ DataRate bitrate_after = bitrate_estimator_->LatestEstimate();
EXPECT_LT(bitrate_after, bitrate_before);
}
} // namespace webrtc
