diff options
author | Markus Handell <handellm@webrtc.org> | 2020-07-08 09:32:42 +0200 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2020-07-08 09:14:22 +0000 |
commit | adbfd1d9856cf1fbd22b3d032ab482d23c6c5f93 (patch) | |
tree | 7029f8f627ce455a27619e1935087d2d4f0f17d8 /video | |
parent | 6cc893ad778a0965e2b7a8e614f3c98aa81bee5b (diff) | |
download | webrtc-adbfd1d9856cf1fbd22b3d032ab482d23c6c5f93.tar.gz |
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}
Diffstat (limited to 'video')
-rw-r--r-- | video/video_analyzer.cc | 7 | ||||
-rw-r--r-- | video/video_analyzer.h | 37 |
2 files changed, 27 insertions, 17 deletions
diff --git a/video/video_analyzer.cc b/video/video_analyzer.cc index f4a1c96d74..e04d59d1dc 100644 --- a/video/video_analyzer.cc +++ b/video/video_analyzer.cc @@ -570,7 +570,7 @@ bool VideoAnalyzer::PopComparison(VideoAnalyzer::FrameComparison* comparison) { // 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 @@ bool VideoAnalyzer::PopComparison(VideoAnalyzer::FrameComparison* comparison) { } 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_; diff --git a/video/video_analyzer.h b/video/video_analyzer.h index 14f77ac53c..3ac72e2c1b 100644 --- a/video/video_analyzer.h +++ b/video/video_analyzer.h @@ -83,9 +83,9 @@ class VideoAnalyzer : public PacketReceiver, 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 @@ class VideoAnalyzer : public PacketReceiver, 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 @@ class VideoAnalyzer : public PacketReceiver, 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 @@ class VideoAnalyzer : public PacketReceiver, 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 @@ class VideoAnalyzer : public PacketReceiver, 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_); |