NetEq: New test for muted state during CNG

webrtc/modules/audio_coding/neteq/neteq_unittest.cc

Index: webrtc/modules/audio_coding/neteq/neteq_unittest.cc
diff --git a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc
index 14a48af6709b43083bd11aeb9a2ec333827ca6a9..12bc2cb09964c25801bfdbaafccad22ca6e3ee2d 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_unittest.cc
@@ -1315,6 +1315,17 @@ class NetEqDecodingTestWithMutedState : public NetEqDecodingTest {
     EXPECT_EQ(0, neteq_->InsertPacket(rtp_info, payload, 0));
   }
 
+  void InsertCngPacket(uint32_t rtp_timestamp) {
+    uint8_t payload[kPayloadBytes] = {0};
+    WebRtcRTPHeader rtp_info;
+    size_t payload_len;
+    PopulateCng(0, rtp_timestamp, &rtp_info, payload, &payload_len);
+    EXPECT_EQ(
+        NetEq::kOK,
+        neteq_->InsertPacket(
+            rtp_info, rtc::ArrayView<const uint8_t>(payload, payload_len), 0));
+  }
+
   bool GetAudioReturnMuted() {
     bool muted;
     EXPECT_EQ(0, neteq_->GetAudio(&out_frame_, &muted));
@@ -1429,6 +1440,26 @@ TEST_F(NetEqDecodingTestWithMutedState, MutedStateOldPacket) {
   EXPECT_EQ(AudioFrame::kNormalSpeech, out_frame_.speech_type_);
 }
 
+// Verifies that NetEq doesn't enter muted state when CNG mode is active and the
+// packet stream is suspended for a long time.
+TEST_F(NetEqDecodingTestWithMutedState, DoNotMuteExtendedCngWithoutPackets) {
+  // Insert one speech packet.
+  InsertCngPacket(0);
+
+  // Pull 10 seconds of audio (10 ms per lap).
+  for (int i = 0; i < 1000; ++i) {

[aleloi, 2016/09/19 11:55:07]

Does it take much time to do 1000 iterations? I.e., does NetEq wait for 10ms
before GetAudio returns or does it happen faster? 10 seconds seems a very long
time to wait for a test.

[hlundin-webrtc, 2016/09/20 08:09:02]

The test completes in 22 ms on my machine. It runs at "full speed", and does not
have to wait 10 ms between each lap. I'll modify the comment to clarify.

+    bool muted;
+    EXPECT_EQ(0, neteq_->GetAudio(&out_frame_, &muted));
+    EXPECT_EQ(AudioFrame::kCNG, out_frame_.speech_type_);
+    ASSERT_FALSE(muted);

[aleloi, 2016/09/19 11:55:07]

I recently read go/unit-test-practices and I think one of the tips for more
resilient tests applies here (link below).

It seems to me that this test checks for multiple behaviors at the same time. It
checks that 
 (A) NetEq outputs un-muted frames with speech type CNG after being fed a CNG
packet and 
 (B) the speech type goes back to un-muted normal speech.

I think that the test could check for these two behaviors separately and e.g.
just have GetAudio with no checks in the loop.

Then there would be less repeated assertions and there is a better separation
between set-up and checks.

Link:
https://engdoc.corp.google.com/eng/doc/devguide/coding/testing/unit-testing-b...

[hlundin-webrtc, 2016/09/20 08:09:02]

How about this?

+  }
+
+  // Insert new data. Timestamp is corrected for the time elapsed since the last
+  // packet. Verify that normal operation resumes.
+  InsertPacket(kSamples * counter_);
+  GetAudioUntilNormal();
+}
+
 class NetEqDecodingTestTwoInstances : public NetEqDecodingTest {
  public:
   NetEqDecodingTestTwoInstances() : NetEqDecodingTest() {}