From f8bc044109216afbd3487a92577b6dd1d8c98da7 Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Thu, 11 Apr 2019 09:20:05 +0000 Subject: [PATCH] Revert "Fix threading model of video quality test with audio enabled" This reverts commit f537da6c194d2c021709a255563c27b261e92488. Reason for revert: Speculative revert to check is it cause of https://crbug.com/950333 Original change's description: > Fix threading model of video quality test with audio enabled > > Bug: None > Change-Id: Ifb7fc57df54ec4d0a6f8c7f0504f3c06de6ac756 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/130514 > Reviewed-by: Ilya Nikolaevskiy > Reviewed-by: Christoffer Rodbro > Commit-Queue: Artem Titov > Cr-Commit-Position: refs/heads/master@{#27413} TBR=ilnik@webrtc.org,crodbro@webrtc.org,titovartem@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: None Change-Id: I89466ea6bc11336bcb08d0d1afe31bba50d6c773 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132543 Reviewed-by: Artem Titov Commit-Queue: Artem Titov Cr-Commit-Position: refs/heads/master@{#27557} --- video/video_analyzer.cc | 142 +++++++++++++++++------------------- video/video_analyzer.h | 9 +-- video/video_quality_test.cc | 15 ++-- 3 files changed, 77 insertions(+), 89 deletions(-) diff --git a/video/video_analyzer.cc b/video/video_analyzer.cc index ecd0125389..cb6c6f1aaf 100644 --- a/video/video_analyzer.cc +++ b/video/video_analyzer.cc @@ -49,23 +49,21 @@ bool IsFlexfec(int payload_type) { } } // namespace -VideoAnalyzer::VideoAnalyzer( - test::LayerFilteringTransport* transport, - const std::string& test_label, - double avg_psnr_threshold, - double avg_ssim_threshold, - int duration_frames, - FILE* graph_data_output_file, - const std::string& graph_title, - uint32_t ssrc_to_analyze, - uint32_t rtx_ssrc_to_analyze, - size_t selected_stream, - int selected_sl, - int selected_tl, - bool is_quick_test_enabled, - Clock* clock, - std::string rtp_dump_name, - test::SingleThreadedTaskQueueForTesting* task_queue) +VideoAnalyzer::VideoAnalyzer(test::LayerFilteringTransport* transport, + const std::string& test_label, + double avg_psnr_threshold, + double avg_ssim_threshold, + int duration_frames, + FILE* graph_data_output_file, + const std::string& graph_title, + uint32_t ssrc_to_analyze, + uint32_t rtx_ssrc_to_analyze, + size_t selected_stream, + int selected_sl, + int selected_tl, + bool is_quick_test_enabled, + Clock* clock, + std::string rtp_dump_name) : transport_(transport), receiver_(nullptr), call_(nullptr), @@ -101,10 +99,10 @@ VideoAnalyzer::VideoAnalyzer( avg_psnr_threshold_(avg_psnr_threshold), avg_ssim_threshold_(avg_ssim_threshold), is_quick_test_enabled_(is_quick_test_enabled), + stats_polling_thread_(&PollStatsThread, this, "StatsPoller"), done_(true, false), clock_(clock), - start_ms_(clock->TimeInMilliseconds()), - task_queue_(task_queue) { + start_ms_(clock->TimeInMilliseconds()) { // Create thread pool for CPU-expensive PSNR/SSIM calculations. // Try to use about as many threads as cores, but leave kMinCoresLeft alone, @@ -338,12 +336,7 @@ void VideoAnalyzer::Wait() { // at time-out check if frames_processed is going up. If so, give it more // time, otherwise fail. Hopefully this will reduce test flakiness. - { - rtc::CritScope lock(&comparison_lock_); - stop_stats_poller_ = false; - stats_polling_task_id_ = task_queue_->PostDelayedTask( - [this]() { PollStats(); }, kSendStatsPollingIntervalMs); - } + stats_polling_thread_.Start(); int last_frames_processed = -1; int last_frames_captured = -1; @@ -388,15 +381,11 @@ void VideoAnalyzer::Wait() { if (iteration > 0) printf("- Farewell, sweet Concorde!\n"); - { - rtc::CritScope lock(&comparison_lock_); - stop_stats_poller_ = true; - task_queue_->CancelTask(stats_polling_task_id_); - } - PrintResults(); if (graph_data_output_file_) PrintSamplesToFile(); + + stats_polling_thread_.Stop(); } void VideoAnalyzer::StartMeasuringCpuProcessTime() { @@ -467,56 +456,57 @@ bool VideoAnalyzer::IsInSelectedSpatialAndTemporalLayer( } } +void VideoAnalyzer::PollStatsThread(void* obj) { + static_cast(obj)->PollStats(); +} + void VideoAnalyzer::PollStats() { - rtc::CritScope crit(&comparison_lock_); - if (stop_stats_poller_) { - return; - } + while (!done_.Wait(kSendStatsPollingIntervalMs)) { + rtc::CritScope crit(&comparison_lock_); - Call::Stats call_stats = call_->GetStats(); - send_bandwidth_bps_.AddSample(call_stats.send_bandwidth_bps); + Call::Stats call_stats = call_->GetStats(); + send_bandwidth_bps_.AddSample(call_stats.send_bandwidth_bps); - VideoSendStream::Stats send_stats = send_stream_->GetStats(); - // It's not certain that we yet have estimates for any of these stats. - // Check that they are positive before mixing them in. - if (send_stats.encode_frame_rate > 0) - encode_frame_rate_.AddSample(send_stats.encode_frame_rate); - if (send_stats.avg_encode_time_ms > 0) - encode_time_ms_.AddSample(send_stats.avg_encode_time_ms); - if (send_stats.encode_usage_percent > 0) - encode_usage_percent_.AddSample(send_stats.encode_usage_percent); - if (send_stats.media_bitrate_bps > 0) - media_bitrate_bps_.AddSample(send_stats.media_bitrate_bps); - size_t fec_bytes = 0; - for (const auto& kv : send_stats.substreams) { - fec_bytes += kv.second.rtp_stats.fec.payload_bytes + - kv.second.rtp_stats.fec.padding_bytes; - } - fec_bitrate_bps_.AddSample((fec_bytes - last_fec_bytes_) * 8); - last_fec_bytes_ = fec_bytes; - - if (receive_stream_ != nullptr) { - VideoReceiveStream::Stats receive_stats = receive_stream_->GetStats(); - if (receive_stats.decode_ms > 0) - decode_time_ms_.AddSample(receive_stats.decode_ms); - if (receive_stats.max_decode_ms > 0) - decode_time_max_ms_.AddSample(receive_stats.max_decode_ms); - if (receive_stats.width > 0 && receive_stats.height > 0) { - pixels_.AddSample(receive_stats.width * receive_stats.height); + VideoSendStream::Stats send_stats = send_stream_->GetStats(); + // It's not certain that we yet have estimates for any of these stats. + // Check that they are positive before mixing them in. + if (send_stats.encode_frame_rate > 0) + encode_frame_rate_.AddSample(send_stats.encode_frame_rate); + if (send_stats.avg_encode_time_ms > 0) + encode_time_ms_.AddSample(send_stats.avg_encode_time_ms); + if (send_stats.encode_usage_percent > 0) + encode_usage_percent_.AddSample(send_stats.encode_usage_percent); + if (send_stats.media_bitrate_bps > 0) + media_bitrate_bps_.AddSample(send_stats.media_bitrate_bps); + size_t fec_bytes = 0; + for (const auto& kv : send_stats.substreams) { + fec_bytes += kv.second.rtp_stats.fec.payload_bytes + + kv.second.rtp_stats.fec.padding_bytes; } + fec_bitrate_bps_.AddSample((fec_bytes - last_fec_bytes_) * 8); + last_fec_bytes_ = fec_bytes; + + if (receive_stream_ != nullptr) { + VideoReceiveStream::Stats receive_stats = receive_stream_->GetStats(); + if (receive_stats.decode_ms > 0) + decode_time_ms_.AddSample(receive_stats.decode_ms); + if (receive_stats.max_decode_ms > 0) + decode_time_max_ms_.AddSample(receive_stats.max_decode_ms); + if (receive_stats.width > 0 && receive_stats.height > 0) { + pixels_.AddSample(receive_stats.width * receive_stats.height); + } + } + + if (audio_receive_stream_ != nullptr) { + AudioReceiveStream::Stats receive_stats = + audio_receive_stream_->GetStats(); + audio_expand_rate_.AddSample(receive_stats.expand_rate); + audio_accelerate_rate_.AddSample(receive_stats.accelerate_rate); + audio_jitter_buffer_ms_.AddSample(receive_stats.jitter_buffer_ms); + } + + memory_usage_.AddSample(rtc::GetProcessResidentSizeBytes()); } - - if (audio_receive_stream_ != nullptr) { - AudioReceiveStream::Stats receive_stats = audio_receive_stream_->GetStats(); - audio_expand_rate_.AddSample(receive_stats.expand_rate); - audio_accelerate_rate_.AddSample(receive_stats.accelerate_rate); - audio_jitter_buffer_ms_.AddSample(receive_stats.jitter_buffer_ms); - } - - memory_usage_.AddSample(rtc::GetProcessResidentSizeBytes()); - - stats_polling_task_id_ = task_queue_->PostDelayedTask( - [this]() { PollStats(); }, kSendStatsPollingIntervalMs); } bool VideoAnalyzer::FrameComparisonThread(void* obj) { diff --git a/video/video_analyzer.h b/video/video_analyzer.h index c40018885f..8caaf1420a 100644 --- a/video/video_analyzer.h +++ b/video/video_analyzer.h @@ -42,8 +42,7 @@ class VideoAnalyzer : public PacketReceiver, int selected_tl, bool is_quick_test_enabled, Clock* clock, - std::string rtp_dump_name, - test::SingleThreadedTaskQueueForTesting* task_queue); + std::string rtp_dump_name); ~VideoAnalyzer(); virtual void SetReceiver(PacketReceiver* receiver); @@ -177,6 +176,7 @@ class VideoAnalyzer : public PacketReceiver, int64_t render_time_ms) RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_); + static void PollStatsThread(void* obj); void PollStats(); static bool FrameComparisonThread(void* obj); bool CompareFrames(); @@ -273,17 +273,14 @@ class VideoAnalyzer : public PacketReceiver, bool is_quick_test_enabled_; std::vector comparison_thread_pool_; + rtc::PlatformThread stats_polling_thread_; rtc::Event comparison_available_event_; std::deque comparisons_ RTC_GUARDED_BY(comparison_lock_); rtc::Event done_; - test::SingleThreadedTaskQueueForTesting::TaskId stats_polling_task_id_ - RTC_GUARDED_BY(comparison_lock_); - bool stop_stats_poller_ RTC_GUARDED_BY(comparison_lock_); std::unique_ptr rtp_file_writer_; Clock* const clock_; const int64_t start_ms_; - test::SingleThreadedTaskQueueForTesting* task_queue_; }; } // namespace webrtc diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 2f0f01d2cf..e6760ee70e 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -1200,11 +1200,13 @@ void VideoQualityTest::RunWithAnalyzer(const Params& params) { recv_event_log_ = RtcEventLog::CreateNull(); } - task_queue_.SendTask([this, ¶ms, &send_transport, &recv_transport]() { - Call::Config send_call_config(send_event_log_.get()); - Call::Config recv_call_config(recv_event_log_.get()); - send_call_config.bitrate_config = params.call.call_bitrate_config; - recv_call_config.bitrate_config = params.call.call_bitrate_config; + Call::Config send_call_config(send_event_log_.get()); + Call::Config recv_call_config(recv_event_log_.get()); + send_call_config.bitrate_config = params.call.call_bitrate_config; + recv_call_config.bitrate_config = params.call.call_bitrate_config; + + task_queue_.SendTask([this, &send_call_config, &recv_call_config, + &send_transport, &recv_transport]() { if (params_.audio.enabled) InitializeAudioDevice(&send_call_config, &recv_call_config, params_.audio.use_real_adm); @@ -1229,8 +1231,7 @@ void VideoQualityTest::RunWithAnalyzer(const Params& params) { kSendRtxSsrcs[params_.ss[0].selected_stream], static_cast(params_.ss[0].selected_stream), params.ss[0].selected_sl, params_.video[0].selected_tl, - is_quick_test_enabled, clock_, params_.logging.rtp_dump_name, - &task_queue_); + is_quick_test_enabled, clock_, params_.logging.rtp_dump_name); task_queue_.SendTask([&]() { analyzer_->SetCall(sender_call_.get());