Reland r8577 "Collapse AudioEncoderDecoderIsacRed into ..."

This effectively reverts r8578.

TBR=jmarusic@webrtc.org

Original commit message:
Collapse AudioEncoderDecoderIsacRed into AudioEncoderDecoderIsac

With this change, support for iSAC-RED is incorporated into the
regular AudioEncoderDecoderIsac class.

COAUTHOR=kwiberg@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/44539004

Cr-Commit-Position: refs/heads/master@{#8589}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8589 4adac7df-926f-26a2-2b94-8c16560cd09d

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

@@ -41,6 +41,7 @@ class AudioEncoderDecoderIsacT : public AudioEncoder, public AudioDecoder {
     int bit_rate;  // Limit on the short-term average bit rate, in bits/second.
     int max_bit_rate;
     int max_payload_size_bytes;
+    bool use_red;
   };
 
   // For constructing an encoder in channel-adaptive mode. Allowed combinations
@@ -60,6 +61,7 @@ class AudioEncoderDecoderIsacT : public AudioEncoder, public AudioDecoder {
     int max_bit_rate;
     bool enforce_frame_size;  // Prevent adaptive changes to the frame size?
     int max_payload_size_bytes;
+    bool use_red;
   };
 
   explicit AudioEncoderDecoderIsacT(const Config& config);
@@ -108,6 +110,7 @@ class AudioEncoderDecoderIsacT : public AudioEncoder, public AudioDecoder {
 
   const int payload_type_;
   const int red_payload_type_;
+  const bool use_red_;
 
   // iSAC encoder/decoder state, guarded by a mutex to ensure that encode calls
   // from one thread won't clash with decode calls from another thread.
@@ -131,11 +134,7 @@ class AudioEncoderDecoderIsacT : public AudioEncoder, public AudioDecoder {
   uint32_t last_encoded_timestamp_ GUARDED_BY(lock_);
 
   // Redundant encoding from last time.
-  // Note: If has_redundant_encoder is false, we set the array length to 1,
+  const rtc::scoped_ptr<uint8_t[]> redundant_payload_ GUARDED_BY(lock_);
-  // since zero-length arrays are not supported by all compilers.
-  uint8_t redundant_payload_[T::has_redundant_encoder
-                                 ? kSufficientEncodeBufferSizeBytes
-                                 : 1] GUARDED_BY(lock_);
   size_t redundant_length_bytes_ GUARDED_BY(lock_);
 
   DISALLOW_COPY_AND_ASSIGN(AudioEncoderDecoderIsacT);

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

@@ -33,7 +33,8 @@ AudioEncoderDecoderIsacT<T>::Config::Config()
       frame_size_ms(30),
       bit_rate(kDefaultBitRate),
       max_bit_rate(-1),
-      max_payload_size_bytes(-1) {
+      max_payload_size_bytes(-1),
+      use_red(false) {
 }
 
 template <typename T>
@@ -42,6 +43,8 @@ bool AudioEncoderDecoderIsacT<T>::Config::IsOk() const {
     return false;
   if (max_payload_size_bytes < 120 && max_payload_size_bytes != -1)
     return false;
+  if (use_red && red_payload_type == kInvalidPayloadType)
+    return false;
   switch (sample_rate_hz) {
     case 16000:
       if (max_bit_rate > 53400)
@@ -73,7 +76,8 @@ AudioEncoderDecoderIsacT<T>::ConfigAdaptive::ConfigAdaptive()
       initial_bit_rate(kDefaultBitRate),
       max_bit_rate(-1),
       enforce_frame_size(false),
-      max_payload_size_bytes(-1) {
+      max_payload_size_bytes(-1),
+      use_red(false) {
 }
 
 template <typename T>
@@ -82,6 +86,8 @@ bool AudioEncoderDecoderIsacT<T>::ConfigAdaptive::IsOk() const {
     return false;
   if (max_payload_size_bytes < 120 && max_payload_size_bytes != -1)
     return false;
+  if (use_red && red_payload_type == kInvalidPayloadType)
+    return false;
   switch (sample_rate_hz) {
     case 16000:
       if (max_bit_rate > 53400)
@@ -108,10 +114,13 @@ template <typename T>
 AudioEncoderDecoderIsacT<T>::AudioEncoderDecoderIsacT(const Config& config)
     : payload_type_(config.payload_type),
       red_payload_type_(config.red_payload_type),
+      use_red_(config.use_red),
       state_lock_(CriticalSectionWrapper::CreateCriticalSection()),
       decoder_sample_rate_hz_(0),
       lock_(CriticalSectionWrapper::CreateCriticalSection()),
       packet_in_progress_(false),
+      redundant_payload_(
+          use_red_ ? new uint8_t[kSufficientEncodeBufferSizeBytes] : nullptr),
       redundant_length_bytes_(0) {
   CHECK(config.IsOk());
   CHECK_EQ(0, T::Create(&isac_state_));
@@ -136,10 +145,13 @@ AudioEncoderDecoderIsacT<T>::AudioEncoderDecoderIsacT(
     const ConfigAdaptive& config)
     : payload_type_(config.payload_type),
       red_payload_type_(config.red_payload_type),
+      use_red_(config.use_red),
       state_lock_(CriticalSectionWrapper::CreateCriticalSection()),
       decoder_sample_rate_hz_(0),
       lock_(CriticalSectionWrapper::CreateCriticalSection()),
       packet_in_progress_(false),
+      redundant_payload_(
+          use_red_ ? new uint8_t[kSufficientEncodeBufferSizeBytes] : nullptr),
       redundant_length_bytes_(0) {
   CHECK(config.IsOk());
   CHECK_EQ(0, T::Create(&isac_state_));
@@ -218,7 +230,7 @@ void AudioEncoderDecoderIsacT<T>::EncodeInternal(uint32_t rtp_timestamp,
   info->encoded_timestamp = packet_timestamp_;
   info->payload_type = payload_type_;
 
-  if (!T::has_redundant_encoder)
+  if (!use_red_)
     return;
 
   if (redundant_length_bytes_ == 0) {
@@ -229,7 +241,7 @@ void AudioEncoderDecoderIsacT<T>::EncodeInternal(uint32_t rtp_timestamp,
     // resulting payload consists of the primary encoding followed by the
     // redundant encoding from last time.
     const size_t primary_length = info->encoded_bytes;
-    memcpy(&encoded[primary_length], redundant_payload_,
+    memcpy(&encoded[primary_length], redundant_payload_.get(),
            redundant_length_bytes_);
     // The EncodedInfo struct |info| will have one root node and two leaves.
     // |info| will be implicitly cast to an EncodedInfoLeaf struct, effectively
@@ -251,9 +263,9 @@ void AudioEncoderDecoderIsacT<T>::EncodeInternal(uint32_t rtp_timestamp,
   {
     CriticalSectionScoped cs(state_lock_.get());
     // Call the encoder's method to get redundant encoding.
-    redundant_length_bytes_ = T::GetRedPayload(isac_state_, redundant_payload_);
+    redundant_length_bytes_ =
+        T::GetRedPayload(isac_state_, redundant_payload_.get());
   }
-  DCHECK_LE(redundant_length_bytes_, sizeof(redundant_payload_));
   DCHECK_GE(redundant_length_bytes_, 0u);
   last_encoded_timestamp_ = packet_timestamp_;
 }

webrtc/modules/audio_coding/codecs/isac/fix/interface/audio_encoder_isacfix.h

@@ -20,7 +20,6 @@ namespace webrtc {
 struct IsacFix {
   typedef ISACFIX_MainStruct instance_type;
   static const bool has_swb = false;
-  static const bool has_redundant_encoder = false;
   static const uint16_t kFixSampleRate = 16000;
   static inline int16_t Control(instance_type* inst,
                                 int32_t rate,

webrtc/modules/audio_coding/codecs/isac/main/interface/audio_encoder_isac.h

@@ -20,7 +20,6 @@ namespace webrtc {
 struct IsacFloat {
   typedef ISACStruct instance_type;
   static const bool has_swb = true;
-  static const bool has_redundant_encoder = false;
   static inline int16_t Control(instance_type* inst,
                                 int32_t rate,
                                 int16_t framesize) {
@@ -100,8 +99,7 @@ struct IsacFloat {
                                        rtp_seq_number, send_ts, arr_ts);
   }
   static inline int16_t GetRedPayload(instance_type* inst, uint8_t* encoded) {
-    FATAL() << "Should never be called.";
+    return WebRtcIsac_GetRedPayload(inst, encoded);
-    return -1;
   }
   static inline int16_t SetMaxPayloadSize(instance_type* inst,
                                           int16_t max_payload_size_bytes) {
@@ -114,15 +112,5 @@ struct IsacFloat {
 
 typedef AudioEncoderDecoderIsacT<IsacFloat> AudioEncoderDecoderIsac;
 
-struct IsacRed : public IsacFloat {
-  static const bool has_redundant_encoder = true;
-
-  static inline int16_t GetRedPayload(instance_type* inst, uint8_t* encoded) {
-    return WebRtcIsac_GetRedPayload(inst, encoded);
-  }
-};
-
-typedef AudioEncoderDecoderIsacT<IsacRed> AudioEncoderDecoderIsacRed;
-
 }  // namespace webrtc
 #endif  // WEBRTC_MODULES_AUDIO_CODING_CODECS_ISAC_MAIN_INTERFACE_AUDIO_ENCODER_ISAC_H_

webrtc/modules/audio_coding/codecs/isac/main/source/audio_encoder_isac.cc

@@ -18,8 +18,4 @@ namespace webrtc {
 // AudioEncoderDecoderIsac.
 template class AudioEncoderDecoderIsacT<IsacFloat>;
 
-// Explicit instantiation of AudioEncoderDecoderIsacT<IsacRed>, a.k.a.
-// AudioEncoderDecoderIsacRed.
-template class AudioEncoderDecoderIsacT<IsacRed>;
-
 }  // namespace webrtc

webrtc/modules/audio_coding/codecs/isac/main/source/audio_encoder_isac_red_unittest.cc

@@ -36,12 +36,13 @@ TEST(AudioEncoderIsacRedTest, CompareRedAndNoRed) {
   AudioEncoderDecoderIsac::Config config;
   config.sample_rate_hz = kSampleRateHz;
   AudioEncoderDecoderIsac isac_encoder(config);
-  AudioEncoderDecoderIsacRed::Config red_config;
+  AudioEncoderDecoderIsac::Config red_config;
   red_config.sample_rate_hz = kSampleRateHz;
   red_config.red_payload_type = kRedPayloadType;
+  red_config.use_red = true;
   ASSERT_NE(red_config.red_payload_type, red_config.payload_type)
       << "iSAC and RED payload types must be different.";
-  AudioEncoderDecoderIsacRed isac_red_encoder(red_config);
+  AudioEncoderDecoderIsac isac_red_encoder(red_config);
   AudioEncoder::EncodedInfo info, red_info;
 
   // Note that we are not expecting any output from the redundant encoder until

webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc

@@ -372,36 +372,7 @@ void ACMGenericCodec::ResetAudioEncoder() {
 #ifdef WEBRTC_CODEC_ISAC
   } else if (!STR_CASE_CMP(codec_inst.plname, "ISAC")) {
     is_isac_ = true;
-    if (copy_red_enabled_) {
+    using_codec_internal_red = copy_red_enabled_;
-      using_codec_internal_red = true;
-      AudioEncoderDecoderIsacRed* enc_dec;
-      if (codec_inst.rate == -1) {
-        // Adaptive mode.
-        AudioEncoderDecoderIsacRed::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;
-        config.red_payload_type = red_payload_type_;
-        enc_dec = new AudioEncoderDecoderIsacRed(config);
-      } else {
-        // Channel independent mode.
-        AudioEncoderDecoderIsacRed::Config config;
-        config.sample_rate_hz = codec_inst.plfreq;
-        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;
-        config.red_payload_type = red_payload_type_;
-        enc_dec = new AudioEncoderDecoderIsacRed(config);
-      }
-      audio_encoder_.reset(enc_dec);
-      decoder_proxy_.SetDecoder(enc_dec);
-    } else {
     AudioEncoderDecoderIsac* enc_dec;
     if (codec_inst.rate == -1) {
       // Adaptive mode.
@@ -412,6 +383,10 @@ void ACMGenericCodec::ResetAudioEncoder() {
       config.max_payload_size_bytes = max_payload_size_bytes_;
       config.max_bit_rate = max_rate_bps_;
       config.payload_type = codec_inst.pltype;
+      if (copy_red_enabled_) {
+        config.red_payload_type = red_payload_type_;
+        config.use_red = true;
+      }
       enc_dec = new AudioEncoderDecoderIsac(config);
     } else {
       // Channel independent mode.
@@ -423,11 +398,14 @@ void ACMGenericCodec::ResetAudioEncoder() {
       config.max_payload_size_bytes = max_payload_size_bytes_;
       config.max_bit_rate = max_rate_bps_;
       config.payload_type = codec_inst.pltype;
+      if (copy_red_enabled_) {
+        config.red_payload_type = red_payload_type_;
+        config.use_red = true;
+      }
       enc_dec = new AudioEncoderDecoderIsac(config);
     }
     audio_encoder_.reset(enc_dec);
     decoder_proxy_.SetDecoder(enc_dec);
-    }
 #endif
   } else {
     FATAL();