Send audio and video codecs to RTPPayloadRegistry

webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc

 /*
  *  Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 17 matching lines @@
 static const size_t kTypicalChannels = 1;
 static const int kTypicalFrequency = 44000;
 static const int kTypicalRate = 32 * 1024;
 
 class RtpPayloadRegistryTest : public ::testing::Test {
  public:
   void SetUp() {
     // Note: the payload registry takes ownership of the strategy.
     mock_payload_strategy_ = new testing::NiceMock<MockRTPPayloadStrategy>();
     rtp_payload_registry_.reset(new RTPPayloadRegistry(mock_payload_strategy_));
+
+    strcpy(typical_audio_codec_.plname, kTypicalPayloadName);
+    typical_audio_codec_.plfreq = kTypicalFrequency;
+    typical_audio_codec_.channels = kTypicalChannels;
+    typical_audio_codec_.rate = kTypicalRate;
   }
 
  protected:
   RtpUtility::Payload* ExpectReturnOfTypicalAudioPayload(uint8_t payload_type,
                                                          uint32_t rate) {
     bool audio = true;
     RtpUtility::Payload returned_payload = {
         "name",
         audio,
         {// Initialize the audio struct in this case.
          {kTypicalFrequency, kTypicalChannels, rate}}};
 
     // Note: we return a new payload since the payload registry takes ownership
     // of the created object.
     RtpUtility::Payload* returned_payload_on_heap =
         new RtpUtility::Payload(returned_payload);
-    EXPECT_CALL(*mock_payload_strategy_,
-                CreatePayloadType(kTypicalPayloadName, payload_type,
-                                  kTypicalFrequency, kTypicalChannels, rate))
+    EXPECT_CALL(
+        *mock_payload_strategy_,
+        CreatePayloadType(testing::StrEq(kTypicalPayloadName), payload_type,

[danilchap, 2016/11/23 15:19:23]

probably nicer to add using ::testing::StrEq above, and use just StrEq here

[magjed_webrtc, 2016/11/23 16:35:53]

Done.

+                          kTypicalFrequency, kTypicalChannels, rate))
         .WillOnce(Return(returned_payload_on_heap));
     return returned_payload_on_heap;
   }
 
   std::unique_ptr<RTPPayloadRegistry> rtp_payload_registry_;
   testing::NiceMock<MockRTPPayloadStrategy>* mock_payload_strategy_;
+  CodecInst typical_audio_codec_;

[danilchap, 2016/11/23 15:19:23]

may be use constant outside of the fixture instead of member to stress it never
change:

const CodecInst kTypicalAudioCodec = {-1, "name", kTypicalFrequency, 0,
kTypicalChannels, kTypicalRate};
or factory:
CodecInst TypicalAudioCodecWithPayloadType(int payload_type) {
  CodecInst codec;
  codec.pltype = payload_type;
  ...
  return codec;
}

[magjed_webrtc, 2016/11/23 16:35:53]

Thanks, much better. I tried to do this but used kTypicalPayloadName to
initialize instead of duplicating "name" and then I got compile errors because
the plname in CodecInst is a char array. But with your suggestion it works :)

 };
 
 TEST_F(RtpPayloadRegistryTest, RegistersAndRemembersPayloadsUntilDeregistered) {
   uint8_t payload_type = 97;
   RtpUtility::Payload* returned_payload_on_heap =
       ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate);
 
   bool new_payload_created = false;
+  CodecInst audio_codec = typical_audio_codec_;
+  audio_codec.pltype = payload_type;
   EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(
-                   kTypicalPayloadName, payload_type, kTypicalFrequency,
-                   kTypicalChannels, kTypicalRate, &new_payload_created));
+                   audio_codec, &new_payload_created));
 
   EXPECT_TRUE(new_payload_created) << "A new payload WAS created.";
 
   const RtpUtility::Payload* retrieved_payload =
       rtp_payload_registry_->PayloadTypeToPayload(payload_type);
   EXPECT_TRUE(retrieved_payload);
 
   // We should get back the exact pointer to the payload returned by the
   // payload strategy.
   EXPECT_EQ(returned_payload_on_heap, retrieved_payload);
 
   // Now forget about it and verify it's gone.
   EXPECT_EQ(0, rtp_payload_registry_->DeRegisterReceivePayload(payload_type));
   EXPECT_FALSE(rtp_payload_registry_->PayloadTypeToPayload(payload_type));
 }
 
 TEST_F(RtpPayloadRegistryTest, AudioRedWorkProperly) {
   const uint8_t kRedPayloadType = 127;
   const int kRedSampleRate = 8000;
   const size_t kRedChannels = 1;
   const int kRedBitRate = 0;
 
   // This creates an audio RTP payload strategy.
   rtp_payload_registry_.reset(
       new RTPPayloadRegistry(RTPPayloadStrategy::CreateStrategy(true)));
 
   bool new_payload_created = false;
+  CodecInst red_audio_codec;
+  strcpy(red_audio_codec.plname, "red");
+  red_audio_codec.pltype = kRedPayloadType;
+  red_audio_codec.plfreq = kRedSampleRate;
+  red_audio_codec.channels = kRedChannels;
+  red_audio_codec.rate = kRedBitRate;
   EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(
-                   "red", kRedPayloadType, kRedSampleRate, kRedChannels,
-                   kRedBitRate, &new_payload_created));
+                   red_audio_codec, &new_payload_created));
   EXPECT_TRUE(new_payload_created);
 
   EXPECT_EQ(kRedPayloadType, rtp_payload_registry_->red_payload_type());
 
   const RtpUtility::Payload* retrieved_payload =
       rtp_payload_registry_->PayloadTypeToPayload(kRedPayloadType);
   EXPECT_TRUE(retrieved_payload);
   EXPECT_TRUE(retrieved_payload->audio);
   EXPECT_STRCASEEQ("red", retrieved_payload->name);
 
   // Sample rate is correctly registered.
   EXPECT_EQ(kRedSampleRate,
             rtp_payload_registry_->GetPayloadTypeFrequency(kRedPayloadType));
 }
 
 TEST_F(RtpPayloadRegistryTest,
        DoesNotAcceptSamePayloadTypeTwiceExceptIfPayloadIsCompatible) {
   uint8_t payload_type = 97;
 
   bool ignored = false;
   RtpUtility::Payload* first_payload_on_heap =
       ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate);
-  EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(
-                   kTypicalPayloadName, payload_type, kTypicalFrequency,
-                   kTypicalChannels, kTypicalRate, &ignored));
+  CodecInst audio_codec = typical_audio_codec_;
+  audio_codec.pltype = payload_type;
+  EXPECT_EQ(
+      0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored));
 
-  EXPECT_EQ(-1, rtp_payload_registry_->RegisterReceivePayload(
-                    kTypicalPayloadName, payload_type, kTypicalFrequency,
-                    kTypicalChannels, kTypicalRate, &ignored))
+  EXPECT_EQ(
+      -1, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored))
       << "Adding same codec twice = bad.";
 
   RtpUtility::Payload* second_payload_on_heap =
       ExpectReturnOfTypicalAudioPayload(payload_type - 1, kTypicalRate);
-  EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(
-                   kTypicalPayloadName, payload_type - 1, kTypicalFrequency,
-                   kTypicalChannels, kTypicalRate, &ignored))
+  CodecInst audio_codec_2 = typical_audio_codec_;
+  audio_codec_2.pltype = payload_type - 1;
+  EXPECT_EQ(
+      0, rtp_payload_registry_->RegisterReceivePayload(audio_codec_2, &ignored))
       << "With a different payload type is fine though.";
 
   // Ensure both payloads are preserved.
   const RtpUtility::Payload* retrieved_payload =
       rtp_payload_registry_->PayloadTypeToPayload(payload_type);
   EXPECT_TRUE(retrieved_payload);
   EXPECT_EQ(first_payload_on_heap, retrieved_payload);
   retrieved_payload =
       rtp_payload_registry_->PayloadTypeToPayload(payload_type - 1);
   EXPECT_TRUE(retrieved_payload);
   EXPECT_EQ(second_payload_on_heap, retrieved_payload);
 
   // Ok, update the rate for one of the codecs. If either the incoming rate or
   // the stored rate is zero it's not really an error to register the same
   // codec twice, and in that case roughly the following happens.
   ON_CALL(*mock_payload_strategy_, PayloadIsCompatible(_, _, _, _))
       .WillByDefault(Return(true));
   EXPECT_CALL(*mock_payload_strategy_,
               UpdatePayloadRate(first_payload_on_heap, kTypicalRate));
-  EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(
-                   kTypicalPayloadName, payload_type, kTypicalFrequency,
-                   kTypicalChannels, kTypicalRate, &ignored));
+  EXPECT_EQ(
+      0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored));
 }
 
 TEST_F(RtpPayloadRegistryTest,
        RemovesCompatibleCodecsOnRegistryIfCodecsMustBeUnique) {
   ON_CALL(*mock_payload_strategy_, PayloadIsCompatible(_, _, _, _))
       .WillByDefault(Return(true));
   ON_CALL(*mock_payload_strategy_, CodecsMustBeUnique())
       .WillByDefault(Return(true));
 
   uint8_t payload_type = 97;
 
   bool ignored = false;
   ExpectReturnOfTypicalAudioPayload(payload_type, kTypicalRate);
-  EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(
-                   kTypicalPayloadName, payload_type, kTypicalFrequency,
-                   kTypicalChannels, kTypicalRate, &ignored));
+  CodecInst audio_codec = typical_audio_codec_;
+  audio_codec.pltype = payload_type;
+  EXPECT_EQ(
+      0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored));
   ExpectReturnOfTypicalAudioPayload(payload_type - 1, kTypicalRate);
-  EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(
-                   kTypicalPayloadName, payload_type - 1, kTypicalFrequency,
-                   kTypicalChannels, kTypicalRate, &ignored));
+  CodecInst audio_codec_2 = typical_audio_codec_;
+  audio_codec_2.pltype = payload_type - 1;
+  EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(audio_codec_2,
+                                                             &ignored));
 
   EXPECT_FALSE(rtp_payload_registry_->PayloadTypeToPayload(payload_type))
       << "The first payload should be "
          "deregistered because the only thing that differs is payload type.";
   EXPECT_TRUE(rtp_payload_registry_->PayloadTypeToPayload(payload_type - 1))
       << "The second payload should still be registered though.";
 
   // Now ensure non-compatible codecs aren't removed.
   ON_CALL(*mock_payload_strategy_, PayloadIsCompatible(_, _, _, _))
       .WillByDefault(Return(false));
   ExpectReturnOfTypicalAudioPayload(payload_type + 1, kTypicalRate);
-  EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(
-                   kTypicalPayloadName, payload_type + 1, kTypicalFrequency,
-                   kTypicalChannels, kTypicalRate, &ignored));
+  CodecInst audio_codec_3 = typical_audio_codec_;
+  audio_codec_3.pltype = payload_type + 1;
+  EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(audio_codec_3,
+                                                             &ignored));
 
   EXPECT_TRUE(rtp_payload_registry_->PayloadTypeToPayload(payload_type - 1))
       << "Not compatible; both payloads should be kept.";
   EXPECT_TRUE(rtp_payload_registry_->PayloadTypeToPayload(payload_type + 1))
       << "Not compatible; both payloads should be kept.";
 }
 
 TEST_F(RtpPayloadRegistryTest,
        LastReceivedCodecTypesAreResetWhenRegisteringNewPayloadTypes) {
   rtp_payload_registry_->set_last_received_payload_type(17);
   EXPECT_EQ(17, rtp_payload_registry_->last_received_payload_type());
 
   bool media_type_unchanged = rtp_payload_registry_->ReportMediaPayloadType(18);
   EXPECT_FALSE(media_type_unchanged);
   media_type_unchanged = rtp_payload_registry_->ReportMediaPayloadType(18);
   EXPECT_TRUE(media_type_unchanged);
 
   bool ignored;
   ExpectReturnOfTypicalAudioPayload(34, kTypicalRate);
-  EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(
-                   kTypicalPayloadName, 34, kTypicalFrequency, kTypicalChannels,
-                   kTypicalRate, &ignored));
+  CodecInst audio_codec = typical_audio_codec_;
+  audio_codec.pltype = 34;
+  EXPECT_EQ(
+      0, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored));
 
   EXPECT_EQ(-1, rtp_payload_registry_->last_received_payload_type());
   media_type_unchanged = rtp_payload_registry_->ReportMediaPayloadType(18);
   EXPECT_FALSE(media_type_unchanged);
 }
 
 class ParameterizedRtpPayloadRegistryTest
     : public RtpPayloadRegistryTest,
       public ::testing::WithParamInterface<int> {};
 
 TEST_P(ParameterizedRtpPayloadRegistryTest,
        FailsToRegisterKnownPayloadsWeAreNotInterestedIn) {
   int payload_type = GetParam();
 
   bool ignored;
-  EXPECT_EQ(-1, rtp_payload_registry_->RegisterReceivePayload(
-                    "whatever", static_cast<uint8_t>(payload_type), 19, 1, 17,
-                    &ignored));
+  CodecInst audio_codec;
+  strcpy(audio_codec.plname, "whatever");
+  audio_codec.pltype = static_cast<uint8_t>(payload_type);
+  audio_codec.plfreq = 19;
+  audio_codec.channels = 1;
+  audio_codec.rate = 17;
+  EXPECT_EQ(
+      -1, rtp_payload_registry_->RegisterReceivePayload(audio_codec, &ignored));
 }
 
 INSTANTIATE_TEST_CASE_P(TestKnownBadPayloadTypes,
                         ParameterizedRtpPayloadRegistryTest,
                         testing::Values(64, 72, 73, 74, 75, 76, 77, 78, 79));
 
 class RtpPayloadRegistryGenericTest
     : public RtpPayloadRegistryTest,
       public ::testing::WithParamInterface<int> {};
 
 TEST_P(RtpPayloadRegistryGenericTest, RegisterGenericReceivePayloadType) {
   int payload_type = GetParam();
 
   bool ignored;
 
-  EXPECT_EQ(0, rtp_payload_registry_->RegisterReceivePayload(
-                   "generic-codec", static_cast<int8_t>(payload_type), 19, 1,
-                   17, &ignored));  // dummy values, except for payload_type
+  CodecInst audio_codec;
+  strcpy(audio_codec.plname, "generic-codec");
+  audio_codec.pltype = static_cast<uint8_t>(payload_type);
+  audio_codec.plfreq = 19;
+  audio_codec.channels = 1;
+  audio_codec.rate = 17;
+  EXPECT_EQ(
+      0, rtp_payload_registry_->RegisterReceivePayload(
+             audio_codec, &ignored));  // dummy values, except for payload_type

[danilchap, 2016/11/23 15:19:23]

May be fix comment as well: add dot in the end, start with uppercase.

[magjed_webrtc, 2016/11/23 16:35:53]

Done.

 }
 
 // Generates an RTX packet for the given length and original sequence number.
 // The RTX sequence number and ssrc will use the default value of 9999. The
 // caller takes ownership of the returned buffer.
 const uint8_t* GenerateRtxPacket(size_t header_length,
                                  size_t payload_length,
                                  uint16_t original_sequence_number) {
   uint8_t* packet =
       new uint8_t[kRtxHeaderSize + header_length + payload_length]();
@@ skipping 83 matching lines @@
   // Succeeds when the mapping is used, but fails for the implicit fallback.
   rtp_payload_registry_->SetRtxPayloadType(105, 95);
   TestRtxPacket(rtp_payload_registry_.get(), 105, 95, true);
   TestRtxPacket(rtp_payload_registry_.get(), 106, 0, false);
 }
 
 INSTANTIATE_TEST_CASE_P(TestDynamicRange,
                         RtpPayloadRegistryGenericTest,
                         testing::Range(96, 127 + 1));
 
 }  // namespace webrtc

[hta-webrtc, 2016/11/23 16:08:24]

There's a new function added that takes a VideoCodec. I don't see tests for that
function here.

(Today it's just a very small shim. Still, test that it continues to behave.)

[magjed_webrtc, 2016/11/23 16:35:53]

Yeah, you are right. I added a default test for video similar to the existing
RegistersAndRemembersPayloadsUntilDeregistered. I will add more tests once I add
the video H264 profile specific logic into RtpPayloadRegistry.