From e16bfde5124fc75dcd294ae1856da820923e18a4 Mon Sep 17 00:00:00 2001 From: "minyue@webrtc.org" Date: Thu, 12 Mar 2015 15:28:41 +0000 Subject: [PATCH] Adding flag to force Opus application and DTX while toggling. Currently, we only allow Opus DTX in combination with Opus kVoip mode. When one of them is toggled, the other might need to change as well. This CL is to introduce a flag to force a co-config. BUG= R=henrik.lundin@webrtc.org, tina.legrand@webrtc.org Review URL: https://webrtc-codereview.appspot.com/40159004 Cr-Commit-Position: refs/heads/master@{#8698} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8698 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../codecs/opus/audio_encoder_opus.cc | 1 + .../opus/interface/audio_encoder_opus.h | 2 + .../main/acm2/acm_generic_codec.cc | 22 +++++--- .../main/acm2/acm_generic_codec.h | 31 ++++++++---- .../main/acm2/acm_generic_codec_opus_test.cc | 38 ++++++++++---- .../main/acm2/audio_coding_module_impl.cc | 10 ++-- .../main/acm2/audio_coding_module_impl.h | 5 +- .../audio_coding_module_unittest_oldapi.cc | 2 +- .../main/interface/audio_coding_module.h | 50 +++++++++++-------- .../audio_coding/main/test/TestVADDTX.cc | 16 ++++-- 10 files changed, 119 insertions(+), 58 deletions(-) diff --git a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc index b98033e06..ae08423db 100644 --- a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc +++ b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc @@ -77,6 +77,7 @@ AudioEncoderOpus::AudioEncoderOpus(const Config& config) num_channels_(config.num_channels), payload_type_(config.payload_type), application_(config.application), + dtx_enabled_(config.dtx_enabled), samples_per_10ms_frame_(rtc::CheckedDivExact(kSampleRateHz, 100) * num_channels_), packet_loss_rate_(0.0) { diff --git a/webrtc/modules/audio_coding/codecs/opus/interface/audio_encoder_opus.h b/webrtc/modules/audio_coding/codecs/opus/interface/audio_encoder_opus.h index bfd4ba86a..0a2a008d1 100644 --- a/webrtc/modules/audio_coding/codecs/opus/interface/audio_encoder_opus.h +++ b/webrtc/modules/audio_coding/codecs/opus/interface/audio_encoder_opus.h @@ -55,6 +55,7 @@ class AudioEncoderOpus final : public AudioEncoder { double packet_loss_rate() const { return packet_loss_rate_; } ApplicationMode application() const { return application_; } + bool dtx_enabled() const { return dtx_enabled_; } protected: void EncodeInternal(uint32_t rtp_timestamp, @@ -69,6 +70,7 @@ class AudioEncoderOpus final : public AudioEncoder { const int payload_type_; const ApplicationMode application_; int bitrate_bps_; + const bool dtx_enabled_; const int samples_per_10ms_frame_; std::vector input_buffer_; OpusEncInst* inst_; 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 e71fa1694..c764d7421 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc +++ b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc @@ -514,14 +514,17 @@ AudioDecoder* ACMGenericCodec::Decoder() { return decoder_proxy_.IsSet() ? &decoder_proxy_ : nullptr; } -int ACMGenericCodec::EnableOpusDtx() { +int ACMGenericCodec::EnableOpusDtx(bool force_voip) { WriteLockScoped wl(codec_wrapper_lock_); if (!is_opus_) return -1; // Needed for tests to pass. - if (GetOpusApplication(encoder_->NumChannels(), true) != kVoip) { - // Opus DTX can only be enabled when application mode is KVoip. - return -1; + if (!force_voip && + GetOpusApplication(encoder_->NumChannels(), true) != kVoip) { + // Opus DTX can only be enabled when application mode is KVoip. + return -1; } + opus_application_ = kVoip; + opus_application_set_ = true; opus_dtx_enabled_ = true; ResetAudioEncoder(); return 0; @@ -547,11 +550,16 @@ int ACMGenericCodec::SetFEC(bool enable_fec) { return 0; } -int ACMGenericCodec::SetOpusApplication(OpusApplicationMode application) { +int ACMGenericCodec::SetOpusApplication(OpusApplicationMode application, + bool disable_dtx_if_needed) { WriteLockScoped wl(codec_wrapper_lock_); if (opus_dtx_enabled_ && application == kAudio) { - // Opus can only be set to kAudio when DTX is off. - return -1; + if (disable_dtx_if_needed) { + opus_dtx_enabled_ = false; + } else { + // Opus can only be set to kAudio when DTX is off. + return -1; + } } opus_application_ = application; opus_application_set_ = true; diff --git a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h index 678f6fef3..9ae99bf86 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h +++ b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h @@ -320,18 +320,25 @@ class ACMGenericCodec { int32_t SetISACMaxRate(const uint32_t max_rate_bps); /////////////////////////////////////////////////////////////////////////// - // int SetOpusApplication() + // int SetOpusApplication(OpusApplicationMode application, + // bool disable_dtx_if_needed) // Sets the intended application for the Opus encoder. Opus uses this to - // optimize the encoding for applications like VOIP and music. + // optimize the encoding for applications like VOIP and music. Currently, two + // modes are supported: kVoip and kAudio. kAudio is only allowed when Opus + // DTX is switched off. If DTX is on, and |application| == kAudio, a failure + // will be triggered unless |disable_dtx_if_needed| == true, for which, the + // DTX will be forced off. // // Input: - // - application : intended application. + // - application : intended application. + // - disable_dtx_if_needed : whether to force Opus DTX to stop when needed. // // Return value: // -1 if failed or on codecs other than Opus. // 0 if succeeded. // - int SetOpusApplication(OpusApplicationMode /*application*/); + int SetOpusApplication(OpusApplicationMode application, + bool disable_dtx_if_needed); /////////////////////////////////////////////////////////////////////////// // int SetOpusMaxPlaybackRate() @@ -344,19 +351,24 @@ class ACMGenericCodec { // -frequency_hz : maximum playback rate in Hz. // // Return value: - // -1 if failed or on codecs other than Opus + // -1 if failed or on codecs other than Opus. // 0 if succeeded. // int SetOpusMaxPlaybackRate(int /* frequency_hz */); /////////////////////////////////////////////////////////////////////////// - // EnableOpusDtx() - // Enable the DTX, if the codec is Opus. If current Opus application mode is - // audio, a failure will be triggered. + // EnableOpusDtx(bool force_voip) + // Enable the DTX, if the codec is Opus. Currently, DTX can only be enabled + // when the application mode is kVoip. If |force_voip| == true, the + // application mode will be forced to kVoip. Otherwise, a failure will be + // triggered if current application mode is kAudio. + // Input: + // - force_voip : whether to force application mode to kVoip. // Return value: // -1 if failed or on codecs other than Opus. // 0 if succeeded. - int EnableOpusDtx(); + // + int EnableOpusDtx(bool force_voip); /////////////////////////////////////////////////////////////////////////// // DisbleOpusDtx() @@ -364,6 +376,7 @@ class ACMGenericCodec { // Return value: // -1 if failed or on codecs other than Opus. // 0 if succeeded. + // int DisableOpusDtx(); /////////////////////////////////////////////////////////////////////////// diff --git a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec_opus_test.cc b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec_opus_test.cc index 29ca5260b..e06a74c0b 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec_opus_test.cc +++ b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec_opus_test.cc @@ -67,7 +67,7 @@ TEST_F(AcmGenericCodecOpusTest, ChangeApplicationMode) { EXPECT_EQ(AudioEncoderOpus::kAudio, opus_ptr->application()); // Change mode. - EXPECT_EQ(0, codec_wrapper_->SetOpusApplication(kVoip)); + EXPECT_EQ(0, codec_wrapper_->SetOpusApplication(kVoip, false)); // Verify that the AudioEncoder object was changed. EXPECT_NE(opus_ptr, GetAudioEncoderOpus()); EXPECT_EQ(AudioEncoderOpus::kVoip, GetAudioEncoderOpus()->application()); @@ -89,7 +89,7 @@ TEST_F(AcmGenericCodecOpusTest, ResetWontChangeApplicationMode) { EXPECT_EQ(AudioEncoderOpus::kAudio, GetAudioEncoderOpus()->application()); // Now change to kVoip. - EXPECT_EQ(0, codec_wrapper_->SetOpusApplication(kVoip)); + EXPECT_EQ(0, codec_wrapper_->SetOpusApplication(kVoip, false)); EXPECT_EQ(AudioEncoderOpus::kVoip, GetAudioEncoderOpus()->application()); opus_ptr = GetAudioEncoderOpus(); @@ -108,18 +108,36 @@ TEST_F(AcmGenericCodecOpusTest, ToggleDtx) { // Verify that the mode is still kAudio. EXPECT_EQ(AudioEncoderOpus::kAudio, GetAudioEncoderOpus()->application()); - // DTX is not allowed in audio mode. - EXPECT_EQ(-1, codec_wrapper_->EnableOpusDtx()); + // DTX is not allowed in audio mode, if mode forcing flag is false. + EXPECT_EQ(-1, codec_wrapper_->EnableOpusDtx(false)); + EXPECT_EQ(AudioEncoderOpus::kAudio, GetAudioEncoderOpus()->application()); - EXPECT_EQ(0, codec_wrapper_->SetOpusApplication(kVoip)); - EXPECT_EQ(0, codec_wrapper_->EnableOpusDtx()); - - // Audio mode is not allowed when DTX is on. - EXPECT_EQ(-1, codec_wrapper_->SetOpusApplication(kAudio)); + // DTX will be on, if mode forcing flag is true. Then application mode is + // switched to kVoip. + EXPECT_EQ(0, codec_wrapper_->EnableOpusDtx(true)); EXPECT_EQ(AudioEncoderOpus::kVoip, GetAudioEncoderOpus()->application()); + // Audio mode is not allowed when DTX is on, and DTX forcing flag is false. + EXPECT_EQ(-1, codec_wrapper_->SetOpusApplication(kAudio, false)); + EXPECT_EQ(true, GetAudioEncoderOpus()->dtx_enabled()); + + // Audio mode will be set, if DTX forcing flag is true. Then DTX is switched + // off. + EXPECT_EQ(0, codec_wrapper_->SetOpusApplication(kAudio, true)); + EXPECT_EQ(false, GetAudioEncoderOpus()->dtx_enabled()); + + // Now we set VOIP mode. The DTX forcing flag has no effect. + EXPECT_EQ(0, codec_wrapper_->SetOpusApplication(kVoip, true)); + EXPECT_EQ(false, GetAudioEncoderOpus()->dtx_enabled()); + + // In VOIP mode, we can enable DTX with mode forcing flag being false. + EXPECT_EQ(0, codec_wrapper_->EnableOpusDtx(false)); + + // Turn off DTX. EXPECT_EQ(0, codec_wrapper_->DisableOpusDtx()); - EXPECT_EQ(0, codec_wrapper_->SetOpusApplication(kAudio)); + + // When DTX is off, we can set Audio mode with DTX forcing flag being false. + EXPECT_EQ(0, codec_wrapper_->SetOpusApplication(kAudio, false)); } #endif // WEBRTC_CODEC_OPUS 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 319c791ea..718992f41 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 @@ -1327,12 +1327,14 @@ int AudioCodingModuleImpl::ConfigISACBandwidthEstimator( // frame_size_ms, rate_bit_per_sec, enforce_frame_size); } -int AudioCodingModuleImpl::SetOpusApplication(OpusApplicationMode application) { +int AudioCodingModuleImpl::SetOpusApplication(OpusApplicationMode application, + bool disable_dtx_if_needed) { CriticalSectionScoped lock(acm_crit_sect_); if (!HaveValidEncoder("SetOpusApplication")) { return -1; } - return codecs_[current_send_codec_idx_]->SetOpusApplication(application); + return codecs_[current_send_codec_idx_]->SetOpusApplication( + application, disable_dtx_if_needed); } // Informs Opus encoder of the maximum playback rate the receiver will render. @@ -1344,12 +1346,12 @@ int AudioCodingModuleImpl::SetOpusMaxPlaybackRate(int frequency_hz) { return codecs_[current_send_codec_idx_]->SetOpusMaxPlaybackRate(frequency_hz); } -int AudioCodingModuleImpl::EnableOpusDtx() { +int AudioCodingModuleImpl::EnableOpusDtx(bool force_voip) { CriticalSectionScoped lock(acm_crit_sect_); if (!HaveValidEncoder("EnableOpusDtx")) { return -1; } - return codecs_[current_send_codec_idx_]->EnableOpusDtx(); + return codecs_[current_send_codec_idx_]->EnableOpusDtx(force_voip); } int AudioCodingModuleImpl::DisableOpusDtx() { 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 ef69ba53c..0b6175256 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 @@ -214,13 +214,14 @@ class AudioCodingModuleImpl : public AudioCodingModule { int rate_bit_per_sec, bool enforce_frame_size = false) override; - int SetOpusApplication(OpusApplicationMode application) override; + int SetOpusApplication(OpusApplicationMode application, + bool disable_dtx_if_needed) override; // If current send codec is Opus, informs it about the maximum playback rate // the receiver will render. int SetOpusMaxPlaybackRate(int frequency_hz) override; - int EnableOpusDtx() override; + int EnableOpusDtx(bool force_voip) override; int DisableOpusDtx() override; diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc index 14a73fd21..e29940162 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc +++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc @@ -1117,7 +1117,7 @@ TEST_F(AcmSenderBitExactnessOldApi, MAYBE_Opus_stereo_20ms) { TEST_F(AcmSenderBitExactnessOldApi, MAYBE_Opus_stereo_20ms_voip) { ASSERT_NO_FATAL_FAILURE(SetUpTest("opus", 48000, 2, 120, 960, 960)); // If not set, default will be kAudio in case of stereo. - send_test_->acm()->SetOpusApplication(kVoip); + EXPECT_EQ(0, send_test_->acm()->SetOpusApplication(kVoip, false)); Run(AcmReceiverBitExactnessOldApi::PlatformChecksum( "9b9e12bc3cc793740966e11cbfa8b35b", "57412a4b5771d19ff03ec35deffe7067", diff --git a/webrtc/modules/audio_coding/main/interface/audio_coding_module.h b/webrtc/modules/audio_coding/main/interface/audio_coding_module.h index f1d0613ad..014b9f3d8 100644 --- a/webrtc/modules/audio_coding/main/interface/audio_coding_module.h +++ b/webrtc/modules/audio_coding/main/interface/audio_coding_module.h @@ -876,18 +876,26 @@ class AudioCodingModule { bool enforce_frame_size = false) = 0; /////////////////////////////////////////////////////////////////////////// - // int SetOpusApplication() - // Sets the intended application for the Opus encoder. Opus uses this to - // optimize the encoding for applications like VOIP and music. + // int SetOpusApplication(OpusApplicationMode application, + // bool disable_dtx_if_needed) + // Sets the intended application if current send codec is Opus. Opus uses this + // to optimize the encoding for applications like VOIP and music. Currently, + // two modes are supported: kVoip and kAudio. kAudio is only allowed when Opus + // DTX is switched off. If DTX is on, and |application| == kAudio, a failure + // will be triggered unless |disable_dtx_if_needed| == true, for which, the + // DTX will be forced off. // // Input: - // - application : intended application. + // - application : intended application. + // - disable_dtx_if_needed : whether to force Opus DTX to stop. // // Return value: - // -1 if failed or on codecs other than Opus. - // 0 if succeeded. + // -1 if current send codec is not Opus or error occurred in setting the + // Opus application mode. + // 0 if the Opus application mode is successfully set. // - virtual int SetOpusApplication(OpusApplicationMode /*application*/) = 0; + virtual int SetOpusApplication(OpusApplicationMode application, + bool force_dtx) = 0; /////////////////////////////////////////////////////////////////////////// // int SetOpusMaxPlaybackRate() @@ -901,31 +909,31 @@ class AudioCodingModule { // Return value: // -1 if current send codec is not Opus or // error occurred in setting the maximum playback rate, - // 0 maximum bandwidth is set successfully. + // 0 if maximum bandwidth is set successfully. // virtual int SetOpusMaxPlaybackRate(int frequency_hz) = 0; /////////////////////////////////////////////////////////////////////////// - // int EnableOpusDtx() - // If current send codec is Opus, enables its internal DTX. - // Currently, this can be only called when Opus application mode is VOIP. - // Use SetOpusApplication() to switch to VOIP mode when necessary. - // + // EnableOpusDtx(bool force_voip) + // Enable the DTX, if current send codec is Opus. Currently, DTX can only be + // enabled when the application mode is kVoip. If |force_voip| == true, + // the application mode will be forced to kVoip. Otherwise, a failure will be + // triggered if current application mode is kAudio. + // Input: + // - force_application : whether to force application mode to kVoip. // Return value: - // -1 if current send codec is not Opus or - // error occurred in enabling DTX. - // 0 Opus DTX is enabled successfully. - // - virtual int EnableOpusDtx() = 0; + // -1 if current send codec is not Opus or error occurred in enabling the + // Opus DTX. + // 0 if Opus DTX is enabled successfully.. + virtual int EnableOpusDtx(bool force_application) = 0; /////////////////////////////////////////////////////////////////////////// // int DisableOpusDtx() // If current send codec is Opus, disables its internal DTX. // // Return value: - // -1 if current send codec is not Opus or - // error occurred in disabling DTX. - // 0 Opus DTX is disabled successfully. + // -1 if current send codec is not Opus or error occurred in disabling DTX. + // 0 if Opus DTX is disabled successfully. // virtual int DisableOpusDtx() = 0; diff --git a/webrtc/modules/audio_coding/main/test/TestVADDTX.cc b/webrtc/modules/audio_coding/main/test/TestVADDTX.cc index 475e69916..e544ae31c 100644 --- a/webrtc/modules/audio_coding/main/test/TestVADDTX.cc +++ b/webrtc/modules/audio_coding/main/test/TestVADDTX.cc @@ -236,6 +236,13 @@ 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. + RegisterCodec(kIsacWb); + EXPECT_EQ(-1, acm_send_->EnableOpusDtx(false)); + EXPECT_EQ(-1, acm_send_->DisableOpusDtx()); +#endif + #ifdef WEBRTC_CODEC_OPUS int expects[] = {0, 1, 0, 0, 0}; @@ -248,7 +255,7 @@ void TestOpusDtx::Perform() { Run(webrtc::test::ResourcePath("audio_coding/testfile32kHz", "pcm"), 32000, 1, out_filename, false, expects); - EXPECT_EQ(0, acm_send_->EnableOpusDtx()); + EXPECT_EQ(0, acm_send_->EnableOpusDtx(false)); expects[kFrameEmpty] = 1; Run(webrtc::test::ResourcePath("audio_coding/testfile32kHz", "pcm"), 32000, 1, out_filename, true, expects); @@ -261,9 +268,10 @@ void TestOpusDtx::Perform() { Run(webrtc::test::ResourcePath("audio_coding/teststereo32kHz", "pcm"), 32000, 2, out_filename, false, expects); - // Opus DTX should only work in Voip mode. - EXPECT_EQ(0, acm_send_->SetOpusApplication(kVoip)); - EXPECT_EQ(0, acm_send_->EnableOpusDtx()); + // Opus should be now in kAudio mode. Opus DTX should not be set without + // forcing kVoip mode. + EXPECT_EQ(-1, acm_send_->EnableOpusDtx(false)); + EXPECT_EQ(0, acm_send_->EnableOpusDtx(true)); expects[kFrameEmpty] = 1; Run(webrtc::test::ResourcePath("audio_coding/teststereo32kHz", "pcm"),