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}
diff --git a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
index 04999c3..e7aae4b 100644
--- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
+++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
@@ -140,7 +140,6 @@
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 @@
}
{
MutexLock lock(&lock_);
+ peers_ = std::make_unique<NamesCollection>(peer_names);
RTC_CHECK(start_time_.IsMinusInfinity());
state_ = State::kActive;
@@ -166,19 +166,22 @@
// |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 @@
}
std::string DefaultVideoQualityAnalyzer::StatsKeyToMetricName(
- const StatsKey& key) {
+ const StatsKey& key) const {
if (peers_->size() <= 2) {
return key.stream_label;
}
diff --git a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h
index f30e61b..de9419d 100644
--- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h
+++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h
@@ -504,7 +504,8 @@
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 @@
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