Fixed sending Rtp packets with non zero csrcs and certain extensions.

webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc

 /*
  *  Copyright (c) 2012 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 105 matching lines @@
                     bool retransmission));
 };
 
 class RtpSenderTest : public ::testing::Test {
  protected:
   RtpSenderTest()
       : fake_clock_(kStartTime),
         mock_paced_sender_(),
         rtp_sender_(),
         payload_(kPayload),
-        transport_(),
-        kMarkerBit(true) {
+        transport_() {
     EXPECT_CALL(mock_paced_sender_, InsertPacket(_, _, _, _, _, _))
         .WillRepeatedly(testing::Return());
   }
 
   void SetUp() override { SetUpRtpSender(true); }
 
   void SetUpRtpSender(bool pacer) {
     rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, nullptr,
                                     pacer ? &mock_paced_sender_ : nullptr,
                                     nullptr, nullptr, nullptr, nullptr,
                                     nullptr));
     rtp_sender_->SetSequenceNumber(kSeqNum);
   }
 
   SimulatedClock fake_clock_;
   MockRtpPacketSender mock_paced_sender_;
   rtc::scoped_ptr<RTPSender> rtp_sender_;
   int payload_;
   LoopbackTransportTest transport_;
-  const bool kMarkerBit;
+  static const bool kMarkerBit = true;
   uint8_t packet_[kMaxPacketLength];
 
-  void VerifyRTPHeaderCommon(const RTPHeader& rtp_header) {
-    VerifyRTPHeaderCommon(rtp_header, kMarkerBit);
-  }
-
-  void VerifyRTPHeaderCommon(const RTPHeader& rtp_header, bool marker_bit) {
+  void VerifyRTPHeaderCommon(const RTPHeader& rtp_header,
+                             bool marker_bit = kMarkerBit,
+                             uint8_t number_of_csrcs = 0) {

[stefan-webrtc, 2016/01/14 14:59:17]

No default values please, instead pass in all arguments, or create two
functions.

[danilchap, 2016/01/14 15:45:04]

Done.

     EXPECT_EQ(marker_bit, rtp_header.markerBit);
     EXPECT_EQ(payload_, rtp_header.payloadType);
     EXPECT_EQ(kSeqNum, rtp_header.sequenceNumber);
     EXPECT_EQ(kTimestamp, rtp_header.timestamp);
     EXPECT_EQ(rtp_sender_->SSRC(), rtp_header.ssrc);
-    EXPECT_EQ(0, rtp_header.numCSRCs);
+    EXPECT_EQ(number_of_csrcs, rtp_header.numCSRCs);
     EXPECT_EQ(0U, rtp_header.paddingLength);
   }
 
   void SendPacket(int64_t capture_time_ms, int payload_length) {
     uint32_t timestamp = capture_time_ms * 90;
     int32_t rtp_length = rtp_sender_->BuildRTPheader(
         packet_, kPayload, kMarkerBit, timestamp, capture_time_ms);
     ASSERT_GE(rtp_length, 0);
 
     // Packet should be stored in a send bucket.
@@ skipping 374 matching lines @@
   const bool valid_rtp_header2 = rtp_parser.Parse(&rtp_header2, nullptr);
 
   ASSERT_TRUE(valid_rtp_header2);
   VerifyRTPHeaderCommon(rtp_header2);
   EXPECT_EQ(length, rtp_header2.headerLength);
   EXPECT_FALSE(rtp_header2.extension.hasAudioLevel);
   EXPECT_FALSE(rtp_header2.extension.voiceActivity);
   EXPECT_EQ(0u, rtp_header2.extension.audioLevel);
 }
 
+TEST_F(RtpSenderTestWithoutPacer,
+       BuildRTPPacketWithCSRCAndAudioLevelExtension) {
+  EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionAudioLevel,
+                                                       kAudioLevelExtensionId));
+  std::vector<uint32_t> csrcs;
+  csrcs.push_back(0x23456789);
+  rtp_sender_->SetCsrcs(csrcs);

[stefan-webrtc, 2016/01/14 14:59:17]

Shouldn't you use at least 2 CSRCs to trigger this bug?

[danilchap, 2016/01/14 15:45:04]

No, one is enough. I checked test was red before creating the fix.

+  size_t length = static_cast<size_t>(rtp_sender_->BuildRTPheader(
+      packet_, kPayload, kMarkerBit, kTimestamp, 0));
+
+  // Verify
+  webrtc::RtpUtility::RtpHeaderParser rtp_parser(packet_, length);
+  webrtc::RTPHeader rtp_header;
+
+  // Updating audio level is done in RTPSenderAudio, so simulate it here.
+  rtp_parser.Parse(&rtp_header);
+  EXPECT_TRUE(rtp_sender_->UpdateAudioLevel(packet_, length, rtp_header, true,
+                                            kAudioLevel));
+
+  RtpHeaderExtensionMap map;
+  map.Register(kRtpExtensionAudioLevel, kAudioLevelExtensionId);
+  const bool valid_rtp_header = rtp_parser.Parse(&rtp_header, &map);
+
+  ASSERT_TRUE(valid_rtp_header);
+  ASSERT_FALSE(rtp_parser.RTCP());
+  VerifyRTPHeaderCommon(rtp_header, kMarkerBit, csrcs.size());
+  EXPECT_EQ(length, rtp_header.headerLength);
+  EXPECT_TRUE(rtp_header.extension.hasAudioLevel);
+  EXPECT_TRUE(rtp_header.extension.voiceActivity);
+  EXPECT_EQ(kAudioLevel, rtp_header.extension.audioLevel);
+  EXPECT_EQ(1u, rtp_header.numCSRCs);
+  EXPECT_EQ(csrcs[0], rtp_header.arrOfCSRCs[0]);
+}
+
 TEST_F(RtpSenderTestWithoutPacer, BuildRTPPacketWithHeaderExtensions) {
   EXPECT_EQ(0, rtp_sender_->SetTransmissionTimeOffset(kTimeOffset));
   EXPECT_EQ(0, rtp_sender_->SetAbsoluteSendTime(kAbsoluteSendTime));
   EXPECT_EQ(0,
             rtp_sender_->SetTransportSequenceNumber(kTransportSequenceNumber));
   EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
                    kRtpExtensionTransmissionTimeOffset,
                    kTransmissionTimeOffsetExtensionId));
   EXPECT_EQ(
       0, rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionAbsoluteSendTime,
@@ skipping 853 matching lines @@
       reinterpret_cast<uint8_t*>(transport_.sent_packets_[0]->data()),
       transport_.sent_packets_[0]->size(), true, &map, kSeqNum, hdr.rotation);
 
   // Verify that this packet does have CVO byte.
   VerifyCVOPacket(
       reinterpret_cast<uint8_t*>(transport_.sent_packets_[1]->data()),
       transport_.sent_packets_[1]->size(), true, &map, kSeqNum + 1,
       hdr.rotation);
 }
 }  // namespace webrtc