(Re-land) AudioEncoderDecoderIsac: Merge the two config structs
This reverts commit 599beb86, which in turn reverted 7c324cac. What
makes it work this time is that we don't remove the option of setting
bit_rate to 0 in order to ask for the default value.
COAUTHOR=henrik.lundin@webrtc.org
BUG=4228, chromium:478161
R=henrik.lundin@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/48199004
Cr-Commit-Position: refs/heads/master@{#9068}
diff --git a/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h b/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h
index cda5548..ad53166 100644
--- a/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h
+++ b/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h
@@ -25,8 +25,7 @@
template <typename T>
class AudioEncoderDecoderIsacT : public AudioEncoder, public AudioDecoder {
public:
- // For constructing an encoder in instantaneous mode. Allowed combinations
- // are
+ // Allowed combinations of sample rate, frame size, and bit rate are
// - 16000 Hz, 30 ms, 10000-32000 bps
// - 16000 Hz, 60 ms, 10000-32000 bps
// - 32000 Hz, 30 ms, 10000-56000 bps (if T has super-wideband support)
@@ -34,34 +33,24 @@
struct Config {
Config();
bool IsOk() const;
+
int payload_type;
int sample_rate_hz;
int frame_size_ms;
- int bit_rate; // Limit on the short-term average bit rate, in bits/second.
- int max_bit_rate;
+ int bit_rate; // Limit on the short-term average bit rate, in bits/s.
int max_payload_size_bytes;
- };
+ int max_bit_rate;
- // For constructing an encoder in channel-adaptive mode. Allowed combinations
- // are
- // - 16000 Hz, 30 ms, 10000-32000 bps
- // - 16000 Hz, 60 ms, 10000-32000 bps
- // - 32000 Hz, 30 ms, 10000-56000 bps (if T has super-wideband support)
- // - 48000 Hz, 30 ms, 10000-56000 bps (if T has super-wideband support)
- struct ConfigAdaptive {
- ConfigAdaptive();
- bool IsOk() const;
- int payload_type;
- int sample_rate_hz;
- int initial_frame_size_ms;
- int initial_bit_rate;
- int max_bit_rate;
- bool enforce_frame_size; // Prevent adaptive changes to the frame size?
- int max_payload_size_bytes;
+ // If true, the encoder will dynamically adjust frame size and bit rate;
+ // the configured values are then merely the starting point.
+ bool adaptive_mode;
+
+ // In adaptive mode, prevent adaptive changes to the frame size. (Not used
+ // in nonadaptive mode.)
+ bool enforce_frame_size;
};
explicit AudioEncoderDecoderIsacT(const Config& config);
- explicit AudioEncoderDecoderIsacT(const ConfigAdaptive& config);
~AudioEncoderDecoderIsacT() override;
// AudioEncoder public methods.
diff --git a/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h b/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h
index e81686a..c5982f5 100644
--- a/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h
+++ b/webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h
@@ -30,8 +30,10 @@
sample_rate_hz(16000),
frame_size_ms(30),
bit_rate(kDefaultBitRate),
+ max_payload_size_bytes(-1),
max_bit_rate(-1),
- max_payload_size_bytes(-1) {
+ adaptive_mode(false),
+ enforce_frame_size(false) {
}
template <typename T>
@@ -47,7 +49,7 @@
if (max_payload_size_bytes > 400)
return false;
return (frame_size_ms == 30 || frame_size_ms == 60) &&
- ((bit_rate >= 10000 && bit_rate <= 32000) || bit_rate == 0);
+ (bit_rate == 0 || (bit_rate >= 10000 && bit_rate <= 32000));
case 32000:
case 48000:
if (max_bit_rate > 160000)
@@ -56,46 +58,7 @@
return false;
return T::has_swb &&
(frame_size_ms == 30 &&
- ((bit_rate >= 10000 && bit_rate <= 56000) || bit_rate == 0));
- default:
- return false;
- }
-}
-
-template <typename T>
-AudioEncoderDecoderIsacT<T>::ConfigAdaptive::ConfigAdaptive()
- : payload_type(kIsacPayloadType),
- sample_rate_hz(16000),
- initial_frame_size_ms(30),
- initial_bit_rate(kDefaultBitRate),
- max_bit_rate(-1),
- enforce_frame_size(false),
- max_payload_size_bytes(-1) {
-}
-
-template <typename T>
-bool AudioEncoderDecoderIsacT<T>::ConfigAdaptive::IsOk() const {
- if (max_bit_rate < 32000 && max_bit_rate != -1)
- return false;
- if (max_payload_size_bytes < 120 && max_payload_size_bytes != -1)
- return false;
- switch (sample_rate_hz) {
- case 16000:
- if (max_bit_rate > 53400)
- return false;
- if (max_payload_size_bytes > 400)
- return false;
- return (initial_frame_size_ms == 30 || initial_frame_size_ms == 60) &&
- initial_bit_rate >= 10000 && initial_bit_rate <= 32000;
- case 32000:
- case 48000:
- if (max_bit_rate > 160000)
- return false;
- if (max_payload_size_bytes > 600)
- return false;
- return T::has_swb &&
- (initial_frame_size_ms == 30 && initial_bit_rate >= 10000 &&
- initial_bit_rate <= 56000);
+ (bit_rate == 0 || (bit_rate >= 10000 && bit_rate <= 56000)));
default:
return false;
}
@@ -110,11 +73,16 @@
packet_in_progress_(false) {
CHECK(config.IsOk());
CHECK_EQ(0, T::Create(&isac_state_));
- CHECK_EQ(0, T::EncoderInit(isac_state_, 1));
+ CHECK_EQ(0, T::EncoderInit(isac_state_, config.adaptive_mode ? 0 : 1));
CHECK_EQ(0, T::SetEncSampRate(isac_state_, config.sample_rate_hz));
- CHECK_EQ(0, T::Control(isac_state_, config.bit_rate == 0 ? kDefaultBitRate
- : config.bit_rate,
- config.frame_size_ms));
+ const int bit_rate = config.bit_rate == 0 ? kDefaultBitRate : config.bit_rate;
+ if (config.adaptive_mode) {
+ CHECK_EQ(0, T::ControlBwe(isac_state_, bit_rate,
+ config.frame_size_ms, config.enforce_frame_size));
+
+ } else {
+ CHECK_EQ(0, T::Control(isac_state_, bit_rate, config.frame_size_ms));
+ }
// When config.sample_rate_hz is set to 48000 Hz (iSAC-fb), the decoder is
// still set to 32000 Hz, since there is no full-band mode in the decoder.
CHECK_EQ(0, T::SetDecSampRate(isac_state_,
@@ -124,29 +92,7 @@
T::SetMaxPayloadSize(isac_state_, config.max_payload_size_bytes));
if (config.max_bit_rate != -1)
CHECK_EQ(0, T::SetMaxRate(isac_state_, config.max_bit_rate));
-}
-
-template <typename T>
-AudioEncoderDecoderIsacT<T>::AudioEncoderDecoderIsacT(
- const ConfigAdaptive& config)
- : payload_type_(config.payload_type),
- state_lock_(CriticalSectionWrapper::CreateCriticalSection()),
- decoder_sample_rate_hz_(0),
- lock_(CriticalSectionWrapper::CreateCriticalSection()),
- packet_in_progress_(false) {
- CHECK(config.IsOk());
- CHECK_EQ(0, T::Create(&isac_state_));
- CHECK_EQ(0, T::EncoderInit(isac_state_, 0));
- CHECK_EQ(0, T::SetEncSampRate(isac_state_, config.sample_rate_hz));
- CHECK_EQ(0, T::ControlBwe(isac_state_, config.initial_bit_rate,
- config.initial_frame_size_ms,
- config.enforce_frame_size));
- CHECK_EQ(0, T::SetDecSampRate(isac_state_, config.sample_rate_hz));
- if (config.max_payload_size_bytes != -1)
- CHECK_EQ(0,
- T::SetMaxPayloadSize(isac_state_, config.max_payload_size_bytes));
- if (config.max_bit_rate != -1)
- CHECK_EQ(0, T::SetMaxRate(isac_state_, config.max_bit_rate));
+ CHECK_EQ(0, T::DecoderInit(isac_state_));
}
template <typename T>
diff --git a/webrtc/modules/audio_coding/codecs/isac/main/source/audio_encoder_isac_unittest.cc b/webrtc/modules/audio_coding/codecs/isac/main/source/audio_encoder_isac_unittest.cc
new file mode 100644
index 0000000..ee5c031
--- /dev/null
+++ b/webrtc/modules/audio_coding/codecs/isac/main/source/audio_encoder_isac_unittest.cc
@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2015 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 <limits>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/modules/audio_coding/codecs/isac/main/interface/audio_encoder_isac.h"
+
+namespace webrtc {
+
+namespace {
+
+void TestBadConfig(const AudioEncoderDecoderIsac::Config& config) {
+ EXPECT_FALSE(config.IsOk());
+}
+
+void TestGoodConfig(const AudioEncoderDecoderIsac::Config& config) {
+ EXPECT_TRUE(config.IsOk());
+ AudioEncoderDecoderIsac ed(config);
+}
+
+// Wrap subroutine calls that test things in this, so that the error messages
+// will be accompanied by stack traces that make it possible to tell which
+// subroutine invocation caused the failure.
+#define S(x) do { SCOPED_TRACE(#x); x; } while (0)
+
+} // namespace
+
+TEST(AudioEncoderIsacTest, TestConfigBitrate) {
+ AudioEncoderDecoderIsac::Config config;
+
+ // The default value is some real, positive value.
+ EXPECT_GT(config.bit_rate, 1);
+ S(TestGoodConfig(config));
+
+ // 0 is another way to ask for the default value.
+ config.bit_rate = 0;
+ S(TestGoodConfig(config));
+
+ // Try some unreasonable values and watch them fail.
+ config.bit_rate = -1;
+ S(TestBadConfig(config));
+ config.bit_rate = 1;
+ S(TestBadConfig(config));
+ config.bit_rate = std::numeric_limits<int>::max();
+ S(TestBadConfig(config));
+}
+
+} // namespace webrtc
diff --git a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc
index 6b04081..c852d8b 100644
--- a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc
+++ b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc
@@ -293,51 +293,34 @@
#endif
#ifdef WEBRTC_CODEC_ISACFX
} else if (!STR_CASE_CMP(codec_inst.plname, "ISAC")) {
- DCHECK_EQ(codec_inst.plfreq, 16000);
is_isac_ = true;
- AudioEncoderDecoderIsacFix* enc_dec;
- if (codec_inst.rate == -1) {
- // Adaptive mode.
- AudioEncoderDecoderIsacFix::ConfigAdaptive config;
- config.payload_type = codec_inst.pltype;
- enc_dec = new AudioEncoderDecoderIsacFix(config);
- } else {
- // Channel independent mode.
- AudioEncoderDecoderIsacFix::Config config;
+ AudioEncoderDecoderIsacFix::Config config;
+ config.payload_type = codec_inst.pltype;
+ config.sample_rate_hz = codec_inst.plfreq;
+ config.frame_size_ms = rtc::CheckedDivExact(codec_inst.pacsize, 16);
+ if (codec_inst.rate != -1)
config.bit_rate = codec_inst.rate;
- config.frame_size_ms = codec_inst.pacsize / 16;
- config.payload_type = codec_inst.pltype;
- enc_dec = new AudioEncoderDecoderIsacFix(config);
- }
+ config.max_payload_size_bytes = max_payload_size_bytes_;
+ config.max_bit_rate = max_rate_bps_;
+ config.adaptive_mode = (codec_inst.rate == -1);
+ auto* enc_dec = new AudioEncoderDecoderIsacFix(config);
decoder_proxy_.SetDecoder(enc_dec);
audio_encoder_.reset(enc_dec);
#endif
#ifdef WEBRTC_CODEC_ISAC
} else if (!STR_CASE_CMP(codec_inst.plname, "ISAC")) {
is_isac_ = true;
- AudioEncoderDecoderIsac* enc_dec;
- if (codec_inst.rate == -1) {
- // Adaptive mode.
- AudioEncoderDecoderIsac::ConfigAdaptive config;
- config.sample_rate_hz = codec_inst.plfreq;
- config.initial_frame_size_ms = rtc::CheckedDivExact(
- 1000 * codec_inst.pacsize, config.sample_rate_hz);
- config.max_payload_size_bytes = max_payload_size_bytes_;
- config.max_bit_rate = max_rate_bps_;
- config.payload_type = codec_inst.pltype;
- enc_dec = new AudioEncoderDecoderIsac(config);
- } else {
- // Channel independent mode.
- AudioEncoderDecoderIsac::Config config;
- config.sample_rate_hz = codec_inst.plfreq;
+ AudioEncoderDecoderIsac::Config config;
+ config.payload_type = codec_inst.pltype;
+ config.sample_rate_hz = codec_inst.plfreq;
+ config.frame_size_ms =
+ rtc::CheckedDivExact(1000 * codec_inst.pacsize, config.sample_rate_hz);
+ if (codec_inst.rate != -1)
config.bit_rate = codec_inst.rate;
- config.frame_size_ms = rtc::CheckedDivExact(1000 * codec_inst.pacsize,
- config.sample_rate_hz);
- config.max_payload_size_bytes = max_payload_size_bytes_;
- config.max_bit_rate = max_rate_bps_;
- config.payload_type = codec_inst.pltype;
- enc_dec = new AudioEncoderDecoderIsac(config);
- }
+ config.max_payload_size_bytes = max_payload_size_bytes_;
+ config.max_bit_rate = max_rate_bps_;
+ config.adaptive_mode = (codec_inst.rate == -1);
+ auto* enc_dec = new AudioEncoderDecoderIsac(config);
decoder_proxy_.SetDecoder(enc_dec);
audio_encoder_.reset(enc_dec);
#endif
diff --git a/webrtc/modules/audio_coding/neteq/audio_decoder_unittest.cc b/webrtc/modules/audio_coding/neteq/audio_decoder_unittest.cc
index 728caef..334a2fc 100644
--- a/webrtc/modules/audio_coding/neteq/audio_decoder_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/audio_decoder_unittest.cc
@@ -361,6 +361,7 @@
AudioEncoderDecoderIsac::Config config;
config.payload_type = payload_type_;
config.sample_rate_hz = codec_input_rate_hz_;
+ config.adaptive_mode = false;
config.frame_size_ms =
1000 * static_cast<int>(frame_size_) / codec_input_rate_hz_;
@@ -380,6 +381,7 @@
AudioEncoderDecoderIsac::Config config;
config.payload_type = payload_type_;
config.sample_rate_hz = codec_input_rate_hz_;
+ config.adaptive_mode = false;
config.frame_size_ms =
1000 * static_cast<int>(frame_size_) / codec_input_rate_hz_;
@@ -399,6 +401,7 @@
AudioEncoderDecoderIsacFix::Config config;
config.payload_type = payload_type_;
config.sample_rate_hz = codec_input_rate_hz_;
+ config.adaptive_mode = false;
config.frame_size_ms =
1000 * static_cast<int>(frame_size_) / codec_input_rate_hz_;
diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp
index dbf0187..771e672 100644
--- a/webrtc/modules/modules.gyp
+++ b/webrtc/modules/modules.gyp
@@ -108,6 +108,7 @@
'audio_coding/codecs/isac/fix/source/filterbanks_unittest.cc',
'audio_coding/codecs/isac/fix/source/lpc_masking_model_unittest.cc',
'audio_coding/codecs/isac/fix/source/transform_unittest.cc',
+ 'audio_coding/codecs/isac/main/source/audio_encoder_isac_unittest.cc',
'audio_coding/codecs/isac/main/source/isac_unittest.cc',
'audio_coding/codecs/opus/audio_encoder_opus_unittest.cc',
'audio_coding/codecs/opus/opus_unittest.cc',
