Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(562)

Unified Diff: webrtc/modules/rtp_rtcp/source/rtp_packet.cc

Issue 2576653003: In RtpPacket do not keep pointer to RtpHeaderExtensionMap (Closed)
Patch Set: Fix bug catched by msan Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « webrtc/modules/rtp_rtcp/source/rtp_packet.h ('k') | webrtc/modules/rtp_rtcp/source/rtp_packet_received.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/modules/rtp_rtcp/source/rtp_packet.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet.cc b/webrtc/modules/rtp_rtcp/source/rtp_packet.cc
index 72fd7892b235f1c1118b07457d18f87f9e83e1e7..b7d71c27898cbb8abf7e860e65736df6377eb125 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_packet.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_packet.cc
@@ -52,26 +52,28 @@ constexpr size_t kDefaultPacketSize = 1500;
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
// | padding | Padding size |
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+Packet::Packet() : Packet(nullptr, kDefaultPacketSize) {}
+
Packet::Packet(const ExtensionManager* extensions)
- : extensions_(extensions), buffer_(kDefaultPacketSize) {
- Clear();
-}
+ : Packet(extensions, kDefaultPacketSize) {}
Packet::Packet(const ExtensionManager* extensions, size_t capacity)
- : extensions_(extensions), buffer_(capacity) {
+ : buffer_(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;
+ }
}
Packet::~Packet() {}
-void Packet::IdentifyExtensions(const ExtensionManager* extensions) {
- RTC_DCHECK(extensions);
- extensions_ = extensions;
- for (size_t i = 0; i < num_extensions_; ++i) {
- uint8_t id = data()[extension_entries_[i].offset - 1] >> 4;
- extension_entries_[i].type = extensions_->GetType(id);
- }
+void Packet::IdentifyExtensions(const ExtensionManager& extensions) {
+ for (size_t i = 0; i < kMaxExtensionHeaders; ++i)
+ extension_entries_[i].type = extensions.GetType(i + 1);
}
bool Packet::Parse(const uint8_t* buffer, size_t buffer_size) {
@@ -211,8 +213,7 @@ void Packet::CopyHeaderFrom(const Packet& packet) {
timestamp_ = packet.timestamp_;
ssrc_ = packet.ssrc_;
payload_offset_ = packet.payload_offset_;
- num_extensions_ = packet.num_extensions_;
- for (size_t i = 0; i < num_extensions_; ++i) {
+ for (size_t i = 0; i < kMaxExtensionHeaders; ++i) {
extension_entries_[i] = packet.extension_entries_[i];
}
extensions_size_ = packet.extensions_size_;
@@ -253,7 +254,7 @@ void Packet::SetSsrc(uint32_t ssrc) {
}
void Packet::SetCsrcs(const std::vector<uint32_t>& csrcs) {
- RTC_DCHECK_EQ(num_extensions_, 0);
+ RTC_DCHECK_EQ(extensions_size_, 0);
RTC_DCHECK_EQ(payload_size_, 0);
RTC_DCHECK_EQ(padding_size_, 0);
RTC_DCHECK_LE(csrcs.size(), 0x0fu);
@@ -322,8 +323,11 @@ void Packet::Clear() {
payload_offset_ = kFixedHeaderSize;
payload_size_ = 0;
padding_size_ = 0;
- num_extensions_ = 0;
extensions_size_ = 0;
+ for (ExtensionInfo& location : extension_entries_) {
+ location.offset = 0;
+ location.length = 0;
+ }
memset(WriteAt(0), 0, kFixedHeaderSize);
buffer_.SetSize(kFixedHeaderSize);
@@ -362,8 +366,11 @@ bool Packet::ParseBuffer(const uint8_t* buffer, size_t size) {
padding_size_ = 0;
}
- num_extensions_ = 0;
extensions_size_ = 0;
+ for (ExtensionInfo& location : extension_entries_) {
+ location.offset = 0;
+ location.length = 0;
+ }
if (has_extension) {
/* RTP header extension, RFC 3550.
0 1 2 3
@@ -392,7 +399,7 @@ bool Packet::ParseBuffer(const uint8_t* buffer, size_t size) {
constexpr uint8_t kPaddingId = 0;
constexpr uint8_t kReservedId = 15;
while (extensions_size_ + kOneByteHeaderSize < extensions_capacity) {
- uint8_t id = buffer[extension_offset + extensions_size_] >> 4;
+ int id = buffer[extension_offset + extensions_size_] >> 4;
if (id == kReservedId) {
break;
} else if (id == kPaddingId) {
@@ -406,18 +413,16 @@ bool Packet::ParseBuffer(const uint8_t* buffer, size_t size) {
LOG(LS_WARNING) << "Oversized rtp header extension.";
break;
}
- if (num_extensions_ >= kMaxExtensionHeaders) {
- LOG(LS_WARNING) << "Too many rtp header extensions.";
- break;
+
+ size_t idx = id - 1;
+ if (extension_entries_[idx].length != 0) {
+ LOG(LS_VERBOSE) << "Duplicate rtp header extension id " << id
+ << ". Overwriting.";
}
+
extensions_size_ += kOneByteHeaderSize;
- extension_entries_[num_extensions_].type =
- extensions_ ? extensions_->GetType(id)
- : ExtensionManager::kInvalidType;
- extension_entries_[num_extensions_].length = length;
- extension_entries_[num_extensions_].offset =
- extension_offset + extensions_size_;
- num_extensions_++;
+ extension_entries_[idx].offset = extension_offset + extensions_size_;
+ extension_entries_[idx].length = length;
extensions_size_ += length;
}
}
@@ -435,16 +440,19 @@ bool Packet::FindExtension(ExtensionType type,
uint8_t length,
uint16_t* offset) const {
RTC_DCHECK(offset);
- for (size_t i = 0; i < num_extensions_; ++i) {
- if (extension_entries_[i].type == type) {
- if (length != extension_entries_[i].length) {
+ for (const ExtensionInfo& extension : extension_entries_) {
+ if (extension.type == type) {
+ if (extension.length == 0) {
+ // Extension is registered but not set.
+ return false;
+ }
+ if (length != extension.length) {
LOG(LS_WARNING) << "Length mismatch for extension '" << type
<< "': expected " << static_cast<int>(length)
- << ", received "
- << static_cast<int>(extension_entries_[i].length);
+ << ", received " << static_cast<int>(extension.length);
return false;
}
- *offset = extension_entries_[i].offset;
+ *offset = extension.offset;
return true;
}
}
@@ -454,10 +462,28 @@ bool Packet::FindExtension(ExtensionType type,
bool Packet::AllocateExtension(ExtensionType type,
uint8_t length,
uint16_t* offset) {
- if (!extensions_) {
- return false;
+ 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;
+ }
}
- if (FindExtension(type, length, offset)) {
+
+ 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;
}
@@ -469,10 +495,6 @@ bool Packet::AllocateExtension(ExtensionType type,
return false;
}
- uint8_t extension_id = extensions_->GetId(type);
- if (extension_id == ExtensionManager::kInvalidId) {
- return false;
- }
RTC_DCHECK_GT(length, 0);
RTC_DCHECK_LE(length, 16);
@@ -491,9 +513,8 @@ bool Packet::AllocateExtension(ExtensionType type,
(new_extensions_size + 3) / 4; // Wrap up to 32bit.
// All checks passed, write down the extension.
- if (num_extensions_ == 0) {
+ if (extensions_size_ == 0) {
RTC_DCHECK_EQ(payload_offset_, kFixedHeaderSize + (num_csrc * 4));
- RTC_DCHECK_EQ(extensions_size_, 0);
WriteAt(0, data()[0] | 0x10); // Set extension bit.
// Profile specific ID always set to OneByteExtensionHeader.
ByteWriter<uint16_t>::WriteBigEndian(WriteAt(extensions_offset - 4),
@@ -502,12 +523,10 @@ bool Packet::AllocateExtension(ExtensionType type,
WriteAt(extensions_offset + extensions_size_,
(extension_id << 4) | (length - 1));
- RTC_DCHECK(num_extensions_ < kMaxExtensionHeaders);
- extension_entries_[num_extensions_].type = type;
- extension_entries_[num_extensions_].length = length;
+
+ extension_entry->length = length;
*offset = extensions_offset + kOneByteHeaderSize + extensions_size_;
- extension_entries_[num_extensions_].offset = *offset;
- ++num_extensions_;
+ extension_entry->offset = *offset;
extensions_size_ = new_extensions_size;
// Update header length field.
« no previous file with comments | « webrtc/modules/rtp_rtcp/source/rtp_packet.h ('k') | webrtc/modules/rtp_rtcp/source/rtp_packet_received.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698