From a46fce2194df8483ed92d62170cfd0a1301034d1 Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Tue, 16 Jun 2020 16:35:28 +0200 Subject: [PATCH] Update rtp_utils to support two-byte RTP header extensions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:11691 Change-Id: Icdd3eeed0fe0c6e1dee387cc03740628ee24e5c3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177343 Reviewed-by: Erik Språng Reviewed-by: Ilya Nikolaevskiy Reviewed-by: Danil Chapovalov Commit-Queue: Johannes Kron Cr-Commit-Position: refs/heads/master@{#31537} --- media/base/rtp_utils.cc | 57 ++++++++++++---- media/base/rtp_utils_unittest.cc | 107 ++++++++++++++++++++----------- 2 files changed, 114 insertions(+), 50 deletions(-) diff --git a/media/base/rtp_utils.cc b/media/base/rtp_utils.cc index 0b45e69410..4a2b3267cc 100644 --- a/media/base/rtp_utils.cc +++ b/media/base/rtp_utils.cc @@ -34,6 +34,7 @@ static const size_t kRtcpPayloadTypeOffset = 1; static const size_t kRtpExtensionHeaderLen = 4; static const size_t kAbsSendTimeExtensionLen = 3; static const size_t kOneByteExtensionHeaderLen = 1; +static const size_t kTwoByteExtensionHeaderLen = 2; namespace { @@ -424,10 +425,13 @@ bool UpdateRtpAbsSendTimeExtension(uint8_t* rtp, rtp += kRtpExtensionHeaderLen; // Moving past extension header. + constexpr uint16_t kOneByteExtensionProfileId = 0xBEDE; + constexpr uint16_t kTwoByteExtensionProfileId = 0x1000; + bool found = false; - // WebRTC is using one byte header extension. - // TODO(mallinath) - Handle two byte header extension. - if (profile_id == 0xBEDE) { // OneByte extension header + if (profile_id == kOneByteExtensionProfileId || + profile_id == kTwoByteExtensionProfileId) { + // OneByte extension header // 0 // 0 1 2 3 4 5 6 7 // +-+-+-+-+-+-+-+-+ @@ -445,24 +449,53 @@ bool UpdateRtpAbsSendTimeExtension(uint8_t* rtp, // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // | data | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + + // TwoByte extension header + // 0 + // 0 1 2 3 4 5 6 7 + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | ID | length | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + + // 0 1 2 3 + // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | 0x10 | 0x00 | length=3 | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | ID | L=1 | data | ID | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | L=2 | data | 0 (pad) | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // | ID | L=2 | data | + // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + + size_t extension_header_length = profile_id == kOneByteExtensionProfileId + ? kOneByteExtensionHeaderLen + : kTwoByteExtensionHeaderLen; + const uint8_t* extension_start = rtp; const uint8_t* extension_end = extension_start + extension_length; - while (rtp < extension_end) { - const int id = (*rtp & 0xF0) >> 4; - const size_t length = (*rtp & 0x0F) + 1; - if (rtp + kOneByteExtensionHeaderLen + length > extension_end) { + // rtp + 1 since the minimum size per header extension is two bytes for both + // one- and two-byte header extensions. + while (rtp + 1 < extension_end) { + // See RFC8285 Section 4.2-4.3 for more information about one- and + // two-byte header extensions. + const int id = + profile_id == kOneByteExtensionProfileId ? (*rtp & 0xF0) >> 4 : *rtp; + const size_t length = profile_id == kOneByteExtensionProfileId + ? (*rtp & 0x0F) + 1 + : *(rtp + 1); + if (rtp + extension_header_length + length > extension_end) { return false; } - // The 4-bit length is the number minus one of data bytes of this header - // extension element following the one-byte header. if (id == extension_id) { - UpdateAbsSendTimeExtensionValue(rtp + kOneByteExtensionHeaderLen, - length, time_us); + UpdateAbsSendTimeExtensionValue(rtp + extension_header_length, length, + time_us); found = true; break; } - rtp += kOneByteExtensionHeaderLen + length; + rtp += extension_header_length + length; // Counting padding bytes. while ((rtp < extension_end) && (*rtp == 0)) { ++rtp; diff --git a/media/base/rtp_utils_unittest.cc b/media/base/rtp_utils_unittest.cc index 051508cd01..a5e8a810f4 100644 --- a/media/base/rtp_utils_unittest.cc +++ b/media/base/rtp_utils_unittest.cc @@ -71,15 +71,25 @@ static uint8_t kRtpMsgWith2ByteExtnHeader[] = { // clang-format on }; -// RTP packet with single byte extension header of length 4 bytes. -// Extension id = 3 and length = 3 -static uint8_t kRtpMsgWithAbsSendTimeExtension[] = { +// RTP packet with two one-byte header extensions. The last 4 bytes consist of +// abs-send-time with extension id = 3 and length = 3. +static uint8_t kRtpMsgWithOneByteAbsSendTimeExtension[] = { 0x90, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xBE, 0xDE, 0x00, 0x02, 0x22, 0x00, 0x02, 0x1c, 0x32, 0xaa, 0xbb, 0xcc, }; -// Index of AbsSendTimeExtn data in message |kRtpMsgWithAbsSendTimeExtension|. -static const int kAstIndexInRtpMsg = 21; +// RTP packet with two two-byte header extensions. The last 5 bytes consist of +// abs-send-time with extension id = 3 and length = 3. +static uint8_t kRtpMsgWithTwoByteAbsSendTimeExtension[] = { + 0x90, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x10, 0x00, 0x00, 0x02, 0x02, 0x01, 0x02, 0x03, 0x03, 0xaa, 0xbb, 0xcc, +}; + +// Index of AbsSendTimeExtn data in message +// |kRtpMsgWithOneByteAbsSendTimeExtension|. +static const int kAstIndexInOneByteRtpMsg = 21; +// and in message |kRtpMsgWithTwoByteAbsSendTimeExtension|. +static const int kAstIndexInTwoByteRtpMsg = 21; static const rtc::ArrayView kPcmuFrameArrayView = rtc::MakeArrayView(reinterpret_cast(kPcmuFrame), @@ -213,19 +223,17 @@ TEST(RtpUtilsTest, Valid2ByteExtnHdrRtpMessage) { } // Valid RTP packet which has 1 byte header AbsSendTime extension in it. -TEST(RtpUtilsTest, ValidRtpPacketWithAbsSendTimeExtension) { - EXPECT_TRUE(ValidateRtpHeader(kRtpMsgWithAbsSendTimeExtension, - sizeof(kRtpMsgWithAbsSendTimeExtension), +TEST(RtpUtilsTest, ValidRtpPacketWithOneByteAbsSendTimeExtension) { + EXPECT_TRUE(ValidateRtpHeader(kRtpMsgWithOneByteAbsSendTimeExtension, + sizeof(kRtpMsgWithOneByteAbsSendTimeExtension), nullptr)); } -// Verify handling of a 2 byte extension header RTP messsage. Currently these -// messages are not supported. -TEST(RtpUtilsTest, UpdateAbsSendTimeExtensionIn2ByteHeaderExtn) { - std::vector data( - kRtpMsgWith2ByteExtnHeader, - kRtpMsgWith2ByteExtnHeader + sizeof(kRtpMsgWith2ByteExtnHeader)); - EXPECT_FALSE(UpdateRtpAbsSendTimeExtension(&data[0], data.size(), 3, 0)); +// Valid RTP packet which has 2 byte header AbsSendTime extension in it. +TEST(RtpUtilsTest, ValidRtpPacketWithTwoByteAbsSendTimeExtension) { + EXPECT_TRUE(ValidateRtpHeader(kRtpMsgWithTwoByteAbsSendTimeExtension, + sizeof(kRtpMsgWithTwoByteAbsSendTimeExtension), + nullptr)); } // Verify finding an extension ID in the TURN send indication message. @@ -276,19 +284,21 @@ TEST(RtpUtilsTest, UpdateAbsSendTimeExtensionInTurnSendIndication) { // without HMAC value in the packet. TEST(RtpUtilsTest, ApplyPacketOptionsWithDefaultValues) { rtc::PacketTimeUpdateParams packet_time_params; - std::vector rtp_packet(kRtpMsgWithAbsSendTimeExtension, - kRtpMsgWithAbsSendTimeExtension + - sizeof(kRtpMsgWithAbsSendTimeExtension)); + std::vector rtp_packet( + kRtpMsgWithOneByteAbsSendTimeExtension, + kRtpMsgWithOneByteAbsSendTimeExtension + + sizeof(kRtpMsgWithOneByteAbsSendTimeExtension)); rtp_packet.insert(rtp_packet.end(), kFakeTag, kFakeTag + sizeof(kFakeTag)); EXPECT_TRUE(ApplyPacketOptions(&rtp_packet[0], rtp_packet.size(), packet_time_params, 0)); // Making sure HMAC wasn't updated.. - EXPECT_EQ(0, memcmp(&rtp_packet[sizeof(kRtpMsgWithAbsSendTimeExtension)], - kFakeTag, 4)); + EXPECT_EQ(0, + memcmp(&rtp_packet[sizeof(kRtpMsgWithOneByteAbsSendTimeExtension)], + kFakeTag, 4)); // Verify AbsouluteSendTime extension field wasn't modified. - EXPECT_EQ(0, memcmp(&rtp_packet[kAstIndexInRtpMsg], kTestAstValue, + EXPECT_EQ(0, memcmp(&rtp_packet[kAstIndexInOneByteRtpMsg], kTestAstValue, sizeof(kTestAstValue))); } @@ -299,34 +309,53 @@ TEST(RtpUtilsTest, ApplyPacketOptionsWithAuthParams) { kTestKey + sizeof(kTestKey)); packet_time_params.srtp_auth_tag_len = 4; - std::vector rtp_packet(kRtpMsgWithAbsSendTimeExtension, - kRtpMsgWithAbsSendTimeExtension + - sizeof(kRtpMsgWithAbsSendTimeExtension)); + std::vector rtp_packet( + kRtpMsgWithOneByteAbsSendTimeExtension, + kRtpMsgWithOneByteAbsSendTimeExtension + + sizeof(kRtpMsgWithOneByteAbsSendTimeExtension)); rtp_packet.insert(rtp_packet.end(), kFakeTag, kFakeTag + sizeof(kFakeTag)); EXPECT_TRUE(ApplyPacketOptions(&rtp_packet[0], rtp_packet.size(), packet_time_params, 0)); uint8_t kExpectedTag[] = {0xc1, 0x7a, 0x8c, 0xa0}; - EXPECT_EQ(0, memcmp(&rtp_packet[sizeof(kRtpMsgWithAbsSendTimeExtension)], - kExpectedTag, sizeof(kExpectedTag))); + EXPECT_EQ(0, + memcmp(&rtp_packet[sizeof(kRtpMsgWithOneByteAbsSendTimeExtension)], + kExpectedTag, sizeof(kExpectedTag))); // Verify AbsouluteSendTime extension field is not modified. - EXPECT_EQ(0, memcmp(&rtp_packet[kAstIndexInRtpMsg], kTestAstValue, + EXPECT_EQ(0, memcmp(&rtp_packet[kAstIndexInOneByteRtpMsg], kTestAstValue, sizeof(kTestAstValue))); } // Verify finding an extension ID in a raw rtp message. -TEST(RtpUtilsTest, UpdateAbsSendTimeExtensionInRtpPacket) { - std::vector rtp_packet(kRtpMsgWithAbsSendTimeExtension, - kRtpMsgWithAbsSendTimeExtension + - sizeof(kRtpMsgWithAbsSendTimeExtension)); +TEST(RtpUtilsTest, UpdateOneByteAbsSendTimeExtensionInRtpPacket) { + std::vector rtp_packet( + kRtpMsgWithOneByteAbsSendTimeExtension, + kRtpMsgWithOneByteAbsSendTimeExtension + + sizeof(kRtpMsgWithOneByteAbsSendTimeExtension)); EXPECT_TRUE(UpdateRtpAbsSendTimeExtension(&rtp_packet[0], rtp_packet.size(), 3, 51183266)); // Verify that the timestamp was updated. const uint8_t kExpectedTimestamp[3] = {0xcc, 0xbb, 0xaa}; - EXPECT_EQ(0, memcmp(&rtp_packet[kAstIndexInRtpMsg], kExpectedTimestamp, + EXPECT_EQ(0, memcmp(&rtp_packet[kAstIndexInOneByteRtpMsg], kExpectedTimestamp, + sizeof(kExpectedTimestamp))); +} + +// Verify finding an extension ID in a raw rtp message. +TEST(RtpUtilsTest, UpdateTwoByteAbsSendTimeExtensionInRtpPacket) { + std::vector rtp_packet( + kRtpMsgWithTwoByteAbsSendTimeExtension, + kRtpMsgWithTwoByteAbsSendTimeExtension + + sizeof(kRtpMsgWithTwoByteAbsSendTimeExtension)); + + EXPECT_TRUE(UpdateRtpAbsSendTimeExtension(&rtp_packet[0], rtp_packet.size(), + 3, 51183266)); + + // Verify that the timestamp was updated. + const uint8_t kExpectedTimestamp[3] = {0xcc, 0xbb, 0xaa}; + EXPECT_EQ(0, memcmp(&rtp_packet[kAstIndexInTwoByteRtpMsg], kExpectedTimestamp, sizeof(kExpectedTimestamp))); } @@ -339,20 +368,22 @@ TEST(RtpUtilsTest, ApplyPacketOptionsWithAuthParamsAndAbsSendTime) { packet_time_params.rtp_sendtime_extension_id = 3; // 3 is also present in the test message. - std::vector rtp_packet(kRtpMsgWithAbsSendTimeExtension, - kRtpMsgWithAbsSendTimeExtension + - sizeof(kRtpMsgWithAbsSendTimeExtension)); + std::vector rtp_packet( + kRtpMsgWithOneByteAbsSendTimeExtension, + kRtpMsgWithOneByteAbsSendTimeExtension + + sizeof(kRtpMsgWithOneByteAbsSendTimeExtension)); rtp_packet.insert(rtp_packet.end(), kFakeTag, kFakeTag + sizeof(kFakeTag)); EXPECT_TRUE(ApplyPacketOptions(&rtp_packet[0], rtp_packet.size(), packet_time_params, 51183266)); const uint8_t kExpectedTag[] = {0x81, 0xd1, 0x2c, 0x0e}; - EXPECT_EQ(0, memcmp(&rtp_packet[sizeof(kRtpMsgWithAbsSendTimeExtension)], - kExpectedTag, sizeof(kExpectedTag))); + EXPECT_EQ(0, + memcmp(&rtp_packet[sizeof(kRtpMsgWithOneByteAbsSendTimeExtension)], + kExpectedTag, sizeof(kExpectedTag))); // Verify that the timestamp was updated. const uint8_t kExpectedTimestamp[3] = {0xcc, 0xbb, 0xaa}; - EXPECT_EQ(0, memcmp(&rtp_packet[kAstIndexInRtpMsg], kExpectedTimestamp, + EXPECT_EQ(0, memcmp(&rtp_packet[kAstIndexInOneByteRtpMsg], kExpectedTimestamp, sizeof(kExpectedTimestamp))); }