From ff7730d2ba2b1d2fcf9ed55f51f20b8256d9c31d Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Tue, 2 Apr 2019 13:46:53 +0200 Subject: [PATCH] Reland "Fix threading model of video quality test with audio enabled" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a reland of f537da6c194d2c021709a255563c27b261e92488 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} Bug: None Change-Id: I4fb793a5a5f636103159ed537847d6f2deb60108 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132797 Reviewed-by: Erik SprÃ¥ng Reviewed-by: Christoffer Rodbro Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Artem Titov Cr-Commit-Position: refs/heads/master@{#27621} --- video/video_analyzer.cc | 148 +++++++++++++++++++----------------- video/video_analyzer.h | 9 ++- video/video_quality_test.cc | 15 ++-- 3 files changed, 92 insertions(+), 80 deletions(-) diff --git a/video/video_analyzer.cc b/video/video_analyzer.cc index 11d2c7b648..d74b79ab60 100644 --- a/video/video_analyzer.cc +++ b/video/video_analyzer.cc @@ -49,21 +49,23 @@ 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) +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) : transport_(transport), receiver_(nullptr), call_(nullptr), @@ -99,10 +101,10 @@ VideoAnalyzer::VideoAnalyzer(test::LayerFilteringTransport* transport, 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()) { + start_ms_(clock->TimeInMilliseconds()), + task_queue_(task_queue) { // Create thread pool for CPU-expensive PSNR/SSIM calculations. // Try to use about as many threads as cores, but leave kMinCoresLeft alone, @@ -336,7 +338,12 @@ 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. - stats_polling_thread_.Start(); + { + rtc::CritScope lock(&comparison_lock_); + stop_stats_poller_ = false; + stats_polling_task_id_ = task_queue_->PostDelayedTask( + [this]() { PollStats(); }, kSendStatsPollingIntervalMs); + } int last_frames_processed = -1; int last_frames_captured = -1; @@ -381,11 +388,15 @@ 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() { @@ -456,57 +467,56 @@ bool VideoAnalyzer::IsInSelectedSpatialAndTemporalLayer( } } -void VideoAnalyzer::PollStatsThread(void* obj) { - static_cast(obj)->PollStats(); -} - void VideoAnalyzer::PollStats() { - while (!done_.Wait(kSendStatsPollingIntervalMs)) { - rtc::CritScope crit(&comparison_lock_); - - 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); - } - } - - 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()); + rtc::CritScope crit(&comparison_lock_); + if (stop_stats_poller_) { + return; } + + 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); + } + } + + 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 36ed03e830..855034565c 100644 --- a/video/video_analyzer.h +++ b/video/video_analyzer.h @@ -44,7 +44,8 @@ class VideoAnalyzer : public PacketReceiver, int selected_tl, bool is_quick_test_enabled, Clock* clock, - std::string rtp_dump_name); + std::string rtp_dump_name, + test::SingleThreadedTaskQueueForTesting* task_queue); ~VideoAnalyzer(); virtual void SetReceiver(PacketReceiver* receiver); @@ -178,7 +179,6 @@ 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,14 +273,17 @@ 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 139be61cb0..10f9699601 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -1203,13 +1203,11 @@ void VideoQualityTest::RunWithAnalyzer(const Params& params) { recv_event_log_ = RtcEventLog::CreateNull(); } - 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]() { + 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; if (params_.audio.enabled) InitializeAudioDevice(&send_call_config, &recv_call_config, params_.audio.use_real_adm); @@ -1234,7 +1232,8 @@ 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); + is_quick_test_enabled, clock_, params_.logging.rtp_dump_name, + &task_queue_); task_queue_.SendTask([&]() { analyzer_->SetCall(sender_call_.get());