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}

webrtc/modules/audio_coding/acm2/audio_coding_module.cc

@@ -48,6 +48,8 @@ class AudioCodingModuleImpl final : public AudioCodingModule {
   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 @@ void AudioCodingModuleImpl::ModifyEncoder(
   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_);

webrtc/modules/audio_coding/codecs/audio_encoder.cc

@@ -50,6 +50,10 @@ bool AudioEncoder::SetDtx(bool enable) {
   return !enable;
 }
 
+bool AudioEncoder::GetDtx() const {
+  return false;
+}
+
 bool AudioEncoder::SetApplication(Application application) {
   return false;
 }

webrtc/modules/audio_coding/codecs/audio_encoder.h

@@ -127,6 +127,10 @@ class AudioEncoder {
   // 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 };

webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc

@@ -152,6 +152,10 @@ bool AudioEncoderOpus::SetDtx(bool enable) {
   return RecreateEncoderInstance(conf);
 }
 
+bool AudioEncoderOpus::GetDtx() const {
+  return config_.dtx_enabled;
+}
+
 bool AudioEncoderOpus::SetApplication(Application application) {
   auto conf = config_;
   switch (application) {

webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h

@@ -75,6 +75,7 @@ class AudioEncoderOpus final : public AudioEncoder {
   // 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 @@ class AudioEncoderOpus final : public AudioEncoder {
   // 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,

webrtc/modules/audio_coding/include/audio_coding_module.h

@@ -250,6 +250,10 @@ class AudioCodingModule {
   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) {

webrtc/voice_engine/channel.cc

@@ -1548,6 +1548,17 @@ int Channel::SetOpusDtx(bool enable_dtx) {
   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()");

webrtc/voice_engine/channel.h

@@ -228,6 +228,7 @@ class Channel
   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);

webrtc/voice_engine/include/voe_codec.h

@@ -131,6 +131,12 @@ class WEBRTC_DLLEXPORT VoECodec {
   // 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() {}

webrtc/voice_engine/voe_codec_impl.cc

@@ -376,6 +376,23 @@ int VoECodecImpl::SetOpusDtx(int channel, bool enable_dtx) {
   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

webrtc/voice_engine/voe_codec_impl.h

@@ -58,6 +58,8 @@ class VoECodecImpl : public VoECodec {
 
   int SetOpusDtx(int channel, bool enable_dtx) override;
 
+  int GetOpusDtxStatus(int channel, bool* enabled) override;
+
  protected:
   VoECodecImpl(voe::SharedData* shared);
   ~VoECodecImpl() override;

webrtc/voice_engine/voe_codec_unittest.cc

@@ -22,85 +22,6 @@ namespace webrtc {
 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 @@ TEST(VoECodecInst, TestCompareCodecInstances) {
   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