Delete class StringRtpHeaderExtension, replaced with std::string
Bug: webrtc:10440
Change-Id: I52f865496f9838ac0981a6cd13f24b5b681b6616
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/128609
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27265}diff --git a/api/rtp_headers.h b/api/rtp_headers.h
index 04c3853..13bc4f9 100644
--- a/api/rtp_headers.h
+++ b/api/rtp_headers.h
@@ -13,7 +13,7 @@
#include <stddef.h>
#include <stdint.h>
-#include <string.h>
+#include <string>
#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<const char> name);
- static bool IsLegalRsidName(rtc::ArrayView<const char> name);
-
- // TODO(bugs.webrtc.org/9537): Deprecate and remove when third parties have
- // migrated to "IsLegalRsidName".
- static bool IsLegalName(rtc::ArrayView<const char> name) {
- return IsLegalRsidName(name);
- }
-
- StringRtpHeaderExtension() { value_[0] = 0; }
- explicit StringRtpHeaderExtension(rtc::ArrayView<const char> 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<const uint8_t> value) {
- Set(reinterpret_cast<const char*>(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 @@
// 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<ColorSpace> color_space;
};
diff --git a/call/rtcp_demuxer_unittest.cc b/call/rtcp_demuxer_unittest.cc
index bed2819..5060c7b 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, 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 3b8200e..fbca494 100644
--- a/call/rtp_demuxer_unittest.cc
+++ b/call/rtp_demuxer_unittest.cc
@@ -1497,13 +1497,13 @@
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 20bd1e7..3b8972f 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp_defines.cc
+++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.cc
@@ -24,47 +24,24 @@
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<const char> 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<const char> 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<StreamId>::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 1d76ff3..9045003 100644
--- a/modules/rtp_rtcp/source/rtp_header_extensions.cc
+++ b/modules/rtp_rtcp/source/rtp_header_extensions.cc
@@ -727,24 +727,6 @@
}
bool BaseRtpStringExtension::Parse(rtc::ArrayView<const uint8_t> 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<uint8_t> 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<const uint8_t> data,
std::string* str) {
if (data.empty() || data[0] == 0) // Valid string extension can't be empty.
return false;
@@ -758,7 +740,7 @@
bool BaseRtpStringExtension::Write(rtc::ArrayView<uint8_t> 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 8aacb9a..c77492e 100644
--- a/modules/rtp_rtcp/source/rtp_header_extensions.h
+++ b/modules/rtp_rtcp/source/rtp_header_extensions.h
@@ -259,14 +259,6 @@
// maximum length that can be encoded with one-byte header extensions.
static constexpr uint8_t kMaxValueSizeBytes = 16;
- static bool Parse(rtc::ArrayView<const uint8_t> data,
- StringRtpHeaderExtension* str);
- static size_t ValueSize(const StringRtpHeaderExtension& str) {
- return str.size();
- }
- static bool Write(rtc::ArrayView<uint8_t> data,
- const StringRtpHeaderExtension& str);
-
static bool Parse(rtc::ArrayView<const uint8_t> data, std::string* str);
static size_t ValueSize(const std::string& str) { return str.size(); }
static bool Write(rtc::ArrayView<uint8_t> data, const std::string& str);
diff --git a/modules/rtp_rtcp/source/rtp_utility.cc b/modules/rtp_rtcp/source/rtp_utility.cc
index a1a3e35..b04ac4b 100644
--- a/modules/rtp_rtcp/source/rtp_utility.cc
+++ b/modules/rtp_rtcp/source/rtp_utility.cc
@@ -12,6 +12,7 @@
#include <assert.h>
#include <stddef.h>
+#include <string>
#include "api/array_view.h"
#include "api/video/video_content_type.h"
@@ -494,16 +495,30 @@
break;
}
case kRtpExtensionRtpStreamId: {
- header->extension.stream_id.Set(rtc::MakeArrayView(ptr, len + 1));
+ std::string name(reinterpret_cast<const char*>(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<const char*>(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<const char*>(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 e1bcfb7..f5ade3d 100644
--- a/modules/rtp_rtcp/source/rtp_utility_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_utility_unittest.cc
@@ -204,8 +204,8 @@
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 @@
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) {
