Propagate field trials into NetEq DelayManager

Bug: webrtc:42220378
Change-Id: Idf261b0966fb76a68ec610544c705f0aa0f026bf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/359243
Reviewed-by: Jakob Ivarsson‎ <jakobi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42779}

api/neteq/BUILD.gn

@@ -56,7 +56,9 @@ rtc_source_set("neteq_controller_api") {
   deps = [
     ":neteq_api",
     ":tick_timer",
+    "../../rtc_base:checks",
     "../../system_wrappers:system_wrappers",
+    "../environment",
     "//third_party/abseil-cpp/absl/types:optional",
   ]
 }
@@ -71,6 +73,7 @@ rtc_source_set("default_neteq_controller_factory") {
   deps = [
     ":neteq_controller_api",
     "../../modules/audio_coding:neteq",
+    "../environment",
   ]
 }
 

api/neteq/default_neteq_controller_factory.cc

@@ -10,6 +10,7 @@
 
 #include "api/neteq/default_neteq_controller_factory.h"
 
+#include "api/environment/environment.h"
 #include "modules/audio_coding/neteq/decision_logic.h"
 
 namespace webrtc {
@@ -17,10 +18,10 @@ namespace webrtc {
 DefaultNetEqControllerFactory::DefaultNetEqControllerFactory() = default;
 DefaultNetEqControllerFactory::~DefaultNetEqControllerFactory() = default;
 
-std::unique_ptr<NetEqController>
+std::unique_ptr<NetEqController> DefaultNetEqControllerFactory::Create(
-DefaultNetEqControllerFactory::CreateNetEqController(
+    const Environment& env,
     const NetEqController::Config& config) const {
-  return std::make_unique<DecisionLogic>(config);
+  return std::make_unique<DecisionLogic>(env, config);
 }
 
 }  // namespace webrtc

api/neteq/default_neteq_controller_factory.h

@@ -13,6 +13,7 @@
 
 #include <memory>
 
+#include "api/environment/environment.h"
 #include "api/neteq/neteq_controller.h"
 #include "api/neteq/neteq_controller_factory.h"
 
@@ -27,7 +28,8 @@ class DefaultNetEqControllerFactory : public NetEqControllerFactory {
   DefaultNetEqControllerFactory& operator=(
       const DefaultNetEqControllerFactory&) = delete;
 
-  std::unique_ptr<NetEqController> CreateNetEqController(
+  std::unique_ptr<NetEqController> Create(
+      const Environment& env,
       const NetEqController::Config& config) const override;
 };
 

api/neteq/neteq_controller.h

@@ -67,6 +67,7 @@ class NetEqController {
     int max_packets_in_buffer;
     int base_min_delay_ms;
     TickTimer* tick_timer;
+    // Deprecated, instead use clock passed along with Config in Environment.
     webrtc::Clock* clock = nullptr;
   };
 

api/neteq/neteq_controller_factory.h

@@ -13,7 +13,9 @@
 
 #include <memory>
 
+#include "api/environment/environment.h"
 #include "api/neteq/neteq_controller.h"
+#include "rtc_base/checks.h"
 
 namespace webrtc {
 
@@ -24,8 +26,19 @@ class NetEqControllerFactory {
   virtual ~NetEqControllerFactory() = default;
 
   // Creates a new NetEqController object, with parameters set in `config`.
+  virtual std::unique_ptr<NetEqController> Create(
+      const Environment& env,
+      const NetEqController::Config& config) const {
+    return CreateNetEqController(config);
+  }
+
+ private:
+  // TODO: bugs.webrtc.org/42220378 - Remove when downstream implements Create
   virtual std::unique_ptr<NetEqController> CreateNetEqController(
-      const NetEqController::Config& config) const = 0;
+      const NetEqController::Config& config) const {
+    RTC_DCHECK_NOTREACHED();
+    return nullptr;
+  }
 };
 
 }  // namespace webrtc

modules/audio_coding/BUILD.gn

@@ -738,7 +738,6 @@ rtc_library("neteq") {
     "../../rtc_base/experiments:field_trial_parser",
     "../../rtc_base/synchronization:mutex",
     "../../system_wrappers",
-    "../../system_wrappers:field_trial",
     "../../system_wrappers:metrics",
     "//third_party/abseil-cpp/absl/strings",
     "//third_party/abseil-cpp/absl/strings:string_view",

modules/audio_coding/neteq/decision_logic.cc

@@ -37,8 +37,9 @@ constexpr int kPacketHistorySizeMs = 2000;
 constexpr size_t kCngTimeoutMs = 1000;
 
 std::unique_ptr<DelayManager> CreateDelayManager(
+    const Environment& env,
     const NetEqController::Config& neteq_config) {
-  DelayManager::Config config;
+  DelayManager::Config config(env.field_trials());
   config.max_packets_in_buffer = neteq_config.max_packets_in_buffer;
   config.base_minimum_delay_ms = neteq_config.base_min_delay_ms;
   config.Log();
@@ -63,9 +64,10 @@ bool IsExpand(NetEq::Mode mode) {
 
 }  // namespace
 
-DecisionLogic::DecisionLogic(NetEqController::Config config)
+DecisionLogic::DecisionLogic(const Environment& env,
+                             NetEqController::Config config)
     : DecisionLogic(config,
-                    CreateDelayManager(config),
+                    CreateDelayManager(env, config),
                     std::make_unique<BufferLevelFilter>()) {}
 
 DecisionLogic::DecisionLogic(

modules/audio_coding/neteq/decision_logic.h

@@ -13,6 +13,7 @@
 
 #include <memory>
 
+#include "api/environment/environment.h"
 #include "api/neteq/neteq.h"
 #include "api/neteq/neteq_controller.h"
 #include "api/neteq/tick_timer.h"
@@ -26,7 +27,7 @@ namespace webrtc {
 // This is the class for the decision tree implementation.
 class DecisionLogic : public NetEqController {
  public:
-  DecisionLogic(NetEqController::Config config);
+  DecisionLogic(const Environment& env, NetEqController::Config config);
   DecisionLogic(
       NetEqController::Config config,
       std::unique_ptr<DelayManager> delay_manager,

modules/audio_coding/neteq/decision_logic_unittest.cc

@@ -18,12 +18,15 @@
 #include "modules/audio_coding/neteq/mock/mock_buffer_level_filter.h"
 #include "modules/audio_coding/neteq/mock/mock_delay_manager.h"
 #include "modules/audio_coding/neteq/mock/mock_packet_arrival_history.h"
+#include "test/explicit_key_value_config.h"
 #include "test/gtest.h"
 
 namespace webrtc {
 
 namespace {
 
+using test::ExplicitKeyValueConfig;
+
 constexpr int kSampleRate = 8000;
 constexpr int kSamplesPerMs = kSampleRate / 1000;
 constexpr int kOutputSizeSamples = kSamplesPerMs * 10;
@@ -59,7 +62,7 @@ class DecisionLogicTest : public ::testing::Test {
     config.tick_timer = &tick_timer_;
     config.allow_time_stretching = true;
     auto delay_manager = std::make_unique<MockDelayManager>(
-        DelayManager::Config(), config.tick_timer);
+        DelayManager::Config(ExplicitKeyValueConfig("")), config.tick_timer);
     mock_delay_manager_ = delay_manager.get();
     auto buffer_level_filter = std::make_unique<MockBufferLevelFilter>();
     mock_buffer_level_filter_ = buffer_level_filter.get();

modules/audio_coding/neteq/delay_manager.cc

@@ -18,13 +18,13 @@
 #include <numeric>
 #include <string>
 
+#include "api/field_trials_view.h"
 #include "modules/include/module_common_types_public.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/experiments/struct_parameters_parser.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/numerics/safe_conversions.h"
 #include "rtc_base/numerics/safe_minmax.h"
-#include "system_wrappers/include/field_trial.h"
 
 namespace webrtc {
 namespace {
@@ -45,7 +45,7 @@ std::unique_ptr<ReorderOptimizer> MaybeCreateReorderOptimizer(
 
 }  // namespace
 
-DelayManager::Config::Config() {
+DelayManager::Config::Config(const FieldTrialsView& field_trials) {
   StructParametersParser::Create(                       //
       "quantile", &quantile,                            //
       "forget_factor", &forget_factor,                  //
@@ -54,8 +54,7 @@ DelayManager::Config::Config() {
       "use_reorder_optimizer", &use_reorder_optimizer,  //
       "reorder_forget_factor", &reorder_forget_factor,  //
       "ms_per_loss_percent", &ms_per_loss_percent)
-      ->Parse(webrtc::field_trial::FindFullName(
+      ->Parse(field_trials.Lookup("WebRTC-Audio-NetEqDelayManagerConfig"));
-          "WebRTC-Audio-NetEqDelayManagerConfig"));
 }
 
 void DelayManager::Config::Log() {

modules/audio_coding/neteq/delay_manager.h

@@ -17,6 +17,7 @@
 #include <memory>
 
 #include "absl/types/optional.h"
+#include "api/field_trials_view.h"
 #include "api/neteq/tick_timer.h"
 #include "modules/audio_coding/neteq/histogram.h"
 #include "modules/audio_coding/neteq/reorder_optimizer.h"
@@ -27,7 +28,7 @@ namespace webrtc {
 class DelayManager {
  public:
   struct Config {
-    Config();
+    explicit Config(const FieldTrialsView& field_trials);
     void Log();
 
     // Options that can be configured via field trial.

modules/audio_coding/neteq/delay_manager_unittest.cc

@@ -21,13 +21,16 @@
 #include "modules/audio_coding/neteq/mock/mock_histogram.h"
 #include "modules/audio_coding/neteq/mock/mock_statistics_calculator.h"
 #include "rtc_base/checks.h"
-#include "test/field_trial.h"
+#include "test/explicit_key_value_config.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
 
 namespace webrtc {
 
 namespace {
+
+using test::ExplicitKeyValueConfig;
+
 constexpr int kMaxNumberOfPackets = 200;
 constexpr int kTimeStepMs = 10;
 constexpr int kFrameSizeMs = 20;
@@ -47,7 +50,7 @@ class DelayManagerTest : public ::testing::Test {
 };
 
 DelayManagerTest::DelayManagerTest()
-    : dm_(DelayManager::Config(), &tick_timer_) {}
+    : dm_(DelayManager::Config(ExplicitKeyValueConfig("")), &tick_timer_) {}
 
 void DelayManagerTest::SetUp() {
   dm_.SetPacketAudioLength(kFrameSizeMs);

modules/audio_coding/neteq/neteq_impl.cc

@@ -50,27 +50,10 @@
 #include "rtc_base/strings/audio_format_to_string.h"
 #include "rtc_base/trace_event.h"
 #include "system_wrappers/include/clock.h"
-#include "system_wrappers/include/field_trial.h"
 
 namespace webrtc {
 namespace {
 
-std::unique_ptr<NetEqController> CreateNetEqController(
-    const NetEqControllerFactory& controller_factory,
-    int base_min_delay,
-    int max_packets_in_buffer,
-    bool allow_time_stretching,
-    TickTimer* tick_timer,
-    webrtc::Clock* clock) {
-  NetEqController::Config config;
-  config.base_min_delay_ms = base_min_delay;
-  config.max_packets_in_buffer = max_packets_in_buffer;
-  config.allow_time_stretching = allow_time_stretching;
-  config.tick_timer = tick_timer;
-  config.clock = clock;
-  return controller_factory.CreateNetEqController(config);
-}
-
 AudioFrame::SpeechType ToSpeechType(NetEqImpl::OutputType type) {
   switch (type) {
     case NetEqImpl::OutputType::kNormalSpeech: {
@@ -123,13 +106,14 @@ NetEqImpl::Dependencies::Dependencies(
       packet_buffer(new PacketBuffer(config.max_packets_in_buffer,
                                      tick_timer.get(),
                                      stats.get())),
-      neteq_controller(
+      neteq_controller(controller_factory.Create(
-          CreateNetEqController(controller_factory,
+          env,
-                                config.min_delay_ms,
+          {.allow_time_stretching = !config.for_test_no_time_stretching,
-                                config.max_packets_in_buffer,
+           .max_packets_in_buffer =
-                                !config.for_test_no_time_stretching,
+               static_cast<int>(config.max_packets_in_buffer),
-                                tick_timer.get(),
+           .base_min_delay_ms = config.min_delay_ms,
-                                &env.clock())),
+           .tick_timer = tick_timer.get(),
+           .clock = &env.clock()})),
       red_payload_splitter(new RedPayloadSplitter),
       timestamp_scaler(new TimestampScaler(*decoder_database)),
       accelerate_factory(new AccelerateFactory),

modules/audio_coding/neteq/neteq_impl_unittest.cc

@@ -129,7 +129,7 @@ class NetEqImplTest : public ::testing::Test {
       controller_config.max_packets_in_buffer = config_.max_packets_in_buffer;
       controller_config.clock = &clock_;
       deps.neteq_controller =
-          std::make_unique<DecisionLogic>(std::move(controller_config));
+          std::make_unique<DecisionLogic>(env_, std::move(controller_config));
     }
     neteq_controller_ = deps.neteq_controller.get();
 

modules/audio_coding/neteq/packet_buffer.cc

@@ -28,7 +28,6 @@
 #include "rtc_base/experiments/struct_parameters_parser.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/numerics/safe_conversions.h"
-#include "system_wrappers/include/field_trial.h"
 
 namespace webrtc {
 namespace {