Remove support for iSAC RCU
The current way that iSAC RCU is packetized and sent as a RED packet,
with the same payload type for primary and redundant payloads, does
not follow the specification for RED. As it is now, it is impossible
for a receiver to know if an incoming RED packet with iSAC payloads
inside consists of two "primary" (but time-shifted) payloads, or one
primary and one RCU payload. The RED standard stipulates that the
former option is the correct interpretation, while our implementation
currently applies the latter.
This CL removes support for iSAC RCU from Audio Coding Module, but
leaves it in the iSAC codec itself (i.e., in the C implementation).
BUG=4402
COAUTHOR=kwiberg@webrtc.org
R=tina.legrand@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/45569004
Cr-Commit-Position: refs/heads/master@{#8713}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8713 4adac7df-926f-26a2-2b94-8c16560cd09d
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 88ead93..acd8c2f 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
@@ -35,13 +35,11 @@
Config();
bool IsOk() const;
int payload_type;
- int red_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 max_payload_size_bytes;
- bool use_red;
};
// For constructing an encoder in channel-adaptive mode. Allowed combinations
@@ -54,14 +52,12 @@
ConfigAdaptive();
bool IsOk() const;
int payload_type;
- int red_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;
- bool use_red;
};
explicit AudioEncoderDecoderIsacT(const Config& config);
@@ -81,11 +77,6 @@
int sample_rate_hz,
int16_t* decoded,
SpeechType* speech_type) override;
- int DecodeRedundant(const uint8_t* encoded,
- size_t encoded_len,
- int sample_rate_hz,
- int16_t* decoded,
- SpeechType* speech_type) override;
bool HasDecodePlc() const override;
int DecodePlc(int num_frames, int16_t* decoded) override;
int Init() override;
@@ -110,8 +101,6 @@
static const size_t kSufficientEncodeBufferSizeBytes = 400;
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.
@@ -134,10 +123,6 @@
// Timestamp of the previously encoded packet.
uint32_t last_encoded_timestamp_ GUARDED_BY(lock_);
- // Redundant encoding from last time.
- const rtc::scoped_ptr<uint8_t[]> redundant_payload_ GUARDED_BY(lock_);
- size_t redundant_length_bytes_ GUARDED_BY(lock_);
-
DISALLOW_COPY_AND_ASSIGN(AudioEncoderDecoderIsacT);
};
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 b9c3e98..a1ffa80 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
@@ -22,19 +22,16 @@
namespace webrtc {
const int kIsacPayloadType = 103;
-const int kInvalidPayloadType = -1;
const int kDefaultBitRate = 32000;
template <typename T>
AudioEncoderDecoderIsacT<T>::Config::Config()
: payload_type(kIsacPayloadType),
- red_payload_type(kInvalidPayloadType),
sample_rate_hz(16000),
frame_size_ms(30),
bit_rate(kDefaultBitRate),
max_bit_rate(-1),
- max_payload_size_bytes(-1),
- use_red(false) {
+ max_payload_size_bytes(-1) {
}
template <typename T>
@@ -43,8 +40,6 @@
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)
@@ -70,14 +65,12 @@
template <typename T>
AudioEncoderDecoderIsacT<T>::ConfigAdaptive::ConfigAdaptive()
: payload_type(kIsacPayloadType),
- red_payload_type(kInvalidPayloadType),
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),
- use_red(false) {
+ max_payload_size_bytes(-1) {
}
template <typename T>
@@ -86,8 +79,6 @@
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)
@@ -113,15 +104,10 @@
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) {
+ packet_in_progress_(false) {
CHECK(config.IsOk());
CHECK_EQ(0, T::Create(&isac_state_));
CHECK_EQ(0, T::EncoderInit(isac_state_, 1));
@@ -144,15 +130,10 @@
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) {
+ packet_in_progress_(false) {
CHECK(config.IsOk());
CHECK_EQ(0, T::Create(&isac_state_));
CHECK_EQ(0, T::EncoderInit(isac_state_, 0));
@@ -234,45 +215,6 @@
packet_in_progress_ = false;
info->encoded_timestamp = packet_timestamp_;
info->payload_type = payload_type_;
-
- if (!use_red_)
- return;
-
- if (redundant_length_bytes_ == 0) {
- // Do not emit the first output frame when using redundant encoding.
- info->encoded_bytes = 0;
- } else {
- // When a redundant payload from the last Encode call is available, the
- // 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_.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
- // discarding the (empty) vector of redundant information. This is
- // intentional.
- info->redundant.push_back(*info);
- EncodedInfoLeaf secondary_info;
- secondary_info.payload_type = info->payload_type;
- secondary_info.encoded_bytes = redundant_length_bytes_;
- secondary_info.encoded_timestamp = last_encoded_timestamp_;
- info->redundant.push_back(secondary_info);
- info->encoded_bytes +=
- redundant_length_bytes_; // Sum of primary and secondary.
- DCHECK_NE(red_payload_type_, kInvalidPayloadType)
- << "Config.red_payload_type must be set for "
- "AudioEncoderDecoderIsacRed.";
- info->payload_type = red_payload_type_;
- }
- {
- CriticalSectionScoped cs(state_lock_.get());
- // Call the encoder's method to get redundant encoding.
- redundant_length_bytes_ =
- T::GetRedPayload(isac_state_, redundant_payload_.get());
- }
- DCHECK_GE(redundant_length_bytes_, 0u);
- last_encoded_timestamp_ = packet_timestamp_;
}
template <typename T>
@@ -297,21 +239,6 @@
}
template <typename T>
-int AudioEncoderDecoderIsacT<T>::DecodeRedundant(const uint8_t* encoded,
- size_t encoded_len,
- int /*sample_rate_hz*/,
- int16_t* decoded,
- SpeechType* speech_type) {
- CriticalSectionScoped cs(state_lock_.get());
- int16_t temp_type = 1; // Default is speech.
- int16_t ret =
- T::DecodeRcu(isac_state_, encoded, static_cast<int16_t>(encoded_len),
- decoded, &temp_type);
- *speech_type = ConvertSpeechType(temp_type);
- return ret;
-}
-
-template <typename T>
bool AudioEncoderDecoderIsacT<T>::HasDecodePlc() const {
return true;
}
diff --git a/webrtc/modules/audio_coding/codecs/isac/fix/interface/audio_encoder_isacfix.h b/webrtc/modules/audio_coding/codecs/isac/fix/interface/audio_encoder_isacfix.h
index 979c7fe..b34f768 100644
--- a/webrtc/modules/audio_coding/codecs/isac/fix/interface/audio_encoder_isacfix.h
+++ b/webrtc/modules/audio_coding/codecs/isac/fix/interface/audio_encoder_isacfix.h
@@ -48,14 +48,6 @@
int16_t num_lost_frames) {
return WebRtcIsacfix_DecodePlc(inst, decoded, num_lost_frames);
}
- static inline int16_t DecodeRcu(instance_type* inst,
- const uint8_t* encoded,
- int16_t len,
- int16_t* decoded,
- int16_t* speech_type) {
- // iSACfix has no DecodeRcu; just call the normal Decode.
- return WebRtcIsacfix_Decode(inst, encoded, len, decoded, speech_type);
- }
static inline int16_t DecoderInit(instance_type* inst) {
return WebRtcIsacfix_DecoderInit(inst);
}
@@ -101,10 +93,6 @@
return WebRtcIsacfix_UpdateBwEstimate(inst, encoded, packet_size,
rtp_seq_number, send_ts, arr_ts);
}
- static inline int16_t GetRedPayload(instance_type* inst, uint8_t* encoded) {
- FATAL() << "Should never be called.";
- return -1;
- }
static inline int16_t SetMaxPayloadSize(instance_type* inst,
int16_t max_payload_size_bytes) {
return WebRtcIsacfix_SetMaxPayloadSize(inst, max_payload_size_bytes);
diff --git a/webrtc/modules/audio_coding/codecs/isac/main/interface/audio_encoder_isac.h b/webrtc/modules/audio_coding/codecs/isac/main/interface/audio_encoder_isac.h
index 5218230..7b9ec9c 100644
--- a/webrtc/modules/audio_coding/codecs/isac/main/interface/audio_encoder_isac.h
+++ b/webrtc/modules/audio_coding/codecs/isac/main/interface/audio_encoder_isac.h
@@ -48,13 +48,6 @@
return WebRtcIsac_DecodePlc(inst, decoded, num_lost_frames);
}
- static inline int16_t DecodeRcu(instance_type* inst,
- const uint8_t* encoded,
- int16_t len,
- int16_t* decoded,
- int16_t* speech_type) {
- return WebRtcIsac_DecodeRcu(inst, encoded, len, decoded, speech_type);
- }
static inline int16_t DecoderInit(instance_type* inst) {
return WebRtcIsac_DecoderInit(inst);
}
@@ -98,9 +91,6 @@
return WebRtcIsac_UpdateBwEstimate(inst, encoded, packet_size,
rtp_seq_number, send_ts, arr_ts);
}
- static inline int16_t GetRedPayload(instance_type* inst, uint8_t* encoded) {
- return WebRtcIsac_GetRedPayload(inst, encoded);
- }
static inline int16_t SetMaxPayloadSize(instance_type* inst,
int16_t max_payload_size_bytes) {
return WebRtcIsac_SetMaxPayloadSize(inst, max_payload_size_bytes);
diff --git a/webrtc/modules/audio_coding/codecs/isac/main/source/audio_encoder_isac_red_unittest.cc b/webrtc/modules/audio_coding/codecs/isac/main/source/audio_encoder_isac_red_unittest.cc
deleted file mode 100644
index 22949a9..0000000
--- a/webrtc/modules/audio_coding/codecs/isac/main/source/audio_encoder_isac_red_unittest.cc
+++ /dev/null
@@ -1,82 +0,0 @@
-/*
- * Copyright (c) 2014 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 <stdlib.h>
-
-#include "testing/gtest/include/gtest/gtest.h"
-#include "webrtc/base/scoped_ptr.h"
-#include "webrtc/modules/audio_coding/codecs/isac/main/interface/audio_encoder_isac.h"
-
-namespace webrtc {
-
-// Simply check that AudioEncoderDecoderIsacRed produces more encoded bytes
-// than AudioEncoderDecoderIsac. Also check that the redundancy information is
-// populated in the EncodedInfo.
-TEST(AudioEncoderIsacRedTest, CompareRedAndNoRed) {
- static const int kSampleRateHz = 16000;
- static const int k10MsSamples = kSampleRateHz / 100;
- static const int kRedPayloadType = 100;
- // Fill the input array with pseudo-random noise in the range [-1000, 1000].
- int16_t input[k10MsSamples];
- srand(1418811752);
- for (int i = 0; i < k10MsSamples; ++i) {
- double r = rand(); // NOLINT(runtime/threadsafe_fn)
- input[i] = (r / RAND_MAX) * 2000 - 1000;
- }
- AudioEncoderDecoderIsac::Config config;
- config.sample_rate_hz = kSampleRateHz;
- AudioEncoderDecoderIsac isac_encoder(config);
- size_t max_encoded_bytes = isac_encoder.MaxEncodedBytes();
- rtc::scoped_ptr<uint8_t[]> encoded(new uint8_t[max_encoded_bytes]);
- 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.";
- AudioEncoderDecoderIsac isac_red_encoder(red_config);
- size_t max_red_encoded_bytes = isac_red_encoder.MaxEncodedBytes();
- rtc::scoped_ptr<uint8_t[]> red_encoded(new uint8_t[max_red_encoded_bytes]);
- AudioEncoder::EncodedInfo info, red_info;
-
- // Note that we are not expecting any output from the redundant encoder until
- // the 6th block of 10 ms has been processed. This is because in RED mode,
- // iSAC will not output the first 30 ms frame.
- for (int i = 0; i < 6; ++i) {
- EXPECT_EQ(0u, red_info.encoded_bytes);
- EXPECT_EQ(0u, red_info.redundant.size());
- const uint32_t timestamp = static_cast<uint32_t>(i);
- isac_encoder.Encode(timestamp, input, k10MsSamples, max_encoded_bytes,
- encoded.get(), &info);
- isac_red_encoder.Encode(timestamp, input, k10MsSamples,
- max_red_encoded_bytes, red_encoded.get(),
- &red_info);
- }
- EXPECT_GT(info.encoded_bytes, 0u)
- << "Regular codec did not produce any output";
- EXPECT_GT(red_info.encoded_bytes, info.encoded_bytes)
- << "Redundant payload seems to be missing";
- ASSERT_EQ(2u, red_info.redundant.size()) << "Redundancy vector not populated";
- ASSERT_EQ(info.encoded_bytes, red_info.redundant[0].encoded_bytes)
- << "Primary payload should be same length as non-redundant payload";
- // Check that |encoded| and the primary part of |red_encoded| are identical.
- EXPECT_EQ(0, memcmp(encoded.get(), red_encoded.get(), info.encoded_bytes));
- EXPECT_GT(red_info.redundant[0].encoded_bytes,
- red_info.redundant[1].encoded_bytes)
- << "Redundant payload should be smaller than primary";
- EXPECT_EQ(red_info.encoded_bytes, red_info.redundant[0].encoded_bytes +
- red_info.redundant[1].encoded_bytes)
- << "Encoded sizes don't add up";
- EXPECT_EQ(3u, red_info.redundant[0].encoded_timestamp)
- << "Primary timestamp is wrong";
- EXPECT_EQ(0u, red_info.redundant[1].encoded_timestamp)
- << "Secondary timestamp is wrong";
-}
-} // 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 c764d74..3256945 100644
--- a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc
+++ b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc
@@ -261,7 +261,6 @@
void ACMGenericCodec::ResetAudioEncoder() {
const CodecInst& codec_inst = acm_codec_params_.codec_inst;
- bool using_codec_internal_red = false;
if (!STR_CASE_CMP(codec_inst.plname, "PCMU")) {
AudioEncoderPcmU::Config config;
config.num_channels = codec_inst.channels;
@@ -344,7 +343,6 @@
#ifdef WEBRTC_CODEC_ISAC
} else if (!STR_CASE_CMP(codec_inst.plname, "ISAC")) {
is_isac_ = true;
- using_codec_internal_red = copy_red_enabled_;
AudioEncoderDecoderIsac* enc_dec;
if (codec_inst.rate == -1) {
// Adaptive mode.
@@ -355,10 +353,6 @@
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.
@@ -370,10 +364,6 @@
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);
@@ -388,7 +378,7 @@
encoder_ = audio_encoder_.get();
// Attach RED if needed.
- if (copy_red_enabled_ && !using_codec_internal_red) {
+ if (copy_red_enabled_) {
CHECK_NE(red_payload_type_, kInvalidPayloadType);
AudioEncoderCopyRed::Config config;
config.payload_type = red_payload_type_;
diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp
index 7cf263c..de21ae6 100644
--- a/webrtc/modules/modules.gyp
+++ b/webrtc/modules/modules.gyp
@@ -108,7 +108,6 @@
'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/isac_unittest.cc',
- 'audio_coding/codecs/isac/main/source/audio_encoder_isac_red_unittest.cc',
'audio_coding/codecs/opus/audio_encoder_opus_unittest.cc',
'audio_coding/codecs/opus/opus_unittest.cc',
'audio_coding/codecs/red/audio_encoder_copy_red_unittest.cc',
