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..3d7c0679070e2d03918dd7c14677dbf5feeba829 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 volatile the
+  // data has to be copied to its own buffer.
+  if (packet.sizeBytes) {
+    uint8_t* payload = new uint8_t[packet.sizeBytes];
+    memcpy(payload, packet.dataPtr, packet.sizeBytes);

[brandtr, 2016/09/01 12:45:13]

In this block, the copied VCMPacket essentially takes ownership of the data by
copying it. Would it make sense to make this functionality a member function of
VCMPacket, or to add an explicit copy constructor?

If the data pointed to by dataPtr is volatile, should it be locked during
copying?

[philipel, 2016/09/01 12:49:07]

We don't need to lock the data at this points.

I guess volatile is the wrong word, but If I don't copy the payload here the
data is lost. Maybe non-persistent is a better word to use, WDYT?

[brandtr, 2016/09/01 13:21:53]

"Non-persistent" sounds good.

[philipel, 2016/09/01 13:28:00]

Done.

+    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;