diff --git a/api/rtp_headers.h b/api/rtp_headers.h index 04c38536b7..13bc4f985e 100644 --- a/api/rtp_headers.h +++ b/api/rtp_headers.h @@ -13,7 +13,7 @@ #include #include -#include +#include #include "absl/types/optional.h" #include "api/array_view.h" @@ -26,62 +26,6 @@ namespace webrtc { -// Class to represent the value of RTP header extensions that are -// variable-length strings (e.g., RtpStreamId and RtpMid). -// Unlike std::string, it can be copied with memcpy and cleared with memset. -// -// Empty value represents unset header extension (use empty() to query). -class StringRtpHeaderExtension { - public: - // String RTP header extensions are limited to 16 bytes because it is the - // maximum length that can be encoded with one-byte header extensions. - static constexpr size_t kMaxSize = 16; - - static bool IsLegalMidName(rtc::ArrayView name); - static bool IsLegalRsidName(rtc::ArrayView name); - - // TODO(bugs.webrtc.org/9537): Deprecate and remove when third parties have - // migrated to "IsLegalRsidName". - static bool IsLegalName(rtc::ArrayView name) { - return IsLegalRsidName(name); - } - - StringRtpHeaderExtension() { value_[0] = 0; } - explicit StringRtpHeaderExtension(rtc::ArrayView value) { - Set(value.data(), value.size()); - } - StringRtpHeaderExtension(const StringRtpHeaderExtension&) = default; - StringRtpHeaderExtension& operator=(const StringRtpHeaderExtension&) = - default; - - bool empty() const { return value_[0] == 0; } - const char* data() const { return value_; } - size_t size() const { return strnlen(value_, kMaxSize); } - - void Set(rtc::ArrayView value) { - Set(reinterpret_cast(value.data()), value.size()); - } - void Set(const char* data, size_t size); - - friend bool operator==(const StringRtpHeaderExtension& lhs, - const StringRtpHeaderExtension& rhs) { - return strncmp(lhs.value_, rhs.value_, kMaxSize) == 0; - } - friend bool operator!=(const StringRtpHeaderExtension& lhs, - const StringRtpHeaderExtension& rhs) { - return !(lhs == rhs); - } - - private: - char value_[kMaxSize]; -}; - -// StreamId represents RtpStreamId which is a string. -typedef StringRtpHeaderExtension StreamId; - -// Mid represents RtpMid which is a string. -typedef StringRtpHeaderExtension Mid; - struct FeedbackRequest { // Determines whether the recv delta as specified in // https://tools.ietf.org/html/draft-holmer-rmcat-transport-wide-cc-extensions-01 @@ -134,12 +78,12 @@ struct RTPHeaderExtension { // For identification of a stream when ssrc is not signaled. See // https://tools.ietf.org/html/draft-ietf-avtext-rid-09 // TODO(danilchap): Update url from draft to release version. - StreamId stream_id; - StreamId repaired_stream_id; + std::string stream_id; + std::string repaired_stream_id; // For identifying the media section used to interpret this RTP packet. See // https://tools.ietf.org/html/draft-ietf-mmusic-sdp-bundle-negotiation-38 - Mid mid; + std::string mid; absl::optional color_space; }; diff --git a/call/rtcp_demuxer_unittest.cc b/call/rtcp_demuxer_unittest.cc index bed2819a63..5060c7b1d3 100644 --- a/call/rtcp_demuxer_unittest.cc +++ b/call/rtcp_demuxer_unittest.cc @@ -16,6 +16,7 @@ #include "api/rtp_headers.h" #include "call/rtcp_packet_sink_interface.h" #include "modules/rtp_rtcp/source/rtcp_packet/bye.h" +#include "modules/rtp_rtcp/source/rtp_header_extensions.h" #include "rtc_base/arraysize.h" #include "rtc_base/buffer.h" #include "rtc_base/checks.h" @@ -494,7 +495,7 @@ TEST_F(RtcpDemuxerTest, RsidMustBeAlphaNumeric) { TEST_F(RtcpDemuxerTest, RsidMustNotExceedMaximumLength) { MockRtcpPacketSink sink; - std::string rsid(StreamId::kMaxSize + 1, 'a'); + std::string rsid(BaseRtpStringExtension::kMaxValueSizeBytes + 1, 'a'); EXPECT_DEATH(AddRsidSink(rsid, &sink), ""); } diff --git a/call/rtp_demuxer_unittest.cc b/call/rtp_demuxer_unittest.cc index 3b8200e781..fbca494c1f 100644 --- a/call/rtp_demuxer_unittest.cc +++ b/call/rtp_demuxer_unittest.cc @@ -1497,13 +1497,13 @@ TEST_F(RtpDemuxerTest, MidMustBeToken) { TEST_F(RtpDemuxerTest, RsidMustNotExceedMaximumLength) { MockRtpPacketSink sink; - std::string rsid(StreamId::kMaxSize + 1, 'a'); + std::string rsid(BaseRtpStringExtension::kMaxValueSizeBytes + 1, 'a'); EXPECT_DEATH(AddSinkOnlyRsid(rsid, &sink), ""); } TEST_F(RtpDemuxerTest, MidMustNotExceedMaximumLength) { MockRtpPacketSink sink; - std::string mid(Mid::kMaxSize + 1, 'a'); + std::string mid(BaseRtpStringExtension::kMaxValueSizeBytes + 1, 'a'); EXPECT_DEATH(AddSinkOnlyMid(mid, &sink), ""); } diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.cc b/modules/rtp_rtcp/include/rtp_rtcp_defines.cc index 20bd1e700d..3b8972f01f 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.cc +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.cc @@ -24,47 +24,24 @@ namespace { constexpr size_t kMidRsidMaxSize = 16; // Check if passed character is a "token-char" from RFC 4566. -static bool IsTokenChar(char ch) { +bool IsTokenChar(char ch) { return ch == 0x21 || (ch >= 0x23 && ch <= 0x27) || ch == 0x2a || ch == 0x2b || ch == 0x2d || ch == 0x2e || (ch >= 0x30 && ch <= 0x39) || (ch >= 0x41 && ch <= 0x5a) || (ch >= 0x5e && ch <= 0x7e); } - } // namespace -StreamDataCounters::StreamDataCounters() : first_packet_time_ms(-1) {} - -constexpr size_t StreamId::kMaxSize; - bool IsLegalMidName(absl::string_view name) { return (name.size() <= kMidRsidMaxSize && name.size() > 0 && std::all_of(name.data(), name.data() + name.size(), IsTokenChar)); } -bool StreamId::IsLegalMidName(rtc::ArrayView name) { - return ::webrtc::IsLegalMidName(absl::string_view(name.data(), name.size())); -} - bool IsLegalRsidName(absl::string_view name) { return (name.size() <= kMidRsidMaxSize && name.size() > 0 && std::all_of(name.data(), name.data() + name.size(), isalnum)); } -bool StreamId::IsLegalRsidName(rtc::ArrayView name) { - return ::webrtc::IsLegalRsidName(absl::string_view(name.data(), name.size())); -} - -void StreamId::Set(const char* data, size_t size) { - // If |data| contains \0, the stream id size might become less than |size|. - RTC_CHECK_LE(size, kMaxSize); - memcpy(value_, data, size); - if (size < kMaxSize) - value_[size] = 0; -} - -// StreamId is used as member of RTPHeader that is sometimes copied with memcpy -// and thus assume trivial destructibility. -static_assert(std::is_trivially_destructible::value, ""); +StreamDataCounters::StreamDataCounters() : first_packet_time_ms(-1) {} PacketFeedback::PacketFeedback(int64_t arrival_time_ms, uint16_t sequence_number) diff --git a/modules/rtp_rtcp/source/rtp_header_extensions.cc b/modules/rtp_rtcp/source/rtp_header_extensions.cc index 1d76ff3f1e..904500337a 100644 --- a/modules/rtp_rtcp/source/rtp_header_extensions.cc +++ b/modules/rtp_rtcp/source/rtp_header_extensions.cc @@ -726,24 +726,6 @@ size_t ColorSpaceExtension::WriteLuminance(uint8_t* data, return 2; // Return number of bytes written. } -bool BaseRtpStringExtension::Parse(rtc::ArrayView data, - StringRtpHeaderExtension* str) { - if (data.empty() || data[0] == 0) // Valid string extension can't be empty. - return false; - str->Set(data); - RTC_DCHECK(!str->empty()); - return true; -} - -bool BaseRtpStringExtension::Write(rtc::ArrayView data, - const StringRtpHeaderExtension& str) { - RTC_DCHECK_EQ(data.size(), str.size()); - RTC_DCHECK_GE(str.size(), 1); - RTC_DCHECK_LE(str.size(), StringRtpHeaderExtension::kMaxSize); - memcpy(data.data(), str.data(), str.size()); - return true; -} - bool BaseRtpStringExtension::Parse(rtc::ArrayView data, std::string* str) { if (data.empty() || data[0] == 0) // Valid string extension can't be empty. @@ -758,7 +740,7 @@ bool BaseRtpStringExtension::Parse(rtc::ArrayView data, bool BaseRtpStringExtension::Write(rtc::ArrayView data, const std::string& str) { - if (str.size() > StringRtpHeaderExtension::kMaxSize) { + if (str.size() > kMaxValueSizeBytes) { return false; } RTC_DCHECK_EQ(data.size(), str.size()); diff --git a/modules/rtp_rtcp/source/rtp_header_extensions.h b/modules/rtp_rtcp/source/rtp_header_extensions.h index 8aacb9acb0..c77492e8ca 100644 --- a/modules/rtp_rtcp/source/rtp_header_extensions.h +++ b/modules/rtp_rtcp/source/rtp_header_extensions.h @@ -259,14 +259,6 @@ class BaseRtpStringExtension { // maximum length that can be encoded with one-byte header extensions. static constexpr uint8_t kMaxValueSizeBytes = 16; - static bool Parse(rtc::ArrayView data, - StringRtpHeaderExtension* str); - static size_t ValueSize(const StringRtpHeaderExtension& str) { - return str.size(); - } - static bool Write(rtc::ArrayView data, - const StringRtpHeaderExtension& str); - static bool Parse(rtc::ArrayView data, std::string* str); static size_t ValueSize(const std::string& str) { return str.size(); } static bool Write(rtc::ArrayView data, const std::string& str); diff --git a/modules/rtp_rtcp/source/rtp_utility.cc b/modules/rtp_rtcp/source/rtp_utility.cc index a1a3e35eb6..b04ac4b5b4 100644 --- a/modules/rtp_rtcp/source/rtp_utility.cc +++ b/modules/rtp_rtcp/source/rtp_utility.cc @@ -12,6 +12,7 @@ #include #include +#include #include "api/array_view.h" #include "api/video/video_content_type.h" @@ -494,16 +495,30 @@ void RtpHeaderParser::ParseOneByteExtensionHeader( break; } case kRtpExtensionRtpStreamId: { - header->extension.stream_id.Set(rtc::MakeArrayView(ptr, len + 1)); + std::string name(reinterpret_cast(ptr), len + 1); + if (IsLegalRsidName(name)) { + header->extension.stream_id = name; + } else { + RTC_LOG(LS_WARNING) << "Incorrect RtpStreamId"; + } break; } case kRtpExtensionRepairedRtpStreamId: { - header->extension.repaired_stream_id.Set( - rtc::MakeArrayView(ptr, len + 1)); + std::string name(reinterpret_cast(ptr), len + 1); + if (IsLegalRsidName(name)) { + header->extension.repaired_stream_id = name; + } else { + RTC_LOG(LS_WARNING) << "Incorrect RepairedRtpStreamId"; + } break; } case kRtpExtensionMid: { - header->extension.mid.Set(rtc::MakeArrayView(ptr, len + 1)); + std::string name(reinterpret_cast(ptr), len + 1); + if (IsLegalMidName(name)) { + header->extension.mid = name; + } else { + RTC_LOG(LS_WARNING) << "Incorrect Mid"; + } break; } case kRtpExtensionGenericFrameDescriptor00: diff --git a/modules/rtp_rtcp/source/rtp_utility_unittest.cc b/modules/rtp_rtcp/source/rtp_utility_unittest.cc index e1bcfb7a4a..f5ade3dd43 100644 --- a/modules/rtp_rtcp/source/rtp_utility_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_utility_unittest.cc @@ -204,8 +204,8 @@ TEST(RtpHeaderParser, ParseAll8Extensions) { header.extension.playout_delay.min_ms); EXPECT_EQ(0x876 * PlayoutDelayLimits::kGranularityMs, header.extension.playout_delay.max_ms); - EXPECT_EQ(header.extension.stream_id, StreamId("rtx")); - EXPECT_EQ(header.extension.repaired_stream_id, StreamId("stream")); + EXPECT_EQ(header.extension.stream_id, "rtx"); + EXPECT_EQ(header.extension.repaired_stream_id, "stream"); } TEST(RtpHeaderParser, ParseMalformedRsidExtensions) { @@ -230,7 +230,7 @@ TEST(RtpHeaderParser, ParseMalformedRsidExtensions) { EXPECT_TRUE(parser.Parse(&header, &extensions)); EXPECT_TRUE(header.extension.stream_id.empty()); - EXPECT_EQ(header.extension.repaired_stream_id, StreamId("str")); + EXPECT_TRUE(header.extension.repaired_stream_id.empty()); } TEST(RtpHeaderParser, ParseWithCsrcsExtensionAndPadding) {