iSAC: Make separate AudioEncoder and AudioDecoder objects

webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h

Index: webrtc/modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h
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 d2b20e3b941c932cf736489b17cf4455154a9d32..ce70db455b511c9508d650855d0bc998fd4339ba 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
@@ -17,7 +17,6 @@
 
 #include "webrtc/base/checks.h"
 #include "webrtc/modules/audio_coding/codecs/isac/main/interface/isac.h"
-#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
 
 namespace webrtc {
 
@@ -25,8 +24,9 @@ const int kIsacPayloadType = 103;
 const int kDefaultBitRate = 32000;
 
 template <typename T>
-AudioEncoderDecoderIsacT<T>::Config::Config()
-    : payload_type(kIsacPayloadType),
+AudioEncoderIsacT<T>::Config::Config()
+    : bwinfo(nullptr),
+      payload_type(kIsacPayloadType),
       sample_rate_hz(16000),
       frame_size_ms(30),
       bit_rate(kDefaultBitRate),
@@ -37,11 +37,13 @@ AudioEncoderDecoderIsacT<T>::Config::Config()
 }
 
 template <typename T>
-bool AudioEncoderDecoderIsacT<T>::Config::IsOk() const {
+bool AudioEncoderIsacT<T>::Config::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;
+  if (adaptive_mode && !bwinfo)
+    return false;
   switch (sample_rate_hz) {
     case 16000:
       if (max_bit_rate > 53400)
@@ -65,11 +67,9 @@ bool AudioEncoderDecoderIsacT<T>::Config::IsOk() const {
 }
 
 template <typename T>
-AudioEncoderDecoderIsacT<T>::AudioEncoderDecoderIsacT(const Config& config)
+AudioEncoderIsacT<T>::AudioEncoderIsacT(const Config& config)
     : payload_type_(config.payload_type),
-      state_lock_(CriticalSectionWrapper::CreateCriticalSection()),
-      decoder_sample_rate_hz_(0),
-      lock_(CriticalSectionWrapper::CreateCriticalSection()),
+      bwinfo_(config.bwinfo),
       packet_in_progress_(false),
       target_bitrate_bps_(config.adaptive_mode ? -1 : (config.bit_rate == 0
                                                            ? kDefaultBitRate
@@ -85,80 +85,82 @@ AudioEncoderDecoderIsacT<T>::AudioEncoderDecoderIsacT(const Config& config)
   } 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_,
-                                std::min(config.sample_rate_hz, 32000)));
   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_));
+
+  // 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.
+  const int decoder_sample_rate_hz = std::min(config.sample_rate_hz, 32000);
+
+  // Set the decoder sample rate even though we just use the encoder. This
+  // doesn't appear to be necessary to produce a valid encoding, but without it
+  // we get an encoding that isn't bit-for-bit identical with what a combined
+  // encoder+decoder object produces.
+  CHECK_EQ(0, T::SetDecSampRate(isac_state_, decoder_sample_rate_hz));
 }
 
 template <typename T>
-AudioEncoderDecoderIsacT<T>::~AudioEncoderDecoderIsacT() {
+AudioEncoderIsacT<T>::~AudioEncoderIsacT() {
   CHECK_EQ(0, T::Free(isac_state_));
 }
 
 template <typename T>
-int AudioEncoderDecoderIsacT<T>::SampleRateHz() const {
-  CriticalSectionScoped cs(state_lock_.get());
+int AudioEncoderIsacT<T>::SampleRateHz() const {
   return T::EncSampRate(isac_state_);
 }
 
 template <typename T>
-int AudioEncoderDecoderIsacT<T>::NumChannels() const {
+int AudioEncoderIsacT<T>::NumChannels() const {
   return 1;
 }
 
 template <typename T>
-size_t AudioEncoderDecoderIsacT<T>::MaxEncodedBytes() const {
+size_t AudioEncoderIsacT<T>::MaxEncodedBytes() const {
   return kSufficientEncodeBufferSizeBytes;
 }
 
 template <typename T>
-int AudioEncoderDecoderIsacT<T>::Num10MsFramesInNextPacket() const {
-  CriticalSectionScoped cs(state_lock_.get());
+int AudioEncoderIsacT<T>::Num10MsFramesInNextPacket() const {
   const int samples_in_next_packet = T::GetNewFrameLen(isac_state_);
   return rtc::CheckedDivExact(samples_in_next_packet,
                               rtc::CheckedDivExact(SampleRateHz(), 100));
 }
 
 template <typename T>
-int AudioEncoderDecoderIsacT<T>::Max10MsFramesInAPacket() const {
+int AudioEncoderIsacT<T>::Max10MsFramesInAPacket() const {
   return 6;  // iSAC puts at most 60 ms in a packet.
 }
 
 template <typename T>
-int AudioEncoderDecoderIsacT<T>::GetTargetBitrate() const {
+int AudioEncoderIsacT<T>::GetTargetBitrate() const {
   return target_bitrate_bps_;
 }
 
 template <typename T>
-AudioEncoder::EncodedInfo AudioEncoderDecoderIsacT<T>::EncodeInternal(
+AudioEncoder::EncodedInfo AudioEncoderIsacT<T>::EncodeInternal(
     uint32_t rtp_timestamp,
     const int16_t* audio,
     size_t max_encoded_bytes,
     uint8_t* encoded) {
-  CriticalSectionScoped cs_lock(lock_.get());
   if (!packet_in_progress_) {
     // Starting a new packet; remember the timestamp for later.
     packet_in_progress_ = true;
     packet_timestamp_ = rtp_timestamp;
   }
-  int r;
-  {
-    CriticalSectionScoped cs(state_lock_.get());
-    r = T::Encode(isac_state_, audio, encoded);
-    CHECK_GE(r, 0) << "Encode failed (error code "
-                   << T::GetErrorCode(isac_state_) << ")";
+  if (bwinfo_) {
+    IsacBandwidthInfo bwinfo = bwinfo_->Get();
+    T::SetBandwidthInfo(isac_state_, &bwinfo);
   }
+  int r = T::Encode(isac_state_, audio, encoded);
+  CHECK_GE(r, 0) << "Encode failed (error code " << T::GetErrorCode(isac_state_)
+                 << ")";
 
   // T::Encode doesn't allow us to tell it the size of the output
   // buffer. All we can do is check for an overrun after the fact.
-  CHECK(static_cast<size_t>(r) <= max_encoded_bytes);
+  CHECK_LE(static_cast<size_t>(r), max_encoded_bytes);
 
   if (r == 0)
     return EncodedInfo();
@@ -174,12 +176,33 @@ AudioEncoder::EncodedInfo AudioEncoderDecoderIsacT<T>::EncodeInternal(
 }
 
 template <typename T>
-int AudioEncoderDecoderIsacT<T>::DecodeInternal(const uint8_t* encoded,
-                                                size_t encoded_len,
-                                                int sample_rate_hz,
-                                                int16_t* decoded,
-                                                SpeechType* speech_type) {
-  CriticalSectionScoped cs(state_lock_.get());
+AudioDecoderIsacT<T>::AudioDecoderIsacT()
+    : AudioDecoderIsacT(nullptr) {
+}
+
+template <typename T>
+AudioDecoderIsacT<T>::AudioDecoderIsacT(LockedIsacBandwidthInfo* bwinfo)
+    : bwinfo_(bwinfo), decoder_sample_rate_hz_(-1) {
+  CHECK_EQ(0, T::Create(&isac_state_));
+  CHECK_EQ(0, T::DecoderInit(isac_state_));
+  if (bwinfo_) {
+    IsacBandwidthInfo bwinfo;

[brucedawson, 2015/08/26 18:45:57]

This variable shadows the function parameter bwinfo. This isn't a bug but it is
confusing, especially given the different type and the similarly named member
variable.

Consider renaming to ibwinfo or something else different.

[kwiberg-webrtc, 2015/08/27 09:01:48]

Good idea. CL here: https://codereview.webrtc.org/1315333003/

+    T::GetBandwidthInfo(isac_state_, &bwinfo);
+    bwinfo_->Set(bwinfo);
+  }
+}
+
+template <typename T>
+AudioDecoderIsacT<T>::~AudioDecoderIsacT() {
+  CHECK_EQ(0, T::Free(isac_state_));
+}
+
+template <typename T>
+int AudioDecoderIsacT<T>::DecodeInternal(const uint8_t* encoded,
+                                         size_t encoded_len,
+                                         int sample_rate_hz,
+                                         int16_t* decoded,
+                                         SpeechType* speech_type) {
   // We want to crate the illusion that iSAC supports 48000 Hz decoding, while
   // in fact it outputs 32000 Hz. This is the iSAC fullband mode.
   if (sample_rate_hz == 48000)
@@ -199,40 +222,47 @@ int AudioEncoderDecoderIsacT<T>::DecodeInternal(const uint8_t* encoded,
 }
 
 template <typename T>
-bool AudioEncoderDecoderIsacT<T>::HasDecodePlc() const {
+bool AudioDecoderIsacT<T>::HasDecodePlc() const {
   return false;
 }
 
 template <typename T>
-int AudioEncoderDecoderIsacT<T>::DecodePlc(int num_frames, int16_t* decoded) {
-  CriticalSectionScoped cs(state_lock_.get());
+int AudioDecoderIsacT<T>::DecodePlc(int num_frames, int16_t* decoded) {
   return T::DecodePlc(isac_state_, decoded, num_frames);
 }
 
 template <typename T>
-int AudioEncoderDecoderIsacT<T>::Init() {
-  CriticalSectionScoped cs(state_lock_.get());
+int AudioDecoderIsacT<T>::Init() {
   return T::DecoderInit(isac_state_);
 }
 
 template <typename T>
-int AudioEncoderDecoderIsacT<T>::IncomingPacket(const uint8_t* payload,
-                                                size_t payload_len,
-                                                uint16_t rtp_sequence_number,
-                                                uint32_t rtp_timestamp,
-                                                uint32_t arrival_timestamp) {
-  CriticalSectionScoped cs(state_lock_.get());
-  return T::UpdateBwEstimate(
+int AudioDecoderIsacT<T>::IncomingPacket(const uint8_t* payload,
+                                         size_t payload_len,
+                                         uint16_t rtp_sequence_number,
+                                         uint32_t rtp_timestamp,
+                                         uint32_t arrival_timestamp) {
+  int ret = T::UpdateBwEstimate(
       isac_state_, payload, static_cast<int32_t>(payload_len),
       rtp_sequence_number, rtp_timestamp, arrival_timestamp);
+  if (bwinfo_) {
+    IsacBandwidthInfo bwinfo;
+    T::GetBandwidthInfo(isac_state_, &bwinfo);
+    bwinfo_->Set(bwinfo);
+  }
+  return ret;
 }
 
 template <typename T>
-int AudioEncoderDecoderIsacT<T>::ErrorCode() {
-  CriticalSectionScoped cs(state_lock_.get());
+int AudioDecoderIsacT<T>::ErrorCode() {
   return T::GetErrorCode(isac_state_);
 }
 
+template <typename T>
+size_t AudioDecoderIsacT<T>::Channels() const {
+  return 1;
+}
+
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_AUDIO_CODING_CODECS_ISAC_AUDIO_ENCODER_ISAC_T_IMPL_H_