From c5744b8b21b627213286f1b6f2c65da5df9ce8d0 Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Mon, 24 Sep 2018 14:50:48 +0200 Subject: [PATCH] Refactor to remove direct memory dependency on kMaxId When two-byte header extensions are enabled, kMaxId will change from 15 to 255. This CL is a refactor to remove the direct dependency between memory allocation and kMaxId. Bug: webrtc:7990 Change-Id: I38974a9c705eb6a0fdba9038a7d909861587d98d Reviewed-on: https://webrtc-review.googlesource.com/101580 Commit-Queue: Johannes Kron Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#24801} --- .../include/rtp_header_extension_map.h | 7 +- modules/rtp_rtcp/include/rtp_rtcp_defines.h | 5 +- .../source/rtp_header_extension_map.cc | 25 ++++-- modules/rtp_rtcp/source/rtp_packet.cc | 90 +++++++++++-------- modules/rtp_rtcp/source/rtp_packet.h | 20 ++++- 5 files changed, 89 insertions(+), 58 deletions(-) diff --git a/modules/rtp_rtcp/include/rtp_header_extension_map.h b/modules/rtp_rtcp/include/rtp_header_extension_map.h index 89a0ed526f..f07da8bf6c 100644 --- a/modules/rtp_rtcp/include/rtp_header_extension_map.h +++ b/modules/rtp_rtcp/include/rtp_header_extension_map.h @@ -44,11 +44,7 @@ class RtpHeaderExtensionMap { return GetId(type) != kInvalidId; } // Return kInvalidType if not found. - RTPExtensionType GetType(int id) const { - RTC_DCHECK_GE(id, kMinId); - RTC_DCHECK_LE(id, kMaxId); - return types_[id]; - } + RTPExtensionType GetType(int id) const; // Return kInvalidId if not found. uint8_t GetId(RTPExtensionType type) const { RTC_DCHECK_GT(type, kRtpExtensionNone); @@ -70,7 +66,6 @@ class RtpHeaderExtensionMap { static constexpr int kMaxId = 14; bool Register(int id, RTPExtensionType type, const char* uri); - RTPExtensionType types_[kMaxId + 1]; uint8_t ids_[kRtpExtensionNumberOfExtensions]; }; diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 0b2a23800a..53df981b10 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -94,7 +94,10 @@ enum ProtectionType { kUnprotectedPacket, kProtectedPacket }; enum StorageType { kDontRetransmit, kAllowRetransmission }; -enum RTPExtensionType { +// This enum must not have any gaps, i.e., all integers between +// kRtpExtensionNone and kRtpExtensionNumberOfExtensions must be valid enum +// entries. +enum RTPExtensionType : int { kRtpExtensionNone, kRtpExtensionTransmissionTimeOffset, kRtpExtensionAudioLevel, diff --git a/modules/rtp_rtcp/source/rtp_header_extension_map.cc b/modules/rtp_rtcp/source/rtp_header_extension_map.cc index 23ccd0ef88..c756310298 100644 --- a/modules/rtp_rtcp/source/rtp_header_extension_map.cc +++ b/modules/rtp_rtcp/source/rtp_header_extension_map.cc @@ -59,8 +59,6 @@ constexpr int RtpHeaderExtensionMap::kMinId; constexpr int RtpHeaderExtensionMap::kMaxId; RtpHeaderExtensionMap::RtpHeaderExtensionMap() { - for (auto& type : types_) - type = kInvalidType; for (auto& id : ids_) id = kInvalidId; } @@ -89,6 +87,18 @@ bool RtpHeaderExtensionMap::RegisterByUri(int id, const std::string& uri) { return false; } +RTPExtensionType RtpHeaderExtensionMap::GetType(int id) const { + RTC_DCHECK_GE(id, kMinId); + RTC_DCHECK_LE(id, kMaxId); + for (int type = kRtpExtensionNone + 1; type < kRtpExtensionNumberOfExtensions; + ++type) { + if (ids_[type] == id) { + return static_cast(type); + } + } + return kInvalidType; +} + size_t RtpHeaderExtensionMap::GetTotalLengthInBytes( rtc::ArrayView extensions) const { // Header size of the extension block, see RFC3550 Section 5.3.1 @@ -110,8 +120,6 @@ size_t RtpHeaderExtensionMap::GetTotalLengthInBytes( int32_t RtpHeaderExtensionMap::Deregister(RTPExtensionType type) { if (IsRegistered(type)) { - uint8_t id = GetId(type); - types_[id] = kInvalidType; ids_[type] = kInvalidId; } return 0; @@ -129,22 +137,23 @@ bool RtpHeaderExtensionMap::Register(int id, return false; } - if (GetType(id) == type) { // Same type/id pair already registered. + RTPExtensionType registered_type = GetType(id); + if (registered_type == type) { // Same type/id pair already registered. RTC_LOG(LS_VERBOSE) << "Reregistering extension uri:'" << uri << "', id:" << id; return true; } - if (GetType(id) != kInvalidType) { // |id| used by another extension type. + if (registered_type != + kInvalidType) { // |id| used by another extension type. RTC_LOG(LS_WARNING) << "Failed to register extension uri:'" << uri << "', id:" << id << ". Id already in use by extension type " - << static_cast(GetType(id)); + << static_cast(registered_type); return false; } RTC_DCHECK(!IsRegistered(type)); - types_[id] = type; // There is a run-time check above id fits into uint8_t. ids_[type] = static_cast(id); return true; diff --git a/modules/rtp_rtcp/source/rtp_packet.cc b/modules/rtp_rtcp/source/rtp_packet.cc index 4387fe74c3..10d4143513 100644 --- a/modules/rtp_rtcp/source/rtp_packet.cc +++ b/modules/rtp_rtcp/source/rtp_packet.cc @@ -68,18 +68,14 @@ RtpPacket::RtpPacket(const ExtensionManager* extensions, size_t capacity) RTC_DCHECK_GE(capacity, kFixedHeaderSize); Clear(); if (extensions) { - IdentifyExtensions(*extensions); - } else { - for (size_t i = 0; i < kMaxExtensionHeaders; ++i) - extension_entries_[i].type = ExtensionManager::kInvalidType; + extensions_ = *extensions; } } RtpPacket::~RtpPacket() {} void RtpPacket::IdentifyExtensions(const ExtensionManager& extensions) { - for (int i = 0; i < kMaxExtensionHeaders; ++i) - extension_entries_[i].type = extensions.GetType(i + 1); + extensions_ = extensions; } bool RtpPacket::Parse(const uint8_t* buffer, size_t buffer_size) { @@ -127,9 +123,8 @@ void RtpPacket::CopyHeaderFrom(const RtpPacket& packet) { timestamp_ = packet.timestamp_; ssrc_ = packet.ssrc_; payload_offset_ = packet.payload_offset_; - for (size_t i = 0; i < kMaxExtensionHeaders; ++i) { - extension_entries_[i] = packet.extension_entries_[i]; - } + extensions_ = packet.extensions_; + extension_entries_ = packet.extension_entries_; extensions_size_ = packet.extensions_size_; buffer_.SetData(packet.data(), packet.headers_size()); // Reset payload and padding. @@ -189,14 +184,13 @@ rtc::ArrayView RtpPacket::AllocateRawExtension(int id, size_t length) { RTC_DCHECK_GE(length, 1); RTC_DCHECK_LE(length, 16); - ExtensionInfo* extension_entry = &extension_entries_[id - 1]; - if (extension_entry->offset != 0) { + const ExtensionInfo* extension_entry = FindExtensionInfo(id); + if (extension_entry != nullptr) { // Extension already reserved. Check if same length is used. if (extension_entry->length == length) return rtc::MakeArrayView(WriteAt(extension_entry->offset), length); - RTC_LOG(LS_ERROR) << "Length mismatch for extension id " << id << " type " - << static_cast(extension_entry->type) + RTC_LOG(LS_ERROR) << "Length mismatch for extension id " << id << ": expected " << static_cast(extension_entry->length) << ". received " << length; @@ -235,9 +229,11 @@ rtc::ArrayView RtpPacket::AllocateRawExtension(int id, size_t length) { one_byte_header |= rtc::dchecked_cast(length - 1); WriteAt(extensions_offset + extensions_size_, one_byte_header); - extension_entry->offset = rtc::dchecked_cast( + const uint16_t extension_info_offset = rtc::dchecked_cast( extensions_offset + extensions_size_ + kOneByteHeaderSize); - extension_entry->length = rtc::dchecked_cast(length); + const uint8_t extension_info_length = rtc::dchecked_cast(length); + extension_entries_.emplace_back(id, extension_info_length, + extension_info_offset); extensions_size_ = new_extensions_size; // Update header length field. @@ -251,7 +247,8 @@ rtc::ArrayView RtpPacket::AllocateRawExtension(int id, size_t length) { extension_padding_size); payload_offset_ = extensions_offset + 4 * extensions_words; buffer_.SetSize(payload_offset_); - return rtc::MakeArrayView(WriteAt(extension_entry->offset), length); + return rtc::MakeArrayView(WriteAt(extension_info_offset), + extension_info_length); } uint8_t* RtpPacket::AllocatePayload(size_t size_bytes) { @@ -306,10 +303,7 @@ void RtpPacket::Clear() { payload_size_ = 0; padding_size_ = 0; extensions_size_ = 0; - for (ExtensionInfo& location : extension_entries_) { - location.offset = 0; - location.length = 0; - } + extension_entries_.clear(); memset(WriteAt(0), 0, kFixedHeaderSize); buffer_.SetSize(kFixedHeaderSize); @@ -396,8 +390,8 @@ bool RtpPacket::ParseBuffer(const uint8_t* buffer, size_t size) { break; } - size_t idx = id - 1; - if (extension_entries_[idx].length != 0) { + ExtensionInfo& extension_info = FindOrCreateExtensionInfo(id); + if (extension_info.length != 0) { RTC_LOG(LS_VERBOSE) << "Duplicate rtp header extension id " << id << ". Overwriting."; } @@ -408,8 +402,8 @@ bool RtpPacket::ParseBuffer(const uint8_t* buffer, size_t size) { RTC_DLOG(LS_WARNING) << "Oversized rtp header extension."; break; } - extension_entries_[idx].offset = static_cast(offset); - extension_entries_[idx].length = length; + extension_info.offset = static_cast(offset); + extension_info.length = length; extensions_size_ += kOneByteHeaderSize + length; } } @@ -423,30 +417,48 @@ bool RtpPacket::ParseBuffer(const uint8_t* buffer, size_t size) { return true; } -rtc::ArrayView RtpPacket::FindExtension( - ExtensionType type) const { +const RtpPacket::ExtensionInfo* RtpPacket::FindExtensionInfo(int id) const { for (const ExtensionInfo& extension : extension_entries_) { - if (extension.type == type) { - if (extension.length == 0) { - // Extension is registered but not set. - return nullptr; - } - return rtc::MakeArrayView(data() + extension.offset, extension.length); + if (extension.id == id) { + return &extension; } } return nullptr; } +RtpPacket::ExtensionInfo& RtpPacket::FindOrCreateExtensionInfo(int id) { + for (ExtensionInfo& extension : extension_entries_) { + if (extension.id == id) { + return extension; + } + } + extension_entries_.emplace_back(id); + return extension_entries_.back(); +} + +rtc::ArrayView RtpPacket::FindExtension( + ExtensionType type) const { + uint8_t id = extensions_.GetId(type); + if (id == ExtensionManager::kInvalidId) { + // Extension not registered. + return nullptr; + } + ExtensionInfo const* extension_info = FindExtensionInfo(id); + if (extension_info == nullptr) { + return nullptr; + } + return rtc::MakeArrayView(data() + extension_info->offset, + extension_info->length); +} + rtc::ArrayView RtpPacket::AllocateExtension(ExtensionType type, size_t length) { - for (int i = 0; i < kMaxExtensionHeaders; ++i) { - if (extension_entries_[i].type == type) { - int extension_id = i + 1; - return AllocateRawExtension(extension_id, length); - } + uint8_t id = extensions_.GetId(type); + if (id == ExtensionManager::kInvalidId) { + // Extension not registered. + return nullptr; } - // Extension not registered. - return nullptr; + return AllocateRawExtension(id, length); } } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_packet.h b/modules/rtp_rtcp/source/rtp_packet.h index 21adf6229b..4a3b7e9543 100644 --- a/modules/rtp_rtcp/source/rtp_packet.h +++ b/modules/rtp_rtcp/source/rtp_packet.h @@ -13,11 +13,11 @@ #include #include "api/array_view.h" +#include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/copyonwritebuffer.h" namespace webrtc { -class RtpHeaderExtensionMap; class Random; class RtpPacket { @@ -114,15 +114,26 @@ class RtpPacket { private: struct ExtensionInfo { - ExtensionType type; - uint16_t offset; + explicit ExtensionInfo(uint8_t id) : ExtensionInfo(id, 0, 0) {} + ExtensionInfo(uint8_t id, uint8_t length, uint16_t offset) + : id(id), length(length), offset(offset) {} + uint8_t id; uint8_t length; + uint16_t offset; }; // Helper function for Parse. Fill header fields using data in given buffer, // but does not touch packet own buffer, leaving packet in invalid state. bool ParseBuffer(const uint8_t* buffer, size_t size); + // Returns pointer to extension info for a given id. Returns nullptr if not + // found. + const ExtensionInfo* FindExtensionInfo(int id) const; + + // Returns reference to extension info for a given id. Creates a new entry + // with the specified id if not found. + ExtensionInfo& FindOrCreateExtensionInfo(int id); + // Find an extension |type|. // Returns view of the raw extension or empty view on failure. rtc::ArrayView FindExtension(ExtensionType type) const; @@ -148,7 +159,8 @@ class RtpPacket { size_t payload_offset_; // Match header size with csrcs and extensions. size_t payload_size_; - ExtensionInfo extension_entries_[kMaxExtensionHeaders]; + ExtensionManager extensions_; + std::vector extension_entries_; size_t extensions_size_ = 0; // Unaligned. rtc::CopyOnWriteBuffer buffer_; };