Change ACM's CodecManager to hold one encoder instead of an array
With this change, the currently used encoder is held in a scoped_ptr.
iSAC is a special case, since the encoder instance is also a decoder
instance, so it may have to be available also if another send codec is
used. This is accomplished by having a separate scoped_ptr for iSAC.
Remove mirror ID from ACM codec database functions, and remove unused
functions from the database.
COAUTHOR=kwiberg@webrtc.org
BUG=4228
R=tina.legrand@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/48729004
Cr-Commit-Position: refs/heads/master@{#8982}
diff --git a/webrtc/modules/audio_coding/main/acm2/acm_codec_database.cc b/webrtc/modules/audio_coding/main/acm2/acm_codec_database.cc
index 7710046..8a9a5de 100644
--- a/webrtc/modules/audio_coding/main/acm2/acm_codec_database.cc
+++ b/webrtc/modules/audio_coding/main/acm2/acm_codec_database.cc
@@ -227,7 +227,7 @@
// Gets the codec id number from the database. If there is some mismatch in
// the codec settings, the function will return an error code.
// NOTE! The first mismatch found will generate the return value.
-int ACMCodecDB::CodecNumber(const CodecInst& codec_inst, int* mirror_id) {
+int ACMCodecDB::CodecNumber(const CodecInst& codec_inst) {
// Look for a matching codec in the database.
int codec_id = CodecId(codec_inst);
@@ -243,13 +243,11 @@
// Comfort Noise is special case, packet-size & rate is not checked.
if (STR_CASE_CMP(database_[codec_id].plname, "CN") == 0) {
- *mirror_id = codec_id;
return codec_id;
}
// RED is special case, packet-size & rate is not checked.
if (STR_CASE_CMP(database_[codec_id].plname, "red") == 0) {
- *mirror_id = codec_id;
return codec_id;
}
@@ -278,16 +276,8 @@
// Check the validity of rate. Codecs with multiple rates have their own
// function for this.
- *mirror_id = codec_id;
if (STR_CASE_CMP("isac", codec_inst.plname) == 0) {
- if (IsISACRateValid(codec_inst.rate)) {
- // Set mirrorID to iSAC WB which is only created once to be used both for
- // iSAC WB and SWB, because they need to share struct.
- *mirror_id = kISAC;
- return codec_id;
- } else {
- return kInvalidRate;
- }
+ return IsISACRateValid(codec_inst.rate) ? codec_id : kInvalidRate;
} else if (STR_CASE_CMP("ilbc", codec_inst.plname) == 0) {
return IsILBCRateValid(codec_inst.rate, codec_inst.pacsize)
? codec_id : kInvalidRate;
@@ -350,22 +340,10 @@
// We didn't find a matching codec.
return -1;
}
-// Gets codec id number, and mirror id, from database for the receiver.
-int ACMCodecDB::ReceiverCodecNumber(const CodecInst& codec_inst,
- int* mirror_id) {
+// Gets codec id number from database for the receiver.
+int ACMCodecDB::ReceiverCodecNumber(const CodecInst& codec_inst) {
// Look for a matching codec in the database.
- int codec_id = CodecId(codec_inst);
-
- // Set |mirror_id| to |codec_id|, except for iSAC. In case of iSAC we always
- // set |mirror_id| to iSAC WB (kISAC) which is only created once to be used
- // both for iSAC WB and SWB, because they need to share struct.
- if (STR_CASE_CMP(codec_inst.plname, "ISAC") != 0) {
- *mirror_id = codec_id;
- } else {
- *mirror_id = kISAC;
- }
-
- return codec_id;
+ return CodecId(codec_inst);
}
// Returns the codec sampling frequency for codec with id = "codec_id" in
@@ -394,16 +372,6 @@
return neteq_decoders_;
}
-// Gets mirror id. The Id is used for codecs sharing struct for settings that
-// need different payload types.
-int ACMCodecDB::MirrorID(int codec_id) {
- if (STR_CASE_CMP(database_[codec_id].plname, "isac") == 0) {
- return kISAC;
- } else {
- return codec_id;
- }
-}
-
// Creates memory/instance for storing codec state.
ACMGenericCodec* ACMCodecDB::CreateCodecInstance(const CodecInst& codec_inst,
int cng_pt_nb,
diff --git a/webrtc/modules/audio_coding/main/acm2/acm_codec_database.h b/webrtc/modules/audio_coding/main/acm2/acm_codec_database.h
index ea7eb23..adfe5c7 100644
--- a/webrtc/modules/audio_coding/main/acm2/acm_codec_database.h
+++ b/webrtc/modules/audio_coding/main/acm2/acm_codec_database.h
@@ -166,36 +166,17 @@
// 0 if successful, otherwise -1.
static int Codec(int codec_id, CodecInst* codec_inst);
- // Returns codec id and mirror id from database, given the information
- // received in the input [codec_inst]. Mirror id is a number that tells
- // where to find the codec's memory (instance). The number is either the
- // same as codec id (most common), or a number pointing at a different
- // entry in the database, if the codec has several entries with different
- // payload types. This is used for codecs that must share one struct even if
- // the payload type differs.
- // One example is the codec iSAC which has the same struct for both 16 and
- // 32 khz, but they have different entries in the database. Let's say the
- // function is called with iSAC 32kHz. The function will return 1 as that is
- // the entry in the data base, and [mirror_id] = 0, as that is the entry for
- // iSAC 16 kHz, which holds the shared memory.
+ // Returns codec id from database, given the information received in the input
+ // [codec_inst].
// Input:
// [codec_inst] - Information about the codec for which we require the
// database id.
- // Output:
- // [mirror_id] - mirror id, which most often is the same as the return
- // value, see above.
- // [err_message] - if present, in the event of a mismatch found between the
- // input and the database, a descriptive error message is
- // written here.
- // [err_message] - if present, the length of error message is returned here.
// Return:
// codec id if successful, otherwise < 0.
- static int CodecNumber(const CodecInst& codec_inst, int* mirror_id,
- char* err_message, int max_message_len_byte);
- static int CodecNumber(const CodecInst& codec_inst, int* mirror_id);
+ static int CodecNumber(const CodecInst& codec_inst);
static int CodecId(const CodecInst& codec_inst);
static int CodecId(const char* payload_name, int frequency, int channels);
- static int ReceiverCodecNumber(const CodecInst& codec_inst, int* mirror_id);
+ static int ReceiverCodecNumber(const CodecInst& codec_inst);
// Returns the codec sampling frequency for codec with id = "codec_id" in
// database.
@@ -221,19 +202,6 @@
// Returns the NetEQ decoder database.
static const NetEqDecoder* NetEQDecoders();
- // Returns mirror id, which is a number that tells where to find the codec's
- // memory (instance). It is either the same as codec id (most common), or a
- // number pointing at a different entry in the database, if the codec have
- // several entries with different payload types. This is used for codecs that
- // must share struct even if the payload type differs.
- // TODO(tlegrand): Check if function is needed, or if we can change
- // to access database directly.
- // Input:
- // [codec_id] - number that specifies codec's position in the database.
- // Return:
- // Mirror id on success, otherwise -1.
- static int MirrorID(int codec_id);
-
// Creates a codec wrapper containing an AudioEncoder object (or an
// ACMGenericCodec subclass during the refactoring time). The type of
// AudioEncoder is decided by looking at the information in |codec_inst|.
diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module.cc b/webrtc/modules/audio_coding/main/acm2/audio_coding_module.cc
index f1d7a5d..fa63491 100644
--- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module.cc
+++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module.cc
@@ -81,9 +81,7 @@
// Checks the validity of the parameters of the given codec
bool AudioCodingModule::IsCodecValid(const CodecInst& codec) {
- int mirror_id;
-
- int codec_number = acm2::ACMCodecDB::CodecNumber(codec, &mirror_id);
+ int codec_number = acm2::ACMCodecDB::CodecNumber(codec);
if (codec_number < 0) {
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, -1,
diff --git a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc
index c3e340e..6e80be8 100644
--- a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc
+++ b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc
@@ -34,9 +34,7 @@
}
// Check if the given codec is a valid to be registered as send codec.
-int IsValidSendCodec(const CodecInst& send_codec,
- bool is_primary_encoder,
- int* mirror_id) {
+int IsValidSendCodec(const CodecInst& send_codec, bool is_primary_encoder) {
int dummy_id = 0;
if ((send_codec.channels != 1) && (send_codec.channels != 2)) {
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, dummy_id,
@@ -47,7 +45,7 @@
return -1;
}
- int codec_id = ACMCodecDB::CodecNumber(send_codec, mirror_id);
+ int codec_id = ACMCodecDB::CodecNumber(send_codec);
if (codec_id < 0) {
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, dummy_id,
"Invalid codec setting for the send codec.");
@@ -68,7 +66,6 @@
if (!STR_CASE_CMP(send_codec.plname, "telephone-event")) {
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, dummy_id,
"telephone-event cannot be a send codec");
- *mirror_id = -1;
return -1;
}
@@ -77,7 +74,6 @@
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, dummy_id,
"%d number of channels not supportedn for %s.",
send_codec.channels, send_codec.plname);
- *mirror_id = -1;
return -1;
}
@@ -87,20 +83,22 @@
if (IsCodecRED(&send_codec)) {
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, dummy_id,
"RED cannot be secondary codec");
- *mirror_id = -1;
return -1;
}
if (IsCodecCN(&send_codec)) {
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, dummy_id,
"DTX cannot be secondary codec");
- *mirror_id = -1;
return -1;
}
}
return codec_id;
}
+bool IsIsac(const CodecInst& codec) {
+ return !STR_CASE_CMP(codec.plname, "isac");
+}
+
const CodecInst kEmptyCodecInst = {-1, "noCodecRegistered", 0, 0, 0, 0};
} // namespace
@@ -119,11 +117,6 @@
send_codec_inst_(kEmptyCodecInst),
red_enabled_(false),
codec_fec_enabled_(false) {
- for (int i = 0; i < ACMCodecDB::kMaxNumCodecs; i++) {
- codecs_[i] = nullptr;
- mirror_codec_idx_[i] = -1;
- }
-
// Register the default payload type for RED and for CNG at sampling rates of
// 8, 16, 32 and 48 kHz.
for (int i = (ACMCodecDB::kNumCodecs - 1); i >= 0; i--) {
@@ -144,25 +137,11 @@
thread_checker_.DetachFromThread();
}
-CodecManager::~CodecManager() {
- for (int i = 0; i < ACMCodecDB::kMaxNumCodecs; i++) {
- if (codecs_[i] != NULL) {
- // Mirror index holds the address of the codec memory.
- assert(mirror_codec_idx_[i] > -1);
- if (codecs_[mirror_codec_idx_[i]] != NULL) {
- delete codecs_[mirror_codec_idx_[i]];
- codecs_[mirror_codec_idx_[i]] = NULL;
- }
-
- codecs_[i] = NULL;
- }
- }
-}
+CodecManager::~CodecManager() = default;
int CodecManager::RegisterSendCodec(const CodecInst& send_codec) {
DCHECK(thread_checker_.CalledOnValidThread());
- int mirror_id;
- int codec_id = IsValidSendCodec(send_codec, true, &mirror_id);
+ int codec_id = IsValidSendCodec(send_codec, true);
// Check for reported errors from function IsValidSendCodec().
if (codec_id < 0) {
@@ -243,36 +222,31 @@
// Check if the codec is already registered as send codec.
bool is_send_codec;
if (current_encoder_) {
- int send_codec_mirror_id;
- int send_codec_id =
- ACMCodecDB::CodecNumber(send_codec_inst_, &send_codec_mirror_id);
+ int send_codec_id = ACMCodecDB::CodecNumber(send_codec_inst_);
assert(send_codec_id >= 0);
- is_send_codec =
- (send_codec_id == codec_id) || (mirror_id == send_codec_mirror_id);
+ is_send_codec = send_codec_id == codec_id;
} else {
is_send_codec = false;
}
// If new codec, or new settings, register.
if (!is_send_codec) {
- if (!codecs_[mirror_id]) {
- codecs_[mirror_id] = ACMCodecDB::CreateCodecInstance(
+ ACMGenericCodec* new_codec;
+ if (!IsIsac(send_codec)) {
+ encoder_.reset(ACMCodecDB::CreateCodecInstance(
send_codec, cng_nb_pltype_, cng_wb_pltype_, cng_swb_pltype_,
- cng_fb_pltype_, red_enabled_, red_nb_pltype_);
- if (!codecs_[mirror_id]) {
- WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, dummy_id,
- "Cannot Create the codec");
- return -1;
+ cng_fb_pltype_, red_enabled_, red_nb_pltype_));
+ new_codec = encoder_.get();
+ } else {
+ if (!isac_enc_dec_) {
+ isac_enc_dec_.reset(ACMCodecDB::CreateCodecInstance(
+ send_codec, cng_nb_pltype_, cng_wb_pltype_, cng_swb_pltype_,
+ cng_fb_pltype_, red_enabled_, red_nb_pltype_));
}
- mirror_codec_idx_[mirror_id] = mirror_id;
+ new_codec = isac_enc_dec_.get();
}
+ DCHECK(new_codec);
- if (mirror_id != codec_id) {
- codecs_[codec_id] = codecs_[mirror_id];
- mirror_codec_idx_[codec_id] = mirror_id;
- }
-
- ACMGenericCodec* codec_ptr = codecs_[codec_id];
WebRtcACMCodecParams codec_params;
memcpy(&(codec_params.codec_inst), &send_codec, sizeof(CodecInst));
@@ -280,7 +254,7 @@
codec_params.enable_dtx = dtx_enabled_;
codec_params.vad_mode = vad_mode_;
// Force initialization.
- if (codec_ptr->InitEncoder(&codec_params, true) < 0) {
+ if (new_codec->InitEncoder(&codec_params, true) < 0) {
// Could not initialize the encoder.
// Check if already have a registered codec.
@@ -306,18 +280,18 @@
// If we change codec we start fresh with RED.
// This is not strictly required by the standard.
- if (codec_ptr->SetCopyRed(red_enabled_) < 0) {
+ if (new_codec->SetCopyRed(red_enabled_) < 0) {
// We tried to preserve the old red status, if failed, it means the
// red status has to be flipped.
red_enabled_ = !red_enabled_;
}
- codec_ptr->SetVAD(&dtx_enabled_, &vad_enabled_, &vad_mode_);
+ new_codec->SetVAD(&dtx_enabled_, &vad_enabled_, &vad_mode_);
- if (!codec_ptr->HasInternalFEC()) {
+ if (!new_codec->HasInternalFEC()) {
codec_fec_enabled_ = false;
} else {
- if (codec_ptr->SetFEC(codec_fec_enabled_) < 0) {
+ if (new_codec->SetFEC(codec_fec_enabled_) < 0) {
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, dummy_id,
"Cannot set codec FEC");
return -1;
@@ -325,7 +299,7 @@
}
}
- current_encoder_ = codecs_[codec_id];
+ current_encoder_ = new_codec;
DCHECK(current_encoder_);
memcpy(&send_codec_inst_, &send_codec, sizeof(CodecInst));
return 0;
@@ -335,11 +309,6 @@
// If any parameter is valid then apply it and record.
bool force_init = false;
- if (mirror_id != codec_id) {
- codecs_[codec_id] = codecs_[mirror_id];
- mirror_codec_idx_[codec_id] = mirror_id;
- }
-
// Check the payload type.
if (send_codec.pltype != send_codec_inst_.pltype) {
// At this point check if the given payload type is valid.
@@ -396,7 +365,7 @@
// Check if a change in Rate is required.
if (send_codec.rate != send_codec_inst_.rate) {
- if (codecs_[codec_id]->SetBitRate(send_codec.rate) < 0) {
+ if (current_encoder_->SetBitRate(send_codec.rate) < 0) {
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, dummy_id,
"Could not change the codec rate.");
return -1;
@@ -404,10 +373,10 @@
send_codec_inst_.rate = send_codec.rate;
}
- if (!codecs_[codec_id]->HasInternalFEC()) {
+ if (!current_encoder_->HasInternalFEC()) {
codec_fec_enabled_ = false;
} else {
- if (codecs_[codec_id]->SetFEC(codec_fec_enabled_) < 0) {
+ if (current_encoder_->SetFEC(codec_fec_enabled_) < 0) {
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, dummy_id,
"Cannot set codec FEC");
return -1;
@@ -445,8 +414,7 @@
return -1;
}
- int mirror_id;
- int codec_id = ACMCodecDB::ReceiverCodecNumber(codec, &mirror_id);
+ int codec_id = ACMCodecDB::ReceiverCodecNumber(codec);
if (codec_id < 0 || codec_id >= ACMCodecDB::kNumCodecs) {
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, 0,
@@ -464,7 +432,7 @@
AudioDecoder* decoder = NULL;
// Get |decoder| associated with |codec|. |decoder| can be NULL if |codec|
// does not own its decoder.
- if (GetAudioDecoder(codec, codec_id, mirror_id, &decoder) < 0) {
+ if (GetAudioDecoder(codec, codec_id, &decoder) < 0) {
WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, 0,
"Wrong codec params to be registered as receive codec");
return -1;
@@ -554,57 +522,43 @@
}
void CodecManager::SetCngPayloadType(int sample_rate_hz, int payload_type) {
- for (auto* codec : codecs_) {
- if (codec) {
- codec->SetCngPt(sample_rate_hz, payload_type);
- }
- }
+ if (isac_enc_dec_)
+ isac_enc_dec_->SetCngPt(sample_rate_hz, payload_type);
+ if (encoder_)
+ encoder_->SetCngPt(sample_rate_hz, payload_type);
}
void CodecManager::SetRedPayloadType(int sample_rate_hz, int payload_type) {
- for (auto* codec : codecs_) {
- if (codec) {
- codec->SetRedPt(sample_rate_hz, payload_type);
- }
- }
+ if (isac_enc_dec_)
+ isac_enc_dec_->SetRedPt(sample_rate_hz, payload_type);
+ if (encoder_)
+ encoder_->SetRedPt(sample_rate_hz, payload_type);
}
int CodecManager::GetAudioDecoder(const CodecInst& codec,
int codec_id,
- int mirror_id,
AudioDecoder** decoder) {
- if (ACMCodecDB::OwnsDecoder(codec_id)) {
- // This codec has to own its own decoder. Therefore, it should create the
- // corresponding AudioDecoder class and insert it into NetEq. If the codec
- // does not exist create it.
- //
- // TODO(turajs): this part of the code is common with RegisterSendCodec(),
- // make a method for it.
- if (codecs_[mirror_id] == NULL) {
- codecs_[mirror_id] = ACMCodecDB::CreateCodecInstance(
- codec, cng_nb_pltype_, cng_wb_pltype_, cng_swb_pltype_,
- cng_fb_pltype_, red_enabled_, red_nb_pltype_);
- if (codecs_[mirror_id] == NULL) {
- WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, 0,
- "Cannot Create the codec");
- return -1;
- }
- mirror_codec_idx_[mirror_id] = mirror_id;
- }
-
- if (mirror_id != codec_id) {
- codecs_[codec_id] = codecs_[mirror_id];
- mirror_codec_idx_[codec_id] = mirror_id;
- }
- *decoder = codecs_[codec_id]->Decoder();
- if (!*decoder) {
- assert(false);
- return -1;
- }
- } else {
- *decoder = NULL;
+ if (!ACMCodecDB::OwnsDecoder(codec_id)) {
+ DCHECK(!IsIsac(codec)) << "Codec must not be iSAC at this point.";
+ *decoder = nullptr;
+ return 0;
}
-
+ DCHECK(IsIsac(codec)) << "Codec must be iSAC at this point.";
+ // This codec has to own its own decoder. Therefore, it should create the
+ // corresponding AudioDecoder class and insert it into NetEq. If the codec
+ // does not exist create it.
+ //
+ // TODO(turajs): this part of the code is common with RegisterSendCodec(),
+ // make a method for it.
+ if (!isac_enc_dec_) {
+ isac_enc_dec_.reset(ACMCodecDB::CreateCodecInstance(
+ codec, cng_nb_pltype_, cng_wb_pltype_, cng_swb_pltype_, cng_fb_pltype_,
+ red_enabled_, red_nb_pltype_));
+ if (!isac_enc_dec_)
+ return -1;
+ }
+ *decoder = isac_enc_dec_->Decoder();
+ DCHECK(*decoder);
return 0;
}
diff --git a/webrtc/modules/audio_coding/main/acm2/codec_manager.h b/webrtc/modules/audio_coding/main/acm2/codec_manager.h
index 8d5350d..3146467 100644
--- a/webrtc/modules/audio_coding/main/acm2/codec_manager.h
+++ b/webrtc/modules/audio_coding/main/acm2/codec_manager.h
@@ -72,7 +72,6 @@
// valid pointer, otherwise it will be NULL.
int GetAudioDecoder(const CodecInst& codec,
int codec_id,
- int mirror_id,
AudioDecoder** decoder);
AudioCodingModuleImpl* acm_;
@@ -90,8 +89,8 @@
CodecInst send_codec_inst_;
bool red_enabled_;
bool codec_fec_enabled_;
- ACMGenericCodec* codecs_[ACMCodecDB::kMaxNumCodecs];
- int mirror_codec_idx_[ACMCodecDB::kMaxNumCodecs];
+ rtc::scoped_ptr<ACMGenericCodec> isac_enc_dec_;
+ rtc::scoped_ptr<ACMGenericCodec> encoder_;
DISALLOW_COPY_AND_ASSIGN(CodecManager);
};
