Copy payload data when inserting packets into video_coding::PacketBuffer.

webrtc/modules/video_coding/packet_buffer.cc

Index: webrtc/modules/video_coding/packet_buffer.cc
diff --git a/webrtc/modules/video_coding/packet_buffer.cc b/webrtc/modules/video_coding/packet_buffer.cc
index 0d36b9c3c97ea439c714cf7f8b37656bc563326d..62ecededc11e8e85967026a77f534da79e9d59c1 100644
--- a/webrtc/modules/video_coding/packet_buffer.cc
+++ b/webrtc/modules/video_coding/packet_buffer.cc
@@ -90,6 +90,14 @@ bool PacketBuffer::InsertPacket(const VCMPacket& packet) {
   sequence_buffer_[index].used = true;
   data_buffer_[index] = packet;
 
+  // Since the data pointed to by |packet.dataPtr| is non-persistent the
+  // data has to be copied to its own buffer.

[stefan-webrtc, 2016/09/13 08:16:10]

Maybe add a TODO mentioning that we'd like to pass the ownership of this data
over from the socket to the packet buffer at some point. That'd avoid the
memcpy, which should be doable since AsyncUDPSocket and friends own the packet
memory at the moment.

[philipel, 2016/09/13 11:12:07]

Done.

+  if (packet.sizeBytes) {
+    uint8_t* payload = new uint8_t[packet.sizeBytes];

[stefan-webrtc, 2016/09/13 08:16:10]

Not super happy about this as we have to ensure we delete the memory at all
possible places (currently only 2 it seems, but still). Can we do something to
make this better by e.g. using unique_ptr?

[philipel, 2016/09/13 11:12:07]

Since the packet buffer have a fixed vector of packets the lifetime management
is manual. I guess using a unique_ptr could help but I'm not sure how much work
that requires.

Also, the lifetime management is fairly contained so it's not particularly
complex, so I'm pretty certain there is no leak (famous last words).

+    memcpy(payload, packet.dataPtr, packet.sizeBytes);
+    data_buffer_[index].dataPtr = payload;
+  }
+
   FindFrames(seq_num);
   return true;
 }
@@ -99,7 +107,9 @@ void PacketBuffer::ClearTo(uint16_t seq_num) {
   size_t index = first_seq_num_ % size_;
   while (AheadOf<uint16_t>(seq_num, first_seq_num_ + 1)) {
     index = (index + 1) % size_;
-    first_seq_num_ = Add<1 << 16>(first_seq_num_, 1);
+    ++first_seq_num_;
+    delete[] data_buffer_[index].dataPtr;
+    data_buffer_[index].dataPtr = nullptr;
     sequence_buffer_[index].used = false;
   }
 }
@@ -191,8 +201,11 @@ void PacketBuffer::ReturnFrame(RtpFrameObject* frame) {
   size_t end = (frame->last_seq_num() + 1) % size_;
   uint16_t seq_num = frame->first_seq_num();
   while (index != end) {
-    if (sequence_buffer_[index].seq_num == seq_num)
+    if (sequence_buffer_[index].seq_num == seq_num) {
+      delete[] data_buffer_[index].dataPtr;
+      data_buffer_[index].dataPtr = nullptr;
       sequence_buffer_[index].used = false;
+    }
 
     index = (index + 1) % size_;
     ++seq_num;