Protect DefaultVideoQualityAnalyzer::peers_ with lock

Protect DefaultVideoQualityAnalyzer::peers_ with lock, because it's now
accessed from multiple threads.

Bug: webrtc:12247
Change-Id: I41932afe678979f6da9e8d0d5fe2e1ffef0fb513
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/203880
Reviewed-by: Andrey Logvin <landrey@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33073}
This commit is contained in:
Artem Titov 2021-01-25 23:52:57 +01:00 committed by Commit Bot
parent c57089a97a
commit 08f46909a8
2 changed files with 12 additions and 8 deletions

View File

@ -140,7 +140,6 @@ void DefaultVideoQualityAnalyzer::Start(
rtc::ArrayView<const std::string> peer_names,
int max_threads_count) {
test_label_ = std::move(test_case_name);
peers_ = std::make_unique<NamesCollection>(peer_names);
for (int i = 0; i < max_threads_count; i++) {
auto thread = std::make_unique<rtc::PlatformThread>(
&DefaultVideoQualityAnalyzer::ProcessComparisonsThread, this,
@ -151,6 +150,7 @@ void DefaultVideoQualityAnalyzer::Start(
}
{
MutexLock lock(&lock_);
peers_ = std::make_unique<NamesCollection>(peer_names);
RTC_CHECK(start_time_.IsMinusInfinity());
state_ = State::kActive;
@ -166,19 +166,22 @@ uint16_t DefaultVideoQualityAnalyzer::OnFrameCaptured(
// |next_frame_id| is atomic, so we needn't lock here.
uint16_t frame_id = next_frame_id_++;
Timestamp start_time = Timestamp::MinusInfinity();
size_t peer_index = peers_->index(peer_name);
size_t peer_index = -1;
size_t peers_count = -1;
size_t stream_index;
{
MutexLock lock(&lock_);
// Create a local copy of start_time_ to access it under
// |comparison_lock_| without holding a |lock_|
// Create a local copy of |start_time_|, peer's index and total peers count
// to access it under |comparison_lock_| without holding a |lock_|
start_time = start_time_;
peer_index = peers_->index(peer_name);
peers_count = peers_->size();
stream_index = streams_.AddIfAbsent(stream_label);
}
{
// Ensure stats for this stream exists.
MutexLock lock(&comparison_lock_);
for (size_t i = 0; i < peers_->size(); ++i) {
for (size_t i = 0; i < peers_count; ++i) {
if (i == peer_index) {
continue;
}
@ -956,7 +959,7 @@ StatsKey DefaultVideoQualityAnalyzer::ToStatsKey(
}
std::string DefaultVideoQualityAnalyzer::StatsKeyToMetricName(
const StatsKey& key) {
const StatsKey& key) const {
if (peers_->size() <= 2) {
return key.stream_label;
}

View File

@ -504,7 +504,8 @@ class DefaultVideoQualityAnalyzer : public VideoQualityAnalyzerInterface {
RTC_EXCLUSIVE_LOCKS_REQUIRED(lock_);
// Returns string representation of stats key for metrics naming. Used for
// backward compatibility by metrics naming for 2 peers cases.
std::string StatsKeyToMetricName(const StatsKey& key);
std::string StatsKeyToMetricName(const StatsKey& key) const
RTC_EXCLUSIVE_LOCKS_REQUIRED(lock_);
void StartMeasuringCpuProcessTime();
void StopMeasuringCpuProcessTime();
@ -517,9 +518,9 @@ class DefaultVideoQualityAnalyzer : public VideoQualityAnalyzerInterface {
std::atomic<uint16_t> next_frame_id_{0};
std::string test_label_;
std::unique_ptr<NamesCollection> peers_;
mutable Mutex lock_;
std::unique_ptr<NamesCollection> peers_ RTC_GUARDED_BY(lock_);
State state_ RTC_GUARDED_BY(lock_) = State::kNew;
Timestamp start_time_ RTC_GUARDED_BY(lock_) = Timestamp::MinusInfinity();
// Mapping from stream label to unique size_t value to use in stats and avoid