[PCLF] Cleanup field trials propagation api
Remove deprecated PeerConfigurer::SetFieldTrials in favor of AddFieldTrials
That is more restricted api, but gives PCLF more freedom in combining
test provided field trials with framework provided field trials.
Delete noop enable_flex_fec_support flag as framework now can enable flexfec per peer without need to rely on global field trials.
Remove fallback to the global field trials.
Bug: webrtc:419453427
Change-Id: I71d9f933dbc0b91ea88e6c78d85679bd5c612b75
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/396082
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#44957}
diff --git a/api/test/pclf/BUILD.gn b/api/test/pclf/BUILD.gn
index 3104a6c..7c9a0de 100644
--- a/api/test/pclf/BUILD.gn
+++ b/api/test/pclf/BUILD.gn
@@ -32,7 +32,7 @@
]
}
-rtc_library("media_quality_test_params") {
+rtc_source_set("media_quality_test_params") {
visibility = [ "*" ]
testonly = true
sources = [ "media_quality_test_params.h" ]
@@ -42,7 +42,6 @@
"../..:async_dns_resolver",
"../..:fec_controller_api",
"../..:field_trials",
- "../..:field_trials_view",
"../..:ice_transport_interface",
"../..:libjingle_peerconnection_api",
"../..:scoped_refptr",
@@ -79,7 +78,6 @@
"../..:create_peer_connection_quality_test_frame_generator",
"../..:fec_controller_api",
"../..:field_trials",
- "../..:field_trials_view",
"../..:frame_generator_api",
"../..:ice_transport_interface",
"../..:libjingle_peerconnection_api",
diff --git a/api/test/pclf/media_quality_test_params.h b/api/test/pclf/media_quality_test_params.h
index 0635717..eb5d9e6 100644
--- a/api/test/pclf/media_quality_test_params.h
+++ b/api/test/pclf/media_quality_test_params.h
@@ -25,7 +25,6 @@
#include "api/audio_codecs/audio_encoder_factory.h"
#include "api/fec_controller.h"
#include "api/field_trials.h"
-#include "api/field_trials_view.h"
#include "api/ice_transport_interface.h"
#include "api/neteq/neteq_factory.h"
#include "api/peer_connection_interface.h"
@@ -70,7 +69,6 @@
scoped_refptr<webrtc::AudioEncoderFactory> audio_encoder_factory;
scoped_refptr<webrtc::AudioDecoderFactory> audio_decoder_factory;
- [[deprecated]] std::unique_ptr<FieldTrialsView> trials;
std::unique_ptr<FieldTrials> field_trials;
std::unique_ptr<AudioProcessingBuilderInterface> audio_processing;
@@ -173,19 +171,12 @@
// test.
struct RunParams {
explicit RunParams(TimeDelta run_duration) : run_duration(run_duration) {}
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
- RunParams(const RunParams&) = default;
- RunParams(RunParams&&) = default;
-#pragma clang diagnostic pop
// Specifies how long the test should be run. This time shows how long
// the media should flow after connection was established and before
// it will be shut downed.
TimeDelta run_duration;
- [[deprecated("Noop flag: Enable FlexFEC per peer.")]]
- bool enable_flex_fec_support = false;
// If true will set conference mode in SDP media section for all video
// tracks for all peers.
bool use_conference_mode = false;
diff --git a/api/test/pclf/peer_configurer.cc b/api/test/pclf/peer_configurer.cc
index 80fc716..668f29a 100644
--- a/api/test/pclf/peer_configurer.cc
+++ b/api/test/pclf/peer_configurer.cc
@@ -25,7 +25,6 @@
#include "api/audio_codecs/audio_encoder_factory.h"
#include "api/fec_controller.h"
#include "api/field_trials.h"
-#include "api/field_trials_view.h"
#include "api/ice_transport_interface.h"
#include "api/neteq/neteq_factory.h"
#include "api/peer_connection_interface.h"
@@ -241,15 +240,6 @@
return this;
}
-PeerConfigurer* PeerConfigurer::SetFieldTrials(
- std::unique_ptr<FieldTrialsView> field_trials) {
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
- components_->pcf_dependencies->trials = std::move(field_trials);
-#pragma clang diagnostic pop
- return this;
-}
-
PeerConfigurer* PeerConfigurer::SetPortAllocatorExtraFlags(
uint32_t extra_flags) {
params_->port_allocator_flags =
diff --git a/api/test/pclf/peer_configurer.h b/api/test/pclf/peer_configurer.h
index 62d3d93..821962d 100644
--- a/api/test/pclf/peer_configurer.h
+++ b/api/test/pclf/peer_configurer.h
@@ -24,7 +24,6 @@
#include "api/audio_codecs/audio_encoder_factory.h"
#include "api/fec_controller.h"
#include "api/field_trials.h"
-#include "api/field_trials_view.h"
#include "api/ice_transport_interface.h"
#include "api/neteq/neteq_factory.h"
#include "api/peer_connection_interface.h"
@@ -177,9 +176,6 @@
// Set bitrate parameters on PeerConnection. This constraints will be
// applied to all summed RTP streams for this peer.
PeerConfigurer* SetBitrateSettings(BitrateSettings bitrate_settings);
- // Set field trials used for this PeerConnection.
- [[deprecated]]
- PeerConfigurer* SetFieldTrials(std::unique_ptr<FieldTrialsView> field_trials);
// Appends field trials for this PeerConnection.
PeerConfigurer* AddFieldTrials(const FieldTrials& field_trials) {
diff --git a/test/pc/e2e/BUILD.gn b/test/pc/e2e/BUILD.gn
index 739456a..4c23f15 100644
--- a/test/pc/e2e/BUILD.gn
+++ b/test/pc/e2e/BUILD.gn
@@ -107,8 +107,6 @@
":test_peer",
"../..:copy_to_file_audio_capturer",
"../../../api:enable_media_with_defaults",
- "../../../api:field_trials",
- "../../../api:field_trials_view",
"../../../api:libjingle_peerconnection_api",
"../../../api:scoped_refptr",
"../../../api:time_controller",
@@ -116,7 +114,6 @@
"../../../api/environment",
"../../../api/environment:environment_factory",
"../../../api/rtc_event_log:rtc_event_log_factory",
- "../../../api/task_queue",
"../../../api/test/pclf:media_configuration",
"../../../api/test/pclf:media_quality_test_params",
"../../../api/test/pclf:peer_configurer",
@@ -128,10 +125,8 @@
"../../../rtc_base:checks",
"../../../rtc_base:threading",
"../../../rtc_base/system:file_wrapper",
- "../../../system_wrappers:field_trial",
"analyzer/video:quality_analyzing_video_encoder",
"analyzer/video:video_quality_analyzer_injection_helper",
- "//third_party/abseil-cpp/absl/base:nullability",
"//third_party/abseil-cpp/absl/memory",
"//third_party/abseil-cpp/absl/strings:string_view",
]
diff --git a/test/pc/e2e/test_peer_factory.cc b/test/pc/e2e/test_peer_factory.cc
index 8f044c8..ca78fa4 100644
--- a/test/pc/e2e/test_peer_factory.cc
+++ b/test/pc/e2e/test_peer_factory.cc
@@ -16,15 +16,12 @@
#include <utility>
#include <vector>
-#include "absl/base/nullability.h"
#include "absl/memory/memory.h"
#include "absl/strings/string_view.h"
#include "api/audio/audio_device.h"
#include "api/enable_media_with_defaults.h"
#include "api/environment/environment.h"
#include "api/environment/environment_factory.h"
-#include "api/field_trials.h"
-#include "api/field_trials_view.h"
#include "api/peer_connection_interface.h"
#include "api/rtc_event_log/rtc_event_log_factory.h"
#include "api/scoped_refptr.h"
@@ -41,7 +38,6 @@
#include "rtc_base/checks.h"
#include "rtc_base/system/file_wrapper.h"
#include "rtc_base/thread.h"
-#include "system_wrappers/include/field_trial.h"
#include "test/pc/e2e/analyzer/video/quality_analyzing_video_encoder.h"
#include "test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h"
#include "test/pc/e2e/echo/echo_emulation.h"
@@ -58,36 +54,6 @@
constexpr int16_t kGeneratedAudioMaxAmplitude = 32000;
constexpr int kDefaultSamplingFrequencyInHz = 48000;
-// TODO: bugs.webrtc.org/419453427 - Remove this combiner when users of the PCLF
-// both stop passing field trials as single FieldTrialsView, and stop relying
-// on the global field trial string.
-class FieldTrialsCombiner : public FieldTrialsView {
- public:
- FieldTrialsCombiner(absl_nullable std::unique_ptr<FieldTrialsView> trials,
- absl_nonnull std::unique_ptr<FieldTrials> field_trials)
- : trials_(std::move(trials)), field_trials_(std::move(field_trials)) {}
-
- std::string Lookup(absl::string_view key) const override {
- if (trials_ != nullptr) {
- if (std::string value = trials_->Lookup(key); !value.empty()) {
- return value;
- }
- }
-
- if (std::string value = field_trials_->Lookup(key); !value.empty()) {
- return value;
- }
-
- // Fallback to the global field trial string while some users of the PCLF
- // sets it.
- return field_trial::FindFullName(key);
- }
-
- private:
- absl_nullable std::unique_ptr<FieldTrialsView> trials_;
- absl_nonnull std::unique_ptr<FieldTrials> field_trials_;
-};
-
// Sets mandatory entities in injectable components like `pcf_dependencies`
// and `pc_dependencies` if they are omitted. Also setup required
// dependencies, that won't be specially provided by factory and will be just
@@ -326,12 +292,7 @@
params->rtc_configuration.sdp_semantics = SdpSemantics::kUnifiedPlan;
const Environment env = CreateEnvironment(
- std::make_unique<FieldTrialsCombiner>(
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
- std::move(components->pcf_dependencies->trials),
-#pragma clang diagnostic pop
- std::move(components->pcf_dependencies->field_trials)),
+ std::move(components->pcf_dependencies->field_trials),
time_controller_.GetClock(), time_controller_.GetTaskQueueFactory());
// Create peer connection factory.
