Pass ownership of external encoders to the ACM

webrtc/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc

Index: webrtc/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc
diff --git a/webrtc/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc b/webrtc/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc
index a4416c955aaf779d34784c20d6c0ef6a649158ba..8f30d783ae4c8fa24170f6074163cae365343390 100644
--- a/webrtc/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc
+++ b/webrtc/modules/audio_coding/codecs/cng/audio_encoder_cng_unittest.cc
@@ -34,42 +34,50 @@ static const int kCngPayloadType = 18;
 class AudioEncoderCngTest : public ::testing::Test {
  protected:
   AudioEncoderCngTest()
-      : mock_vad_(new MockVad),
+      : mock_encoder_owner_(new MockAudioEncoder),
+        mock_encoder_(mock_encoder_owner_.get()),
+        mock_vad_(new MockVad),
         timestamp_(4711),
         num_audio_samples_10ms_(0),
         sample_rate_hz_(8000) {
     memset(audio_, 0, kMaxNumSamples * 2);
-    config_.speech_encoder = &mock_encoder_;
-    EXPECT_CALL(mock_encoder_, NumChannels()).WillRepeatedly(Return(1));
-    // Let the AudioEncoderCng object use a MockVad instead of its internally
-    // created Vad object.
-    config_.vad = mock_vad_;
-    config_.payload_type = kCngPayloadType;
+    EXPECT_CALL(*mock_encoder_, NumChannels()).WillRepeatedly(Return(1));
+    EXPECT_CALL(*mock_encoder_, Die()).Times(1);
   }
 
   void TearDown() override {
     EXPECT_CALL(*mock_vad_, Die()).Times(1);
     cng_.reset();
-    // Don't expect the cng_ object to delete the AudioEncoder object. But it
-    // will be deleted with the test fixture. This is why we explicitly delete
-    // the cng_ object above, and set expectations on mock_encoder_ afterwards.
-    EXPECT_CALL(mock_encoder_, Die()).Times(1);
   }
 
-  void CreateCng() {
-    // The config_ parameters may be changed by the TEST_Fs up until CreateCng()
-    // is called, thus we cannot use the values until now.
+  AudioEncoderCng::Config MakeCngConfig() {
+    AudioEncoderCng::Config config;
+    config.speech_encoder = std::move(mock_encoder_owner_);
+    EXPECT_TRUE(config.speech_encoder);
+
+    // Let the AudioEncoderCng object use a MockVad instead of its internally
+    // created Vad object.
+    config.vad = mock_vad_;
+    config.payload_type = kCngPayloadType;
+
+    return config;
+  }
+
+  void CreateCng(AudioEncoderCng::Config&& config) {
     num_audio_samples_10ms_ = static_cast<size_t>(10 * sample_rate_hz_ / 1000);
     ASSERT_LE(num_audio_samples_10ms_, kMaxNumSamples);
-    EXPECT_CALL(mock_encoder_, SampleRateHz())
-        .WillRepeatedly(Return(sample_rate_hz_));
-    // Max10MsFramesInAPacket() is just used to verify that the SID frame period
-    // is not too small. The return value does not matter that much, as long as
-    // it is smaller than 10.
-    EXPECT_CALL(mock_encoder_, Max10MsFramesInAPacket()).WillOnce(Return(1u));
-    EXPECT_CALL(mock_encoder_, MaxEncodedBytes())
-        .WillRepeatedly(Return(kMockMaxEncodedBytes));
-    cng_.reset(new AudioEncoderCng(config_));
+    if (config.speech_encoder) {
+      EXPECT_CALL(*mock_encoder_, SampleRateHz())
+          .WillRepeatedly(Return(sample_rate_hz_));
+      // Max10MsFramesInAPacket() is just used to verify that the SID frame
+      // period is not too small. The return value does not matter that much,
+      // as long as it is smaller than 10.
+      EXPECT_CALL(*mock_encoder_, Max10MsFramesInAPacket())
+          .WillOnce(Return(1u));
+      EXPECT_CALL(*mock_encoder_, MaxEncodedBytes())
+          .WillRepeatedly(Return(kMockMaxEncodedBytes));
+    }
+    cng_.reset(new AudioEncoderCng(std::move(config)));
   }
 
   void Encode() {
@@ -88,27 +96,29 @@ class AudioEncoderCngTest : public ::testing::Test {
     InSequence s;
     AudioEncoder::EncodedInfo info;
     for (size_t j = 0; j < num_calls - 1; ++j) {
-      EXPECT_CALL(mock_encoder_, EncodeImpl(_, _, _))
+      EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _))
           .WillOnce(Return(info));
     }
     info.encoded_bytes = kMockReturnEncodedBytes;
-    EXPECT_CALL(mock_encoder_, EncodeImpl(_, _, _))
-        .WillOnce(Invoke(
-            MockAudioEncoder::FakeEncoding(kMockReturnEncodedBytes)));
+    EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _))
+        .WillOnce(
+            Invoke(MockAudioEncoder::FakeEncoding(kMockReturnEncodedBytes)));
   }
 
   // Verifies that the cng_ object waits until it has collected
   // |blocks_per_frame| blocks of audio, and then dispatches all of them to
   // the underlying codec (speech or cng).
   void CheckBlockGrouping(size_t blocks_per_frame, bool active_speech) {
-    EXPECT_CALL(mock_encoder_, Num10MsFramesInNextPacket())
+    EXPECT_CALL(*mock_encoder_, Num10MsFramesInNextPacket())
         .WillRepeatedly(Return(blocks_per_frame));
-    CreateCng();
+    auto config = MakeCngConfig();
+    const int num_cng_coefficients = config.num_cng_coefficients;
+    CreateCng(std::move(config));
     EXPECT_CALL(*mock_vad_, VoiceActivity(_, _, _))
         .WillRepeatedly(Return(active_speech ? Vad::kActive : Vad::kPassive));
 
     // Don't expect any calls to the encoder yet.
-    EXPECT_CALL(mock_encoder_, EncodeImpl(_, _, _)).Times(0);
+    EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _)).Times(0);
     for (size_t i = 0; i < blocks_per_frame - 1; ++i) {
       Encode();
       EXPECT_EQ(0u, encoded_info_.encoded_bytes);
@@ -119,7 +129,7 @@ class AudioEncoderCngTest : public ::testing::Test {
     if (active_speech) {
       EXPECT_EQ(kMockReturnEncodedBytes, encoded_info_.encoded_bytes);
     } else {
-      EXPECT_EQ(static_cast<size_t>(config_.num_cng_coefficients + 1),
+      EXPECT_EQ(static_cast<size_t>(num_cng_coefficients + 1),
                 encoded_info_.encoded_bytes);
     }
   }
@@ -132,7 +142,7 @@ class AudioEncoderCngTest : public ::testing::Test {
     const size_t blocks_per_frame =
         static_cast<size_t>(input_frame_size_ms / 10);
 
-    EXPECT_CALL(mock_encoder_, Num10MsFramesInNextPacket())
+    EXPECT_CALL(*mock_encoder_, Num10MsFramesInNextPacket())
         .WillRepeatedly(Return(blocks_per_frame));
 
     // Expect nothing to happen before the last block is sent to cng_.
@@ -167,7 +177,7 @@ class AudioEncoderCngTest : public ::testing::Test {
     // Set the speech encoder frame size to 60 ms, to ensure that the VAD will
     // be called twice.
     const size_t blocks_per_frame = 6;
-    EXPECT_CALL(mock_encoder_, Num10MsFramesInNextPacket())
+    EXPECT_CALL(*mock_encoder_, Num10MsFramesInNextPacket())
         .WillRepeatedly(Return(blocks_per_frame));
     InSequence s;
     EXPECT_CALL(*mock_vad_, VoiceActivity(_, _, _))
@@ -184,9 +194,9 @@ class AudioEncoderCngTest : public ::testing::Test {
     return encoded_info_.payload_type != kCngPayloadType;
   }
 
-  AudioEncoderCng::Config config_;
   std::unique_ptr<AudioEncoderCng> cng_;
-  MockAudioEncoder mock_encoder_;
+  std::unique_ptr<MockAudioEncoder> mock_encoder_owner_;
+  MockAudioEncoder* mock_encoder_;
   MockVad* mock_vad_;  // Ownership is transferred to |cng_|.
   uint32_t timestamp_;
   int16_t audio_[kMaxNumSamples];
@@ -194,34 +204,37 @@ class AudioEncoderCngTest : public ::testing::Test {
   rtc::Buffer encoded_;
   AudioEncoder::EncodedInfo encoded_info_;
   int sample_rate_hz_;
+
+  RTC_DISALLOW_COPY_AND_ASSIGN(AudioEncoderCngTest);
 };
 
 TEST_F(AudioEncoderCngTest, CreateAndDestroy) {
-  CreateCng();
+  CreateCng(MakeCngConfig());
 }
 
 TEST_F(AudioEncoderCngTest, CheckFrameSizePropagation) {
-  CreateCng();
-  EXPECT_CALL(mock_encoder_, Num10MsFramesInNextPacket()).WillOnce(Return(17U));
+  CreateCng(MakeCngConfig());
+  EXPECT_CALL(*mock_encoder_, Num10MsFramesInNextPacket())
+      .WillOnce(Return(17U));
   EXPECT_EQ(17U, cng_->Num10MsFramesInNextPacket());
 }
 
 TEST_F(AudioEncoderCngTest, CheckChangeBitratePropagation) {
-  CreateCng();
-  EXPECT_CALL(mock_encoder_, SetTargetBitrate(4711));
+  CreateCng(MakeCngConfig());
+  EXPECT_CALL(*mock_encoder_, SetTargetBitrate(4711));
   cng_->SetTargetBitrate(4711);
 }
 
 TEST_F(AudioEncoderCngTest, CheckProjectedPacketLossRatePropagation) {
-  CreateCng();
-  EXPECT_CALL(mock_encoder_, SetProjectedPacketLossRate(0.5));
+  CreateCng(MakeCngConfig());
+  EXPECT_CALL(*mock_encoder_, SetProjectedPacketLossRate(0.5));
   cng_->SetProjectedPacketLossRate(0.5);
 }
 
 TEST_F(AudioEncoderCngTest, EncodeCallsVad) {
-  EXPECT_CALL(mock_encoder_, Num10MsFramesInNextPacket())
+  EXPECT_CALL(*mock_encoder_, Num10MsFramesInNextPacket())
       .WillRepeatedly(Return(1U));
-  CreateCng();
+  CreateCng(MakeCngConfig());
   EXPECT_CALL(*mock_vad_, VoiceActivity(_, _, _))
       .WillOnce(Return(Vad::kPassive));
   Encode();
@@ -253,13 +266,16 @@ TEST_F(AudioEncoderCngTest, EncodeCollects3BlocksActiveSpeech) {
 
 TEST_F(AudioEncoderCngTest, EncodePassive) {
   const size_t kBlocksPerFrame = 3;
-  EXPECT_CALL(mock_encoder_, Num10MsFramesInNextPacket())
+  EXPECT_CALL(*mock_encoder_, Num10MsFramesInNextPacket())
       .WillRepeatedly(Return(kBlocksPerFrame));
-  CreateCng();
+  auto config = MakeCngConfig();
+  const auto sid_frame_interval_ms = config.sid_frame_interval_ms;
+  const auto num_cng_coefficients = config.num_cng_coefficients;
+  CreateCng(std::move(config));
   EXPECT_CALL(*mock_vad_, VoiceActivity(_, _, _))
       .WillRepeatedly(Return(Vad::kPassive));
   // Expect no calls at all to the speech encoder mock.
-  EXPECT_CALL(mock_encoder_, EncodeImpl(_, _, _)).Times(0);
+  EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _)).Times(0);
   uint32_t expected_timestamp = timestamp_;
   for (size_t i = 0; i < 100; ++i) {
     Encode();
@@ -267,11 +283,11 @@ TEST_F(AudioEncoderCngTest, EncodePassive) {
     // |kBlocksPerFrame| calls.
     if ((i + 1) % kBlocksPerFrame == 0) {
       // Now check if a SID interval has elapsed.
-      if ((i % (config_.sid_frame_interval_ms / 10)) < kBlocksPerFrame) {
+      if ((i % (sid_frame_interval_ms / 10)) < kBlocksPerFrame) {
         // If so, verify that we got a CNG encoding.
         EXPECT_EQ(kCngPayloadType, encoded_info_.payload_type);
         EXPECT_FALSE(encoded_info_.speech);
-        EXPECT_EQ(static_cast<size_t>(config_.num_cng_coefficients) + 1,
+        EXPECT_EQ(static_cast<size_t>(num_cng_coefficients) + 1,
                   encoded_info_.encoded_bytes);
         EXPECT_EQ(expected_timestamp, encoded_info_.encoded_timestamp);
       }
@@ -286,7 +302,7 @@ TEST_F(AudioEncoderCngTest, EncodePassive) {
 // Verifies that the correct action is taken for frames with both active and
 // passive speech.
 TEST_F(AudioEncoderCngTest, MixedActivePassive) {
-  CreateCng();
+  CreateCng(MakeCngConfig());
 
   // All of the frame is active speech.
   ExpectEncodeCalls(6);
@@ -314,35 +330,35 @@ TEST_F(AudioEncoderCngTest, MixedActivePassive) {
 // CheckVadInputSize(frame_size, expected_first_block_size,
 //                   expected_second_block_size);
 TEST_F(AudioEncoderCngTest, VadInputSize10Ms) {
-  CreateCng();
+  CreateCng(MakeCngConfig());
   CheckVadInputSize(10, 10, 0);
 }
 TEST_F(AudioEncoderCngTest, VadInputSize20Ms) {
-  CreateCng();
+  CreateCng(MakeCngConfig());
   CheckVadInputSize(20, 20, 0);
 }
 TEST_F(AudioEncoderCngTest, VadInputSize30Ms) {
-  CreateCng();
+  CreateCng(MakeCngConfig());
   CheckVadInputSize(30, 30, 0);
 }
 TEST_F(AudioEncoderCngTest, VadInputSize40Ms) {
-  CreateCng();
+  CreateCng(MakeCngConfig());
   CheckVadInputSize(40, 20, 20);
 }
 TEST_F(AudioEncoderCngTest, VadInputSize50Ms) {
-  CreateCng();
+  CreateCng(MakeCngConfig());
   CheckVadInputSize(50, 30, 20);
 }
 TEST_F(AudioEncoderCngTest, VadInputSize60Ms) {
-  CreateCng();
+  CreateCng(MakeCngConfig());
   CheckVadInputSize(60, 30, 30);
 }
 
 // Verifies that the correct payload type is set when CNG is encoded.
 TEST_F(AudioEncoderCngTest, VerifyCngPayloadType) {
-  CreateCng();
-  EXPECT_CALL(mock_encoder_, EncodeImpl(_, _, _)).Times(0);
-  EXPECT_CALL(mock_encoder_, Num10MsFramesInNextPacket()).WillOnce(Return(1U));
+  CreateCng(MakeCngConfig());
+  EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _)).Times(0);
+  EXPECT_CALL(*mock_encoder_, Num10MsFramesInNextPacket()).WillOnce(Return(1U));
   EXPECT_CALL(*mock_vad_, VoiceActivity(_, _, _))
       .WillOnce(Return(Vad::kPassive));
   encoded_info_.payload_type = 0;
@@ -353,8 +369,10 @@ TEST_F(AudioEncoderCngTest, VerifyCngPayloadType) {
 // Verifies that a SID frame is encoded immediately as the signal changes from
 // active speech to passive.
 TEST_F(AudioEncoderCngTest, VerifySidFrameAfterSpeech) {
-  CreateCng();
-  EXPECT_CALL(mock_encoder_, Num10MsFramesInNextPacket())
+  auto config = MakeCngConfig();
+  const auto num_cng_coefficients = config.num_cng_coefficients;
+  CreateCng(std::move(config));
+  EXPECT_CALL(*mock_encoder_, Num10MsFramesInNextPacket())
       .WillRepeatedly(Return(1U));
   // Start with encoding noise.
   EXPECT_CALL(*mock_vad_, VoiceActivity(_, _, _))
@@ -362,7 +380,7 @@ TEST_F(AudioEncoderCngTest, VerifySidFrameAfterSpeech) {
       .WillRepeatedly(Return(Vad::kPassive));
   Encode();
   EXPECT_EQ(kCngPayloadType, encoded_info_.payload_type);
-  EXPECT_EQ(static_cast<size_t>(config_.num_cng_coefficients) + 1,
+  EXPECT_EQ(static_cast<size_t>(num_cng_coefficients) + 1,
             encoded_info_.encoded_bytes);
   // Encode again, and make sure we got no frame at all (since the SID frame
   // period is 100 ms by default).
@@ -373,8 +391,9 @@ TEST_F(AudioEncoderCngTest, VerifySidFrameAfterSpeech) {
   encoded_info_.payload_type = 0;
   EXPECT_CALL(*mock_vad_, VoiceActivity(_, _, _))
       .WillOnce(Return(Vad::kActive));
-  EXPECT_CALL(mock_encoder_, EncodeImpl(_, _, _)).WillOnce(
-      Invoke(MockAudioEncoder::FakeEncoding(kMockReturnEncodedBytes)));
+  EXPECT_CALL(*mock_encoder_, EncodeImpl(_, _, _))
+      .WillOnce(
+          Invoke(MockAudioEncoder::FakeEncoding(kMockReturnEncodedBytes)));
   Encode();
   EXPECT_EQ(kMockReturnEncodedBytes, encoded_info_.encoded_bytes);
 
@@ -383,14 +402,14 @@ TEST_F(AudioEncoderCngTest, VerifySidFrameAfterSpeech) {
       .WillOnce(Return(Vad::kPassive));
   Encode();
   EXPECT_EQ(kCngPayloadType, encoded_info_.payload_type);
-  EXPECT_EQ(static_cast<size_t>(config_.num_cng_coefficients) + 1,
+  EXPECT_EQ(static_cast<size_t>(num_cng_coefficients) + 1,
             encoded_info_.encoded_bytes);
 }
 
 // Resetting the CNG should reset both the VAD and the encoder.
 TEST_F(AudioEncoderCngTest, Reset) {
-  CreateCng();
-  EXPECT_CALL(mock_encoder_, Reset()).Times(1);
+  CreateCng(MakeCngConfig());
+  EXPECT_CALL(*mock_encoder_, Reset()).Times(1);
   EXPECT_CALL(*mock_vad_, Reset()).Times(1);
   cng_->Reset();
 }
@@ -402,11 +421,9 @@ TEST_F(AudioEncoderCngTest, Reset) {
 class AudioEncoderCngDeathTest : public AudioEncoderCngTest {
  protected:
   AudioEncoderCngDeathTest() : AudioEncoderCngTest() {
-    // Don't provide a Vad mock object, since it will leak when the test dies.
-    config_.vad = NULL;
     EXPECT_CALL(*mock_vad_, Die()).Times(1);
     delete mock_vad_;
-    mock_vad_ = NULL;
+    mock_vad_ = nullptr;
   }
 
   // Override AudioEncoderCngTest::TearDown, since that one expects a call to
@@ -414,45 +431,70 @@ class AudioEncoderCngDeathTest : public AudioEncoderCngTest {
   // deleted.
   void TearDown() override {
     cng_.reset();
-    // Don't expect the cng_ object to delete the AudioEncoder object. But it
-    // will be deleted with the test fixture. This is why we explicitly delete
-    // the cng_ object above, and set expectations on mock_encoder_ afterwards.
-    EXPECT_CALL(mock_encoder_, Die()).Times(1);
+  }
+
+  AudioEncoderCng::Config MakeCngConfig() {
+    // Don't provide a Vad mock object, since it would leak when the test dies.
+    auto config = AudioEncoderCngTest::MakeCngConfig();
+    config.vad = nullptr;
+    return config;
+  }
+
+  void TryWrongNumCoefficients(int num) {
+    EXPECT_DEATH(
+        [&] {
+          auto config = MakeCngConfig();
+          config.num_cng_coefficients = num;
+          CreateCng(std::move(config));
+        }(),
+        "Invalid configuration");
   }
 };
 
 TEST_F(AudioEncoderCngDeathTest, WrongFrameSize) {
-  CreateCng();
+  CreateCng(MakeCngConfig());
   num_audio_samples_10ms_ *= 2;  // 20 ms frame.
   EXPECT_DEATH(Encode(), "");
   num_audio_samples_10ms_ = 0;  // Zero samples.
   EXPECT_DEATH(Encode(), "");
 }
 
-TEST_F(AudioEncoderCngDeathTest, WrongNumCoefficients) {
-  config_.num_cng_coefficients = -1;
-  EXPECT_DEATH(CreateCng(), "Invalid configuration");
-  config_.num_cng_coefficients = 0;
-  EXPECT_DEATH(CreateCng(), "Invalid configuration");
-  config_.num_cng_coefficients = 13;
-  EXPECT_DEATH(CreateCng(), "Invalid configuration");
+TEST_F(AudioEncoderCngDeathTest, WrongNumCoefficientsA) {
+  TryWrongNumCoefficients(-1);
+}
+
+TEST_F(AudioEncoderCngDeathTest, WrongNumCoefficientsB) {
+  TryWrongNumCoefficients(0);
+}
+
+TEST_F(AudioEncoderCngDeathTest, WrongNumCoefficientsC) {
+  TryWrongNumCoefficients(13);
 }
 
 TEST_F(AudioEncoderCngDeathTest, NullSpeechEncoder) {
-  config_.speech_encoder = NULL;
-  EXPECT_DEATH(CreateCng(), "Invalid configuration");
+  auto config = MakeCngConfig();
+  config.speech_encoder = nullptr;
+  EXPECT_DEATH(CreateCng(std::move(config)), "");
+}
+
+TEST_F(AudioEncoderCngDeathTest, StereoEncoder) {
+  EXPECT_CALL(*mock_encoder_, NumChannels()).WillRepeatedly(Return(2));
+  EXPECT_DEATH(CreateCng(MakeCngConfig()), "Invalid configuration");
 }
 
-TEST_F(AudioEncoderCngDeathTest, Stereo) {
-  EXPECT_CALL(mock_encoder_, NumChannels()).WillRepeatedly(Return(2));
-  EXPECT_DEATH(CreateCng(), "Invalid configuration");
-  config_.num_channels = 2;
-  EXPECT_DEATH(CreateCng(), "Invalid configuration");
+TEST_F(AudioEncoderCngDeathTest, StereoConfig) {
+  EXPECT_DEATH(
+      [&] {
+        auto config = MakeCngConfig();
+        config.num_channels = 2;
+        CreateCng(std::move(config));
+      }(),
+      "Invalid configuration");
 }
 
 TEST_F(AudioEncoderCngDeathTest, EncoderFrameSizeTooLarge) {
-  CreateCng();
-  EXPECT_CALL(mock_encoder_, Num10MsFramesInNextPacket())
+  CreateCng(MakeCngConfig());
+  EXPECT_CALL(*mock_encoder_, Num10MsFramesInNextPacket())
       .WillRepeatedly(Return(7U));
   for (int i = 0; i < 6; ++i)
     Encode();