Regression test for issue where Opus DTX status was being forgotten.
BUG=webrtc:6020
Review-Url: https://codereview.webrtc.org/2177263002
Cr-Commit-Position: refs/heads/master@{#13539}
diff --git a/webrtc/modules/audio_coding/acm2/audio_coding_module.cc b/webrtc/modules/audio_coding/acm2/audio_coding_module.cc
index b394c59..3f404f7 100644
--- a/webrtc/modules/audio_coding/acm2/audio_coding_module.cc
+++ b/webrtc/modules/audio_coding/acm2/audio_coding_module.cc
@@ -48,6 +48,8 @@
void ModifyEncoder(
FunctionView<void(std::unique_ptr<AudioEncoder>*)> modifier) override;
+ void QueryEncoder(FunctionView<void(const AudioEncoder*)> query) override;
+
// Get current send codec.
rtc::Optional<CodecInst> SendCodec() const override;
@@ -596,6 +598,12 @@
modifier(&encoder_stack_);
}
+void AudioCodingModuleImpl::QueryEncoder(
+ FunctionView<void(const AudioEncoder*)> query) {
+ rtc::CritScope lock(&acm_crit_sect_);
+ query(encoder_stack_.get());
+}
+
// Get current send codec.
rtc::Optional<CodecInst> AudioCodingModuleImpl::SendCodec() const {
rtc::CritScope lock(&acm_crit_sect_);
diff --git a/webrtc/modules/audio_coding/codecs/audio_encoder.cc b/webrtc/modules/audio_coding/codecs/audio_encoder.cc
index 6b7f5f8..c433dcd 100644
--- a/webrtc/modules/audio_coding/codecs/audio_encoder.cc
+++ b/webrtc/modules/audio_coding/codecs/audio_encoder.cc
@@ -50,6 +50,10 @@
return !enable;
}
+bool AudioEncoder::GetDtx() const {
+ return false;
+}
+
bool AudioEncoder::SetApplication(Application application) {
return false;
}
diff --git a/webrtc/modules/audio_coding/codecs/audio_encoder.h b/webrtc/modules/audio_coding/codecs/audio_encoder.h
index ecc28d9..f09525f 100644
--- a/webrtc/modules/audio_coding/codecs/audio_encoder.h
+++ b/webrtc/modules/audio_coding/codecs/audio_encoder.h
@@ -127,6 +127,10 @@
// supported).
virtual bool SetDtx(bool enable);
+ // Returns the status of codec-internal DTX. The default implementation always
+ // returns false.
+ virtual bool GetDtx() const;
+
// Sets the application mode. Returns true if the codec was able to comply.
// The default implementation just returns false.
enum class Application { kSpeech, kAudio };
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 edafbbd..d03f2d3 100644
--- a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
+++ b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
@@ -152,6 +152,10 @@
return RecreateEncoderInstance(conf);
}
+bool AudioEncoderOpus::GetDtx() const {
+ return config_.dtx_enabled;
+}
+
bool AudioEncoderOpus::SetApplication(Application application) {
auto conf = config_;
switch (application) {
diff --git a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h
index 367c9b9..48fb494 100644
--- a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h
+++ b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h
@@ -75,6 +75,7 @@
// being inactive. During that, it still sends 2 packets (one for content, one
// for signaling) about every 400 ms.
bool SetDtx(bool enable) override;
+ bool GetDtx() const override;
bool SetApplication(Application application) override;
void SetMaxPlaybackRate(int frequency_hz) override;
@@ -84,7 +85,6 @@
// Getters for testing.
double packet_loss_rate() const { return packet_loss_rate_; }
ApplicationMode application() const { return config_.application; }
- bool dtx_enabled() const { return config_.dtx_enabled; }
protected:
EncodedInfo EncodeImpl(uint32_t rtp_timestamp,
diff --git a/webrtc/modules/audio_coding/include/audio_coding_module.h b/webrtc/modules/audio_coding/include/audio_coding_module.h
index 5b35bcf..30a17f7 100644
--- a/webrtc/modules/audio_coding/include/audio_coding_module.h
+++ b/webrtc/modules/audio_coding/include/audio_coding_module.h
@@ -250,6 +250,10 @@
virtual void ModifyEncoder(
FunctionView<void(std::unique_ptr<AudioEncoder>*)> modifier) = 0;
+ // |modifier| is called exactly once with one argument: a const pointer to the
+ // current encoder (which is null if there is no current encoder).
+ virtual void QueryEncoder(FunctionView<void(AudioEncoder const*)> query) = 0;
+
// Utility method for simply replacing the existing encoder with a new one.
void SetEncoder(std::unique_ptr<AudioEncoder> new_encoder) {
ModifyEncoder([&](std::unique_ptr<AudioEncoder>* encoder) {
diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc
index 424d291..dcedd75 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -1548,6 +1548,17 @@
return 0;
}
+int Channel::GetOpusDtx(bool* enabled) {
+ int success = -1;
+ audio_coding_->QueryEncoder([&](AudioEncoder const* encoder) {
+ if (encoder) {
+ *enabled = encoder->GetDtx();
+ success = 0;
+ }
+ });
+ return success;
+}
+
int32_t Channel::RegisterExternalTransport(Transport* transport) {
WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
"Channel::RegisterExternalTransport()");
diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h
index 68211a6..b51b7ec 100644
--- a/webrtc/voice_engine/channel.h
+++ b/webrtc/voice_engine/channel.h
@@ -228,6 +228,7 @@
int32_t SetSendCNPayloadType(int type, PayloadFrequencies frequency);
int SetOpusMaxPlaybackRate(int frequency_hz);
int SetOpusDtx(bool enable_dtx);
+ int GetOpusDtx(bool* enabled);
// VoENetwork
int32_t RegisterExternalTransport(Transport* transport);
diff --git a/webrtc/voice_engine/include/voe_codec.h b/webrtc/voice_engine/include/voe_codec.h
index 5d94ac2..cedf47d 100644
--- a/webrtc/voice_engine/include/voe_codec.h
+++ b/webrtc/voice_engine/include/voe_codec.h
@@ -131,6 +131,12 @@
// success, and -1 if failed.
virtual int SetOpusDtx(int channel, bool enable_dtx) = 0;
+ // If send codec is Opus on a specified |channel|, return its DTX status.
+ // Returns 0 on success, and -1 if failed.
+ // TODO(ivoc): Make GetOpusDtxStatus() pure virtual when all deriving classes
+ // are updated.
+ virtual int GetOpusDtxStatus(int channel, bool* enabled) { return -1; }
+
protected:
VoECodec() {}
virtual ~VoECodec() {}
diff --git a/webrtc/voice_engine/voe_codec_impl.cc b/webrtc/voice_engine/voe_codec_impl.cc
index 19891bc..8ac24da 100644
--- a/webrtc/voice_engine/voe_codec_impl.cc
+++ b/webrtc/voice_engine/voe_codec_impl.cc
@@ -376,6 +376,23 @@
return channelPtr->SetOpusDtx(enable_dtx);
}
+int VoECodecImpl::GetOpusDtxStatus(int channel, bool* enabled) {
+ WEBRTC_TRACE(kTraceApiCall, kTraceVoice, VoEId(_shared->instance_id(), -1),
+ "GetOpusDtx(channel=%d)", channel);
+ if (!_shared->statistics().Initialized()) {
+ _shared->SetLastError(VE_NOT_INITED, kTraceError);
+ return -1;
+ }
+ voe::ChannelOwner ch = _shared->channel_manager().GetChannel(channel);
+ voe::Channel* channelPtr = ch.channel();
+ if (channelPtr == NULL) {
+ _shared->SetLastError(VE_CHANNEL_NOT_VALID, kTraceError,
+ "GetOpusDtx failed to locate channel");
+ return -1;
+ }
+ return channelPtr->GetOpusDtx(enabled);
+}
+
#endif // WEBRTC_VOICE_ENGINE_CODEC_API
} // namespace webrtc
diff --git a/webrtc/voice_engine/voe_codec_impl.h b/webrtc/voice_engine/voe_codec_impl.h
index 5519232..d24bbac 100644
--- a/webrtc/voice_engine/voe_codec_impl.h
+++ b/webrtc/voice_engine/voe_codec_impl.h
@@ -58,6 +58,8 @@
int SetOpusDtx(int channel, bool enable_dtx) override;
+ int GetOpusDtxStatus(int channel, bool* enabled) override;
+
protected:
VoECodecImpl(voe::SharedData* shared);
~VoECodecImpl() override;
diff --git a/webrtc/voice_engine/voe_codec_unittest.cc b/webrtc/voice_engine/voe_codec_unittest.cc
index 73e576b..c056ba0 100644
--- a/webrtc/voice_engine/voe_codec_unittest.cc
+++ b/webrtc/voice_engine/voe_codec_unittest.cc
@@ -22,85 +22,6 @@
namespace voe {
namespace {
-class VoECodecTest : public ::testing::Test {
- protected:
- VoECodecTest()
- : voe_(VoiceEngine::Create()),
- base_(VoEBase::GetInterface(voe_)),
- voe_codec_(VoECodec::GetInterface(voe_)),
- channel_(-1),
- adm_(new FakeAudioDeviceModule),
- red_payload_type_(-1) {}
-
- ~VoECodecTest() {}
-
- void TearDown() {
- base_->DeleteChannel(channel_);
- base_->Terminate();
- base_->Release();
- voe_codec_->Release();
- VoiceEngine::Delete(voe_);
- }
-
- void SetUp() {
- // Check if all components are valid.
- ASSERT_TRUE(voe_ != NULL);
- ASSERT_TRUE(base_ != NULL);
- ASSERT_TRUE(voe_codec_ != NULL);
- ASSERT_TRUE(adm_.get() != NULL);
- ASSERT_EQ(0, base_->Init(adm_.get()));
- channel_ = base_->CreateChannel();
- ASSERT_NE(-1, channel_);
-
- CodecInst my_codec;
-
- bool primary_found = false;
- bool valid_secondary_found = false;
- bool invalid_secondary_found = false;
-
- // Find primary and secondary codecs.
- int num_codecs = voe_codec_->NumOfCodecs();
- int n = 0;
- while (n < num_codecs &&
- (!primary_found || !valid_secondary_found ||
- !invalid_secondary_found || red_payload_type_ < 0)) {
- EXPECT_EQ(0, voe_codec_->GetCodec(n, my_codec));
- if (!STR_CASE_CMP(my_codec.plname, "isac") && my_codec.plfreq == 16000) {
- memcpy(&valid_secondary_, &my_codec, sizeof(my_codec));
- valid_secondary_found = true;
- } else if (!STR_CASE_CMP(my_codec.plname, "isac") &&
- my_codec.plfreq == 32000) {
- memcpy(&invalid_secondary_, &my_codec, sizeof(my_codec));
- invalid_secondary_found = true;
- } else if (!STR_CASE_CMP(my_codec.plname, "L16") &&
- my_codec.plfreq == 16000) {
- memcpy(&primary_, &my_codec, sizeof(my_codec));
- primary_found = true;
- } else if (!STR_CASE_CMP(my_codec.plname, "RED")) {
- red_payload_type_ = my_codec.pltype;
- }
- n++;
- }
-
- EXPECT_TRUE(primary_found);
- EXPECT_TRUE(valid_secondary_found);
- EXPECT_TRUE(invalid_secondary_found);
- EXPECT_NE(-1, red_payload_type_);
- }
-
- VoiceEngine* voe_;
- VoEBase* base_;
- VoECodec* voe_codec_;
- int channel_;
- CodecInst primary_;
- CodecInst valid_secondary_;
- std::unique_ptr<FakeAudioDeviceModule> adm_;
-
- // A codec which is not valid to be registered as secondary codec.
- CodecInst invalid_secondary_;
- int red_payload_type_;
-};
-
TEST(VoECodecInst, TestCompareCodecInstances) {
CodecInst codec1, codec2;
memset(&codec1, 0, sizeof(CodecInst));
@@ -151,6 +72,36 @@
EXPECT_FALSE(codec1 == codec2);
}
+// This is a regression test for
+// https://bugs.chromium.org/p/webrtc/issues/detail?id=6020
+// The Opus DTX setting was being forgotten after unrelated VoE calls.
+TEST(VoECodecInst, RememberOpusDtxAfterSettingChange) {
+ VoiceEngine* voe(VoiceEngine::Create());
+ VoEBase* base(VoEBase::GetInterface(voe));
+ VoECodec* voe_codec(VoECodec::GetInterface(voe));
+ std::unique_ptr<FakeAudioDeviceModule> adm(new FakeAudioDeviceModule);
+
+ base->Init(adm.get());
+
+ CodecInst codec = {111, "opus", 48000, 960, 1, 32000};
+
+ int channel = base->CreateChannel();
+
+ bool DTX = false;
+
+ EXPECT_EQ(0, voe_codec->SetSendCodec(channel, codec));
+ EXPECT_EQ(0, voe_codec->SetOpusDtx(channel, true));
+ EXPECT_EQ(0, voe_codec->SetFECStatus(channel, true));
+ EXPECT_EQ(0, voe_codec->GetOpusDtxStatus(channel, &DTX));
+ EXPECT_TRUE(DTX);
+
+ base->DeleteChannel(channel);
+ base->Terminate();
+ base->Release();
+ voe_codec->Release();
+ VoiceEngine::Delete(voe);
+}
+
} // namespace
} // namespace voe
} // namespace webrtc