Resurrected test_api_audio.cc

webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc

Index: webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc
diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc
index 7d72478df0516df12dffdf25fe4dc701c540c049..80423cd50256b00ac63106d4ecda7d9d22d0fc1d 100644
--- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc
+++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api_audio.cc
@@ -23,7 +23,9 @@
 
 namespace webrtc {
 namespace {
-#define test_rate 64000u
+
+const uint32_t kTestRate = 64000u;
+const uint8_t kTestPayload[] = { 't', 'e', 's', 't' };
 
 class VerifyingAudioReceiver : public NullRtpData {
  public:
@@ -31,31 +33,27 @@ class VerifyingAudioReceiver : public NullRtpData {
       const uint8_t* payloadData,
       size_t payloadSize,
       const webrtc::WebRtcRTPHeader* rtpHeader) override {
-    if (rtpHeader->header.payloadType == 98 ||
-        rtpHeader->header.payloadType == 99) {
-      EXPECT_EQ(4u, payloadSize);
-      char str[5];
-      memcpy(str, payloadData, payloadSize);
-      str[4] = 0;
-      // All our test vectors for payload type 96 and 97 even the stereo is on
-      // a per channel base equal to the 4 chars "test".
-      // Note there is no null termination so we add that to use the
-      // test EXPECT_STRCASEEQ.
-      EXPECT_STRCASEEQ("test", str);
-      return 0;
-    }
-    if (rtpHeader->header.payloadType == 100 ||
-        rtpHeader->header.payloadType == 101 ||
-        rtpHeader->header.payloadType == 102) {
-      if (rtpHeader->type.Audio.channel == 1) {
-        if (payloadData[0] == 0xff) {
-          // All our test vectors for payload type 100, 101 and 102 have the
-          // first channel data being equal to 0xff.
-          return 0;
-        }
+    switch (rtpHeader->header.payloadType) {
+      case 96:

[danilchap, 2016/10/03 13:51:19]

may be define these constants too.

[ossu, 2016/10/03 14:12:03]

Hmm... perhaps I could extract the payload types and their proper parameters
rather than folding it all in here...

+      case 97: {
+        EXPECT_EQ(sizeof(kTestPayload), payloadSize);
+        // All our test vectors for payload type 96 and 97 even the stereo is on
+        // a per channel base equal to the 4 chars "test".
+        const size_t min_size = std::min(sizeof(kTestPayload), payloadSize);
+        EXPECT_EQ(0, memcmp(payloadData, kTestPayload, min_size));
+        break;
+      }
+      // CNG types should be recognized properly.
+      case 13:
+      case 103:
+      case 104:
+      case 105:
+        EXPECT_EQ(kAudioFrameCN, rtpHeader->frameType);
+        EXPECT_TRUE(rtpHeader->type.Audio.isCNG);
+        break;
+      default: {
+        // Do nothing
       }
-      ADD_FAILURE() << "This code path should never happen.";
-      return -1;
     }
     return 0;
   }
@@ -69,7 +67,7 @@ class RTPCallback : public NullRtpFeedback {
                               const size_t channels,
                               const uint32_t rate) override {
     if (payloadType == 96) {
-      EXPECT_EQ(test_rate, rate) <<
+      EXPECT_EQ(kTestRate, rate) <<
           "The rate should be 64K for this payloadType";
     }
     return 0;
@@ -189,7 +187,7 @@ TEST_F(RtpRtcpAudioTest, Basic) {
       voice_codec.channels,
       (voice_codec.rate < 0) ? 0 : voice_codec.rate));
   EXPECT_EQ(0, module2->RegisterSendPayload(voice_codec));
-  voice_codec.rate = test_rate;
+  voice_codec.rate = kTestRate;
   EXPECT_EQ(0, rtp_receiver2_->RegisterReceivePayload(
       voice_codec.plname,
       voice_codec.pltype,
@@ -197,10 +195,9 @@ TEST_F(RtpRtcpAudioTest, Basic) {
       voice_codec.channels,
       (voice_codec.rate < 0) ? 0 : voice_codec.rate));
 
-  const uint8_t test[5] = "test";
-  EXPECT_EQ(true,
-            module1->SendOutgoingData(webrtc::kAudioFrameSpeech, 96, 0, -1,
-                                      test, 4, nullptr, nullptr, nullptr));
+  EXPECT_EQ(true, module1->SendOutgoingData(webrtc::kAudioFrameSpeech, 96, 0,
+                                            -1, kTestPayload, 4, nullptr,
+                                            nullptr, nullptr));
 
   EXPECT_EQ(test_ssrc, rtp_receiver2_->SSRC());
   uint32_t timestamp;
@@ -223,7 +220,7 @@ TEST_F(RtpRtcpAudioTest, DTMF) {
       voice_codec.channels,
       (voice_codec.rate < 0) ? 0 : voice_codec.rate));
   EXPECT_EQ(0, module2->RegisterSendPayload(voice_codec));
-  voice_codec.rate = test_rate;
+  voice_codec.rate = kTestRate;
   EXPECT_EQ(0, rtp_receiver2_->RegisterReceivePayload(
       voice_codec.plname,
       voice_codec.pltype,
@@ -257,14 +254,12 @@ TEST_F(RtpRtcpAudioTest, DTMF) {
   }
   timeStamp += 160;  // Prepare for next packet.
 
-  const uint8_t test[9] = "test";
-
   // Send RTP packets for 16 tones a 160 ms  100ms
   // pause between = 2560ms + 1600ms = 4160ms
   for (; timeStamp <= 250 * 160; timeStamp += 160) {
     EXPECT_TRUE(module1->SendOutgoingData(webrtc::kAudioFrameSpeech, 96,
-                                          timeStamp, -1, test, 4, nullptr,
-                                          nullptr, nullptr));
+                                          timeStamp, -1, kTestPayload, 4,
+                                          nullptr, nullptr, nullptr));
     fake_clock.AdvanceTimeMilliseconds(20);
     module1->Process();
   }
@@ -272,12 +267,97 @@ TEST_F(RtpRtcpAudioTest, DTMF) {
 
   for (; timeStamp <= 740 * 160; timeStamp += 160) {
     EXPECT_TRUE(module1->SendOutgoingData(webrtc::kAudioFrameSpeech, 96,
-                                          timeStamp, -1, test, 4, nullptr,
-                                          nullptr, nullptr));
+                                          timeStamp, -1, kTestPayload, 4,
+                                          nullptr, nullptr, nullptr));
     fake_clock.AdvanceTimeMilliseconds(20);
     module1->Process();
   }
 }
 
+
+TEST_F(RtpRtcpAudioTest, CNG) {

[danilchap, 2016/10/03 13:51:19]

Can you move all unimportant setup into helper method and use several smaller
tests instead of loops?
btw, what CNG is? what this test actually tests?

[ossu, 2016/10/03 14:12:03]

CNG is short for ComfortNoiseGenerator. In this case I want to register several
comfort noise payload types at the same time and make sure they are resolved
correctly, so that one comfort noise payload type doesn't get mapped to the
wrong parameters. I should probably make sure the frequency is correct as well.
Will look into expanding the test a bit.

+  module1->SetSSRC(test_ssrc);
+  module1->SetStartTimestamp(test_timestamp);
+
+  EXPECT_EQ(0, module1->SetSendingStatus(true));
+
+  // Start basic RTP test.
+
+  CodecInst voice_codec;
+  memset(&voice_codec, 0, sizeof(voice_codec));
+  voice_codec.pltype = 96;
+  voice_codec.plfreq = 8000;
+  memcpy(voice_codec.plname, "PCMU", 5);
+
+  EXPECT_EQ(0, module1->RegisterSendPayload(voice_codec));
+  EXPECT_EQ(0, rtp_receiver1_->RegisterReceivePayload(
+      voice_codec.plname,
+      voice_codec.pltype,
+      voice_codec.plfreq,
+      voice_codec.channels,
+      (voice_codec.rate < 0) ? 0 : voice_codec.rate));
+  EXPECT_EQ(0, module2->RegisterSendPayload(voice_codec));
+  voice_codec.rate = kTestRate;
+  EXPECT_EQ(0, rtp_receiver2_->RegisterReceivePayload(
+      voice_codec.plname,
+      voice_codec.pltype,
+      voice_codec.plfreq,
+      voice_codec.channels,
+      (voice_codec.rate < 0) ? 0 : voice_codec.rate));
+
+  struct CngCodecSpec {
+    int payload_type;
+    int clockrate_hz;
+  };
+
+  CngCodecSpec cng_codecs[] = {
+      {13, 8000}, {103, 16000}, {104, 32000}, {105, 48000}};
+
+  for (const auto& c : cng_codecs) {
+    CodecInst cng_codec = {0};
+    cng_codec.pltype = c.payload_type;
+    cng_codec.plfreq = c.clockrate_hz;
+    memcpy(cng_codec.plname, "CN", 3);
+
+    EXPECT_EQ(0, module1->RegisterSendPayload(cng_codec));
+    EXPECT_EQ(0, rtp_receiver1_->RegisterReceivePayload(
+        cng_codec.plname,
+        cng_codec.pltype,
+        cng_codec.plfreq,
+        cng_codec.channels,
+        cng_codec.rate));
+    EXPECT_EQ(0, module2->RegisterSendPayload(cng_codec));
+    EXPECT_EQ(0, rtp_receiver2_->RegisterReceivePayload(
+        cng_codec.plname,
+        cng_codec.pltype,
+        cng_codec.plfreq,
+        cng_codec.channels,
+        cng_codec.rate));
+  }
+
+  uint32_t in_timestamp = 0;
+
+  for (const auto& c : cng_codecs) {
+    uint32_t timestamp;
+    EXPECT_EQ(true, module1->SendOutgoingData(webrtc::kAudioFrameSpeech, 96,
+                                              in_timestamp, -1, kTestPayload, 4,
+                                              nullptr, nullptr, nullptr));
+
+    EXPECT_EQ(test_ssrc, rtp_receiver2_->SSRC());
+    EXPECT_TRUE(rtp_receiver2_->Timestamp(&timestamp));
+    EXPECT_EQ(test_timestamp + in_timestamp, timestamp);
+    in_timestamp += 10;
+
+    EXPECT_EQ(true, module1->SendOutgoingData(

[danilchap, 2016/10/03 13:51:19]

EXPECT_TRUE(...) instead of EXPECT_EQ(true,...) looks cleaner.

[ossu, 2016/10/03 14:12:03]

Acknowledged.

+                        webrtc::kAudioFrameCN, c.payload_type, in_timestamp, -1,
+                        kTestPayload, 1, nullptr, nullptr, nullptr));
+
+    EXPECT_EQ(test_ssrc, rtp_receiver2_->SSRC());
+    EXPECT_TRUE(rtp_receiver2_->Timestamp(&timestamp));
+    EXPECT_EQ(test_timestamp + in_timestamp, timestamp);
+    in_timestamp += 10;
+  }
+}
+
 }  // namespace
 }  // namespace webrtc