Add a CreateNetEq method that takes an AudioDecoderFactory

The NetEqFactory is currently expected to wrap the AudioDecoderFactory,
but this turns out not to be a good idea. Instead, it makes more sense
to pass the AudioDecoderFactory through the CreateNetEq method.

Bug: webrtc:11005
Change-Id: I8027ff6593f40c92072e7e88157631dcf329a984
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160644
Commit-Queue: Ivo Creusen <ivoc@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29918}

api/neteq/DEPS

@@ -5,4 +5,7 @@ specific_include_rules = {
   "custom_neteq_factory\.h": [
     "+system_wrappers/include/clock.h",
   ],
+  "default_neteq_factory\.h": [
+    "+system_wrappers/include/clock.h",
+  ],
 }

api/neteq/custom_neteq_factory.cc

@@ -16,6 +16,10 @@
 
 namespace webrtc {
 
+CustomNetEqFactory::CustomNetEqFactory(
+    std::unique_ptr<NetEqControllerFactory> controller_factory)
+    : controller_factory_(std::move(controller_factory)) {}
+
 CustomNetEqFactory::CustomNetEqFactory(
     rtc::scoped_refptr<AudioDecoderFactory> decoder_factory,
     std::unique_ptr<NetEqControllerFactory> controller_factory)
@@ -31,4 +35,13 @@ std::unique_ptr<NetEq> CustomNetEqFactory::CreateNetEq(
                                       *controller_factory_));
 }
 
+std::unique_ptr<NetEq> CustomNetEqFactory::CreateNetEq(
+    const NetEq::Config& config,
+    const rtc::scoped_refptr<AudioDecoderFactory>& decoder_factory,
+    Clock* clock) const {
+  return std::make_unique<NetEqImpl>(
+      config, NetEqImpl::Dependencies(config, clock, decoder_factory,
+                                      *controller_factory_));
+}
+
 }  // namespace webrtc

api/neteq/custom_neteq_factory.h

@@ -22,9 +22,13 @@
 namespace webrtc {
 
 // This factory can be used to generate NetEq instances that make use of a
-// custom AudioDecoderFactory and/or NetEqControllerFactory.
+// custom AudioDecoderFactory and/or NetEqControllerFactory. Using a custom
+// AudioDecoderFactory is deprecated and the functionality will be removed soon.
 class CustomNetEqFactory : public NetEqFactory {
  public:
+  explicit CustomNetEqFactory(
+      std::unique_ptr<NetEqControllerFactory> controller_factory);
+  // This constructor is deprecated and will be removed soon.
   CustomNetEqFactory(
       rtc::scoped_refptr<AudioDecoderFactory> decoder_factory,
       std::unique_ptr<NetEqControllerFactory> controller_factory);
@@ -35,6 +39,11 @@ class CustomNetEqFactory : public NetEqFactory {
   std::unique_ptr<NetEq> CreateNetEq(const NetEq::Config& config,
                                      Clock* clock) const override;
 
+  std::unique_ptr<NetEq> CreateNetEq(
+      const NetEq::Config& config,
+      const rtc::scoped_refptr<AudioDecoderFactory>& decoder_factory,
+      Clock* clock) const override;
+
  private:
   rtc::scoped_refptr<AudioDecoderFactory> decoder_factory_;
   std::unique_ptr<NetEqControllerFactory> controller_factory_;

api/neteq/neteq_factory.h

@@ -13,6 +13,7 @@
 
 #include <memory>
 
+#include "api/audio_codecs/audio_decoder_factory.h"
 #include "api/neteq/neteq.h"
 #include "system_wrappers/include/clock.h"
 
@@ -26,6 +27,11 @@ class NetEqFactory {
   // Creates a new NetEq object, with parameters set in |config|. The |config|
   // object will only have to be valid for the duration of the call to this
   // method.
+  virtual std::unique_ptr<NetEq> CreateNetEq(
+      const NetEq::Config& config,
+      const rtc::scoped_refptr<AudioDecoderFactory>& decoder_factory,
+      Clock* clock) const = 0;
+  // This method is deprecated and will be removed.
   virtual std::unique_ptr<NetEq> CreateNetEq(const NetEq::Config& config,
                                              Clock* clock) const = 0;
 };

api/test/neteq_factory_with_codecs.cc

@@ -30,6 +30,14 @@ class NetEqFactoryWithCodecs final : public NetEqFactory {
         config, NetEqImpl::Dependencies(config, clock, decoder_factory_,
                                         *controller_factory_));
   }
+  std::unique_ptr<NetEq> CreateNetEq(
+      const NetEq::Config& config,
+      const rtc::scoped_refptr<AudioDecoderFactory>& decoder_factory,
+      Clock* clock) const override {
+    return std::make_unique<NetEqImpl>(
+        config, NetEqImpl::Dependencies(config, clock, decoder_factory,
+                                        *controller_factory_));
+  }
 
  private:
   const rtc::scoped_refptr<AudioDecoderFactory> decoder_factory_ =

modules/audio_coding/BUILD.gn

@@ -43,6 +43,7 @@ rtc_library("audio_coding") {
 
   deps = [
     ":audio_coding_module_typedefs",
+    ":default_neteq_factory",
     ":neteq",
     "..:module_api",
     "..:module_api_public",
@@ -50,8 +51,6 @@ rtc_library("audio_coding") {
     "../../api:function_view",
     "../../api/audio:audio_frame_api",
     "../../api/audio_codecs:audio_codecs_api",
-    "../../api/neteq:custom_neteq_factory",
-    "../../api/neteq:default_neteq_controller_factory",
     "../../api/neteq:neteq_api",
     "../../common_audio",
     "../../common_audio:common_audio_c",
@@ -1028,6 +1027,22 @@ rtc_library("neteq") {
   ]
 }
 
+rtc_source_set("default_neteq_factory") {
+  visibility += webrtc_default_visibility
+  sources = [
+    "neteq/default_neteq_factory.cc",
+    "neteq/default_neteq_factory.h",
+  ]
+  deps = [
+    ":neteq",
+    "../../api:scoped_refptr",
+    "../../api/audio_codecs:audio_codecs_api",
+    "../../api/neteq:default_neteq_controller_factory",
+    "../../api/neteq:neteq_api",
+    "../../system_wrappers:system_wrappers",
+  ]
+}
+
 # Although providing only test support, this target must be outside of the
 # rtc_include_tests conditional. The reason is that it supports fuzzer tests
 # that ultimately are built and run as a part of the Chromium ecosystem, which

modules/audio_coding/acm2/acm_receiver.cc

@@ -19,11 +19,10 @@
 #include "absl/strings/match.h"
 #include "api/audio/audio_frame.h"
 #include "api/audio_codecs/audio_decoder.h"
-#include "api/neteq/custom_neteq_factory.h"
-#include "api/neteq/default_neteq_controller_factory.h"
 #include "api/neteq/neteq.h"
 #include "modules/audio_coding/acm2/acm_resampler.h"
 #include "modules/audio_coding/acm2/call_statistics.h"
+#include "modules/audio_coding/neteq/default_neteq_factory.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/numerics/safe_conversions.h"
@@ -41,16 +40,10 @@ std::unique_ptr<NetEq> CreateNetEq(
     const NetEq::Config& config,
     Clock* clock,
     const rtc::scoped_refptr<AudioDecoderFactory>& decoder_factory) {
-  RTC_CHECK((neteq_factory == nullptr) || (decoder_factory.get() == nullptr))
-      << "Either a NetEqFactory or a AudioDecoderFactory should be injected, "
-         "supplying both is not supported. Please wrap the AudioDecoderFactory "
-         "inside the NetEqFactory when using both.";
   if (neteq_factory) {
-    return neteq_factory->CreateNetEq(config, clock);
+    return neteq_factory->CreateNetEq(config, decoder_factory, clock);
   }
-  CustomNetEqFactory custom_factory(
+  return DefaultNetEqFactory().CreateNetEq(config, decoder_factory, clock);
-      decoder_factory, std::make_unique<DefaultNetEqControllerFactory>());
-  return custom_factory.CreateNetEq(config, clock);
 }
 
 }  // namespace

modules/audio_coding/neteq/default_neteq_factory.cc

@@ -0,0 +1,39 @@
+/*
+ *  Copyright (c) 2019 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "modules/audio_coding/neteq/default_neteq_factory.h"
+
+#include <utility>
+
+#include "modules/audio_coding/neteq/neteq_impl.h"
+
+namespace webrtc {
+
+DefaultNetEqFactory::DefaultNetEqFactory() = default;
+DefaultNetEqFactory::~DefaultNetEqFactory() = default;
+
+std::unique_ptr<NetEq> DefaultNetEqFactory::CreateNetEq(
+    const NetEq::Config& config,
+    const rtc::scoped_refptr<AudioDecoderFactory>& decoder_factory,
+    Clock* clock) const {
+  return std::make_unique<NetEqImpl>(
+      config, NetEqImpl::Dependencies(config, clock, decoder_factory,
+                                      controller_factory_));
+}
+
+std::unique_ptr<NetEq> DefaultNetEqFactory::CreateNetEq(
+    const NetEq::Config& /*config*/,
+    Clock* /*clock*/) const {
+  RTC_NOTREACHED() << "Calling CreateNetEq without an AudioDecoderFactory on "
+                      "DefaultNetEqFactory is not supported.";
+  return nullptr;
+}
+
+}  // namespace webrtc

modules/audio_coding/neteq/default_neteq_factory.h

@@ -0,0 +1,43 @@
+/*
+ *  Copyright (c) 2019 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef MODULES_AUDIO_CODING_NETEQ_DEFAULT_NETEQ_FACTORY_H_
+#define MODULES_AUDIO_CODING_NETEQ_DEFAULT_NETEQ_FACTORY_H_
+
+#include <memory>
+
+#include "api/audio_codecs/audio_decoder_factory.h"
+#include "api/neteq/default_neteq_controller_factory.h"
+#include "api/neteq/neteq_factory.h"
+#include "api/scoped_refptr.h"
+#include "system_wrappers/include/clock.h"
+
+namespace webrtc {
+
+class DefaultNetEqFactory : public NetEqFactory {
+ public:
+  DefaultNetEqFactory();
+  ~DefaultNetEqFactory() override;
+  DefaultNetEqFactory(const DefaultNetEqFactory&) = delete;
+  DefaultNetEqFactory& operator=(const DefaultNetEqFactory&) = delete;
+
+  std::unique_ptr<NetEq> CreateNetEq(
+      const NetEq::Config& config,
+      const rtc::scoped_refptr<AudioDecoderFactory>& decoder_factory,
+      Clock* clock) const override;
+  std::unique_ptr<NetEq> CreateNetEq(const NetEq::Config& config,
+                                     Clock* clock) const override;
+
+ private:
+  const DefaultNetEqControllerFactory controller_factory_;
+};
+
+}  // namespace webrtc
+#endif  // MODULES_AUDIO_CODING_NETEQ_DEFAULT_NETEQ_FACTORY_H_

sdk/android/api/org/webrtc/PeerConnectionFactory.java

@@ -167,7 +167,6 @@ public class PeerConnectionFactory {
     @Nullable private AudioDeviceModule audioDeviceModule;
     private AudioEncoderFactoryFactory audioEncoderFactoryFactory =
         new BuiltinAudioEncoderFactoryFactory();
-    @Nullable
     private AudioDecoderFactoryFactory audioDecoderFactoryFactory =
         new BuiltinAudioDecoderFactoryFactory();
     @Nullable private VideoEncoderFactory videoEncoderFactory;
@@ -201,7 +200,6 @@ public class PeerConnectionFactory {
       return this;
     }
 
-    @Deprecated
     public Builder setAudioDecoderFactoryFactory(
         AudioDecoderFactoryFactory audioDecoderFactoryFactory) {
       if (audioDecoderFactoryFactory == null) {
@@ -263,7 +261,6 @@ public class PeerConnectionFactory {
      * NetEqFactoryFactory.
      */
     public Builder setNetEqFactoryFactory(NetEqFactoryFactory neteqFactoryFactory) {
-      this.audioDecoderFactoryFactory = null;
       this.neteqFactoryFactory = neteqFactoryFactory;
       return this;
     }
@@ -274,18 +271,11 @@ public class PeerConnectionFactory {
         audioDeviceModule = JavaAudioDeviceModule.builder(ContextUtils.getApplicationContext())
                                 .createAudioDeviceModule();
       }
-      if (neteqFactoryFactory == null && audioDecoderFactoryFactory == null) {
-        throw new IllegalStateException(
-            "Setting both audioDecoderFactoryFactory and neteqFactoryFactory "
-            + "to null is not allowed.");
-      }
       return nativeCreatePeerConnectionFactory(ContextUtils.getApplicationContext(), options,
           audioDeviceModule.getNativeAudioDeviceModulePointer(),
           audioEncoderFactoryFactory.createNativeAudioEncoderFactory(),
-          audioDecoderFactoryFactory == null
+          audioDecoderFactoryFactory.createNativeAudioDecoderFactory(), videoEncoderFactory,
-              ? 0
+          videoDecoderFactory,
-              : audioDecoderFactoryFactory.createNativeAudioDecoderFactory(),
-          videoEncoderFactory, videoDecoderFactory,
           audioProcessingFactory == null ? 0 : audioProcessingFactory.createNative(),
           fecControllerFactoryFactory == null ? 0 : fecControllerFactoryFactory.createNative(),
           networkControllerFactoryFactory == null