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}
This commit is contained in:
Markus Handell
2020-07-08 09:32:42 +02:00
committed by Commit Bot
parent 6cc893ad77
commit adbfd1d985
2 changed files with 27 additions and 17 deletions

View File

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

View File

@ -83,9 +83,9 @@ class VideoAnalyzer : public PacketReceiver,
void StartMeasuringCpuProcessTime(); void StartMeasuringCpuProcessTime();
void StopMeasuringCpuProcessTime(); void StopMeasuringCpuProcessTime();
void StartExcludingCpuThreadTime(); void StartExcludingCpuThreadTime() RTC_LOCKS_EXCLUDED(cpu_measurement_lock_);
void StopExcludingCpuThreadTime(); void StopExcludingCpuThreadTime() RTC_LOCKS_EXCLUDED(cpu_measurement_lock_);
double GetCpuUsagePercent(); double GetCpuUsagePercent() RTC_LOCKS_EXCLUDED(cpu_measurement_lock_);
test::LayerFilteringTransport* const transport_; test::LayerFilteringTransport* const transport_;
PacketReceiver* receiver_; PacketReceiver* receiver_;
@ -153,14 +153,17 @@ class VideoAnalyzer : public PacketReceiver,
void SetSource(rtc::VideoSourceInterface<VideoFrame>* video_source); void SetSource(rtc::VideoSourceInterface<VideoFrame>* video_source);
private: 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. // Called when |send_stream_.SetSource()| is called.
void AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink, 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. // 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_; VideoAnalyzer* const analyzer_;
rtc::CriticalSection crit_; rtc::CriticalSection crit_;
@ -186,19 +189,21 @@ class VideoAnalyzer : public PacketReceiver,
int64_t render_time_ms) int64_t render_time_ms)
RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_); RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
void PollStats(); void PollStats() RTC_LOCKS_EXCLUDED(comparison_lock_);
static void FrameComparisonThread(void* obj); static void FrameComparisonThread(void* obj);
bool CompareFrames(); bool CompareFrames();
bool PopComparison(FrameComparison* comparison); bool PopComparison(FrameComparison* comparison);
// Increment counter for number of frames received for 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. // 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 // Increase count of number of frames processed. Returns true if this was the
// last frame to be processed. // last frame to be processed.
bool FrameProcessed(); bool FrameProcessed() RTC_LOCKS_EXCLUDED(comparison_lock_);
void PrintResults(); void PrintResults() RTC_LOCKS_EXCLUDED(crit_, comparison_lock_);
void PerformFrameComparison(const FrameComparison& comparison); void PerformFrameComparison(const FrameComparison& comparison)
RTC_LOCKS_EXCLUDED(comparison_lock_);
void PrintResult(const char* result_type, void PrintResult(const char* result_type,
Statistics stats, Statistics stats,
const char* unit, const char* unit,
@ -209,8 +214,9 @@ class VideoAnalyzer : public PacketReceiver,
Statistics stats, Statistics stats,
const char* unit, const char* unit,
webrtc::test::ImproveDirection improve_direction); webrtc::test::ImproveDirection improve_direction);
void PrintSamplesToFile(void); void PrintSamplesToFile(void) RTC_LOCKS_EXCLUDED(comparison_lock_);
void AddCapturedFrameForComparison(const VideoFrame& video_frame); void AddCapturedFrameForComparison(const VideoFrame& video_frame)
RTC_LOCKS_EXCLUDED(crit_, comparison_lock_);
Call* call_; Call* call_;
VideoSendStream* send_stream_; VideoSendStream* send_stream_;
@ -264,7 +270,8 @@ class VideoAnalyzer : public PacketReceiver,
size_t last_fec_bytes_; 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 int frames_to_process_;
const Timestamp test_end_; const Timestamp test_end_;
int frames_recorded_ RTC_GUARDED_BY(comparison_lock_); int frames_recorded_ RTC_GUARDED_BY(comparison_lock_);