From 9c296e5b9ebf8609e56ff21f117597eaffe53096 Mon Sep 17 00:00:00 2001
From: Andrey Logvin <landrey@webrtc.org>
Date: Wed, 2 Dec 2020 18:45:09 +0000
Subject: [PATCH] Fix DVQA cpu usage when Stop is called multiple times

Bug: webrtc:12247
Change-Id: I946338e0ecf58f91c87c8638977a8bc52e648fd2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/196083
Commit-Queue: Andrey Logvin <landrey@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32747}
---
 .../video/default_video_quality_analyzer.cc   |  2 +-
 .../video/default_video_quality_analyzer.h    |  2 +-
 .../default_video_quality_analyzer_test.cc    | 58 +++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)

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 3765f3dec8..23f94339e1 100644
--- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
+++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.cc
@@ -496,7 +496,6 @@ void DefaultVideoQualityAnalyzer::OnDecoderError(absl::string_view peer_name,
 }
 
 void DefaultVideoQualityAnalyzer::Stop() {
-  StopMeasuringCpuProcessTime();
   {
     MutexLock lock(&lock_);
     if (state_ == State::kStopped) {
@@ -504,6 +503,7 @@ void DefaultVideoQualityAnalyzer::Stop() {
     }
     state_ = State::kStopped;
   }
+  StopMeasuringCpuProcessTime();
   comparison_available_event_.Set();
   for (auto& thread : thread_pool_) {
     thread->Stop();
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 08fc466bed..a1811338e0 100644
--- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h
+++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer.h
@@ -237,6 +237,7 @@ class DefaultVideoQualityAnalyzer : public VideoQualityAnalyzerInterface {
   // obtained by calling GetKnownVideoStreams()
   std::map<StatsKey, StreamStats> GetStats() const;
   AnalyzerStats GetAnalyzerStats() const;
+  double GetCpuUsagePercent();
 
  private:
   struct FrameStats {
@@ -500,7 +501,6 @@ class DefaultVideoQualityAnalyzer : public VideoQualityAnalyzerInterface {
   void StopMeasuringCpuProcessTime();
   void StartExcludingCpuThreadTime();
   void StopExcludingCpuThreadTime();
-  double GetCpuUsagePercent();
 
   // TODO(titovartem) restore const when old constructor will be removed.
   DefaultVideoQualityAnalyzerOptions options_;
diff --git a/test/pc/e2e/analyzer/video/default_video_quality_analyzer_test.cc b/test/pc/e2e/analyzer/video/default_video_quality_analyzer_test.cc
index 20155bb099..9e6e5e4de2 100644
--- a/test/pc/e2e/analyzer/video/default_video_quality_analyzer_test.cc
+++ b/test/pc/e2e/analyzer/video/default_video_quality_analyzer_test.cc
@@ -687,6 +687,64 @@ TEST(DefaultVideoQualityAnalyzerTest,
   EXPECT_GE(it->second.ssim.GetMin(), kMaxSsim);
 }
 
+TEST(DefaultVideoQualityAnalyzerTest, CpuUsage) {
+  std::unique_ptr<test::FrameGeneratorInterface> frame_generator =
+      test::CreateSquareFrameGenerator(kFrameWidth, kFrameHeight,
+                                       /*type=*/absl::nullopt,
+                                       /*num_squares=*/absl::nullopt);
+
+  DefaultVideoQualityAnalyzer analyzer(Clock::GetRealTimeClock(),
+                                       AnalyzerOptionsForTest());
+  analyzer.Start("test_case",
+                 std::vector<std::string>{kSenderPeerName, kReceiverPeerName},
+                 kAnalyzerMaxThreadsCount);
+
+  std::map<uint16_t, VideoFrame> captured_frames;
+  std::vector<uint16_t> frames_order;
+  for (int i = 0; i < kMaxFramesInFlightPerStream; ++i) {
+    VideoFrame frame = NextFrame(frame_generator.get(), i);
+    frame.set_id(
+        analyzer.OnFrameCaptured(kSenderPeerName, kStreamLabel, frame));
+    frames_order.push_back(frame.id());
+    captured_frames.insert({frame.id(), frame});
+    analyzer.OnFramePreEncode(kSenderPeerName, frame);
+    analyzer.OnFrameEncoded(kSenderPeerName, frame.id(), FakeEncode(frame),
+                            VideoQualityAnalyzerInterface::EncoderStats());
+  }
+
+  for (size_t i = 1; i < frames_order.size(); i += 2) {
+    uint16_t frame_id = frames_order.at(i);
+    VideoFrame received_frame = DeepCopy(captured_frames.at(frame_id));
+    analyzer.OnFramePreDecode(kReceiverPeerName, received_frame.id(),
+                              FakeEncode(received_frame));
+    analyzer.OnFrameDecoded(kReceiverPeerName, received_frame,
+                            VideoQualityAnalyzerInterface::DecoderStats());
+    analyzer.OnFrameRendered(kReceiverPeerName, received_frame);
+  }
+
+  // Give analyzer some time to process frames on async thread. The computations
+  // have to be fast (heavy metrics are disabled!), so if doesn't fit 100ms it
+  // means we have an issue!
+  SleepMs(100);
+  analyzer.Stop();
+
+  double cpu_usage = analyzer.GetCpuUsagePercent();
+  // On windows bots GetProcessCpuTimeNanos doesn't work properly (returns the
+  // same number over the whole run). Adhoc solution to prevent them from
+  // failing.
+  // TODO(12249): remove it after issue is fixed.
+#if defined(WEBRTC_WIN)
+  ASSERT_GE(cpu_usage, 0);
+#else
+  ASSERT_GT(cpu_usage, 0);
+#endif
+
+  SleepMs(100);
+  analyzer.Stop();
+
+  EXPECT_EQ(analyzer.GetCpuUsagePercent(), cpu_usage);
+}
+
 }  // namespace
 }  // namespace webrtc_pc_e2e
 }  // namespace webrtc