Bug 1358224 - pt 3 - fix leak in RTPHeaderExtension's rid char buffer. r=drno
Turns out since Firefox doesn't receive simulcast streams, we never
noticed this leak. Convert RTPHeaderExtension.rid from a char* to
rtc::scoped_ptr<char[]> so it gets deleted properly. This also
requires a new copy constructor and assignment operator.
MozReview-Commit-ID: Jh4Gp4dAl9g
--- a/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp
@@ -28,17 +28,17 @@ bool MediaPipelineFilter::Filter(const w
// Some other stream; it is possible that an SSRC has moved, so make sure
// we don't have that SSRC in our filter any more.
remote_ssrc_set_.erase(header.ssrc);
return false;
}
if (header.extension.hasRID &&
remote_rid_set_.size() &&
- remote_rid_set_.count(header.extension.rid)) {
+ remote_rid_set_.count(header.extension.rid.get())) {
return true;
}
if (remote_ssrc_set_.count(header.ssrc)) {
return true;
}
// Last ditch effort...
--- a/media/webrtc/trunk/webrtc/common_types.cc
+++ b/media/webrtc/trunk/webrtc/common_types.cc
@@ -28,17 +28,45 @@ RTPHeaderExtension::RTPHeaderExtension()
hasTransportSequenceNumber(false),
transportSequenceNumber(0),
hasAudioLevel(false),
voiceActivity(false),
audioLevel(0),
hasVideoRotation(false),
videoRotation(0),
hasRID(false),
- rid(NULL) {
+ rid(nullptr) {
+}
+
+RTPHeaderExtension::RTPHeaderExtension(const RTPHeaderExtension& rhs) {
+ *this = rhs;
+}
+
+RTPHeaderExtension&
+RTPHeaderExtension::operator=(const RTPHeaderExtension& rhs) {
+ hasTransmissionTimeOffset = rhs.hasTransmissionTimeOffset;
+ transmissionTimeOffset = rhs.transmissionTimeOffset;
+ hasAbsoluteSendTime = rhs.hasAbsoluteSendTime;
+ absoluteSendTime = rhs.absoluteSendTime;
+ hasTransportSequenceNumber = rhs.hasTransportSequenceNumber;
+ transportSequenceNumber = rhs.transportSequenceNumber;
+
+ hasAudioLevel = rhs.hasAudioLevel;
+ voiceActivity = rhs.voiceActivity;
+ audioLevel = rhs.audioLevel;
+
+ hasVideoRotation = rhs.hasVideoRotation;
+ videoRotation = rhs.videoRotation;
+
+ hasRID = rhs.hasRID;
+ if (rhs.rid) {
+ rid.reset(new char[strlen(rhs.rid.get())+1]);
+ strcpy(rid.get(), rhs.rid.get());
+ }
+ return *this;
}
RTPHeader::RTPHeader()
: markerBit(false),
payloadType(0),
sequenceNumber(0),
timestamp(0),
ssrc(0),
--- a/media/webrtc/trunk/webrtc/common_types.h
+++ b/media/webrtc/trunk/webrtc/common_types.h
@@ -12,16 +12,17 @@
#define WEBRTC_COMMON_TYPES_H_
#include <stddef.h>
#include <string.h>
#include <string>
#include <vector>
+#include "webrtc/base/scoped_ptr.h"
#include "webrtc/typedefs.h"
#if defined(_MSC_VER)
// Disable "new behavior: elements of array will be default initialized"
// warning. Affects OverUseDetectorOptions.
#pragma warning(disable:4351)
#endif
@@ -825,16 +826,18 @@ struct PacketTime {
// indicating the potential error in the |timestamp|
// value,in case the system is busy.
// For example, the time of the last select() call.
// If unknown, this value will be set to zero.
};
struct RTPHeaderExtension {
RTPHeaderExtension();
+ RTPHeaderExtension(const RTPHeaderExtension& rhs);
+ RTPHeaderExtension& operator=(const RTPHeaderExtension& rhs);
bool hasTransmissionTimeOffset;
int32_t transmissionTimeOffset;
bool hasAbsoluteSendTime;
uint32_t absoluteSendTime;
bool hasTransportSequenceNumber;
uint16_t transportSequenceNumber;
@@ -847,17 +850,17 @@ struct RTPHeaderExtension {
// For Coordination of Video Orientation. See
// http://www.etsi.org/deliver/etsi_ts/126100_126199/126114/12.07.00_60/
// ts_126114v120700p.pdf
bool hasVideoRotation;
uint8_t videoRotation;
// RID values for simulcast; see draft-roach-avtext-rid
bool hasRID;
- char *rid; // UTF8 string
+ rtc::scoped_ptr<char[]> rid; // UTF8 string
};
struct RTPHeader {
RTPHeader();
bool markerBit;
uint8_t payloadType;
uint16_t sequenceNumber;
--- a/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc
+++ b/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc
@@ -218,20 +218,20 @@ bool RtpReceiverImpl::IncomingRtpPacket(
{
CriticalSectionScoped lock(critical_section_rtp_receiver_.get());
last_receive_time_ = clock_->TimeInMilliseconds();
last_received_payload_length_ = payload_data_length;
// RID rarely if ever changes
if (rtp_header.extension.hasRID &&
- (!rid_ || strcmp(rtp_header.extension.rid, rid_) != 0)) {
+ (!rid_ || strcmp(rtp_header.extension.rid.get(), rid_) != 0)) {
delete [] rid_;
- rid_ = new char[strlen(rtp_header.extension.rid)+1];
- strcpy(rid_, rtp_header.extension.rid);
+ rid_ = new char[strlen(rtp_header.extension.rid.get())+1];
+ strcpy(rid_, rtp_header.extension.rid.get());
}
if (in_order) {
if (last_received_timestamp_ != rtp_header.timestamp) {
last_received_timestamp_ = rtp_header.timestamp;
last_received_frame_time_ms_ = clock_->TimeInMilliseconds();
}
last_received_sequence_number_ = rtp_header.sequenceNumber;
}
--- a/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_utility.cc
+++ b/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_utility.cc
@@ -439,22 +439,19 @@ void RtpHeaderParser::ParseOneByteExtens
// - 1. E.G. a len of 0 indicates a header data length of 1
if ( &ptr[len + 1] > ptrRTPDataExtensionEnd ) {
LOG(LS_WARNING) << "Extension RtpStreamId data length " << (len + 1)
<< " is longer than remaining input parse buffer "
<< static_cast<size_t>(ptrRTPDataExtensionEnd - ptr);
return;
}
- // TODO(jesup) - avoid allocating on each packet - high watermark the
- // RID buffer?
- char* ptrRID = new char[len + 2];
- memcpy(ptrRID, ptr, len + 1);
- ptrRID[len + 1] = '\0';
- header->extension.rid = ptrRID;
+ header->extension.rid.reset(new char[len + 2]);
+ memcpy(header->extension.rid.get(), ptr, len + 1);
+ header->extension.rid.get()[len + 1] = '\0';
header->extension.hasRID = true;
break;
}
default: {
LOG(LS_WARNING) << "Extension type not implemented: " << type;
return;
}
}