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}diff --git a/audio/BUILD.gn b/audio/BUILD.gn
index 654e810..078d6ab 100644
--- a/audio/BUILD.gn
+++ b/audio/BUILD.gn
@@ -67,9 +67,9 @@
"../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",
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index f11dace..a357520 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -623,12 +623,12 @@
// 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 @@
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);
}
diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn
index 6a630e0..f02a847 100644
--- a/modules/audio_coding/BUILD.gn
+++ b/modules/audio_coding/BUILD.gn
@@ -15,7 +15,6 @@
visibility = [ ":*" ]
audio_codec_deps = [
- ":cng",
":g711",
":pcm16b",
]
@@ -85,6 +84,7 @@
":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("cng") {
- visibility += [ "*" ]
+rtc_static_library("webrtc_cng") {
+ 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 @@
deps = [
":audio_coding_module_typedefs",
- ":cng",
":neteq_decoder_enum",
+ ":webrtc_cng",
"..:module_api",
"..:module_api_public",
"../..:webrtc_common",
@@ -1352,8 +1362,8 @@
deps = [
":audio_coding",
":audio_coding_module_typedefs",
+ ":audio_encoder_cng",
":audio_format_conversion",
- ":cng",
":pcm16b_c",
":red",
"..:module_api",
@@ -1687,6 +1697,7 @@
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 @@
":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 @@
":pcm16b",
":red",
":rent_a_codec",
+ ":webrtc_cng",
":webrtc_opus",
"..:module_api",
"../..:webrtc_common",
diff --git a/modules/audio_coding/acm2/acm_receiver_unittest.cc b/modules/audio_coding/acm2/acm_receiver_unittest.cc
index 29f2a45..46384fe 100644
--- a/modules/audio_coding/acm2/acm_receiver_unittest.cc
+++ b/modules/audio_coding/acm2/acm_receiver_unittest.cc
@@ -73,12 +73,12 @@
// 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.
diff --git a/modules/audio_coding/acm2/audio_coding_module_unittest.cc b/modules/audio_coding/acm2/audio_coding_module_unittest.cc
index 302b19c..1534a03 100644
--- a/modules/audio_coding/acm2/audio_coding_module_unittest.cc
+++ b/modules/audio_coding/acm2/audio_coding_module_unittest.cc
@@ -408,12 +408,12 @@
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));
});
}
diff --git a/modules/audio_coding/acm2/rent_a_codec.cc b/modules/audio_coding/acm2/rent_a_codec.cc
index b904eb8..7601519 100644
--- a/modules/audio_coding/acm2/rent_a_codec.cc
+++ b/modules/audio_coding/acm2/rent_a_codec.cc
@@ -194,7 +194,7 @@
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 @@
default:
FATAL();
}
- return std::unique_ptr<AudioEncoder>(new AudioEncoderCng(std::move(config)));
+ return CreateComfortNoiseEncoder(std::move(config));
}
std::unique_ptr<AudioDecoder> CreateIsacDecoder(
diff --git a/modules/audio_coding/codecs/cng/audio_encoder_cng.cc b/modules/audio_coding/codecs/cng/audio_encoder_cng.cc
index cb89216..5385020 100644
--- a/modules/audio_coding/codecs/cng/audio_encoder_cng.cc
+++ b/modules/audio_coding/codecs/cng/audio_encoder_cng.cc
@@ -23,29 +23,58 @@
const int kMaxFrameSizeMs = 60;
-} // namespace
+class AudioEncoderCng final : public AudioEncoder {
+ public:
+ explicit AudioEncoderCng(AudioEncoderCngConfig&& config);
+ ~AudioEncoderCng() override;
-AudioEncoderCng::Config::Config() = default;
-AudioEncoderCng::Config::Config(Config&&) = default;
-AudioEncoderCng::Config::~Config() = default;
+ // Not copyable or moveable.
+ AudioEncoderCng(const AudioEncoderCng&) = delete;
+ AudioEncoderCng(AudioEncoderCng&&) = delete;
+ AudioEncoderCng& operator=(const AudioEncoderCng&) = delete;
+ AudioEncoderCng& operator=(AudioEncoderCng&&) = delete;
-bool AudioEncoderCng::Config::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;
-}
+ 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;
-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 @@
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
diff --git a/modules/audio_coding/codecs/cng/audio_encoder_cng.h b/modules/audio_coding/codecs/cng/audio_encoder_cng.h
index 67bea7c..2ef3236 100644
--- a/modules/audio_coding/codecs/cng/audio_encoder_cng.h
+++ b/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 {
- public:
- struct Config {
- Config();
- Config(Config&&);
- ~Config();
- bool IsOk() const;
+ bool IsOk() const;
- size_t num_channels = 1;
- int payload_type = 13;
- std::unique_ptr<AudioEncoder> speech_encoder;
- Vad::Aggressiveness vad_mode = Vad::kVadNormal;
- int sid_frame_interval_ms = 100;
- int num_cng_coefficients = 8;
- // The Vad pointer is mainly for testing. If a NULL pointer is passed, the
- // AudioEncoderCng creates (and destroys) a Vad object internally. If an
- // object is passed, the AudioEncoderCng assumes ownership of the Vad
- // object.
- 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);
+ size_t num_channels = 1;
+ int payload_type = 13;
+ std::unique_ptr<AudioEncoder> speech_encoder;
+ Vad::Aggressiveness vad_mode = Vad::kVadNormal;
+ int sid_frame_interval_ms = 100;
+ int num_cng_coefficients = 8;
+ // The Vad pointer is mainly for testing. If a NULL pointer is passed, the
+ // AudioEncoderCng creates (and destroys) a Vad object internally. If an
+ // object is passed, the AudioEncoderCng assumes ownership of the Vad
+ // object.
+ Vad* vad = nullptr;
};
+std::unique_ptr<AudioEncoder> CreateComfortNoiseEncoder(
+ AudioEncoderCngConfig&& config);
+
} // namespace webrtc
#endif // MODULES_AUDIO_CODING_CODECS_CNG_AUDIO_ENCODER_CNG_H_
diff --git a/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc b/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc
index a76dcbd..e3655b4 100644
--- a/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc
+++ b/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc
@@ -50,8 +50,8 @@
cng_.reset();
}
- AudioEncoderCng::Config MakeCngConfig() {
- AudioEncoderCng::Config config;
+ AudioEncoderCngConfig MakeCngConfig() {
+ AudioEncoderCngConfig config;
config.speech_encoder = std::move(mock_encoder_owner_);
EXPECT_TRUE(config.speech_encoder);
@@ -63,7 +63,7 @@
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 @@
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 @@
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 @@
// 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;
diff --git a/modules/audio_coding/neteq/tools/rtp_encode.cc b/modules/audio_coding/neteq/tools/rtp_encode.cc
index f48b04d..14c6e58 100644
--- a/modules/audio_coding/neteq/tools/rtp_encode.cc
+++ b/modules/audio_coding/neteq/tools/rtp_encode.cc
@@ -246,8 +246,8 @@
return nullptr;
}
-AudioEncoderCng::Config GetCngConfig(int sample_rate_hz) {
- AudioEncoderCng::Config cng_config;
+AudioEncoderCngConfig GetCngConfig(int sample_rate_hz) {
+ AudioEncoderCngConfig cng_config;
const auto default_payload_type = [&] {
switch (sample_rate_hz) {
case 8000:
@@ -313,10 +313,10 @@
// 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);
diff --git a/modules/audio_coding/test/TestRedFec.cc b/modules/audio_coding/test/TestRedFec.cc
index 4866118..8bb3971 100644
--- a/modules/audio_coding/test/TestRedFec.cc
+++ b/modules/audio_coding/test/TestRedFec.cc
@@ -175,12 +175,12 @@
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}));
diff --git a/modules/audio_coding/test/TestVADDTX.cc b/modules/audio_coding/test/TestVADDTX.cc
index aa91ee0..09c69f9 100644
--- a/modules/audio_coding/test/TestVADDTX.cc
+++ b/modules/audio_coding/test/TestVADDTX.cc
@@ -83,12 +83,12 @@
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);
diff --git a/test/fuzzers/BUILD.gn b/test/fuzzers/BUILD.gn
index 08eb3fa..ae26420 100644
--- a/test/fuzzers/BUILD.gn
+++ b/test/fuzzers/BUILD.gn
@@ -517,7 +517,7 @@
]
deps = [
"../../api:array_view",
- "../../modules/audio_coding:cng",
+ "../../modules/audio_coding:webrtc_cng",
"../../rtc_base:rtc_base_approved",
]
}
