Hide the AudioEncoderCng class behind a create function

And put codecs/cng/webrtc_cng.h in a non-public build target while
we're at it.

Bug: webrtc:8396
Change-Id: I9f51dffadfb645cd1454617fad30e09d639ff53c
Reviewed-on: https://webrtc-review.googlesource.com/c/108782
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25486}

audio/BUILD.gn

@@ -67,9 +67,9 @@ rtc_static_library("audio") {
     "../logging:rtc_event_log_api",
     "../logging:rtc_stream_config",
     "../modules/audio_coding",
+    "../modules/audio_coding:audio_encoder_cng",
     "../modules/audio_coding:audio_format_conversion",
     "../modules/audio_coding:audio_network_adaptor_config",
-    "../modules/audio_coding:cng",
     "../modules/audio_device",
     "../modules/audio_processing",
     "../modules/bitrate_controller:bitrate_controller",

audio/audio_send_stream.cc

@@ -623,12 +623,12 @@ bool AudioSendStream::SetupSendCodec(AudioSendStream* stream,
 
   // Wrap the encoder in a an AudioEncoderCNG, if VAD is enabled.
   if (spec.cng_payload_type) {
-    AudioEncoderCng::Config cng_config;
+    AudioEncoderCngConfig cng_config;
     cng_config.num_channels = encoder->NumChannels();
     cng_config.payload_type = *spec.cng_payload_type;
     cng_config.speech_encoder = std::move(encoder);
     cng_config.vad_mode = Vad::kVadNormal;
-    encoder.reset(new AudioEncoderCng(std::move(cng_config)));
+    encoder = CreateComfortNoiseEncoder(std::move(cng_config));
 
     stream->RegisterCngPayloadType(
         *spec.cng_payload_type,
@@ -750,12 +750,12 @@ void AudioSendStream::ReconfigureCNG(AudioSendStream* stream,
           old_encoder = std::move(tmp);
         }
         if (new_config.send_codec_spec->cng_payload_type) {
-          AudioEncoderCng::Config config;
+          AudioEncoderCngConfig config;
           config.speech_encoder = std::move(old_encoder);
           config.num_channels = config.speech_encoder->NumChannels();
           config.payload_type = *new_config.send_codec_spec->cng_payload_type;
           config.vad_mode = Vad::kVadNormal;
-          encoder_ptr->reset(new AudioEncoderCng(std::move(config)));
+          *encoder_ptr = CreateComfortNoiseEncoder(std::move(config));
         } else {
           *encoder_ptr = std::move(old_encoder);
         }

modules/audio_coding/BUILD.gn

@@ -15,7 +15,6 @@ if (!build_with_mozilla) {
 visibility = [ ":*" ]
 
 audio_codec_deps = [
-  ":cng",
   ":g711",
   ":pcm16b",
 ]
@@ -85,6 +84,7 @@ rtc_static_library("rent_a_codec") {
            ":audio_coding_module_typedefs",
            ":isac_common",
            ":isac_fix_c",
+           ":audio_encoder_cng",
            ":neteq_decoder_enum",
          ] + audio_codec_deps
 
@@ -161,24 +161,34 @@ rtc_static_library("legacy_encoded_audio_frame") {
   ]
 }
 
-rtc_static_library("cng") {
+rtc_static_library("webrtc_cng") {
-  visibility += [ "*" ]
+  visibility += webrtc_default_visibility
   sources = [
-    "codecs/cng/audio_encoder_cng.cc",
-    "codecs/cng/audio_encoder_cng.h",
     "codecs/cng/webrtc_cng.cc",
     "codecs/cng/webrtc_cng.h",
   ]
 
   deps = [
-    "../..:webrtc_common",
     "../../api:array_view",
-    "../../api/audio_codecs:audio_codecs_api",
-    "../../common_audio",
     "../../common_audio:common_audio_c",
     "../../rtc_base:checks",
     "../../rtc_base:rtc_base_approved",
-    "//third_party/abseil-cpp/absl/types:optional",
+    "../../rtc_base:safe_conversions",
+  ]
+}
+
+rtc_static_library("audio_encoder_cng") {
+  visibility += [ "*" ]
+  sources = [
+    "codecs/cng/audio_encoder_cng.cc",
+    "codecs/cng/audio_encoder_cng.h",
+  ]
+
+  deps = [
+    ":webrtc_cng",
+    "../../api/audio_codecs:audio_codecs_api",
+    "../../common_audio",
+    "../../rtc_base:checks",
   ]
 }
 
@@ -1066,8 +1076,8 @@ rtc_static_library("neteq") {
 
   deps = [
     ":audio_coding_module_typedefs",
-    ":cng",
     ":neteq_decoder_enum",
+    ":webrtc_cng",
     "..:module_api",
     "..:module_api_public",
     "../..:webrtc_common",
@@ -1352,8 +1362,8 @@ if (rtc_include_tests) {
     deps = [
       ":audio_coding",
       ":audio_coding_module_typedefs",
+      ":audio_encoder_cng",
       ":audio_format_conversion",
-      ":cng",
       ":pcm16b_c",
       ":red",
       "..:module_api",
@@ -1687,6 +1697,7 @@ if (rtc_include_tests) {
     deps = audio_coding_deps + [
              "//third_party/abseil-cpp/absl/memory",
              ":audio_coding",
+             ":audio_encoder_cng",
              ":neteq_input_audio_tools",
              "../../api/audio:audio_frame_api",
              "../../api/audio_codecs/g711:audio_encoder_g711",
@@ -2074,9 +2085,9 @@ if (rtc_include_tests) {
       ":acm_send_test",
       ":audio_coding",
       ":audio_coding_module_typedefs",
+      ":audio_encoder_cng",
       ":audio_format_conversion",
       ":audio_network_adaptor",
-      ":cng",
       ":g711",
       ":ilbc",
       ":isac",
@@ -2090,6 +2101,7 @@ if (rtc_include_tests) {
       ":pcm16b",
       ":red",
       ":rent_a_codec",
+      ":webrtc_cng",
       ":webrtc_opus",
       "..:module_api",
       "../..:webrtc_common",

modules/audio_coding/acm2/acm_receiver_unittest.cc

@@ -73,12 +73,12 @@ class AcmReceiverTestOldApi : public AudioPacketizationCallback,
     // If we have a compatible CN specification, stack a CNG on top.
     auto it = cng_payload_types.find(info.sample_rate_hz);
     if (it != cng_payload_types.end()) {
-      AudioEncoderCng::Config config;
+      AudioEncoderCngConfig config;
       config.speech_encoder = std::move(enc);
       config.num_channels = 1;
       config.payload_type = it->second;
       config.vad_mode = Vad::kVadNormal;
-      enc = absl::make_unique<AudioEncoderCng>(std::move(config));
+      enc = CreateComfortNoiseEncoder(std::move(config));
     }
 
     // Actually start using the new encoder.

modules/audio_coding/acm2/audio_coding_module_unittest.cc

@@ -408,12 +408,12 @@ class AudioCodingModuleTestWithComfortNoiseOldApi
               acm_->RegisterReceiveCodec(
                   rtp_payload_type, SdpAudioFormat("cn", kSampleRateHz, 1)));
     acm_->ModifyEncoder([&](std::unique_ptr<AudioEncoder>* enc) {
-      AudioEncoderCng::Config config;
+      AudioEncoderCngConfig config;
       config.speech_encoder = std::move(*enc);
       config.num_channels = 1;
       config.payload_type = rtp_payload_type;
       config.vad_mode = Vad::kVadNormal;
-      *enc = absl::make_unique<AudioEncoderCng>(std::move(config));
+      *enc = CreateComfortNoiseEncoder(std::move(config));
     });
   }
 

modules/audio_coding/acm2/rent_a_codec.cc

@@ -194,7 +194,7 @@ std::unique_ptr<AudioEncoder> CreateCngEncoder(
     std::unique_ptr<AudioEncoder> encoder,
     int payload_type,
     ACMVADMode vad_mode) {
-  AudioEncoderCng::Config config;
+  AudioEncoderCngConfig config;
   config.num_channels = encoder->NumChannels();
   config.payload_type = payload_type;
   config.speech_encoder = std::move(encoder);
@@ -214,7 +214,7 @@ std::unique_ptr<AudioEncoder> CreateCngEncoder(
     default:
       FATAL();
   }
-  return std::unique_ptr<AudioEncoder>(new AudioEncoderCng(std::move(config)));
+  return CreateComfortNoiseEncoder(std::move(config));
 }
 
 std::unique_ptr<AudioDecoder> CreateIsacDecoder(

modules/audio_coding/codecs/cng/audio_encoder_cng.cc

@@ -23,29 +23,58 @@ namespace {
 
 const int kMaxFrameSizeMs = 60;
 
-}  // namespace
+class AudioEncoderCng final : public AudioEncoder {
+ public:
+  explicit AudioEncoderCng(AudioEncoderCngConfig&& config);
+  ~AudioEncoderCng() override;
 
-AudioEncoderCng::Config::Config() = default;
+  // Not copyable or moveable.
-AudioEncoderCng::Config::Config(Config&&) = default;
+  AudioEncoderCng(const AudioEncoderCng&) = delete;
-AudioEncoderCng::Config::~Config() = default;
+  AudioEncoderCng(AudioEncoderCng&&) = delete;
+  AudioEncoderCng& operator=(const AudioEncoderCng&) = delete;
+  AudioEncoderCng& operator=(AudioEncoderCng&&) = delete;
 
-bool AudioEncoderCng::Config::IsOk() const {
+  int SampleRateHz() const override;
-  if (num_channels != 1)
+  size_t NumChannels() const override;
-    return false;
+  int RtpTimestampRateHz() const override;
-  if (!speech_encoder)
+  size_t Num10MsFramesInNextPacket() const override;
-    return false;
+  size_t Max10MsFramesInAPacket() const override;
-  if (num_channels != speech_encoder->NumChannels())
+  int GetTargetBitrate() const override;
-    return false;
+  EncodedInfo EncodeImpl(uint32_t rtp_timestamp,
-  if (sid_frame_interval_ms <
+                         rtc::ArrayView<const int16_t> audio,
-      static_cast<int>(speech_encoder->Max10MsFramesInAPacket() * 10))
+                         rtc::Buffer* encoded) override;
-    return false;
+  void Reset() override;
-  if (num_cng_coefficients > WEBRTC_CNG_MAX_LPC_ORDER ||
+  bool SetFec(bool enable) override;
-      num_cng_coefficients <= 0)
+  bool SetDtx(bool enable) override;
-    return false;
+  bool SetApplication(Application application) override;
-  return true;
+  void SetMaxPlaybackRate(int frequency_hz) override;
-}
+  rtc::ArrayView<std::unique_ptr<AudioEncoder>> ReclaimContainedEncoders()
+      override;
+  void OnReceivedUplinkPacketLossFraction(
+      float uplink_packet_loss_fraction) override;
+  void OnReceivedUplinkRecoverablePacketLossFraction(
+      float uplink_recoverable_packet_loss_fraction) override;
+  void OnReceivedUplinkBandwidth(
+      int target_audio_bitrate_bps,
+      absl::optional<int64_t> bwe_period_ms) override;
 
-AudioEncoderCng::AudioEncoderCng(Config&& config)
+ private:
+  EncodedInfo EncodePassive(size_t frames_to_encode, rtc::Buffer* encoded);
+  EncodedInfo EncodeActive(size_t frames_to_encode, rtc::Buffer* encoded);
+  size_t SamplesPer10msFrame() const;
+
+  std::unique_ptr<AudioEncoder> speech_encoder_;
+  const int cng_payload_type_;
+  const int num_cng_coefficients_;
+  const int sid_frame_interval_ms_;
+  std::vector<int16_t> speech_buffer_;
+  std::vector<uint32_t> rtp_timestamps_;
+  bool last_frame_active_;
+  std::unique_ptr<Vad> vad_;
+  std::unique_ptr<ComfortNoiseEncoder> cng_encoder_;
+};
+
+AudioEncoderCng::AudioEncoderCng(AudioEncoderCngConfig&& config)
     : speech_encoder_((static_cast<void>([&] {
                          RTC_CHECK(config.IsOk()) << "Invalid configuration.";
                        }()),
@@ -263,4 +292,31 @@ size_t AudioEncoderCng::SamplesPer10msFrame() const {
   return rtc::CheckedDivExact(10 * SampleRateHz(), 1000);
 }
 
+}  // namespace
+
+AudioEncoderCngConfig::AudioEncoderCngConfig() = default;
+AudioEncoderCngConfig::AudioEncoderCngConfig(AudioEncoderCngConfig&&) = default;
+AudioEncoderCngConfig::~AudioEncoderCngConfig() = default;
+
+bool AudioEncoderCngConfig::IsOk() const {
+  if (num_channels != 1)
+    return false;
+  if (!speech_encoder)
+    return false;
+  if (num_channels != speech_encoder->NumChannels())
+    return false;
+  if (sid_frame_interval_ms <
+      static_cast<int>(speech_encoder->Max10MsFramesInAPacket() * 10))
+    return false;
+  if (num_cng_coefficients > WEBRTC_CNG_MAX_LPC_ORDER ||
+      num_cng_coefficients <= 0)
+    return false;
+  return true;
+}
+
+std::unique_ptr<AudioEncoder> CreateComfortNoiseEncoder(
+    AudioEncoderCngConfig&& config) {
+  return absl::make_unique<AudioEncoderCng>(std::move(config));
+}
+
 }  // namespace webrtc

modules/audio_coding/codecs/cng/audio_encoder_cng.h

@@ -12,89 +12,37 @@
 #define MODULES_AUDIO_CODING_CODECS_CNG_AUDIO_ENCODER_CNG_H_
 
 #include <stddef.h>
-#include <stdint.h>
 #include <memory>
-#include <vector>
 
-#include "absl/types/optional.h"
-#include "api/array_view.h"
 #include "api/audio_codecs/audio_encoder.h"
 #include "common_audio/vad/include/vad.h"
-#include "rtc_base/buffer.h"
-#include "rtc_base/constructormagic.h"
 
 namespace webrtc {
 
-class ComfortNoiseEncoder;
+struct AudioEncoderCngConfig {
+  // Moveable, not copyable.
+  AudioEncoderCngConfig();
+  AudioEncoderCngConfig(AudioEncoderCngConfig&&);
+  ~AudioEncoderCngConfig();
 
-class AudioEncoderCng final : public AudioEncoder {
+  bool IsOk() const;
- public:
-  struct Config {
-    Config();
-    Config(Config&&);
-    ~Config();
-    bool IsOk() const;
 
-    size_t num_channels = 1;
+  size_t num_channels = 1;
-    int payload_type = 13;
+  int payload_type = 13;
-    std::unique_ptr<AudioEncoder> speech_encoder;
+  std::unique_ptr<AudioEncoder> speech_encoder;
-    Vad::Aggressiveness vad_mode = Vad::kVadNormal;
+  Vad::Aggressiveness vad_mode = Vad::kVadNormal;
-    int sid_frame_interval_ms = 100;
+  int sid_frame_interval_ms = 100;
-    int num_cng_coefficients = 8;
+  int num_cng_coefficients = 8;
-    // The Vad pointer is mainly for testing. If a NULL pointer is passed, the
+  // The Vad pointer is mainly for testing. If a NULL pointer is passed, the
-    // AudioEncoderCng creates (and destroys) a Vad object internally. If an
+  // AudioEncoderCng creates (and destroys) a Vad object internally. If an
-    // object is passed, the AudioEncoderCng assumes ownership of the Vad
+  // object is passed, the AudioEncoderCng assumes ownership of the Vad
-    // object.
+  // object.
-    Vad* vad = nullptr;
+  Vad* vad = nullptr;
-  };
-
-  explicit AudioEncoderCng(Config&& config);
-  ~AudioEncoderCng() override;
-
-  int SampleRateHz() const override;
-  size_t NumChannels() const override;
-  int RtpTimestampRateHz() const override;
-  size_t Num10MsFramesInNextPacket() const override;
-  size_t Max10MsFramesInAPacket() const override;
-  int GetTargetBitrate() const override;
-  EncodedInfo EncodeImpl(uint32_t rtp_timestamp,
-                         rtc::ArrayView<const int16_t> audio,
-                         rtc::Buffer* encoded) override;
-  void Reset() override;
-  bool SetFec(bool enable) override;
-  bool SetDtx(bool enable) override;
-  bool SetApplication(Application application) override;
-  void SetMaxPlaybackRate(int frequency_hz) override;
-  rtc::ArrayView<std::unique_ptr<AudioEncoder>> ReclaimContainedEncoders()
-      override;
-  void OnReceivedUplinkPacketLossFraction(
-      float uplink_packet_loss_fraction) override;
-  void OnReceivedUplinkRecoverablePacketLossFraction(
-      float uplink_recoverable_packet_loss_fraction) override;
-  void OnReceivedUplinkBandwidth(
-      int target_audio_bitrate_bps,
-      absl::optional<int64_t> bwe_period_ms) override;
-
- private:
-  EncodedInfo EncodePassive(size_t frames_to_encode,
-                            rtc::Buffer* encoded);
-  EncodedInfo EncodeActive(size_t frames_to_encode,
-                           rtc::Buffer* encoded);
-  size_t SamplesPer10msFrame() const;
-
-  std::unique_ptr<AudioEncoder> speech_encoder_;
-  const int cng_payload_type_;
-  const int num_cng_coefficients_;
-  const int sid_frame_interval_ms_;
-  std::vector<int16_t> speech_buffer_;
-  std::vector<uint32_t> rtp_timestamps_;
-  bool last_frame_active_;
-  std::unique_ptr<Vad> vad_;
-  std::unique_ptr<ComfortNoiseEncoder> cng_encoder_;
-
-  RTC_DISALLOW_COPY_AND_ASSIGN(AudioEncoderCng);
 };
 
+std::unique_ptr<AudioEncoder> CreateComfortNoiseEncoder(
+    AudioEncoderCngConfig&& config);
+
 }  // namespace webrtc
 
 #endif  // MODULES_AUDIO_CODING_CODECS_CNG_AUDIO_ENCODER_CNG_H_

modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc

@@ -50,8 +50,8 @@ class AudioEncoderCngTest : public ::testing::Test {
     cng_.reset();
   }
 
-  AudioEncoderCng::Config MakeCngConfig() {
+  AudioEncoderCngConfig MakeCngConfig() {
-    AudioEncoderCng::Config config;
+    AudioEncoderCngConfig config;
     config.speech_encoder = std::move(mock_encoder_owner_);
     EXPECT_TRUE(config.speech_encoder);
 
@@ -63,7 +63,7 @@ class AudioEncoderCngTest : public ::testing::Test {
     return config;
   }
 
-  void CreateCng(AudioEncoderCng::Config&& config) {
+  void CreateCng(AudioEncoderCngConfig&& config) {
     num_audio_samples_10ms_ = static_cast<size_t>(10 * sample_rate_hz_ / 1000);
     ASSERT_LE(num_audio_samples_10ms_, kMaxNumSamples);
     if (config.speech_encoder) {
@@ -75,7 +75,7 @@ class AudioEncoderCngTest : public ::testing::Test {
       EXPECT_CALL(*mock_encoder_, Max10MsFramesInAPacket())
           .WillOnce(Return(1u));
     }
-    cng_.reset(new AudioEncoderCng(std::move(config)));
+    cng_ = CreateComfortNoiseEncoder(std::move(config));
   }
 
   void Encode() {
@@ -193,7 +193,7 @@ class AudioEncoderCngTest : public ::testing::Test {
     return encoded_info_.payload_type != kCngPayloadType;
   }
 
-  std::unique_ptr<AudioEncoderCng> cng_;
+  std::unique_ptr<AudioEncoder> cng_;
   std::unique_ptr<MockAudioEncoder> mock_encoder_owner_;
   MockAudioEncoder* mock_encoder_;
   MockVad* mock_vad_;  // Ownership is transferred to |cng_|.
@@ -432,7 +432,7 @@ class AudioEncoderCngDeathTest : public AudioEncoderCngTest {
   // deleted.
   void TearDown() override { cng_.reset(); }
 
-  AudioEncoderCng::Config MakeCngConfig() {
+  AudioEncoderCngConfig MakeCngConfig() {
     // Don't provide a Vad mock object, since it would leak when the test dies.
     auto config = AudioEncoderCngTest::MakeCngConfig();
     config.vad = nullptr;

modules/audio_coding/neteq/tools/rtp_encode.cc

@@ -246,8 +246,8 @@ std::unique_ptr<AudioEncoder> CreateEncoder(CodecType codec_type,
   return nullptr;
 }
 
-AudioEncoderCng::Config GetCngConfig(int sample_rate_hz) {
+AudioEncoderCngConfig GetCngConfig(int sample_rate_hz) {
-  AudioEncoderCng::Config cng_config;
+  AudioEncoderCngConfig cng_config;
   const auto default_payload_type = [&] {
     switch (sample_rate_hz) {
       case 8000:
@@ -313,10 +313,10 @@ int RunRtpEncode(int argc, char* argv[]) {
 
   // Create an external VAD/CNG encoder if needed.
   if (FLAG_dtx && !codec_it->second.internal_dtx) {
-    AudioEncoderCng::Config cng_config = GetCngConfig(codec->SampleRateHz());
+    AudioEncoderCngConfig cng_config = GetCngConfig(codec->SampleRateHz());
     RTC_DCHECK(codec);
     cng_config.speech_encoder = std::move(codec);
-    codec = absl::make_unique<AudioEncoderCng>(std::move(cng_config));
+    codec = CreateComfortNoiseEncoder(std::move(cng_config));
   }
   RTC_DCHECK(codec);
 

modules/audio_coding/test/TestRedFec.cc

@@ -175,12 +175,12 @@ void TestRedFec::RegisterSendCodec(
   EXPECT_NE(encoder, nullptr);
   if (!absl::EqualsIgnoreCase(codec_format.name, "opus")) {
     if (vad_mode.has_value()) {
-      AudioEncoderCng::Config config;
+      AudioEncoderCngConfig config;
       config.speech_encoder = std::move(encoder);
       config.num_channels = 1;
       config.payload_type = cn_payload_type;
       config.vad_mode = vad_mode.value();
-      encoder = absl::make_unique<AudioEncoderCng>(std::move(config));
+      encoder = CreateComfortNoiseEncoder(std::move(config));
       EXPECT_EQ(true,
                 other_acm->RegisterReceiveCodec(
                     cn_payload_type, {"CN", codec_format.clockrate_hz, 1}));

modules/audio_coding/test/TestVADDTX.cc

@@ -83,12 +83,12 @@ bool TestVadDtx::RegisterCodec(const SdpAudioFormat& codec_format,
                                                     absl::nullopt);
   if (vad_mode.has_value() &&
       !absl::EqualsIgnoreCase(codec_format.name, "opus")) {
-    AudioEncoderCng::Config config;
+    AudioEncoderCngConfig config;
     config.speech_encoder = std::move(encoder);
     config.num_channels = 1;
     config.payload_type = cn_payload_type;
     config.vad_mode = vad_mode.value();
-    encoder = absl::make_unique<AudioEncoderCng>(std::move(config));
+    encoder = CreateComfortNoiseEncoder(std::move(config));
     added_comfort_noise = true;
   }
   channel_->SetIsStereo(encoder->NumChannels() > 1);

test/fuzzers/BUILD.gn

@@ -517,7 +517,7 @@ webrtc_fuzzer_test("comfort_noise_decoder_fuzzer") {
   ]
   deps = [
     "../../api:array_view",
-    "../../modules/audio_coding:cng",
+    "../../modules/audio_coding:webrtc_cng",
     "../../rtc_base:rtc_base_approved",
   ]
 }