diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index 537b6a5d5b..ca26c02e21 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -193,6 +193,7 @@ if (rtc_include_tests) { "../../../api/video:video_frame", "../../../api/video:video_rtp_headers", "../../../api/video_codecs:video_codecs_api", + "../../../rtc_base:criticalsection", "../../../test:video_test_common", "../../../test:video_test_support", "//third_party/abseil-cpp/absl/memory", @@ -272,7 +273,6 @@ if (rtc_include_tests) { "../..:fileutils", "../..:platform_video_capturer", "../..:video_test_common", - "../..:video_test_support", "../../../api:create_frame_generator", "../../../api:frame_generator_api", "../../../api:peer_connection_quality_test_fixture_api", diff --git a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc index 70dbcd265e..074188439b 100644 --- a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc +++ b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc @@ -70,35 +70,6 @@ class AnalyzingFramePreprocessor sinks_; }; -// Implements the video sink, that forwards rendered frames to the video quality -// analyzer and provided sinks. -class AnalyzingVideoSink final : public rtc::VideoSinkInterface { - public: - AnalyzingVideoSink( - VideoQualityAnalyzerInterface* analyzer, - std::vector>> sinks) - : analyzer_(analyzer), sinks_(std::move(sinks)) { - RTC_DCHECK(analyzer_); - } - ~AnalyzingVideoSink() override = default; - - void OnFrame(const VideoFrame& frame) override { - if (IsDummyFrameBuffer(frame.video_frame_buffer()->ToI420())) { - // This is dummy frame, so we don't need to process it further. - return; - } - analyzer_->OnFrameRendered(frame); - for (auto& sink : sinks_) { - sink->OnFrame(frame); - } - } - - private: - VideoQualityAnalyzerInterface* const analyzer_; - const std::vector>> - sinks_; -}; - } // namespace VideoQualityAnalyzerInjectionHelper::VideoQualityAnalyzerInjectionHelper( @@ -137,9 +108,10 @@ VideoQualityAnalyzerInjectionHelper::WrapVideoDecoderFactory( std::unique_ptr VideoQualityAnalyzerInjectionHelper::CreateFramePreprocessor( - const VideoConfig& config, - test::VideoFrameWriter* writer) const { + const VideoConfig& config) { std::vector>> sinks; + test::VideoFrameWriter* writer = + MaybeCreateVideoWriter(config.input_dump_file_name, config); if (writer) { sinks.push_back(std::make_unique(writer)); } @@ -148,25 +120,17 @@ VideoQualityAnalyzerInjectionHelper::CreateFramePreprocessor( test::VideoRenderer::Create((*config.stream_label + "-capture").c_str(), config.width, config.height))); } + { + rtc::CritScope crit(&lock_); + known_video_configs_.insert({*config.stream_label, config}); + } return std::make_unique( std::move(*config.stream_label), analyzer_.get(), std::move(sinks)); } std::unique_ptr> -VideoQualityAnalyzerInjectionHelper::CreateVideoSink( - const VideoConfig& config, - test::VideoFrameWriter* writer) const { - std::vector>> sinks; - if (writer) { - sinks.push_back(std::make_unique(writer)); - } - if (config.show_on_screen) { - sinks.push_back(absl::WrapUnique( - test::VideoRenderer::Create((*config.stream_label + "-render").c_str(), - config.width, config.height))); - } - return std::make_unique(analyzer_.get(), - std::move(sinks)); +VideoQualityAnalyzerInjectionHelper::CreateVideoSink() { + return std::make_unique(this); } void VideoQualityAnalyzerInjectionHelper::Start(std::string test_case_name, @@ -182,6 +146,72 @@ void VideoQualityAnalyzerInjectionHelper::OnStatsReports( void VideoQualityAnalyzerInjectionHelper::Stop() { analyzer_->Stop(); + for (const auto& video_writer : video_writers_) { + video_writer->Close(); + } + video_writers_.clear(); +} + +test::VideoFrameWriter* +VideoQualityAnalyzerInjectionHelper::MaybeCreateVideoWriter( + absl::optional file_name, + const PeerConnectionE2EQualityTestFixture::VideoConfig& config) { + if (!file_name.has_value()) { + return nullptr; + } + // TODO(titovartem) create only one file writer for simulcast video track. + // For now this code will be invoked for each simulcast stream separately, but + // only one file will be used. + auto video_writer = std::make_unique( + file_name.value(), config.width, config.height, config.fps); + test::VideoFrameWriter* out = video_writer.get(); + video_writers_.push_back(std::move(video_writer)); + return out; +} + +void VideoQualityAnalyzerInjectionHelper::OnFrame(const VideoFrame& frame) { + if (IsDummyFrameBuffer(frame.video_frame_buffer()->ToI420())) { + // This is dummy frame, so we don't need to process it further. + return; + } + analyzer_->OnFrameRendered(frame); + std::string stream_label = analyzer_->GetStreamLabel(frame.id()); + std::vector>>* sinks = + PopulateSinks(stream_label); + if (sinks == nullptr) { + return; + } + for (auto& sink : *sinks) { + sink->OnFrame(frame); + } +} + +std::vector>>* +VideoQualityAnalyzerInjectionHelper::PopulateSinks( + const std::string& stream_label) { + rtc::CritScope crit(&lock_); + auto sinks_it = sinks_.find(stream_label); + if (sinks_it != sinks_.end()) { + return &sinks_it->second; + } + auto it = known_video_configs_.find(stream_label); + RTC_DCHECK(it != known_video_configs_.end()) + << "No video config for stream " << stream_label; + const VideoConfig& config = it->second; + + std::vector>> sinks; + test::VideoFrameWriter* writer = + MaybeCreateVideoWriter(config.output_dump_file_name, config); + if (writer) { + sinks.push_back(std::make_unique(writer)); + } + if (config.show_on_screen) { + sinks.push_back(absl::WrapUnique( + test::VideoRenderer::Create((*config.stream_label + "-render").c_str(), + config.width, config.height))); + } + sinks_.insert({stream_label, std::move(sinks)}); + return &(sinks_.find(stream_label)->second); } } // namespace webrtc_pc_e2e diff --git a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h index ccda57baaf..a0daa9ff18 100644 --- a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h +++ b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h @@ -22,6 +22,7 @@ #include "api/video/video_sink_interface.h" #include "api/video_codecs/video_decoder_factory.h" #include "api/video_codecs/video_encoder_factory.h" +#include "rtc_base/critical_section.h" #include "test/pc/e2e/analyzer/video/encoded_image_data_injector.h" #include "test/pc/e2e/analyzer/video/id_generator.h" #include "test/test_video_capturer.h" @@ -55,17 +56,15 @@ class VideoQualityAnalyzerInjectionHelper : public StatsObserverInterface { std::unique_ptr delegate) const; // Creates VideoFrame preprocessor, that will allow video quality analyzer to - // get access to the captured frames. If |writer| in not nullptr, will dump - // captured frames with provided writer. + // get access to the captured frames. If provided config also specifies + // |input_dump_file_name|, video will be written into that file. std::unique_ptr - CreateFramePreprocessor(const VideoConfig& config, - test::VideoFrameWriter* writer) const; + CreateFramePreprocessor(const VideoConfig& config); // Creates sink, that will allow video quality analyzer to get access to - // the rendered frames. If |writer| in not nullptr, will dump rendered - // frames with provided writer. - std::unique_ptr> CreateVideoSink( - const VideoConfig& config, - test::VideoFrameWriter* writer) const; + // the rendered frames. If corresponding video track has + // |output_dump_file_name| in its VideoConfig, then video also will be written + // into that file. + std::unique_ptr> CreateVideoSink(); void Start(std::string test_case_name, int max_threads_count); @@ -75,13 +74,41 @@ class VideoQualityAnalyzerInjectionHelper : public StatsObserverInterface { const StatsReports& stats_reports) override; // Stops VideoQualityAnalyzerInterface to populate final data and metrics. + // Should be invoked after analyzed video tracks are disposed. void Stop(); private: + class AnalyzingVideoSink final : public rtc::VideoSinkInterface { + public: + explicit AnalyzingVideoSink(VideoQualityAnalyzerInjectionHelper* helper) + : helper_(helper) {} + ~AnalyzingVideoSink() override = default; + + void OnFrame(const VideoFrame& frame) override { helper_->OnFrame(frame); } + + private: + VideoQualityAnalyzerInjectionHelper* const helper_; + }; + + test::VideoFrameWriter* MaybeCreateVideoWriter( + absl::optional file_name, + const PeerConnectionE2EQualityTestFixture::VideoConfig& config); + void OnFrame(const VideoFrame& frame); + std::vector>>* + PopulateSinks(const std::string& stream_label); + std::unique_ptr analyzer_; EncodedImageDataInjector* injector_; EncodedImageDataExtractor* extractor_; + std::vector> video_writers_; + + rtc::CriticalSection lock_; + std::map known_video_configs_ RTC_GUARDED_BY(lock_); + std::map>>> + sinks_ RTC_GUARDED_BY(lock_); + std::unique_ptr> encoding_entities_id_generator_; }; diff --git a/test/pc/e2e/media/media_helper.cc b/test/pc/e2e/media/media_helper.cc index e584795a3a..38179d2b40 100644 --- a/test/pc/e2e/media/media_helper.cc +++ b/test/pc/e2e/media/media_helper.cc @@ -9,6 +9,7 @@ */ #include "test/pc/e2e/media/media_helper.h" +#include #include #include "api/test/create_frame_generator.h" @@ -29,13 +30,6 @@ using VideoGeneratorType = ::webrtc::webrtc_pc_e2e:: } // namespace -MediaHelper::~MediaHelper() { - for (const auto& video_writer : video_writers_) { - video_writer->Close(); - } - video_writers_.clear(); -} - void MediaHelper::MaybeAddAudio(TestPeer* peer) { if (!peer->params()->audio_config) { return; @@ -59,12 +53,10 @@ MediaHelper::MaybeAddVideo(TestPeer* peer) { for (size_t i = 0; i < params->video_configs.size(); ++i) { auto video_config = params->video_configs[i]; // Setup input video source into peer connection. - test::VideoFrameWriter* writer = - MaybeCreateVideoWriter(video_config.input_dump_file_name, video_config); std::unique_ptr capturer = CreateVideoCapturer( video_config, peer->ReleaseVideoGenerator(i), video_quality_analyzer_injection_helper_->CreateFramePreprocessor( - video_config, writer)); + video_config)); rtc::scoped_refptr source = new rtc::RefCountedObject( std::move(capturer), @@ -99,22 +91,6 @@ MediaHelper::MaybeAddVideo(TestPeer* peer) { return out; } -test::VideoFrameWriter* MediaHelper::MaybeCreateVideoWriter( - absl::optional file_name, - const VideoConfig& config) { - if (!file_name) { - return nullptr; - } - // TODO(titovartem) create only one file writer for simulcast video track. - // For now this code will be invoked for each simulcast stream separately, but - // only one file will be used. - auto video_writer = std::make_unique( - file_name.value(), config.width, config.height, config.fps); - test::VideoFrameWriter* out = video_writer.get(); - video_writers_.push_back(std::move(video_writer)); - return out; -} - std::unique_ptr MediaHelper::CreateVideoCapturer( const VideoConfig& video_config, std::unique_ptr generator, diff --git a/test/pc/e2e/media/media_helper.h b/test/pc/e2e/media/media_helper.h index 740cd2c00a..e10a9fabd1 100644 --- a/test/pc/e2e/media/media_helper.h +++ b/test/pc/e2e/media/media_helper.h @@ -12,7 +12,6 @@ #define TEST_PC_E2E_MEDIA_MEDIA_HELPER_H_ #include -#include #include #include "api/test/frame_generator_interface.h" @@ -20,7 +19,6 @@ #include "test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h" #include "test/pc/e2e/media/test_video_capturer_video_track_source.h" #include "test/pc/e2e/test_peer.h" -#include "test/testsupport/video_frame_writer.h" namespace webrtc { namespace webrtc_pc_e2e { @@ -34,20 +32,12 @@ class MediaHelper { task_queue_factory_(task_queue_factory), video_quality_analyzer_injection_helper_( video_quality_analyzer_injection_helper) {} - ~MediaHelper(); void MaybeAddAudio(TestPeer* peer); std::vector> MaybeAddVideo(TestPeer* peer); - // Creates a video file writer if |file_name| is not empty. Created writer - // will be owned by MediaHelper and will be closed on MediaHelper destruction. - // If |file_name| is empty will return nullptr. - test::VideoFrameWriter* MaybeCreateVideoWriter( - absl::optional file_name, - const PeerConnectionE2EQualityTestFixture::VideoConfig& config); - private: std::unique_ptr CreateVideoCapturer( const PeerConnectionE2EQualityTestFixture::VideoConfig& video_config, @@ -61,7 +51,6 @@ class MediaHelper { Clock* const clock_; TaskQueueFactory* const task_queue_factory_; VideoQualityAnalyzerInjectionHelper* video_quality_analyzer_injection_helper_; - std::vector> video_writers_; }; } // namespace webrtc_pc_e2e diff --git a/test/pc/e2e/peer_connection_quality_test.cc b/test/pc/e2e/peer_connection_quality_test.cc index d60856a0a9..c5c9388f15 100644 --- a/test/pc/e2e/peer_connection_quality_test.cc +++ b/test/pc/e2e/peer_connection_quality_test.cc @@ -386,22 +386,11 @@ void PeerConnectionE2EQualityTest::OnTrackCallback( return; } - VideoConfig* video_config = nullptr; - for (auto& config : remote_video_configs) { - if (config.stream_label == stream_label) { - video_config = &config; - break; - } - } - RTC_CHECK(video_config); - test::VideoFrameWriter* writer = media_helper_->MaybeCreateVideoWriter( - video_config->output_dump_file_name, *video_config); // It is safe to cast here, because it is checked above that // track->kind() is kVideoKind. auto* video_track = static_cast(track.get()); std::unique_ptr> video_sink = - video_quality_analyzer_injection_helper_->CreateVideoSink(*video_config, - writer); + video_quality_analyzer_injection_helper_->CreateVideoSink(); video_track->AddOrUpdateSink(video_sink.get(), rtc::VideoSinkWants()); output_video_sinks_.push_back(std::move(video_sink)); }