From bd1bc473958981995258b7cfff50b94c8a99e762 Mon Sep 17 00:00:00 2001 From: Karl Wiberg Date: Mon, 18 May 2015 12:18:54 +0200 Subject: [PATCH] Restructure decoder registration in ACM Before this change, a decoder was registered into ACMReceiver through the CodecOwner; the CodecOwner had to have a pointer back to the AudioCodingModuleImpl object to make this call. With this change, the AudioCodingModuleImpl object asks the CodecOwner for a decoder pointer instead, making the chain of calls more straightforward. COAUTHOR=henrik.lundin@webrtc.org BUG=4474 R=jmarusic@webrtc.org, minyue@webrtc.org Review URL: https://webrtc-codereview.appspot.com/52439004 Cr-Commit-Position: refs/heads/master@{#9204} --- .../main/acm2/audio_coding_module_impl.cc | 33 +++++++++++----- .../main/acm2/audio_coding_module_impl.h | 5 --- .../audio_coding/main/acm2/codec_manager.cc | 39 ++----------------- .../audio_coding/main/acm2/codec_manager.h | 26 +++++-------- 4 files changed, 37 insertions(+), 66 deletions(-) diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc index 18ff8b326e..fbc27a976c 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc +++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc @@ -22,6 +22,7 @@ #include "webrtc/modules/audio_coding/main/acm2/acm_resampler.h" #include "webrtc/modules/audio_coding/main/acm2/call_statistics.h" #include "webrtc/system_wrappers/interface/critical_section_wrapper.h" +#include "webrtc/system_wrappers/interface/logging.h" #include "webrtc/system_wrappers/interface/rw_lock_wrapper.h" #include "webrtc/system_wrappers/interface/trace.h" #include "webrtc/typedefs.h" @@ -125,7 +126,6 @@ AudioCodingModuleImpl::AudioCodingModuleImpl( expected_codec_ts_(0xD87F3F9F), expected_in_ts_(0xD87F3F9F), receiver_(config), - codec_manager_(this), previous_pltype_(255), aux_rtp_header_(NULL), receiver_initialized_(false), @@ -617,7 +617,28 @@ int AudioCodingModuleImpl::PlayoutFrequency() const { int AudioCodingModuleImpl::RegisterReceiveCodec(const CodecInst& codec) { CriticalSectionScoped lock(acm_crit_sect_); DCHECK(receiver_initialized_); - return codec_manager_.RegisterReceiveCodec(codec); + if (codec.channels > 2 || codec.channels < 0) { + LOG_F(LS_ERROR) << "Unsupported number of channels: " << codec.channels; + return -1; + } + + int codec_id = ACMCodecDB::ReceiverCodecNumber(codec); + if (codec_id < 0 || codec_id >= ACMCodecDB::kNumCodecs) { + LOG_F(LS_ERROR) << "Wrong codec params to be registered as receive codec"; + return -1; + } + + // Check if the payload-type is valid. + if (!ACMCodecDB::ValidPayloadType(codec.pltype)) { + LOG_F(LS_ERROR) << "Invalid payload type " << codec.pltype << " for " + << codec.plname; + return -1; + } + + // Get |decoder| associated with |codec|. |decoder| is NULL if |codec| does + // not own its decoder. + return receiver_.AddCodec(codec_id, codec.pltype, codec.channels, + codec_manager_.GetAudioDecoder(codec)); } // Get current received codec. @@ -626,14 +647,6 @@ int AudioCodingModuleImpl::ReceiveCodec(CodecInst* current_codec) const { return receiver_.LastAudioCodec(current_codec); } -int AudioCodingModuleImpl::RegisterDecoder(int acm_codec_id, - uint8_t payload_type, - int channels, - AudioDecoder* audio_decoder) { - return receiver_.AddCodec(acm_codec_id, payload_type, channels, - audio_decoder); -} - // Incoming packet from network parsed and ready for decode. int AudioCodingModuleImpl::IncomingPacket(const uint8_t* incoming_payload, const size_t payload_length, diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h index a657f4f974..19934858ac 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h +++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h @@ -137,11 +137,6 @@ class AudioCodingModuleImpl : public AudioCodingModule { // Get current received codec. int ReceiveCodec(CodecInst* current_codec) const override; - int RegisterDecoder(int acm_codec_id, - uint8_t payload_type, - int channels, - AudioDecoder* audio_decoder); - // Incoming packet from network parsed and ready for decode. int IncomingPacket(const uint8_t* incoming_payload, const size_t payload_length, diff --git a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc index e61a69a53c..dbbf8f4f67 100644 --- a/webrtc/modules/audio_coding/main/acm2/codec_manager.cc +++ b/webrtc/modules/audio_coding/main/acm2/codec_manager.cc @@ -11,7 +11,8 @@ #include "webrtc/modules/audio_coding/main/acm2/codec_manager.h" #include "webrtc/base/checks.h" -#include "webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h" +#include "webrtc/engine_configurations.h" +#include "webrtc/modules/audio_coding/main/acm2/acm_codec_database.h" #include "webrtc/system_wrappers/interface/trace.h" namespace webrtc { @@ -152,9 +153,8 @@ bool CodecSupported(const CodecInst& codec) { const CodecInst kEmptyCodecInst = {-1, "noCodecRegistered", 0, 0, 0, 0}; } // namespace -CodecManager::CodecManager(AudioCodingModuleImpl* acm) - : acm_(acm), - cng_nb_pltype_(255), +CodecManager::CodecManager() + : cng_nb_pltype_(255), cng_wb_pltype_(255), cng_swb_pltype_(255), cng_fb_pltype_(255), @@ -335,37 +335,6 @@ int CodecManager::SendCodec(CodecInst* current_codec) const { return 0; } -// Register possible receive codecs, can be called multiple times, -// for codecs, CNG (NB, WB and SWB), DTMF, RED. -int CodecManager::RegisterReceiveCodec(const CodecInst& codec) { - if (codec.channels > 2 || codec.channels < 0) { - WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, 0, - "Unsupported number of channels, %d.", codec.channels); - return -1; - } - - int codec_id = ACMCodecDB::ReceiverCodecNumber(codec); - - if (codec_id < 0 || codec_id >= ACMCodecDB::kNumCodecs) { - WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, 0, - "Wrong codec params to be registered as receive codec"); - return -1; - } - - // Check if the payload-type is valid. - if (!ACMCodecDB::ValidPayloadType(codec.pltype)) { - WEBRTC_TRACE(webrtc::kTraceError, webrtc::kTraceAudioCoding, 0, - "Invalid payload-type %d for %s.", codec.pltype, codec.plname); - return -1; - } - - // Get |decoder| associated with |codec|. |decoder| can be NULL if |codec| - // does not own its decoder. - // uint8_t payload_type = static_cast(codec.pltype); - return acm_->RegisterDecoder(codec_id, codec.pltype, codec.channels, - GetAudioDecoder(codec)); -} - bool CodecManager::SetCopyRed(bool enable) { if (enable && codec_fec_enabled_) { WEBRTC_TRACE(webrtc::kTraceWarning, webrtc::kTraceAudioCoding, 0, diff --git a/webrtc/modules/audio_coding/main/acm2/codec_manager.h b/webrtc/modules/audio_coding/main/acm2/codec_manager.h index 610810c9b7..2c54512c30 100644 --- a/webrtc/modules/audio_coding/main/acm2/codec_manager.h +++ b/webrtc/modules/audio_coding/main/acm2/codec_manager.h @@ -14,9 +14,6 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/base/thread_checker.h" -#include "webrtc/modules/audio_coding/codecs/audio_encoder.h" -#include "webrtc/modules/audio_coding/codecs/isac/main/interface/audio_encoder_isac.h" -#include "webrtc/modules/audio_coding/main/acm2/acm_codec_database.h" #include "webrtc/modules/audio_coding/main/acm2/codec_owner.h" #include "webrtc/modules/audio_coding/main/interface/audio_coding_module_typedefs.h" #include "webrtc/common_types.h" @@ -24,22 +21,20 @@ namespace webrtc { class AudioDecoder; +class AudioEncoder; +class AudioEncoderMutable; namespace acm2 { -class AudioCodingModuleImpl; - class CodecManager final { public: - explicit CodecManager(AudioCodingModuleImpl* acm); + CodecManager(); ~CodecManager(); int RegisterSendCodec(const CodecInst& send_codec); int SendCodec(CodecInst* current_codec) const; - int RegisterReceiveCodec(const CodecInst& receive_codec); - bool SetCopyRed(bool enable); int SetVAD(bool enable, ACMVADMode mode); @@ -48,6 +43,13 @@ class CodecManager final { int SetCodecFEC(bool enable_codec_fec); + // Returns a pointer to AudioDecoder of the given codec. For iSAC, encoding + // and decoding have to be performed on a shared codec instance. By calling + // this method, we get the codec instance that ACM owns. + // If |codec| does not share an instance between encoder and decoder, returns + // null. + AudioDecoder* GetAudioDecoder(const CodecInst& codec); + bool stereo_send() const { return stereo_send_; } bool red_enabled() const { return red_enabled_; } @@ -61,18 +63,10 @@ class CodecManager final { const AudioEncoder* CurrentEncoder() const { return codec_owner_.Encoder(); } private: - // Returns a pointer to AudioDecoder of the given codec. For iSAC, encoding - // and decoding have to be performed on a shared codec instance. By calling - // this method, we get the codec instance that ACM owns. - // If |codec| does not share an instance between encoder and decoder, returns - // null. - AudioDecoder* GetAudioDecoder(const CodecInst& codec); - int CngPayloadType(int sample_rate_hz) const; int RedPayloadType(int sample_rate_hz) const; - AudioCodingModuleImpl* acm_; rtc::ThreadChecker thread_checker_; uint8_t cng_nb_pltype_; uint8_t cng_wb_pltype_;