From d8fea51d65c7ecf52e6099f89e510b952b02b07c Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Tue, 28 Jan 2025 02:16:28 -0800 Subject: [PATCH] Revert "Cleanup usage of the global field trials in the PeerConnectionE2EQualityTest helper" This reverts commit a97304ca03c2aeb4267dc1bd794c50aa8bdb9a69. Reason for revert: performance tests still rely in on global field trials to configure PC created by this test fixture Original change's description: > Cleanup usage of the global field trials in the PeerConnectionE2EQualityTest helper > > Bug: webrtc:42220378 > Change-Id: I3dc1a71c043ef506b6d592673b04e49f4a022d17 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374901 > Reviewed-by: Jeremy Leconte > Commit-Queue: Danil Chapovalov > Cr-Commit-Position: refs/heads/main@{#43803} Bug: webrtc:42220378, webrtc:392672060 Change-Id: Ide265c1284f9d53c0b652ed5e144dfb0a532f87a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375621 Reviewed-by: Artem Titov Auto-Submit: Danil Chapovalov Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Commit-Queue: Artem Titov Commit-Queue: rubber-stamper@appspot.gserviceaccount.com Cr-Commit-Position: refs/heads/main@{#43812} --- test/pc/e2e/BUILD.gn | 3 +- test/pc/e2e/peer_connection_quality_test.cc | 41 +++++++++++++++------ test/pc/e2e/peer_connection_quality_test.h | 16 +++----- 3 files changed, 36 insertions(+), 24 deletions(-) diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn index 8a14703bb7..94e6c62a1f 100644 --- a/test/pc/e2e/BUILD.gn +++ b/test/pc/e2e/BUILD.gn @@ -247,7 +247,6 @@ if (!build_with_chromium) { "../..:test_flags", "../..:test_support", "../../../api:audio_quality_analyzer_api", - "../../../api:field_trials", "../../../api:libjingle_peerconnection_api", "../../../api:media_stream_interface", "../../../api:peer_connection_quality_test_fixture_api", @@ -283,8 +282,10 @@ if (!build_with_chromium) { "../../../rtc_base/synchronization:mutex", "../../../rtc_base/task_utils:repeating_task", "../../../system_wrappers", + "../../../system_wrappers:field_trial", "analyzer/video:default_video_quality_analyzer", "analyzer/video:single_process_encoded_image_data_injector", + "analyzer/video:video_frame_tracking_id_injector", "analyzer/video:video_quality_analyzer_injection_helper", "analyzer/video:video_quality_metrics_reporter", "//third_party/abseil-cpp/absl/flags:flag", diff --git a/test/pc/e2e/peer_connection_quality_test.cc b/test/pc/e2e/peer_connection_quality_test.cc index 040f8b1b3f..8fa99826df 100644 --- a/test/pc/e2e/peer_connection_quality_test.cc +++ b/test/pc/e2e/peer_connection_quality_test.cc @@ -22,7 +22,6 @@ #include "absl/flags/flag.h" #include "absl/strings/string_view.h" -#include "api/field_trials.h" #include "api/jsep.h" #include "api/media_stream_interface.h" #include "api/media_types.h" @@ -53,13 +52,18 @@ #include "pc/test/mock_peer_connection_observers.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" +#include "rtc_base/strings/string_builder.h" #include "rtc_base/synchronization/mutex.h" #include "rtc_base/task_queue_for_test.h" #include "rtc_base/task_utils/repeating_task.h" #include "system_wrappers/include/cpu_info.h" +#include "system_wrappers/include/field_trial.h" +#include "test/field_trial.h" #include "test/gtest.h" #include "test/pc/e2e/analyzer/audio/default_audio_quality_analyzer.h" #include "test/pc/e2e/analyzer/video/default_video_quality_analyzer.h" +#include "test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.h" +#include "test/pc/e2e/analyzer/video/video_frame_tracking_id_injector.h" #include "test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h" #include "test/pc/e2e/analyzer/video/video_quality_metrics_reporter.h" #include "test/pc/e2e/cross_media_metrics_reporter.h" @@ -185,10 +189,18 @@ PeerConnectionE2EQualityTest::PeerConnectionE2EQualityTest( video_quality_analyzer = std::make_unique( time_controller_.GetClock(), metrics_logger_); } + if (field_trial::IsEnabled("WebRTC-VideoFrameTrackingIdAdvertised")) { + encoded_image_data_propagator_ = + std::make_unique(); + } else { + encoded_image_data_propagator_ = + std::make_unique(); + } video_quality_analyzer_injection_helper_ = std::make_unique( time_controller_.GetClock(), std::move(video_quality_analyzer), - &encoded_image_data_propagator_, &encoded_image_data_propagator_); + encoded_image_data_propagator_.get(), + encoded_image_data_propagator_.get()); if (audio_quality_analyzer == nullptr) { audio_quality_analyzer = @@ -247,9 +259,7 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) { << "Only simulcast stream from first peer is supported"; } - std::string field_trials = GetFieldTrials(run_params); - alice_configurer->SetFieldTrials(FieldTrials::CreateNoGlobal(field_trials)); - bob_configurer->SetFieldTrials(FieldTrials::CreateNoGlobal(field_trials)); + test::ScopedFieldTrials field_trials(GetFieldTrials(run_params)); // Print test summary RTC_LOG(LS_INFO) @@ -270,8 +280,9 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) { time_controller_.GetClock()); // Create a `task_queue_`. - task_queue_ = time_controller_.GetTaskQueueFactory()->CreateTaskQueue( - "pc_e2e_quality_test", webrtc::TaskQueueFactory::Priority::NORMAL); + task_queue_ = std::make_unique( + time_controller_.GetTaskQueueFactory()->CreateTaskQueue( + "pc_e2e_quality_test", webrtc::TaskQueueFactory::Priority::NORMAL)); // Create call participants: Alice and Bob. // Audio streams are intercepted in AudioDeviceModule, so if it is required to @@ -361,8 +372,8 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) { // Setup alive logging. It is done to prevent test infra to think that test is // dead. - RepeatingTaskHandle::DelayedStart(task_queue_.get(), kAliveMessageLogInterval, - []() { + RepeatingTaskHandle::DelayedStart(task_queue_->Get(), + kAliveMessageLogInterval, []() { std::printf("Test is still running...\n"); return kAliveMessageLogInterval; }); @@ -420,7 +431,7 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) { // There is no guarantee, that last stats collection will happen at the end // of the call, so we force it after executor, which is among others is doing // stats collection, was stopped. - SendTask(task_queue_.get(), [&stats_poller]() { + task_queue_->SendTask([&stats_poller]() { // Get final end-of-call stats. stats_poller.PollStatsAndNotifyObservers(); }); @@ -458,10 +469,16 @@ void PeerConnectionE2EQualityTest::Run(RunParams run_params) { std::string PeerConnectionE2EQualityTest::GetFieldTrials( const RunParams& run_params) { + std::vector default_field_trials = {}; if (run_params.enable_flex_fec_support) { - return kFlexFecEnabledFieldTrials; + default_field_trials.push_back(kFlexFecEnabledFieldTrials); } - return ""; + rtc::StringBuilder sb; + sb << field_trial::GetFieldTrialString(); + for (const absl::string_view& field_trial : default_field_trials) { + sb << field_trial; + } + return sb.Release(); } void PeerConnectionE2EQualityTest::OnTrackCallback( diff --git a/test/pc/e2e/peer_connection_quality_test.h b/test/pc/e2e/peer_connection_quality_test.h index 7045c7d5db..6cbf232874 100644 --- a/test/pc/e2e/peer_connection_quality_test.h +++ b/test/pc/e2e/peer_connection_quality_test.h @@ -10,15 +10,12 @@ #ifndef TEST_PC_E2E_PEER_CONNECTION_QUALITY_TEST_H_ #define TEST_PC_E2E_PEER_CONNECTION_QUALITY_TEST_H_ -#include #include +#include #include #include #include "absl/strings/string_view.h" -#include "api/rtp_transceiver_interface.h" -#include "api/scoped_refptr.h" -#include "api/task_queue/task_queue_base.h" #include "api/task_queue/task_queue_factory.h" #include "api/test/audio_quality_analyzer_interface.h" #include "api/test/metrics/metrics_logger.h" @@ -27,20 +24,17 @@ #include "api/test/pclf/peer_configurer.h" #include "api/test/peerconnection_quality_test_fixture.h" #include "api/test/time_controller.h" -#include "api/test/video_quality_analyzer_interface.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" -#include "api/video/video_frame.h" -#include "api/video/video_sink_interface.h" -#include "rtc_base/checks.h" #include "rtc_base/synchronization/mutex.h" +#include "rtc_base/task_queue_for_test.h" #include "rtc_base/thread.h" #include "rtc_base/thread_annotations.h" +#include "system_wrappers/include/clock.h" #include "test/pc/e2e/analyzer/video/single_process_encoded_image_data_injector.h" #include "test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h" #include "test/pc/e2e/analyzer_helper.h" #include "test/pc/e2e/media/media_helper.h" -#include "test/pc/e2e/media/test_video_capturer_video_track_source.h" #include "test/pc/e2e/sdp/sdp_changer.h" #include "test/pc/e2e/test_activities_executor.h" #include "test/pc/e2e/test_peer.h" @@ -122,7 +116,7 @@ class PeerConnectionE2EQualityTest std::unique_ptr video_quality_analyzer_injection_helper_; std::unique_ptr media_helper_; - SingleProcessEncodedImageDataInjector encoded_image_data_propagator_; + std::unique_ptr encoded_image_data_propagator_; std::unique_ptr audio_quality_analyzer_; std::unique_ptr executor_; test::MetricsLogger* const metrics_logger_; @@ -149,7 +143,7 @@ class PeerConnectionE2EQualityTest // Task queue, that is used for running activities during test call. // This task queue will be created before call set up and will be destroyed // immediately before call tear down. - std::unique_ptr task_queue_; + std::unique_ptr task_queue_; bool alice_connected_ = false; bool bob_connected_ = false;