Add functions to get/set rtp header extension by id.

webrtc/modules/rtp_rtcp/source/rtp_packet.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 12 matching lines @@
 
 namespace webrtc {
 namespace rtp {
 namespace {
 constexpr size_t kFixedHeaderSize = 12;
 constexpr uint8_t kRtpVersion = 2;
 constexpr uint16_t kOneByteExtensionId = 0xBEDE;
 constexpr size_t kOneByteHeaderSize = 1;
 constexpr size_t kDefaultPacketSize = 1500;
 }  // namespace
+
+constexpr size_t Packet::kMaxExtensionHeaders;

[nisse-webrtc, 2017/04/03 09:38:00]

Why is this needed? 

(And not part of this cl, but isn't the usual convention that upper limits like
this is a limit on the size, i.e., maximum index value + 1?)

[danilchap, 2017/04/03 10:57:00]

In theory - to make program well-formed.
In practice - to allow use of the constant in CHECK_EQ macros.

https://github.com/google/googletest/blob/master/googletest/docs/FAQ.md#the-c...
0 is not a valid id, extensions ids are enumerated from 1, so in this particular
case max index and max number of supported extensions are same.... doesn't mean
same constant should be used.

[nisse-webrtc, 2017/04/03 11:21:19]

The FAQ entry talks about static const, but here we have constexpr, which I
thought was intended to make things more sane...

[danilchap, 2017/04/03 12:10:53]

longer story:
you can get away without this construct if you only use the constant by value,
but if you try to take address of the variable, then it should be defined
somewhere too. That what this line do.

CHECK_EQ and similar macroses take parameters by const&, i.e. they need address
of the variable.

+
 //  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
 // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 // |V=2|P|X|  CC   |M|     PT      |       sequence number         |
 // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 // |                           timestamp                           |
 // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 // |           synchronization source (SSRC) identifier            |
 // +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+
 // |            Contributing source (CSRC) identifiers             |
@@ skipping 223 matching lines @@
   payload_offset_ = kFixedHeaderSize + 4 * csrcs.size();
   WriteAt(0, (data()[0] & 0xF0) | csrcs.size());
   size_t offset = kFixedHeaderSize;
   for (uint32_t csrc : csrcs) {
     ByteWriter<uint32_t>::WriteBigEndian(WriteAt(offset), csrc);
     offset += 4;
   }
   buffer_.SetSize(payload_offset_);
 }
 
+bool Packet::HasRawExtension(int id) const {
+  RTC_DCHECK_GT(id, 0);

[nisse-webrtc, 2017/04/03 09:38:00]

I think it's unusual and confusing with a exclusive lower bound and inclusive
upper bound. I'd suggest RTC_DCHECK_GT(id, 1), or maybe even use a symbolic
constant, similar to RtpExtension::kMinId.

[danilchap, 2017/04/03 10:57:00]

Good point. It is also bad idea to use kMaxExtensionHeader as kMaxExtensionId
(following your other comment).
Fixed.

+  RTC_DCHECK_LE(id, kMaxExtensionHeaders);
+  return extension_entries_[id - 1].offset != 0;
+}
+
+rtc::ArrayView<const uint8_t> Packet::GetRawExtension(int id) const {
+  RTC_DCHECK_GT(id, 0);
+  RTC_DCHECK_LE(id, kMaxExtensionHeaders);
+  const ExtensionInfo& extension = extension_entries_[id - 1];
+  if (extension.offset == 0)
+    return nullptr;
+  return rtc::MakeArrayView(data() + extension.offset, extension.length);
+}
+
+bool Packet::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> Packet::AllocateRawExtension(int id, size_t length) {
+  RTC_DCHECK_GT(id, 0);
+  RTC_DCHECK_LE(id, kMaxExtensionHeaders);
+  RTC_DCHECK_GT(length, 0);
+  RTC_DCHECK_LE(length, 16);
+
+  ExtensionInfo* extension_entry = &extension_entries_[id - 1];
+  if (extension_entry->offset != 0) {
+    // Extension already reserved. Check if same length is used.
+    if (extension_entry->length == length)
+      return rtc::MakeArrayView(WriteAt(extension_entry->offset), length);
+
+    LOG(LS_ERROR) << "Length mismatch for extension id " << id << " type "
+                  << static_cast<int>(extension_entry->type) << ": expected "
+                  << static_cast<int>(extension_entry->length) << ". received "
+                  << length;
+    return nullptr;
+  }
+  if (payload_size_ > 0) {
+    LOG(LS_ERROR) << "Can't add new extension id " << id
+                  << " after payload was set.";
+    return nullptr;
+  }
+  if (padding_size_ > 0) {
+    LOG(LS_ERROR) << "Can't add new extension id " << id
+                  << " after padding was set.";
+    return nullptr;
+  }
+
+  size_t num_csrc = data()[0] & 0x0F;
+  size_t extensions_offset = kFixedHeaderSize + (num_csrc * 4) + 4;
+  size_t new_extensions_size = extensions_size_ + kOneByteHeaderSize + length;
+  if (extensions_offset + new_extensions_size > capacity()) {
+    LOG(LS_ERROR)
+        << "Extension cannot be registered: Not enough space left in buffer.";
+    return nullptr;
+  }
+
+  // All checks passed, write down the extension headers.
+  if (extensions_size_ == 0) {
+    RTC_DCHECK_EQ(payload_offset_, kFixedHeaderSize + (num_csrc * 4));
+    WriteAt(0, data()[0] | 0x10);  // Set extension bit.
+    // Profile specific ID always set to OneByteExtensionHeader.
+    ByteWriter<uint16_t>::WriteBigEndian(WriteAt(extensions_offset - 4),
+                                         kOneByteExtensionId);
+  }
+
+  WriteAt(extensions_offset + extensions_size_, (id << 4) | (length - 1));
+
+  extension_entry->offset =
+      extensions_offset + extensions_size_ + kOneByteHeaderSize;
+  extension_entry->length = length;
+  extensions_size_ = new_extensions_size;
+
+  // Update header length field.
+  uint16_t extensions_words = (extensions_size_ + 3) / 4;  // Wrap up to 32bit.
+  ByteWriter<uint16_t>::WriteBigEndian(WriteAt(extensions_offset - 2),
+                                       extensions_words);
+  // Fill extension padding place with zeroes.
+  size_t extension_padding_size = 4 * extensions_words - extensions_size_;
+  memset(WriteAt(extensions_offset + extensions_size_), 0,
+         extension_padding_size);
+  payload_offset_ = extensions_offset + 4 * extensions_words;
+  buffer_.SetSize(payload_offset_);
+  return rtc::MakeArrayView(WriteAt(extension_entry->offset), length);
+}
+
 uint8_t* Packet::AllocatePayload(size_t size_bytes) {
   // Reset payload size to 0. If CopyOnWrite buffer_ was shared, this will cause
   // reallocation and memcpy. Keeping just header reduces memcpy size.
   SetPayloadSize(0);
   return SetPayloadSize(size_bytes);
 }
 
 uint8_t* Packet::SetPayloadSize(size_t size_bytes) {
   RTC_DCHECK_EQ(padding_size_, 0);
   if (payload_offset_ + size_bytes > capacity()) {
@@ skipping 167 matching lines @@
                         << ", received " << static_cast<int>(extension.length);
         return false;
       }
       *offset = extension.offset;
       return true;
     }
   }
   return false;
 }
 
 bool Packet::AllocateExtension(ExtensionType type,

[nisse-webrtc, 2017/04/03 09:38:00]

For consistency, add an AllocateExtension method which returns an ArrayView, and
deprecate this method. (TODO-item plus followup cl would be nice).

[danilchap, 2017/04/03 10:57:00]

this is a private function, so can just replace it.
Done.

                                uint8_t length,
                                uint16_t* offset) {
-  uint8_t extension_id = ExtensionManager::kInvalidId;
-  ExtensionInfo* extension_entry = nullptr;
   for (size_t i = 0; i < kMaxExtensionHeaders; ++i) {
     if (extension_entries_[i].type == type) {
-      extension_id = i + 1;
-      extension_entry = &extension_entries_[i];
-      break;
+      int extension_id = i + 1;
+      auto buffer = AllocateRawExtension(extension_id, length);
+      if (buffer.empty())
+        return false;
+      *offset = buffer.data() - WriteAt(0);
+      return true;
     }
   }
-
-  if (!extension_entry)  // Extension not registered.
-    return false;
-
-  if (extension_entry->length != 0) {  // Already allocated.
-    if (length != extension_entry->length) {
-      LOG(LS_WARNING) << "Length mismatch for extension '" << type
-                      << "': expected " << static_cast<int>(length)
-                      << ", received "
-                      << static_cast<int>(extension_entry->length);
-      return false;
-    }
-    *offset = extension_entry->offset;
-    return true;
-  }
-
-  // Can't add new extension after payload/padding was set.
-  if (payload_size_ > 0) {
-    return false;
-  }
-  if (padding_size_ > 0) {
-    return false;
-  }
-
-  RTC_DCHECK_GT(length, 0);
-  RTC_DCHECK_LE(length, 16);
-
-  size_t num_csrc = data()[0] & 0x0F;
-  size_t extensions_offset = kFixedHeaderSize + (num_csrc * 4) + 4;
-  if (extensions_offset + extensions_size_ + kOneByteHeaderSize + length >
-      capacity()) {
-    LOG(LS_WARNING) << "Extension cannot be registered: "
-                       "Not enough space left in buffer.";
-    return false;
-  }
-
-  uint16_t new_extensions_size =
-      extensions_size_ + kOneByteHeaderSize + length;
-  uint16_t extensions_words =
-      (new_extensions_size + 3) / 4;  // Wrap up to 32bit.
-
-  // All checks passed, write down the extension.
-  if (extensions_size_ == 0) {
-    RTC_DCHECK_EQ(payload_offset_, kFixedHeaderSize + (num_csrc * 4));
-    WriteAt(0, data()[0] | 0x10);  // Set extension bit.
-    // Profile specific ID always set to OneByteExtensionHeader.
-    ByteWriter<uint16_t>::WriteBigEndian(WriteAt(extensions_offset - 4),
-                                         kOneByteExtensionId);
-  }
-
-  WriteAt(extensions_offset + extensions_size_,
-          (extension_id << 4) | (length - 1));
-
-  extension_entry->length = length;
-  *offset = extensions_offset + kOneByteHeaderSize + extensions_size_;
-  extension_entry->offset = *offset;
-  extensions_size_ = new_extensions_size;
-
-  // Update header length field.
-  ByteWriter<uint16_t>::WriteBigEndian(WriteAt(extensions_offset - 2),
-                                       extensions_words);
-  // Fill extension padding place with zeroes.
-  size_t extension_padding_size = 4 * extensions_words - extensions_size_;
-  memset(WriteAt(extensions_offset + extensions_size_), 0,
-         extension_padding_size);
-  payload_offset_ = extensions_offset + 4 * extensions_words;
-  buffer_.SetSize(payload_offset_);
-  return true;
+  // Extension not registered.
+  return false;
 }
 
 uint8_t* Packet::WriteAt(size_t offset) {
   return buffer_.data() + offset;
 }
 
 void Packet::WriteAt(size_t offset, uint8_t byte) {
   buffer_.data()[offset] = byte;
 }
 
 }  // namespace rtp
 }  // namespace webrtc