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

BUG=webrtc:6788

Review-Url: https://codereview.webrtc.org/2535203002
Cr-Commit-Position: refs/heads/master@{#15314}
diff --git a/webrtc/modules/video_coding/packet_buffer.cc b/webrtc/modules/video_coding/packet_buffer.cc
index c9d787b..03aeb5a 100644
--- a/webrtc/modules/video_coding/packet_buffer.cc
+++ b/webrtc/modules/video_coding/packet_buffer.cc
@@ -56,11 +56,11 @@
   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 @@
     } 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 @@
       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);
   }
diff --git a/webrtc/modules/video_coding/packet_buffer.h b/webrtc/modules/video_coding/packet_buffer.h
index 36b3027..da7e80f 100644
--- a/webrtc/modules/video_coding/packet_buffer.h
+++ b/webrtc/modules/video_coding/packet_buffer.h
@@ -48,9 +48,10 @@
 
   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();
 
diff --git a/webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc b/webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc
index 551b06b..d67e1ba 100644
--- a/webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc
+++ b/webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc
@@ -33,8 +33,8 @@
     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 @@
     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 @@
     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 @@
       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 @@
     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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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));
diff --git a/webrtc/modules/video_coding/video_packet_buffer_unittest.cc b/webrtc/modules/video_coding/video_packet_buffer_unittest.cc
index a937af6..1e2ef3e 100644
--- a/webrtc/modules/video_coding/video_packet_buffer_unittest.cc
+++ b/webrtc/modules/video_coding/video_packet_buffer_unittest.cc
@@ -64,7 +64,7 @@
     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 @@
   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 @@
   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 @@
   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
diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc
index bc4aa37..2f27d8d 100644
--- a/webrtc/video/rtp_stream_receiver.cc
+++ b/webrtc/video/rtp_stream_receiver.cc
@@ -274,7 +274,7 @@
       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) {