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

Fix: DefaultVideoQualityAnalyzerFramesComparator::Stop() may not block

Browse Source 
DefaultVideoQualityAnalyzerFramesComparator::Stop() may not block until
all frames comparisons are processed in case when new comparison was
added after worker thread checked for available comparisons and Stop()
was invoked before worker thread checked the state_.

Bug: webrtc:13277, webrtc:13240
Change-Id: Ic16fdc01e43c04529cd83e5d9ef66d7573973cfd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/235205
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35212}
This commit is contained in:
Artem Titov 2021-10-14 15:17:48 +02:00 committed by WebRTC LUCI CQ
parent 6f8fa5af77
commit 46536e3da4
2 changed files with 7 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
test/pc/e2e/analyzer/video/default_video_quality_analyzer_frames_comparator.cc
View File

@ -327,6 +327,7 @@ void DefaultVideoQualityAnalyzerFramesComparator::ProcessComparisons() {
while (true) {
// Try to pick next comparison to perform from the queue.
absl::optional<FrameComparison> comparison = absl::nullopt;
bool more_new_comparisons_expected;
{
MutexLock lock(&mutex_);
if (!comparisons_.empty()) {
@ -336,16 +337,11 @@ void DefaultVideoQualityAnalyzerFramesComparator::ProcessComparisons() {
comparison_available_event_.Set();
}
}
// If state is stopped => no new frame comparisons are expected.
more_new_comparisons_expected = state_ != State::kStopped;
}
if (!comparison) {
bool more_frames_expected;
{
// If there are no comparisons and state is stopped =>
// no more frames expected.
MutexLock lock(&mutex_);
more_frames_expected = state_ != State::kStopped;
}
if (!more_frames_expected) {
if (!more_new_comparisons_expected) {
comparison_available_event_.Set();
return;
}

6
test/pc/e2e/analyzer/video/default_video_quality_analyzer_frames_comparator_test.cc
View File

@ -117,7 +117,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
/*captured=*/absl::nullopt,
/*rendered=*/absl::nullopt,
FrameComparisonType::kRegular, frame_stats);
comparator.Stop({});
comparator.Stop(/*last_rendered_frame_times=*/{});
std::map<InternalStatsKey, StreamStats> stats = comparator.stream_stats();
EXPECT_DOUBLE_EQ(GetFirstOrDie(stats.at(stats_key).transport_time_ms), 20.0);
@ -161,7 +161,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
/*captured=*/absl::nullopt,
/*rendered=*/absl::nullopt,
FrameComparisonType::kRegular, frame_stats2);
comparator.Stop({});
comparator.Stop(/*last_rendered_frame_times=*/{});
std::map<InternalStatsKey, StreamStats> stats = comparator.stream_stats();
EXPECT_DOUBLE_EQ(
@ -244,7 +244,7 @@ TEST(DefaultVideoQualityAnalyzerFramesComparatorTest,
/*rendered=*/absl::nullopt,
FrameComparisonType::kRegular,
stats[stats.size() - 1]);
comparator.Stop({});
comparator.Stop(/*last_rendered_frame_times=*/{});
EXPECT_EQ(comparator.stream_stats().size(), 1lu);
StreamStats result_stats = comparator.stream_stats().at(stats_key);