admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix memory leak in video_coding::PacketBuffer::InsertPacket.

Browse Source 
BUG=webrtc:6788

Review-Url: https://codereview.webrtc.org/2535203002
Cr-Commit-Position: refs/heads/master@{#15314}
This commit is contained in:
philipel 2016-11-30 01:32:05 -08:00 committed by Commit bot
parent be74270ebe
commit 759e0b7241
5 changed files with 77 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
webrtc/modules/video_coding/packet_buffer.cc
View File

@ -56,11 +56,11 @@ PacketBuffer::~PacketBuffer() {
Clear();
}
bool PacketBuffer::InsertPacket(const VCMPacket& packet) {
bool PacketBuffer::InsertPacket(VCMPacket* packet) {
std::vector<std::unique_ptr<RtpFrameObject>> found_frames;
{
rtc::CritScope lock(&crit_);
uint16_t seq_num = packet.seqNum;
uint16_t seq_num = packet->seqNum;
size_t index = seq_num % size_;
if (!first_packet_received_) {
@ -70,16 +70,22 @@ bool PacketBuffer::InsertPacket(const VCMPacket& packet) {
} else if (AheadOf(first_seq_num_, seq_num)) {
// If we have explicitly cleared past this packet then it's old,
// don't insert it.
if (is_cleared_to_first_seq_num_)
if (is_cleared_to_first_seq_num_) {
delete[] packet->dataPtr;
packet->dataPtr = nullptr;
return false;
}
first_seq_num_ = seq_num;
}
if (sequence_buffer_[index].used) {
// Duplicate packet, do nothing.
if (data_buffer_[index].seqNum == packet.seqNum)
// Duplicate packet, just delete the payload.
if (data_buffer_[index].seqNum == packet->seqNum) {
delete[] packet->dataPtr;
packet->dataPtr = nullptr;
return true;
}
// The packet buffer is full, try to expand the buffer.
while (ExpandBufferSize() && sequence_buffer_[seq_num % size_].used) {
@ -87,20 +93,24 @@ bool PacketBuffer::InsertPacket(const VCMPacket& packet) {
index = seq_num % size_;
// Packet buffer is still full.
if (sequence_buffer_[index].used)
if (sequence_buffer_[index].used) {
delete[] packet->dataPtr;
packet->dataPtr = nullptr;
return false;
}
}
if (AheadOf(seq_num, last_seq_num_))
last_seq_num_ = seq_num;
sequence_buffer_[index].frame_begin = packet.isFirstPacket;
sequence_buffer_[index].frame_end = packet.markerBit;
sequence_buffer_[index].seq_num = packet.seqNum;
sequence_buffer_[index].frame_begin = packet->isFirstPacket;
sequence_buffer_[index].frame_end = packet->markerBit;
sequence_buffer_[index].seq_num = packet->seqNum;
sequence_buffer_[index].continuous = false;
sequence_buffer_[index].frame_created = false;
sequence_buffer_[index].used = true;
data_buffer_[index] = packet;
data_buffer_[index] = *packet;
packet->dataPtr = nullptr;
found_frames = FindFrames(seq_num);
}

7
webrtc/modules/video_coding/packet_buffer.h
View File

@ -48,9 +48,10 @@ class PacketBuffer {
virtual ~PacketBuffer();
// Returns true if |packet| is inserted into the packet buffer,
// false otherwise. Made virtual for testing.
virtual bool InsertPacket(const VCMPacket& packet);
// Returns true if |packet| is inserted into the packet buffer, false
// otherwise. The PacketBuffer will always take ownership of the
// |packet.dataPtr| when this function is called. Made virtual for testing.
virtual bool InsertPacket(VCMPacket* packet);
void ClearTo(uint16_t seq_num);
void Clear();

48
webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc
View File

@ -33,8 +33,8 @@ class FakePacketBuffer : public PacketBuffer {
return packet_it == packets_.end() ? nullptr : &packet_it->second;
}
bool InsertPacket(const VCMPacket& packet) override {
packets_[packet.seqNum] = packet;
bool InsertPacket(VCMPacket* packet) override {
packets_[packet->seqNum] = *packet;
return true;
}
@ -83,11 +83,11 @@ class TestRtpFrameReferenceFinder : public ::testing::Test,
packet.codec = kVideoCodecGeneric;
packet.seqNum = seq_num_start;
packet.frameType = keyframe ? kVideoFrameKey : kVideoFrameDelta;
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
packet.seqNum = seq_num_end;
packet.markerBit = true;
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(new RtpFrameObject(
ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0, 0));
@ -110,12 +110,12 @@ class TestRtpFrameReferenceFinder : public ::testing::Test,
packet.video_header.codecHeader.VP8.temporalIdx = tid;
packet.video_header.codecHeader.VP8.tl0PicIdx = tl0;
packet.video_header.codecHeader.VP8.layerSync = sync;
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
if (seq_num_start != seq_num_end) {
packet.seqNum = seq_num_end;
packet.markerBit = true;
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
}
std::unique_ptr<RtpFrameObject> frame(new RtpFrameObject(
@ -148,13 +148,13 @@ class TestRtpFrameReferenceFinder : public ::testing::Test,
packet.video_header.codecHeader.VP9.ss_data_available = true;
packet.video_header.codecHeader.VP9.gof = *ss;
}
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
if (seq_num_start != seq_num_end) {
packet.markerBit = true;
packet.video_header.codecHeader.VP9.ss_data_available = false;
packet.seqNum = seq_num_end;
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
}
std::unique_ptr<RtpFrameObject> frame(new RtpFrameObject(
@ -186,12 +186,12 @@ class TestRtpFrameReferenceFinder : public ::testing::Test,
packet.video_header.codecHeader.VP9.num_ref_pics = refs.size();
for (size_t i = 0; i < refs.size(); ++i)
packet.video_header.codecHeader.VP9.pid_diff[i] = refs[i];
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
if (seq_num_start != seq_num_end) {
packet.seqNum = seq_num_end;
packet.markerBit = true;
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
}
std::unique_ptr<RtpFrameObject> frame(new RtpFrameObject(
@ -1270,7 +1270,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_PidJumpsBackwardThenForward) {
packet.video_header.codecHeader.VP9.gof = ss;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1281,7 +1281,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_PidJumpsBackwardThenForward) {
packet.video_header.codecHeader.VP9.picture_id = 0;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1291,7 +1291,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_PidJumpsBackwardThenForward) {
packet.video_header.codecHeader.VP9.picture_id = 5000;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1323,7 +1323,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_Tl0JumpsBackwardThenForward) {
packet.video_header.codecHeader.VP9.ss_data_available = true;
packet.video_header.codecHeader.VP9.gof = ss;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1333,7 +1333,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_Tl0JumpsBackwardThenForward) {
packet.video_header.codecHeader.VP9.picture_id = 1;
packet.video_header.codecHeader.VP9.tl0_pic_idx = 0;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1344,7 +1344,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_Tl0JumpsBackwardThenForward) {
packet.video_header.codecHeader.VP9.picture_id = 2;
packet.video_header.codecHeader.VP9.tl0_pic_idx = 2;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1356,7 +1356,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_Tl0JumpsBackwardThenForward) {
packet.video_header.codecHeader.VP9.picture_id = 3;
packet.video_header.codecHeader.VP9.tl0_pic_idx = 129;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1389,7 +1389,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_PidSmallJumpForward) {
packet.video_header.codecHeader.VP9.ss_data_available = true;
packet.video_header.codecHeader.VP9.gof = ss;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1399,7 +1399,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_PidSmallJumpForward) {
packet.video_header.codecHeader.VP9.picture_id = 2;
packet.video_header.codecHeader.VP9.tl0_pic_idx = 2;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1409,7 +1409,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_PidSmallJumpForward) {
packet.video_header.codecHeader.VP9.picture_id = 3;
packet.video_header.codecHeader.VP9.tl0_pic_idx = 2;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1419,7 +1419,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_PidSmallJumpForward) {
packet.video_header.codecHeader.VP9.picture_id = 4;
packet.video_header.codecHeader.VP9.tl0_pic_idx = 1;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1452,7 +1452,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_DropOldFrame) {
packet.video_header.codecHeader.VP9.ss_data_available = true;
packet.video_header.codecHeader.VP9.gof = ss;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1462,7 +1462,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_DropOldFrame) {
packet.video_header.codecHeader.VP9.picture_id = 0;
packet.video_header.codecHeader.VP9.tl0_pic_idx = 2;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));
@ -1472,7 +1472,7 @@ TEST_F(TestRtpFrameReferenceFinder, Vp9PidFix_DropOldFrame) {
packet.video_header.codecHeader.VP9.picture_id = 3;
packet.video_header.codecHeader.VP9.tl0_pic_idx = 2;
{
ref_packet_buffer_->InsertPacket(packet);
ref_packet_buffer_->InsertPacket(&packet);
std::unique_ptr<RtpFrameObject> frame(
new RtpFrameObject(ref_packet_buffer_, 0, 0, 0, 0, 0));
reference_finder_->ManageFrame(std::move(frame));

34
webrtc/modules/video_coding/video_packet_buffer_unittest.cc
View File

@ -64,7 +64,7 @@ class TestPacketBuffer : public ::testing::Test,
packet.sizeBytes = data_size;
packet.dataPtr = data;
return packet_buffer_->InsertPacket(packet);
return packet_buffer_->InsertPacket(&packet);
}
void CheckFrame(uint16_t first_seq_num) {
@ -142,21 +142,21 @@ TEST_F(TestPacketBuffer, NackCount) {
packet.markerBit = false;
packet.timesNacked = 0;
packet_buffer_->InsertPacket(packet);
packet_buffer_->InsertPacket(&packet);
packet.seqNum++;
packet.isFirstPacket = false;
packet.timesNacked = 1;
packet_buffer_->InsertPacket(packet);
packet_buffer_->InsertPacket(&packet);
packet.seqNum++;
packet.timesNacked = 3;
packet_buffer_->InsertPacket(packet);
packet_buffer_->InsertPacket(&packet);
packet.seqNum++;
packet.markerBit = true;
packet.timesNacked = 1;
packet_buffer_->InsertPacket(packet);
packet_buffer_->InsertPacket(&packet);
ASSERT_EQ(1UL, frames_from_callback_.size());
RtpFrameObject* frame = frames_from_callback_.begin()->second.get();
@ -365,7 +365,7 @@ TEST_F(TestPacketBuffer, GetBitstreamH264BufferPadding) {
packet.sizeBytes = sizeof(data_data);
packet.isFirstPacket = true;
packet.markerBit = true;
packet_buffer_->InsertPacket(packet);
packet_buffer_->InsertPacket(&packet);
ASSERT_EQ(1UL, frames_from_callback_.size());
EXPECT_EQ(frames_from_callback_[seq_num]->EncodedImage()._length,
@ -438,5 +438,27 @@ TEST_F(TestPacketBuffer, FramesAfterClear) {
CheckFrame(9057);
}
TEST_F(TestPacketBuffer, DontLeakPayloadData) {
// NOTE! Any eventual leak is suppose to be detected by valgrind
// or any other similar tool.
uint8_t* data1 = new uint8_t[5];
uint8_t* data2 = new uint8_t[5];
uint8_t* data3 = new uint8_t[5];
uint8_t* data4 = new uint8_t[5];
// Expected to free data1 upon PacketBuffer destruction.
EXPECT_TRUE(Insert(2, kKeyFrame, kFirst, kNotLast, 5, data1));
// Expect to free data2 upon insertion.
EXPECT_TRUE(Insert(2, kKeyFrame, kFirst, kNotLast, 5, data2));
// Expect to free data3 upon insertion (old packet).
packet_buffer_->ClearTo(1);
EXPECT_FALSE(Insert(1, kKeyFrame, kFirst, kNotLast, 5, data3));
// Expect to free data4 upon insertion (packet buffer is full).
EXPECT_FALSE(Insert(2 + kMaxSize, kKeyFrame, kFirst, kNotLast, 5, data4));
}
} // namespace video_coding
} // namespace webrtc

2
webrtc/video/rtp_stream_receiver.cc
View File

@ -274,7 +274,7 @@ int32_t RtpStreamReceiver::OnReceivedPayloadData(
packet.dataPtr = data;
}
packet_buffer_->InsertPacket(packet);
packet_buffer_->InsertPacket(&packet);
} else {
if (video_receiver_->IncomingPacket(payload_data, payload_size,
rtp_header_with_ntp) != 0) {