From d9e62f58371d1bcafc94edfa1eed836d90627bfe Mon Sep 17 00:00:00 2001 From: danilchap Date: Thu, 14 Jan 2016 14:55:19 -0800 Subject: [PATCH] Fixed sending Rtp packets with non zero csrcs and certain extensions. Added test that fails because of given issue. BUG=webrtc:5413 R=stefan@webrtc.org Review URL: https://codereview.webrtc.org/1586523003 Cr-Commit-Position: refs/heads/master@{#11258} --- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 17 +++---- .../rtp_rtcp/source/rtp_sender_unittest.cc | 44 ++++++++++++++++++- 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index f4933afdd9..19d53c5459 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -1456,8 +1456,9 @@ bool RTPSender::FindHeaderExtensionPosition(RTPExtensionType type, HeaderExtension header_extension(type); - size_t block_pos = - kRtpHeaderLength + rtp_header.numCSRCs + extension_block_pos; + size_t extension_pos = + kRtpHeaderLength + rtp_header.numCSRCs * sizeof(uint32_t); + size_t block_pos = extension_pos + extension_block_pos; if (rtp_packet_length < block_pos + header_extension.length || rtp_header.headerLength < block_pos + header_extension.length) { LOG(LS_WARNING) << "Failed to find extension position for " << type @@ -1466,8 +1467,8 @@ bool RTPSender::FindHeaderExtensionPosition(RTPExtensionType type, } // Verify that header contains extension. - if (!((rtp_packet[kRtpHeaderLength + rtp_header.numCSRCs] == 0xBE) && - (rtp_packet[kRtpHeaderLength + rtp_header.numCSRCs + 1] == 0xDE))) { + if (!(rtp_packet[extension_pos] == 0xBE && + rtp_packet[extension_pos + 1] == 0xDE)) { LOG(LS_WARNING) << "Failed to find extension position for " << type << "as hdr extension not found."; return false; @@ -1494,14 +1495,6 @@ RTPSender::ExtensionStatus RTPSender::VerifyExtension( rtp_packet_length, rtp_header, &block_pos)) return ExtensionStatus::kError; - // Verify that header contains extension. - if (!((rtp_packet[kRtpHeaderLength + rtp_header.numCSRCs] == 0xBE) && - (rtp_packet[kRtpHeaderLength + rtp_header.numCSRCs + 1] == 0xDE))) { - LOG(LS_WARNING) - << "Failed to update absolute send time, hdr extension not found."; - return ExtensionStatus::kError; - } - // Verify first byte in block. const uint8_t first_block_byte = (id << 4) + (extension_length_bytes - 2); if (rtp_packet[block_pos] != first_block_byte) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index c0703244af..4e652d7b95 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -146,16 +146,22 @@ class RtpSenderTest : public ::testing::Test { uint8_t packet_[kMaxPacketLength]; void VerifyRTPHeaderCommon(const RTPHeader& rtp_header) { - VerifyRTPHeaderCommon(rtp_header, kMarkerBit); + VerifyRTPHeaderCommon(rtp_header, kMarkerBit, 0); } void VerifyRTPHeaderCommon(const RTPHeader& rtp_header, bool marker_bit) { + VerifyRTPHeaderCommon(rtp_header, marker_bit, 0); + } + + void VerifyRTPHeaderCommon(const RTPHeader& rtp_header, + bool marker_bit, + uint8_t number_of_csrcs) { 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); } @@ -550,6 +556,40 @@ TEST_F(RtpSenderTestWithoutPacer, BuildRTPPacketWithAudioLevelExtension) { EXPECT_EQ(0u, rtp_header2.extension.audioLevel); } +TEST_F(RtpSenderTestWithoutPacer, + BuildRTPPacketWithCSRCAndAudioLevelExtension) { + EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionAudioLevel, + kAudioLevelExtensionId)); + std::vector csrcs; + csrcs.push_back(0x23456789); + rtp_sender_->SetCsrcs(csrcs); + size_t length = static_cast(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));