commit adbfd1d9856cf1fbd22b3d032ab482d23c6c5f93
author Markus Handell <handellm@webrtc.org> Wed Jul 08 07:32:42 2020
committer Commit Bot <commit-bot@chromium.org> Wed Jul 08 09:14:22 2020
tree 7029f8f627ce455a27619e1935087d2d4f0f17d8
parent 6cc893ad778a0965e2b7a8e614f3c98aa81bee5b

VideoAnalyzer: remove lock recursions.

This change adds thread annotations and fixes lock recursions discovered when trying to land https://webrtc-review.googlesource.com/c/src/+/178813.

Bug: webrtc:11567
Change-Id: Ib6b6dcdade063af2579664536db23d40a5949031
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178860
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31663}

video/video_analyzer.cc

diff --git a/video/video_analyzer.cc b/video/video_analyzer.cc
index f4a1c96..e04d59d 100644
--- a/video/video_analyzer.cc
+++ b/video/video_analyzer.cc
@@ -570,7 +570,7 @@
   // for this thread to be done. frames_processed_ might still be lower if
   // all comparisons are not done, but those frames are currently being
   // worked on by other threads.
-  if (comparisons_.empty() || AllFramesRecorded())
+  if (comparisons_.empty() || AllFramesRecordedLocked())
     return false;
 
   *comparison = comparisons_.front();
@@ -581,12 +581,15 @@
 }
 
 void VideoAnalyzer::FrameRecorded() {
-  rtc::CritScope crit(&comparison_lock_);
   ++frames_recorded_;
 }
 
 bool VideoAnalyzer::AllFramesRecorded() {
   rtc::CritScope crit(&comparison_lock_);
+  return AllFramesRecordedLocked();
+}
+
+bool VideoAnalyzer::AllFramesRecordedLocked() {
   RTC_DCHECK(frames_recorded_ <= frames_to_process_);
   return frames_recorded_ == frames_to_process_ ||
          (clock_->CurrentTime() > test_end_ && comparisons_.empty()) || quit_;

video/video_analyzer.h

diff --git a/video/video_analyzer.h b/video/video_analyzer.h
index 14f77ac..3ac72e2 100644
--- a/video/video_analyzer.h
+++ b/video/video_analyzer.h
@@ -83,9 +83,9 @@
 
   void StartMeasuringCpuProcessTime();
   void StopMeasuringCpuProcessTime();
-  void StartExcludingCpuThreadTime();
-  void StopExcludingCpuThreadTime();
-  double GetCpuUsagePercent();
+  void StartExcludingCpuThreadTime() RTC_LOCKS_EXCLUDED(cpu_measurement_lock_);
+  void StopExcludingCpuThreadTime() RTC_LOCKS_EXCLUDED(cpu_measurement_lock_);
+  double GetCpuUsagePercent() RTC_LOCKS_EXCLUDED(cpu_measurement_lock_);
 
   test::LayerFilteringTransport* const transport_;
   PacketReceiver* receiver_;
@@ -153,14 +153,17 @@
     void SetSource(rtc::VideoSourceInterface<VideoFrame>* video_source);
 
    private:
-    void OnFrame(const VideoFrame& video_frame) override;
+    void OnFrame(const VideoFrame& video_frame)
+        RTC_LOCKS_EXCLUDED(crit_) override;
 
     // Called when |send_stream_.SetSource()| is called.
     void AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
-                         const rtc::VideoSinkWants& wants) override;
+                         const rtc::VideoSinkWants& wants)
+        RTC_LOCKS_EXCLUDED(crit_) override;
 
     // Called by |send_stream_| when |send_stream_.SetSource()| is called.
-    void RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink) override;
+    void RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink)
+        RTC_LOCKS_EXCLUDED(crit_) override;
 
     VideoAnalyzer* const analyzer_;
     rtc::CriticalSection crit_;
@@ -186,19 +189,21 @@
                           int64_t render_time_ms)
       RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
-  void PollStats();
+  void PollStats() RTC_LOCKS_EXCLUDED(comparison_lock_);
   static void FrameComparisonThread(void* obj);
   bool CompareFrames();
   bool PopComparison(FrameComparison* comparison);
   // Increment counter for number of frames received for comparison.
-  void FrameRecorded();
+  void FrameRecorded() RTC_EXCLUSIVE_LOCKS_REQUIRED(comparison_lock_);
   // Returns true if all frames to be compared have been taken from the queue.
-  bool AllFramesRecorded();
+  bool AllFramesRecorded() RTC_LOCKS_EXCLUDED(comparison_lock_);
+  bool AllFramesRecordedLocked() RTC_EXCLUSIVE_LOCKS_REQUIRED(comparison_lock_);
   // Increase count of number of frames processed. Returns true if this was the
   // last frame to be processed.
-  bool FrameProcessed();
-  void PrintResults();
-  void PerformFrameComparison(const FrameComparison& comparison);
+  bool FrameProcessed() RTC_LOCKS_EXCLUDED(comparison_lock_);
+  void PrintResults() RTC_LOCKS_EXCLUDED(crit_, comparison_lock_);
+  void PerformFrameComparison(const FrameComparison& comparison)
+      RTC_LOCKS_EXCLUDED(comparison_lock_);
   void PrintResult(const char* result_type,
                    Statistics stats,
                    const char* unit,
@@ -209,8 +214,9 @@
       Statistics stats,
       const char* unit,
       webrtc::test::ImproveDirection improve_direction);
-  void PrintSamplesToFile(void);
-  void AddCapturedFrameForComparison(const VideoFrame& video_frame);
+  void PrintSamplesToFile(void) RTC_LOCKS_EXCLUDED(comparison_lock_);
+  void AddCapturedFrameForComparison(const VideoFrame& video_frame)
+      RTC_LOCKS_EXCLUDED(crit_, comparison_lock_);
 
   Call* call_;
   VideoSendStream* send_stream_;
@@ -264,7 +270,8 @@
 
   size_t last_fec_bytes_;
 
-  rtc::CriticalSection crit_;
+  rtc::CriticalSection crit_ RTC_ACQUIRED_BEFORE(comparison_lock_)
+      RTC_ACQUIRED_BEFORE(cpu_measurement_lock_);
   const int frames_to_process_;
   const Timestamp test_end_;
   int frames_recorded_ RTC_GUARDED_BY(comparison_lock_);