admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove mutexes from remote bitrate estimators

Browse Source 
They are called only by ReceivedSideCongestionController that already
ensures all access is synchronized.

Bug: None
Change-Id: I0f87e24e3fbb0bd8f6ff679fb949d2373c554fba
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269300
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39622}
This commit is contained in:
Danil Chapovalov 2023-03-21 13:18:37 +01:00 committed by WebRTC LUCI CQ
parent 6a78e93346
commit 198d0d7fd5
5 changed files with 85 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
modules/remote_bitrate_estimator/BUILD.gn
View File

@ -32,14 +32,6 @@ rtc_library("remote_bitrate_estimator") {
"test/bwe_test_logging.h",
]
if (rtc_enable_bwe_test_logging) {
defines = [ "BWE_TEST_LOGGING_COMPILE_TIME_ENABLE=1" ]
sources += [ "test/bwe_test_logging.cc" ]
} else {
defines = [ "BWE_TEST_LOGGING_COMPILE_TIME_ENABLE=0" ]
}
deps = [
"../../api:field_trials_view",
"../../api:network_state_predictor_api",
@ -56,9 +48,6 @@ rtc_library("remote_bitrate_estimator") {
"../../modules/rtp_rtcp:rtp_rtcp_format",
"../../rtc_base:checks",
"../../rtc_base:logging",
"../../rtc_base:macromagic",
"../../rtc_base:platform_thread",
"../../rtc_base:race_checker",
"../../rtc_base:rate_statistics",
"../../rtc_base:rtc_numerics",
"../../rtc_base:safe_minmax",
@ -73,6 +62,14 @@ rtc_library("remote_bitrate_estimator") {
"//third_party/abseil-cpp/absl/strings",
"//third_party/abseil-cpp/absl/types:optional",
]
if (rtc_enable_bwe_test_logging) {
defines = [ "BWE_TEST_LOGGING_COMPILE_TIME_ENABLE=1" ]
sources += [ "test/bwe_test_logging.cc" ]
deps += [ "../../rtc_base:platform_thread" ]
} else {
defines = [ "BWE_TEST_LOGGING_COMPILE_TIME_ENABLE=0" ]
}
}
if (!build_with_chromium) {

137
modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
View File

@ -25,7 +25,6 @@
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/thread_annotations.h"
#include "system_wrappers/include/metrics.h"
namespace webrtc {
@ -215,7 +214,6 @@ void RemoteBitrateEstimatorAbsSendTime::IncomingPacket(
int64_t arrival_time_ms,
size_t payload_size,
const RTPHeader& header) {
RTC_DCHECK_RUNS_SERIALIZED(&network_race_);
if (!header.extension.hasAbsoluteSendTime) {
RTC_LOG(LS_WARNING)
<< "RemoteBitrateEstimatorAbsSendTimeImpl: Incoming packet "
@ -271,85 +269,81 @@ void RemoteBitrateEstimatorAbsSendTime::IncomingPacketInfo(
int size_delta = 0;
bool update_estimate = false;
DataRate target_bitrate = DataRate::Zero();
std::vector<uint32_t> ssrcs;
{
MutexLock lock(&mutex_);
TimeoutStreams(now);
RTC_DCHECK(inter_arrival_);
RTC_DCHECK(estimator_);
ssrcs_.insert_or_assign(ssrc, now);
TimeoutStreams(now);
RTC_DCHECK(inter_arrival_);
RTC_DCHECK(estimator_);
ssrcs_.insert_or_assign(ssrc, now);
// For now only try to detect probes while we don't have a valid estimate.
// We currently assume that only packets larger than 200 bytes are paced by
// the sender.
static constexpr DataSize kMinProbePacketSize = DataSize::Bytes(200);
if (payload_size > kMinProbePacketSize &&
(!remote_rate_.ValidEstimate() ||
now - first_packet_time_ < kInitialProbingInterval)) {
// TODO(holmer): Use a map instead to get correct order?
if (total_probes_received_ < kMaxProbePackets) {
TimeDelta send_delta = TimeDelta::Millis(-1);
TimeDelta recv_delta = TimeDelta::Millis(-1);
if (!probes_.empty()) {
send_delta = send_time - probes_.back().send_time;
recv_delta = arrival_time - probes_.back().recv_time;
}
RTC_LOG(LS_INFO) << "Probe packet received: send time="
<< send_time.ms()
<< " ms, recv time=" << arrival_time.ms()
<< " ms, send delta=" << send_delta.ms()
<< " ms, recv delta=" << recv_delta.ms() << " ms.";
// For now only try to detect probes while we don't have a valid estimate.
// We currently assume that only packets larger than 200 bytes are paced by
// the sender.
static constexpr DataSize kMinProbePacketSize = DataSize::Bytes(200);
if (payload_size > kMinProbePacketSize &&
(!remote_rate_.ValidEstimate() ||
now - first_packet_time_ < kInitialProbingInterval)) {
// TODO(holmer): Use a map instead to get correct order?
if (total_probes_received_ < kMaxProbePackets) {
TimeDelta send_delta = TimeDelta::Millis(-1);
TimeDelta recv_delta = TimeDelta::Millis(-1);
if (!probes_.empty()) {
send_delta = send_time - probes_.back().send_time;
recv_delta = arrival_time - probes_.back().recv_time;
}
probes_.emplace_back(send_time, arrival_time, payload_size);
++total_probes_received_;
// Make sure that a probe which updated the bitrate immediately has an
// effect by calling the OnReceiveBitrateChanged callback.
if (ProcessClusters(now) == ProbeResult::kBitrateUpdated)
update_estimate = true;
}
if (inter_arrival_->ComputeDeltas(timestamp, arrival_time.ms(), now.ms(),
payload_size.bytes(), &ts_delta, &t_delta,
&size_delta)) {
double ts_delta_ms = (1000.0 * ts_delta) / (1 << kInterArrivalShift);
estimator_->Update(t_delta, ts_delta_ms, size_delta, detector_.State(),
arrival_time.ms());
detector_.Detect(estimator_->offset(), ts_delta_ms,
estimator_->num_of_deltas(), arrival_time.ms());
RTC_LOG(LS_INFO) << "Probe packet received: send time=" << send_time.ms()
<< " ms, recv time=" << arrival_time.ms()
<< " ms, send delta=" << send_delta.ms()
<< " ms, recv delta=" << recv_delta.ms() << " ms.";
}
probes_.emplace_back(send_time, arrival_time, payload_size);
++total_probes_received_;
// Make sure that a probe which updated the bitrate immediately has an
// effect by calling the OnReceiveBitrateChanged callback.
if (ProcessClusters(now) == ProbeResult::kBitrateUpdated)
update_estimate = true;
}
if (inter_arrival_->ComputeDeltas(timestamp, arrival_time.ms(), now.ms(),
payload_size.bytes(), &ts_delta, &t_delta,
&size_delta)) {
double ts_delta_ms = (1000.0 * ts_delta) / (1 << kInterArrivalShift);
estimator_->Update(t_delta, ts_delta_ms, size_delta, detector_.State(),
arrival_time.ms());
detector_.Detect(estimator_->offset(), ts_delta_ms,
estimator_->num_of_deltas(), arrival_time.ms());
}
if (!update_estimate) {
// Check if it's time for a periodic update or if we should update because
// of an over-use.
if (last_update_.IsInfinite() ||
now.ms() - last_update_.ms() >
remote_rate_.GetFeedbackInterval().ms()) {
if (!update_estimate) {
// Check if it's time for a periodic update or if we should update because
// of an over-use.
if (last_update_.IsInfinite() ||
now.ms() - last_update_.ms() >
remote_rate_.GetFeedbackInterval().ms()) {
update_estimate = true;
} else if (detector_.State() == BandwidthUsage::kBwOverusing) {
absl::optional<uint32_t> incoming_rate =
incoming_bitrate_.Rate(arrival_time.ms());
if (incoming_rate && remote_rate_.TimeToReduceFurther(
now, DataRate::BitsPerSec(*incoming_rate))) {
update_estimate = true;
} else if (detector_.State() == BandwidthUsage::kBwOverusing) {
absl::optional<uint32_t> incoming_rate =
incoming_bitrate_.Rate(arrival_time.ms());
if (incoming_rate && remote_rate_.TimeToReduceFurther(
now, DataRate::BitsPerSec(*incoming_rate))) {
update_estimate = true;
}
}
}
if (update_estimate) {
// The first overuse should immediately trigger a new estimate.
// We also have to update the estimate immediately if we are overusing
// and the target bitrate is too high compared to what we are receiving.
const RateControlInput input(
detector_.State(), OptionalRateFromOptionalBps(
incoming_bitrate_.Rate(arrival_time.ms())));
target_bitrate = remote_rate_.Update(&input, now);
update_estimate = remote_rate_.ValidEstimate();
ssrcs = Keys(ssrcs_);
}
}
if (update_estimate) {
// The first overuse should immediately trigger a new estimate.
// We also have to update the estimate immediately if we are overusing
// and the target bitrate is too high compared to what we are receiving.
const RateControlInput input(
detector_.State(),
OptionalRateFromOptionalBps(incoming_bitrate_.Rate(arrival_time.ms())));
target_bitrate = remote_rate_.Update(&input, now);
update_estimate = remote_rate_.ValidEstimate();
}
if (update_estimate) {
last_update_ = now;
observer_->OnReceiveBitrateChanged(ssrcs, target_bitrate.bps<uint32_t>());
observer_->OnReceiveBitrateChanged(Keys(ssrcs_),
target_bitrate.bps<uint32_t>());
}
}
@ -378,18 +372,15 @@ void RemoteBitrateEstimatorAbsSendTime::TimeoutStreams(Timestamp now) {
void RemoteBitrateEstimatorAbsSendTime::OnRttUpdate(int64_t avg_rtt_ms,
int64_t /*max_rtt_ms*/) {
MutexLock lock(&mutex_);
remote_rate_.SetRtt(TimeDelta::Millis(avg_rtt_ms));
}
void RemoteBitrateEstimatorAbsSendTime::RemoveStream(uint32_t ssrc) {
MutexLock lock(&mutex_);
ssrcs_.erase(ssrc);
}
DataRate RemoteBitrateEstimatorAbsSendTime::LatestEstimate() const {
// Currently accessed only from the worker thread (see Call::GetStats()).
MutexLock lock(&mutex_);
if (!remote_rate_.ValidEstimate() || ssrcs_.empty()) {
return DataRate::Zero();
}

17
modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
View File

@ -31,10 +31,7 @@
#include "modules/remote_bitrate_estimator/overuse_detector.h"
#include "modules/remote_bitrate_estimator/overuse_estimator.h"
#include "rtc_base/checks.h"
#include "rtc_base/race_checker.h"
#include "rtc_base/rate_statistics.h"
#include "rtc_base/synchronization/mutex.h"
#include "rtc_base/thread_annotations.h"
#include "system_wrappers/include/clock.h"
namespace webrtc {
@ -102,15 +99,12 @@ class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator {
const Cluster* FindBestProbe(const std::list<Cluster>& clusters) const;
// Returns true if a probe which changed the estimate was detected.
ProbeResult ProcessClusters(Timestamp now)
RTC_EXCLUSIVE_LOCKS_REQUIRED(&mutex_);
ProbeResult ProcessClusters(Timestamp now);
bool IsBitrateImproving(DataRate probe_bitrate) const
RTC_EXCLUSIVE_LOCKS_REQUIRED(&mutex_);
bool IsBitrateImproving(DataRate probe_bitrate) const;
void TimeoutStreams(Timestamp now) RTC_EXCLUSIVE_LOCKS_REQUIRED(&mutex_);
void TimeoutStreams(Timestamp now);
rtc::RaceChecker network_race_;
Clock* const clock_;
const FieldTrialBasedConfig field_trials_;
RemoteBitrateObserver* const observer_;
@ -125,9 +119,8 @@ class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator {
Timestamp last_update_ = Timestamp::MinusInfinity();
bool uma_recorded_ = false;
mutable Mutex mutex_;
std::map<uint32_t, Timestamp> ssrcs_ RTC_GUARDED_BY(&mutex_);
AimdRateControl remote_rate_ RTC_GUARDED_BY(&mutex_);
std::map<uint32_t, Timestamp> ssrcs_;
AimdRateControl remote_rate_;
};
} // namespace webrtc

5
modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
View File

@ -94,7 +94,6 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(
uint32_t rtp_timestamp =
header.timestamp + header.extension.transmissionTimeOffset;
int64_t now_ms = clock_->TimeInMilliseconds();
MutexLock lock(&mutex_);
SsrcOveruseEstimatorMap::iterator it = overuse_detectors_.find(ssrc);
if (it == overuse_detectors_.end()) {
// This is a new SSRC. Adding to map.
@ -156,7 +155,6 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(
}
TimeDelta RemoteBitrateEstimatorSingleStream::Process() {
MutexLock lock(&mutex_);
int64_t now_ms = clock_->TimeInMilliseconds();
int64_t next_process_time_ms = last_process_time_ + process_interval_ms_;
if (last_process_time_ == -1 || now_ms >= next_process_time_ms) {
@ -210,12 +208,10 @@ void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t now_ms) {
void RemoteBitrateEstimatorSingleStream::OnRttUpdate(int64_t avg_rtt_ms,
int64_t max_rtt_ms) {
MutexLock lock(&mutex_);
remote_rate_.SetRtt(TimeDelta::Millis(avg_rtt_ms));
}
void RemoteBitrateEstimatorSingleStream::RemoveStream(unsigned int ssrc) {
MutexLock lock(&mutex_);
SsrcOveruseEstimatorMap::iterator it = overuse_detectors_.find(ssrc);
if (it != overuse_detectors_.end()) {
delete it->second;
@ -224,7 +220,6 @@ void RemoteBitrateEstimatorSingleStream::RemoveStream(unsigned int ssrc) {
}
DataRate RemoteBitrateEstimatorSingleStream::LatestEstimate() const {
MutexLock lock(&mutex_);
if (!remote_rate_.ValidEstimate() || overuse_detectors_.empty()) {
return DataRate::Zero();
}

20
modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
View File

@ -24,8 +24,6 @@
#include "modules/remote_bitrate_estimator/aimd_rate_control.h"
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "rtc_base/rate_statistics.h"
#include "rtc_base/synchronization/mutex.h"
#include "rtc_base/thread_annotations.h"
namespace webrtc {
@ -59,21 +57,19 @@ class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator {
typedef std::map<uint32_t, Detector*> SsrcOveruseEstimatorMap;
// Triggers a new estimate calculation.
void UpdateEstimate(int64_t time_now) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
void UpdateEstimate(int64_t time_now);
void GetSsrcs(std::vector<uint32_t>* ssrcs) const
RTC_SHARED_LOCKS_REQUIRED(mutex_);
void GetSsrcs(std::vector<uint32_t>* ssrcs) const;
Clock* const clock_;
const FieldTrialBasedConfig field_trials_;
SsrcOveruseEstimatorMap overuse_detectors_ RTC_GUARDED_BY(mutex_);
RateStatistics incoming_bitrate_ RTC_GUARDED_BY(mutex_);
uint32_t last_valid_incoming_bitrate_ RTC_GUARDED_BY(mutex_);
AimdRateControl remote_rate_ RTC_GUARDED_BY(mutex_);
RemoteBitrateObserver* const observer_ RTC_GUARDED_BY(mutex_);
mutable Mutex mutex_;
SsrcOveruseEstimatorMap overuse_detectors_;
RateStatistics incoming_bitrate_;
uint32_t last_valid_incoming_bitrate_;
AimdRateControl remote_rate_;
RemoteBitrateObserver* const observer_;
int64_t last_process_time_;
int64_t process_interval_ms_ RTC_GUARDED_BY(mutex_);
int64_t process_interval_ms_;
bool uma_recorded_;
};