diff --git a/webrtc/modules/audio_coding/acm2/codec_manager.h b/webrtc/modules/audio_coding/acm2/codec_manager.h index fbd3a18b18..f6c6cd46d2 100644 --- a/webrtc/modules/audio_coding/acm2/codec_manager.h +++ b/webrtc/modules/audio_coding/acm2/codec_manager.h @@ -17,6 +17,7 @@ #include "webrtc/base/optional.h" #include "webrtc/base/thread_checker.h" #include "webrtc/modules/audio_coding/acm2/rent_a_codec.h" +#include "webrtc/modules/audio_coding/include/audio_coding_module.h" #include "webrtc/modules/audio_coding/include/audio_coding_module_typedefs.h" #include "webrtc/common_types.h" @@ -55,6 +56,30 @@ class CodecManager final { bool SetCodecFEC(bool enable_codec_fec); + // Uses the provided Rent-A-Codec to create a new encoder stack, if we have a + // complete specification; if so, it is then passed to set_encoder. On error, + // returns false. + bool MakeEncoder(RentACodec* rac, AudioCodingModule* acm) { + RTC_DCHECK(rac); + RTC_DCHECK(acm); + if (!codec_stack_params_.speech_encoder && send_codec_inst_) { + // We have no speech encoder, but we have a specification for making one. + auto enc = rac->RentEncoder(*send_codec_inst_); + if (!enc) + return false; + codec_stack_params_.speech_encoder = std::move(enc); + } + auto stack = rac->RentEncoderStack(&codec_stack_params_); + if (stack) { + // Give new encoder stack to the ACM. + acm->SetEncoder(std::move(stack)); + } else { + // The specification was good but incomplete, so we have no encoder stack + // to give to the ACM. + } + return true; + } + private: rtc::ThreadChecker thread_checker_; rtc::Optional send_codec_inst_; diff --git a/webrtc/modules/audio_coding/acm2/rent_a_codec.cc b/webrtc/modules/audio_coding/acm2/rent_a_codec.cc index 3bf959e901..7f1e52030d 100644 --- a/webrtc/modules/audio_coding/acm2/rent_a_codec.cc +++ b/webrtc/modules/audio_coding/acm2/rent_a_codec.cc @@ -263,7 +263,8 @@ RentACodec::StackParameters::~StackParameters() = default; std::unique_ptr RentACodec::RentEncoderStack( StackParameters* param) { - RTC_DCHECK(param->speech_encoder); + if (!param->speech_encoder) + return nullptr; if (param->use_codec_fec) { // Switch FEC on. On failure, remember that FEC is off. diff --git a/webrtc/modules/audio_coding/acm2/rent_a_codec.h b/webrtc/modules/audio_coding/acm2/rent_a_codec.h index 2c5873d616..a4026acd28 100644 --- a/webrtc/modules/audio_coding/acm2/rent_a_codec.h +++ b/webrtc/modules/audio_coding/acm2/rent_a_codec.h @@ -212,7 +212,7 @@ class RentACodec { // Creates and returns an audio encoder stack constructed to the given // specification. If the specification isn't compatible with the encoder, it // will be changed to match (things will be switched off). The speech encoder - // will be stolen. + // will be stolen. If the specification isn't complete, returns nullptr. std::unique_ptr RentEncoderStack(StackParameters* param); // Creates and returns an iSAC decoder. diff --git a/webrtc/modules/audio_coding/acm2/rent_a_codec_unittest.cc b/webrtc/modules/audio_coding/acm2/rent_a_codec_unittest.cc index d0a94cf2d2..028a27108c 100644 --- a/webrtc/modules/audio_coding/acm2/rent_a_codec_unittest.cc +++ b/webrtc/modules/audio_coding/acm2/rent_a_codec_unittest.cc @@ -222,13 +222,11 @@ TEST(RentACodecTest, RentEncoderError) { EXPECT_FALSE(rent_a_codec.RentEncoder(codec_inst)); } -#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) TEST(RentACodecTest, RentEncoderStackWithoutSpeechEncoder) { RentACodec::StackParameters sp; EXPECT_EQ(nullptr, sp.speech_encoder); - EXPECT_DEATH(RentACodec().RentEncoderStack(&sp), ""); + EXPECT_EQ(nullptr, RentACodec().RentEncoderStack(&sp)); } -#endif } // namespace acm2 } // namespace webrtc diff --git a/webrtc/modules/utility/source/coder.cc b/webrtc/modules/utility/source/coder.cc index 18b690dc67..1476e02d9c 100644 --- a/webrtc/modules/utility/source/coder.cc +++ b/webrtc/modules/utility/source/coder.cc @@ -29,23 +29,19 @@ AudioCoder::~AudioCoder() { } -int32_t AudioCoder::SetEncodeCodec(const CodecInst& codecInst) -{ - if(_acm->RegisterSendCodec((CodecInst&)codecInst) == -1) - { - return -1; - } - return 0; +int32_t AudioCoder::SetEncodeCodec(const CodecInst& codecInst) { + const bool success = codec_manager_.RegisterEncoder(codecInst) && + codec_manager_.MakeEncoder(&rent_a_codec_, _acm.get()); + return success ? 0 : -1; } -int32_t AudioCoder::SetDecodeCodec(const CodecInst& codecInst) -{ - if(_acm->RegisterReceiveCodec((CodecInst&)codecInst) == -1) - { - return -1; - } - memcpy(&_receiveCodec,&codecInst,sizeof(CodecInst)); - return 0; +int32_t AudioCoder::SetDecodeCodec(const CodecInst& codecInst) { + if (_acm->RegisterReceiveCodec( + codecInst, [&] { return rent_a_codec_.RentIsacDecoder(); }) == -1) { + return -1; + } + memcpy(&_receiveCodec, &codecInst, sizeof(CodecInst)); + return 0; } int32_t AudioCoder::Decode(AudioFrame& decodedAudio, diff --git a/webrtc/modules/utility/source/coder.h b/webrtc/modules/utility/source/coder.h index 399edbd7fb..9536a027d0 100644 --- a/webrtc/modules/utility/source/coder.h +++ b/webrtc/modules/utility/source/coder.h @@ -14,6 +14,8 @@ #include #include "webrtc/common_types.h" +#include "webrtc/modules/audio_coding/acm2/codec_manager.h" +#include "webrtc/modules/audio_coding/acm2/rent_a_codec.h" #include "webrtc/modules/audio_coding/include/audio_coding_module.h" #include "webrtc/typedefs.h" @@ -48,6 +50,8 @@ protected: private: std::unique_ptr _acm; + acm2::CodecManager codec_manager_; + acm2::RentACodec rent_a_codec_; CodecInst _receiveCodec; diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index ccf2455d24..b9adde7e3e 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -43,6 +43,18 @@ namespace webrtc { namespace voe { +namespace { + +bool RegisterReceiveCodec(std::unique_ptr* acm, + acm2::RentACodec* rac, + const CodecInst& ci) { + const int result = + (*acm)->RegisterReceiveCodec(ci, [&] { return rac->RentIsacDecoder(); }); + return result == 0; +} + +} // namespace + const int kTelephoneEventAttenuationdB = 10; class TransportFeedbackProxy : public TransportFeedbackObserver { @@ -391,7 +403,7 @@ int32_t Channel::OnInitializeDecoder( receiveCodec.pacsize = dummyCodec.pacsize; // Register the new codec to the ACM - if (audio_coding_->RegisterReceiveCodec(receiveCodec) == -1) { + if (!RegisterReceiveCodec(&audio_coding_, &rent_a_codec_, receiveCodec)) { WEBRTC_TRACE(kTraceWarning, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::OnInitializeDecoder() invalid codec (" "pt=%d, name=%s) received - 1", @@ -965,8 +977,8 @@ int32_t Channel::Init() { // Register default PT for outband 'telephone-event' if (!STR_CASE_CMP(codec.plname, "telephone-event")) { - if ((_rtpRtcpModule->RegisterSendPayload(codec) == -1) || - (audio_coding_->RegisterReceiveCodec(codec) == -1)) { + if (_rtpRtcpModule->RegisterSendPayload(codec) == -1 || + !RegisterReceiveCodec(&audio_coding_, &rent_a_codec_, codec)) { WEBRTC_TRACE(kTraceWarning, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::Init() failed to register outband " "'telephone-event' (%d/%d) correctly", @@ -975,9 +987,10 @@ int32_t Channel::Init() { } if (!STR_CASE_CMP(codec.plname, "CN")) { - if ((audio_coding_->RegisterSendCodec(codec) == -1) || - (audio_coding_->RegisterReceiveCodec(codec) == -1) || - (_rtpRtcpModule->RegisterSendPayload(codec) == -1)) { + if (!codec_manager_.RegisterEncoder(codec) || + !codec_manager_.MakeEncoder(&rent_a_codec_, audio_coding_.get()) || + !RegisterReceiveCodec(&audio_coding_, &rent_a_codec_, codec) || + _rtpRtcpModule->RegisterSendPayload(codec) == -1) { WEBRTC_TRACE(kTraceWarning, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::Init() failed to register CN (%d/%d) " "correctly - 1", @@ -988,7 +1001,7 @@ int32_t Channel::Init() { // Register RED to the receiving side of the ACM. // We will not receive an OnInitializeDecoder() callback for RED. if (!STR_CASE_CMP(codec.plname, "RED")) { - if (audio_coding_->RegisterReceiveCodec(codec) == -1) { + if (!RegisterReceiveCodec(&audio_coding_, &rent_a_codec_, codec)) { WEBRTC_TRACE(kTraceWarning, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::Init() failed to register RED (%d/%d) " "correctly", @@ -1194,7 +1207,7 @@ int32_t Channel::DeRegisterVoiceEngineObserver() { } int32_t Channel::GetSendCodec(CodecInst& codec) { - auto send_codec = audio_coding_->SendCodec(); + auto send_codec = codec_manager_.GetCodecInst(); if (send_codec) { codec = *send_codec; return 0; @@ -1210,7 +1223,8 @@ int32_t Channel::SetSendCodec(const CodecInst& codec) { WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::SetSendCodec()"); - if (audio_coding_->RegisterSendCodec(codec) != 0) { + if (!codec_manager_.RegisterEncoder(codec) || + !codec_manager_.MakeEncoder(&rent_a_codec_, audio_coding_.get())) { WEBRTC_TRACE(kTraceError, kTraceVoice, VoEId(_instanceId, _channelId), "SetSendCodec() failed to register codec to ACM"); return -1; @@ -1257,10 +1271,9 @@ int32_t Channel::SetVADStatus(bool enableVAD, bool disableDTX) { WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::SetVADStatus(mode=%d)", mode); - assert(!(disableDTX && enableVAD)); // disableDTX mode is deprecated. - // To disable VAD, DTX must be disabled too - disableDTX = ((enableVAD == false) ? true : disableDTX); - if (audio_coding_->SetVAD(!disableDTX, enableVAD, mode) != 0) { + RTC_DCHECK(!(disableDTX && enableVAD)); // disableDTX mode is deprecated. + if (!codec_manager_.SetVAD(enableVAD, mode) || + !codec_manager_.MakeEncoder(&rent_a_codec_, audio_coding_.get())) { _engineStatisticsPtr->SetLastError(VE_AUDIO_CODING_MODULE_ERROR, kTraceError, "SetVADStatus() failed to set VAD"); @@ -1272,13 +1285,10 @@ int32_t Channel::SetVADStatus(bool enableVAD, int32_t Channel::GetVADStatus(bool& enabledVAD, ACMVADMode& mode, bool& disabledDTX) { - if (audio_coding_->VAD(&disabledDTX, &enabledVAD, &mode) != 0) { - _engineStatisticsPtr->SetLastError( - VE_AUDIO_CODING_MODULE_ERROR, kTraceError, - "GetVADStatus() failed to get VAD status"); - return -1; - } - disabledDTX = !disabledDTX; + const auto* params = codec_manager_.GetStackParams(); + enabledVAD = params->use_cng; + mode = params->vad_mode; + disabledDTX = !params->use_cng; return 0; } @@ -1331,6 +1341,8 @@ int32_t Channel::SetRecPayloadType(const CodecInst& codec) { codec.plname, codec.pltype, codec.plfreq, codec.channels, (codec.rate < 0) ? 0 : codec.rate) != 0) { // First attempt to register failed => de-register and try again + // TODO(kwiberg): Retrying is probably not necessary, since + // AcmReceiver::AddCodec also retries. rtp_receiver_->DeRegisterReceivePayload(codec.pltype); if (rtp_receiver_->RegisterReceivePayload( codec.plname, codec.pltype, codec.plfreq, codec.channels, @@ -1341,9 +1353,9 @@ int32_t Channel::SetRecPayloadType(const CodecInst& codec) { return -1; } } - if (audio_coding_->RegisterReceiveCodec(codec) != 0) { + if (!RegisterReceiveCodec(&audio_coding_, &rent_a_codec_, codec)) { audio_coding_->UnregisterReceiveCodec(codec.pltype); - if (audio_coding_->RegisterReceiveCodec(codec) != 0) { + if (!RegisterReceiveCodec(&audio_coding_, &rent_a_codec_, codec)) { _engineStatisticsPtr->SetLastError( VE_AUDIO_CODING_MODULE_ERROR, kTraceError, "SetRecPayloadType() ACM registration failed - 1"); @@ -1390,7 +1402,8 @@ int32_t Channel::SetSendCNPayloadType(int type, PayloadFrequencies frequency) { // Modify the payload type (must be set to dynamic range) codec.pltype = type; - if (audio_coding_->RegisterSendCodec(codec) != 0) { + if (!codec_manager_.RegisterEncoder(codec) || + !codec_manager_.MakeEncoder(&rent_a_codec_, audio_coding_.get())) { _engineStatisticsPtr->SetLastError( VE_AUDIO_CODING_MODULE_ERROR, kTraceError, "SetSendCNPayloadType() failed to register CN to ACM"); @@ -2853,7 +2866,8 @@ int Channel::SetREDStatus(bool enable, int redPayloadtype) { } } - if (audio_coding_->SetREDStatus(enable) != 0) { + if (!codec_manager_.SetCopyRed(enable) || + !codec_manager_.MakeEncoder(&rent_a_codec_, audio_coding_.get())) { _engineStatisticsPtr->SetLastError( VE_AUDIO_CODING_MODULE_ERROR, kTraceError, "SetREDStatus() failed to set RED state in the ACM"); @@ -2863,7 +2877,7 @@ int Channel::SetREDStatus(bool enable, int redPayloadtype) { } int Channel::GetREDStatus(bool& enabled, int& redPayloadtype) { - enabled = audio_coding_->REDStatus(); + enabled = codec_manager_.GetStackParams()->use_red; if (enabled) { int8_t payloadType = 0; if (_rtpRtcpModule->SendREDPayloadType(&payloadType) != 0) { @@ -2883,7 +2897,8 @@ int Channel::SetCodecFECStatus(bool enable) { WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::SetCodecFECStatus()"); - if (audio_coding_->SetCodecFEC(enable) != 0) { + if (!codec_manager_.SetCodecFEC(enable) || + !codec_manager_.MakeEncoder(&rent_a_codec_, audio_coding_.get())) { _engineStatisticsPtr->SetLastError( VE_AUDIO_CODING_MODULE_ERROR, kTraceError, "SetCodecFECStatus() failed to set FEC state"); @@ -2893,8 +2908,7 @@ int Channel::SetCodecFECStatus(bool enable) { } bool Channel::GetCodecFECStatus() { - bool enabled = audio_coding_->CodecFEC(); - return enabled; + return codec_manager_.GetStackParams()->use_codec_fec; } void Channel::SetNACKStatus(bool enable, int maxNumberOfPackets) { @@ -3442,7 +3456,8 @@ int Channel::SetRedPayloadType(int red_payload_type) { } codec.pltype = red_payload_type; - if (audio_coding_->RegisterSendCodec(codec) < 0) { + if (!codec_manager_.RegisterEncoder(codec) || + !codec_manager_.MakeEncoder(&rent_a_codec_, audio_coding_.get())) { _engineStatisticsPtr->SetLastError( VE_AUDIO_CODING_MODULE_ERROR, kTraceError, "SetRedPayloadType() RED registration in ACM module failed"); diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h index 2626df969b..d22da74ae5 100644 --- a/webrtc/voice_engine/channel.h +++ b/webrtc/voice_engine/channel.h @@ -18,6 +18,8 @@ #include "webrtc/base/optional.h" #include "webrtc/common_audio/resampler/include/push_resampler.h" #include "webrtc/common_types.h" +#include "webrtc/modules/audio_coding/acm2/codec_manager.h" +#include "webrtc/modules/audio_coding/acm2/rent_a_codec.h" #include "webrtc/modules/audio_coding/include/audio_coding_module.h" #include "webrtc/modules/audio_conference_mixer/include/audio_conference_mixer_defines.h" #include "webrtc/modules/audio_processing/rms_level.h" @@ -481,6 +483,8 @@ class Channel TelephoneEventHandler* telephone_event_handler_; std::unique_ptr _rtpRtcpModule; std::unique_ptr audio_coding_; + acm2::CodecManager codec_manager_; + acm2::RentACodec rent_a_codec_; std::unique_ptr audio_sink_; AudioLevel _outputAudioLevel; bool _externalTransport;