Remove raw extensions accessors from rtp packet

These accessors were introduced in https://codereview.webrtc.org/2789773004
for dynamic size extensions.
They are now implemented without need of these raw functions

Bug: None
Change-Id: Id43f0bcbf951d60ebeece49556b093956c5ad2bf
Reviewed-on: https://webrtc-review.googlesource.com/92626
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24242}

modules/rtp_rtcp/source/rtp_packet.cc

@@ -251,37 +251,7 @@ void RtpPacket::SetCsrcs(const std::vector<uint32_t>& csrcs) {
   buffer_.SetSize(payload_offset_);
 }
 
-bool RtpPacket::HasRawExtension(int id) const {
-  if (id == ExtensionManager::kInvalidId)
-    return false;
-  RTC_DCHECK_GE(id, kMinExtensionId);
-  RTC_DCHECK_LE(id, kMaxExtensionId);
-  return extension_entries_[id - 1].offset != 0;
-}
-
-rtc::ArrayView<const uint8_t> RtpPacket::GetRawExtension(int id) const {
-  if (id == ExtensionManager::kInvalidId)
-    return nullptr;
-  RTC_DCHECK_GE(id, kMinExtensionId);
-  RTC_DCHECK_LE(id, kMaxExtensionId);
-  const ExtensionInfo& extension = extension_entries_[id - 1];
-  if (extension.offset == 0)
-    return nullptr;
-  return rtc::MakeArrayView(data() + extension.offset, extension.length);
-}
-
-bool RtpPacket::SetRawExtension(int id, rtc::ArrayView<const uint8_t> data) {
-  auto buffer = AllocateRawExtension(id, data.size());
-  if (buffer.empty())
-    return false;
-  RTC_DCHECK_EQ(buffer.size(), data.size());
-  memcpy(buffer.data(), data.data(), data.size());
-  return true;
-}
-
 rtc::ArrayView<uint8_t> RtpPacket::AllocateRawExtension(int id, size_t length) {
-  if (id == ExtensionManager::kInvalidId)
-    return nullptr;
   RTC_DCHECK_GE(id, kMinExtensionId);
   RTC_DCHECK_LE(id, kMaxExtensionId);
   RTC_DCHECK_GE(length, 1);

modules/rtp_rtcp/source/rtp_packet.h

@@ -102,21 +102,6 @@ class RtpPacket {
   template <typename Extension>
   bool ReserveExtension();
 
-  // Following 4 helpers identify rtp header extension by |id| negotiated with
-  // remote peer and written in an rtp packet.
-  bool HasRawExtension(int id) const;
-
-  // Returns place where extension with |id| is stored.
-  // Returns empty arrayview if extension is not present.
-  rtc::ArrayView<const uint8_t> GetRawExtension(int id) const;
-
-  // Allocates and store header extension. Returns true on success.
-  bool SetRawExtension(int id, rtc::ArrayView<const uint8_t> data);
-
-  // Allocates and returns place to store rtp header extension.
-  // Returns empty arrayview on failure.
-  rtc::ArrayView<uint8_t> AllocateRawExtension(int id, size_t length);
-
   // Reserve size_bytes for payload. Returns nullptr on failure.
   uint8_t* SetPayloadSize(size_t size_bytes);
   // Same as SetPayloadSize but doesn't guarantee to keep current payload.
@@ -138,6 +123,10 @@ class RtpPacket {
   // Returns view of the raw extension or empty view on failure.
   rtc::ArrayView<const uint8_t> FindExtension(ExtensionType type) const;
 
+  // Allocates and returns place to store rtp header extension.
+  // Returns empty arrayview on failure.
+  rtc::ArrayView<uint8_t> AllocateRawExtension(int id, size_t length);
+
   // Find or allocate an extension |type|. Returns view of size |length|
   // to write raw extension to or an empty view on failure.
   rtc::ArrayView<uint8_t> AllocateExtension(ExtensionType type, size_t length);

modules/rtp_rtcp/source/rtp_packet_unittest.cc

@@ -196,50 +196,27 @@ TEST(RtpPacketTest, TryToCreateWithLongMid) {
   EXPECT_FALSE(packet.SetExtension<RtpMid>(kLongMid));
 }
 
-TEST(RtpPacketTest, CreateWithExtensionsWithoutManager) {
-  RtpPacketToSend packet(nullptr);
-  packet.SetPayloadType(kPayloadType);
-  packet.SetSequenceNumber(kSeqNum);
-  packet.SetTimestamp(kTimestamp);
-  packet.SetSsrc(kSsrc);
-
-  auto raw = packet.AllocateRawExtension(kTransmissionOffsetExtensionId,
-                                         TransmissionOffset::kValueSizeBytes);
-  EXPECT_EQ(raw.size(), TransmissionOffset::kValueSizeBytes);
-  TransmissionOffset::Write(raw, kTimeOffset);
-
-  raw = packet.AllocateRawExtension(kAudioLevelExtensionId,
-                                    AudioLevel::kValueSizeBytes);
-  EXPECT_EQ(raw.size(), AudioLevel::kValueSizeBytes);
-  AudioLevel::Write(raw, kVoiceActive, kAudioLevel);
-
-  EXPECT_THAT(kPacketWithTOAndAL,
-              ElementsAreArray(packet.data(), packet.size()));
-}
-
 TEST(RtpPacketTest, CreateWithMaxSizeHeaderExtension) {
-  const size_t kMaxExtensionSize = 16;
+  const std::string kValue = "123456789abcdef";
-  const int kId = 1;
+  RtpPacket::ExtensionManager extensions;
-  const uint8_t kValue[16] = "123456789abcdef";
+  extensions.Register<RtpMid>(1);
+  extensions.Register<RtpStreamId>(2);
 
-  // Write packet with a custom extension.
+  RtpPacket packet(&extensions);
-  RtpPacketToSend packet(nullptr);
+  EXPECT_TRUE(packet.SetExtension<RtpMid>(kValue));
-  packet.SetRawExtension(kId, kValue);
-  // Using different size for same id is not allowed.
-  EXPECT_TRUE(packet.AllocateRawExtension(kId, kMaxExtensionSize - 1).empty());
 
   packet.SetPayloadSize(42);
   // Rewriting allocated extension is allowed.
-  EXPECT_EQ(packet.AllocateRawExtension(kId, kMaxExtensionSize).size(),
+  EXPECT_TRUE(packet.SetExtension<RtpMid>(kValue));
-            kMaxExtensionSize);
   // Adding another extension after payload is set is not allowed.
-  EXPECT_TRUE(packet.AllocateRawExtension(kId + 1, kMaxExtensionSize).empty());
+  EXPECT_FALSE(packet.SetExtension<RtpStreamId>(kValue));
 
-  // Read packet with the custom extension.
+  // Read packet with the extension.
-  RtpPacketReceived parsed;
+  RtpPacketReceived parsed(&extensions);
   EXPECT_TRUE(parsed.Parse(packet.Buffer()));
-  auto read_raw = parsed.GetRawExtension(kId);
+  std::string read;
-  EXPECT_THAT(read_raw, ElementsAreArray(kValue, kMaxExtensionSize));
+  EXPECT_TRUE(parsed.GetExtension<RtpMid>(&read));
+  EXPECT_EQ(read, kValue);
 }
 
 TEST(RtpPacketTest, SetReservedExtensionsAfterPayload) {
@@ -426,23 +403,6 @@ TEST(RtpPacketTest, ParseWithExtensionDelayed) {
   EXPECT_EQ(0u, packet.padding_size());
 }
 
-TEST(RtpPacketTest, ParseWithoutExtensionManager) {
-  RtpPacketReceived packet;
-  EXPECT_TRUE(packet.Parse(kPacketWithTO, sizeof(kPacketWithTO)));
-
-  EXPECT_FALSE(packet.HasRawExtension(kAudioLevelExtensionId));
-  EXPECT_TRUE(packet.GetRawExtension(kAudioLevelExtensionId).empty());
-
-  EXPECT_TRUE(packet.HasRawExtension(kTransmissionOffsetExtensionId));
-
-  int32_t time_offset = 0;
-  auto raw_extension = packet.GetRawExtension(kTransmissionOffsetExtensionId);
-  EXPECT_EQ(raw_extension.size(), TransmissionOffset::kValueSizeBytes);
-  EXPECT_TRUE(TransmissionOffset::Parse(raw_extension, &time_offset));
-
-  EXPECT_EQ(time_offset, kTimeOffset);
-}
-
 TEST(RtpPacketTest, ParseDynamicSizeExtension) {
   // clang-format off
   const uint8_t kPacket1[] = {
@@ -493,23 +453,6 @@ TEST(RtpPacketTest, ParseWithMid) {
   EXPECT_EQ(mid, kMid);
 }
 
-TEST(RtpPacketTest, RawExtensionFunctionsAcceptZeroIdAndReturnFalse) {
-  RtpPacketReceived::ExtensionManager extensions;
-  RtpPacketReceived packet(&extensions);
-  // Use ExtensionManager to set kInvalidId to 0 to demonstrate natural way for
-  // using zero value as a parameter to Packet::*RawExtension functions.
-  const int kInvalidId = extensions.GetId(TransmissionOffset::kId);
-  ASSERT_EQ(kInvalidId, 0);
-
-  ASSERT_TRUE(packet.Parse(kPacket, sizeof(kPacket)));
-
-  EXPECT_FALSE(packet.HasRawExtension(kInvalidId));
-  EXPECT_THAT(packet.GetRawExtension(kInvalidId), IsEmpty());
-  const uint8_t kExtension[] = {'e', 'x', 't'};
-  EXPECT_FALSE(packet.SetRawExtension(kInvalidId, kExtension));
-  EXPECT_THAT(packet.AllocateRawExtension(kInvalidId, 3), IsEmpty());
-}
-
 TEST(RtpPacketTest, CreateAndParseTimingFrameExtension) {
   // Create a packet with video frame timing extension populated.
   RtpPacketToSend::ExtensionManager send_extensions;