From 0c67c80ac3fd82352edaba1f014a3928727cf2b7 Mon Sep 17 00:00:00 2001 From: Yves Gerey Date: Thu, 1 Aug 2019 17:45:54 +0200 Subject: [PATCH] Guard video analyzer against race conditions. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL adds thread annotations and ensure that neither data races nor deadlocks occur. It prevents weird results and helps detecting other concurrency issues. As a bonus, some dead code has been removed. Bug: webrtc:10834 Change-Id: Ibd140db9e4dbf81b212044647e2d85bd18ef8d78 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147278 Reviewed-by: Åsa Persson Commit-Queue: Yves Gerey Cr-Commit-Position: refs/heads/master@{#28737} --- video/video_analyzer.cc | 41 +++++++++++++++-------------------------- video/video_analyzer.h | 28 ++++++++++++---------------- 2 files changed, 27 insertions(+), 42 deletions(-) diff --git a/video/video_analyzer.cc b/video/video_analyzer.cc index be48726814..018d7ac5d0 100644 --- a/video/video_analyzer.cc +++ b/video/video_analyzer.cc @@ -93,17 +93,14 @@ VideoAnalyzer::VideoAnalyzer( frames_to_process_(duration_frames), frames_recorded_(0), frames_processed_(0), - dropped_frames_(0), captured_frames_(0), + dropped_frames_(0), dropped_frames_before_first_encode_(0), dropped_frames_before_rendering_(0), last_render_time_(0), last_render_delta_ms_(0), last_unfreeze_time_ms_(0), rtp_timestamp_delta_(0), - total_media_bytes_(0), - first_sending_time_(0), - last_sending_time_(0), cpu_time_(0), wallclock_time_(0), avg_psnr_threshold_(avg_psnr_threshold), @@ -290,12 +287,7 @@ bool VideoAnalyzer::SendRtp(const uint8_t* packet, if (IsInSelectedSpatialAndTemporalLayer(packet, length, header)) { encoded_frame_sizes_[timestamp] += length - (header.headerLength + header.paddingLength); - total_media_bytes_ += - length - (header.headerLength + header.paddingLength); } - if (first_sending_time_ == 0) - first_sending_time_ = current_time; - last_sending_time_ = current_time; } } return result; @@ -631,10 +623,11 @@ bool VideoAnalyzer::FrameProcessed() { void VideoAnalyzer::PrintResults() { StopMeasuringCpuProcessTime(); - int frames_left; + int dropped_frames_diff; { rtc::CritScope crit(&crit_); - frames_left = frames_.size(); + dropped_frames_diff = dropped_frames_before_first_encode_ + + dropped_frames_before_rendering_ + frames_.size(); } rtc::CritScope crit(&comparison_lock_); PrintResult("psnr", psnr_, "dB"); @@ -710,8 +703,7 @@ void VideoAnalyzer::PrintResults() { PrintResultWithExternalMean("decode_time", mean_decode_time_ms_, decode_time_ms_, "ms"); } - dropped_frames_ += dropped_frames_before_first_encode_ + - dropped_frames_before_rendering_ + frames_left; + dropped_frames_ += dropped_frames_diff; test::PrintResult("dropped_frames", "", test_label_.c_str(), dropped_frames_, "count", false); test::PrintResult("cpu_usage", "", test_label_.c_str(), GetCpuUsagePercent(), @@ -873,21 +865,18 @@ void VideoAnalyzer::PrintSamplesToFile() { } } -double VideoAnalyzer::GetAverageMediaBitrateBps() { - if (last_sending_time_ == first_sending_time_) { - return 0; - } else { - return static_cast(total_media_bytes_) * 8 / - (last_sending_time_ - first_sending_time_) * - rtc::kNumMillisecsPerSec; - } -} - void VideoAnalyzer::AddCapturedFrameForComparison( const VideoFrame& video_frame) { - rtc::CritScope lock(&crit_); - if (captured_frames_ < frames_to_process_) { - ++captured_frames_; + bool must_capture = false; + { + rtc::CritScope lock(&comparison_lock_); + must_capture = captured_frames_ < frames_to_process_; + if (must_capture) { + ++captured_frames_; + } + } + if (must_capture) { + rtc::CritScope lock(&crit_); frames_.push_back(video_frame); } } diff --git a/video/video_analyzer.h b/video/video_analyzer.h index 7cc3a86e5c..9fb3ea11ce 100644 --- a/video/video_analyzer.h +++ b/video/video_analyzer.h @@ -161,7 +161,7 @@ class VideoAnalyzer : public PacketReceiver, VideoSourceInterface* video_source_; Clock* clock_; int captured_frames_ RTC_GUARDED_BY(crit_); - int frames_to_process_ RTC_GUARDED_BY(crit_); + const int frames_to_process_; }; struct FrameWithPsnr { @@ -198,7 +198,6 @@ class VideoAnalyzer : public PacketReceiver, Statistics stats, const char* unit); void PrintSamplesToFile(void); - double GetAverageMediaBitrateBps(); void AddCapturedFrameForComparison(const VideoFrame& video_frame); Call* call_; @@ -253,26 +252,23 @@ class VideoAnalyzer : public PacketReceiver, size_t last_fec_bytes_; + rtc::CriticalSection crit_; const int frames_to_process_; - int frames_recorded_; - int frames_processed_; - int dropped_frames_; - int captured_frames_; - int dropped_frames_before_first_encode_; - int dropped_frames_before_rendering_; - int64_t last_render_time_; - int64_t last_render_delta_ms_; - int64_t last_unfreeze_time_ms_; - uint32_t rtp_timestamp_delta_; - int64_t total_media_bytes_; - int64_t first_sending_time_; - int64_t last_sending_time_; + int frames_recorded_ RTC_GUARDED_BY(comparison_lock_); + int frames_processed_ RTC_GUARDED_BY(comparison_lock_); + int captured_frames_ RTC_GUARDED_BY(comparison_lock_); + int dropped_frames_ RTC_GUARDED_BY(comparison_lock_); + int dropped_frames_before_first_encode_ RTC_GUARDED_BY(crit_); + int dropped_frames_before_rendering_ RTC_GUARDED_BY(crit_); + int64_t last_render_time_ RTC_GUARDED_BY(comparison_lock_); + int64_t last_render_delta_ms_ RTC_GUARDED_BY(comparison_lock_); + int64_t last_unfreeze_time_ms_ RTC_GUARDED_BY(comparison_lock_); + uint32_t rtp_timestamp_delta_ RTC_GUARDED_BY(crit_); rtc::CriticalSection cpu_measurement_lock_; int64_t cpu_time_ RTC_GUARDED_BY(cpu_measurement_lock_); int64_t wallclock_time_ RTC_GUARDED_BY(cpu_measurement_lock_); - rtc::CriticalSection crit_; std::deque frames_ RTC_GUARDED_BY(crit_); absl::optional last_rendered_frame_ RTC_GUARDED_BY(crit_); rtc::TimestampWrapAroundHandler wrap_handler_ RTC_GUARDED_BY(crit_);