From 44307630d3ad6dcdd7b7fd07e78881b50a92ced4 Mon Sep 17 00:00:00 2001 From: kwiberg Date: Wed, 16 Dec 2015 06:24:05 -0800 Subject: [PATCH] AudioCodingModuleImpl: Stop failing artificially for non-Opus encoders All encoders already handle the "Opus-specific" requests sanely (by failing nicely), so we don't need extra checks to protect them. BUG=webrtc:5028 Review URL: https://codereview.webrtc.org/1527453005 Cr-Commit-Position: refs/heads/master@{#11051} --- .../audio_coding/acm2/audio_coding_module_impl.cc | 8 -------- webrtc/modules/audio_coding/acm2/codec_manager.cc | 8 +++----- webrtc/modules/audio_coding/acm2/codec_manager.h | 2 -- webrtc/modules/audio_coding/test/TestVADDTX.cc | 4 ++-- .../test/auto_test/standard/codec_test.cc | 12 ------------ 5 files changed, 5 insertions(+), 29 deletions(-) diff --git a/webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc b/webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc index d8384aa9fc..0572b26221 100644 --- a/webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc +++ b/webrtc/modules/audio_coding/acm2/audio_coding_module_impl.cc @@ -744,8 +744,6 @@ int AudioCodingModuleImpl::SetOpusApplication(OpusApplicationMode application) { if (!HaveValidEncoder("SetOpusApplication")) { return -1; } - if (!codec_manager_.CurrentEncoderIsOpus()) - return -1; AudioEncoder::Application app; switch (application) { case kVoip: @@ -767,8 +765,6 @@ int AudioCodingModuleImpl::SetOpusMaxPlaybackRate(int frequency_hz) { if (!HaveValidEncoder("SetOpusMaxPlaybackRate")) { return -1; } - if (!codec_manager_.CurrentEncoderIsOpus()) - return -1; rent_a_codec_.GetEncoderStack()->SetMaxPlaybackRate(frequency_hz); return 0; } @@ -778,8 +774,6 @@ int AudioCodingModuleImpl::EnableOpusDtx() { if (!HaveValidEncoder("EnableOpusDtx")) { return -1; } - if (!codec_manager_.CurrentEncoderIsOpus()) - return -1; return rent_a_codec_.GetEncoderStack()->SetDtx(true) ? 0 : -1; } @@ -788,8 +782,6 @@ int AudioCodingModuleImpl::DisableOpusDtx() { if (!HaveValidEncoder("DisableOpusDtx")) { return -1; } - if (!codec_manager_.CurrentEncoderIsOpus()) - return -1; return rent_a_codec_.GetEncoderStack()->SetDtx(false) ? 0 : -1; } diff --git a/webrtc/modules/audio_coding/acm2/codec_manager.cc b/webrtc/modules/audio_coding/acm2/codec_manager.cc index bc507dd8c7..d8ef2bf9d5 100644 --- a/webrtc/modules/audio_coding/acm2/codec_manager.cc +++ b/webrtc/modules/audio_coding/acm2/codec_manager.cc @@ -166,7 +166,9 @@ bool CodecManager::SetVAD(bool enable, ACMVADMode mode) { return false; } - if (CurrentEncoderIsOpus()) { + // TODO(kwiberg): This doesn't protect Opus when injected as an external + // encoder. + if (send_codec_inst_ && IsOpus(*send_codec_inst_)) { // VAD/DTX not supported, but don't fail. enable = false; } @@ -187,9 +189,5 @@ bool CodecManager::SetCodecFEC(bool enable_codec_fec) { return true; } -bool CodecManager::CurrentEncoderIsOpus() const { - return send_codec_inst_ ? IsOpus(*send_codec_inst_) : false; -} - } // namespace acm2 } // namespace webrtc diff --git a/webrtc/modules/audio_coding/acm2/codec_manager.h b/webrtc/modules/audio_coding/acm2/codec_manager.h index ad96d1d9f0..9227e13f09 100644 --- a/webrtc/modules/audio_coding/acm2/codec_manager.h +++ b/webrtc/modules/audio_coding/acm2/codec_manager.h @@ -53,8 +53,6 @@ class CodecManager final { bool SetCodecFEC(bool enable_codec_fec); - bool CurrentEncoderIsOpus() const; - private: rtc::ThreadChecker thread_checker_; rtc::Optional send_codec_inst_; diff --git a/webrtc/modules/audio_coding/test/TestVADDTX.cc b/webrtc/modules/audio_coding/test/TestVADDTX.cc index a7d2c2134b..229dc2d474 100644 --- a/webrtc/modules/audio_coding/test/TestVADDTX.cc +++ b/webrtc/modules/audio_coding/test/TestVADDTX.cc @@ -234,10 +234,10 @@ void TestWebRtcVadDtx::SetVAD(bool enable_dtx, bool enable_vad, // Following is the implementation of TestOpusDtx. void TestOpusDtx::Perform() { #ifdef WEBRTC_CODEC_ISAC - // If we set other codec than Opus, DTX cannot be toggled. + // If we set other codec than Opus, DTX cannot be switched on. RegisterCodec(kIsacWb); EXPECT_EQ(-1, acm_send_->EnableOpusDtx()); - EXPECT_EQ(-1, acm_send_->DisableOpusDtx()); + EXPECT_EQ(0, acm_send_->DisableOpusDtx()); #endif #ifdef WEBRTC_CODEC_OPUS diff --git a/webrtc/voice_engine/test/auto_test/standard/codec_test.cc b/webrtc/voice_engine/test/auto_test/standard/codec_test.cc index eeb12aba04..5ab6d58c1d 100644 --- a/webrtc/voice_engine/test/auto_test/standard/codec_test.cc +++ b/webrtc/voice_engine/test/auto_test/standard/codec_test.cc @@ -153,17 +153,6 @@ TEST_F(CodecTest, OpusMaxPlaybackRateCanBeSet) { } } -TEST_F(CodecTest, OpusMaxPlaybackRateCannotBeSetForNonOpus) { - for (int i = 0; i < voe_codec_->NumOfCodecs(); ++i) { - voe_codec_->GetCodec(i, codec_instance_); - if (!_stricmp("opus", codec_instance_.plname)) { - continue; - } - voe_codec_->SetSendCodec(channel_, codec_instance_); - EXPECT_EQ(-1, voe_codec_->SetOpusMaxPlaybackRate(channel_, 16000)); - } -} - TEST_F(CodecTest, OpusDtxCanBeSetForOpus) { for (int i = 0; i < voe_codec_->NumOfCodecs(); ++i) { voe_codec_->GetCodec(i, codec_instance_); @@ -183,7 +172,6 @@ TEST_F(CodecTest, OpusDtxCannotBeSetForNonOpus) { continue; } voe_codec_->SetSendCodec(channel_, codec_instance_); - EXPECT_EQ(-1, voe_codec_->SetOpusDtx(channel_, false)); EXPECT_EQ(-1, voe_codec_->SetOpusDtx(channel_, true)); } }