commit 082696efd9c78be1e0c0ee6f94c0400f45523f17
author Ilya Nikolaevskiy <ilnik@webrtc.org> Tue Sep 03 07:52:52 2019 +0000
committer Commit Bot <commit-bot@chromium.org> Tue Sep 03 07:53:05 2019 +0000
tree 627530746b03f3cf2853b07776e473d837fe85c2
parent ce202a0f987c51f70a3643a2d9dd05b7abfb01ba

Revert "Refactor FEC code to use COW buffers"

This reverts commit eec5fff4df92b2330e5fec67ff08c7cbb4c4ab8d.

Reason for revert: Some crashes found by the fuzzer

Original change's description:
> Refactor FEC code to use COW buffers
> 
> This refactoring helps to reduce unnecessary memcpy calls on the receive
> side.
> 
> This CL replaces
> |uint8 data[IP_PACKET_SIZE]| with |rtc::CopyOnWriteBuffer data| in Packet class,
> removes |length| field there, and does necessary changes.
> 
> This is a reland of these two CLs with fixes:
> https://webrtc-review.googlesource.com/c/src/+/144942
> https://webrtc-review.googlesource.com/c/src/+/144881
> 
> Bug: webrtc:10750
> Change-Id: I76f6dee5a57ade59942ea2822ca4737edfe6438b
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145332
> Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#29035}

TBR=brandtr@webrtc.org,ilnik@webrtc.org,stefan@webrtc.org

Change-Id: Id3d65fb1324b9f1b0446fe217012115ecacf2b40
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10750
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/151130
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29043}

modules/rtp_rtcp/source/ulpfec_receiver_impl.cc

diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
index 1974923..42d7af0 100644
--- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
+++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc
@@ -131,30 +131,31 @@
     ++packet_counter_.num_fec_packets;
 
     // everything behind the RED header
-    received_packet->pkt->data.SetData(
-        incoming_rtp_packet + header.headerLength + red_header_length,
-        payload_data_length - red_header_length);
+    memcpy(received_packet->pkt->data,
+           incoming_rtp_packet + header.headerLength + red_header_length,
+           payload_data_length - red_header_length);
+    received_packet->pkt->length = payload_data_length - red_header_length;
     received_packet->ssrc =
         ByteReader<uint32_t>::ReadBigEndian(&incoming_rtp_packet[8]);
 
   } else {
-    received_packet->pkt->data.SetSize(header.headerLength +
-                                       payload_data_length - red_header_length);
     // Copy RTP header.
-    memcpy(received_packet->pkt->data.data(), incoming_rtp_packet,
+    memcpy(received_packet->pkt->data, incoming_rtp_packet,
            header.headerLength);
+
     // Set payload type.
     received_packet->pkt->data[1] &= 0x80;          // Reset RED payload type.
     received_packet->pkt->data[1] += payload_type;  // Set media payload type.
+
     // Copy payload data.
-    if (payload_data_length > red_header_length) {
-      memcpy(received_packet->pkt->data.data() + header.headerLength,
-             incoming_rtp_packet + header.headerLength + red_header_length,
-             payload_data_length - red_header_length);
-    }
+    memcpy(received_packet->pkt->data + header.headerLength,
+           incoming_rtp_packet + header.headerLength + red_header_length,
+           payload_data_length - red_header_length);
+    received_packet->pkt->length =
+        header.headerLength + payload_data_length - red_header_length;
   }
 
-  if (received_packet->pkt->data.size() == 0) {
+  if (received_packet->pkt->length == 0) {
     return 0;
   }
 
@@ -182,18 +183,16 @@
     if (!received_packet->is_fec) {
       ForwardErrorCorrection::Packet* packet = received_packet->pkt;
       crit_sect_.Leave();
-      recovered_packet_callback_->OnRecoveredPacket(packet->data.data(),
-                                                    packet->data.size());
+      recovered_packet_callback_->OnRecoveredPacket(packet->data,
+                                                    packet->length);
       crit_sect_.Enter();
-      // Create a packet with the buffer to modify it.
       RtpPacketReceived rtp_packet;
-      rtp_packet.Parse(packet->data);
+      // TODO(ilnik): move extension nullifying out of RtpPacket, so there's no
+      // need to create one here, and avoid two memcpy calls below.
+      rtp_packet.Parse(packet->data, packet->length);  // Does memcopy.
       rtp_packet.IdentifyExtensions(extensions_);
-      // Reset buffer reference, so zeroing would work on a buffer with a
-      // single reference.
-      packet->data = rtc::CopyOnWriteBuffer(0);
-      rtp_packet.ZeroMutableExtensions();
-      packet->data = rtp_packet.Buffer();
+      rtp_packet.CopyAndZeroMutableExtensions(  // Does memcopy.
+          rtc::MakeArrayView(packet->data, packet->length));
     }
     fec_->DecodeFec(*received_packet, &recovered_packets_);
   }
@@ -210,8 +209,7 @@
     // header, OnRecoveredPacket will recurse back here.
     recovered_packet->returned = true;
     crit_sect_.Leave();
-    recovered_packet_callback_->OnRecoveredPacket(packet->data.data(),
-                                                  packet->data.size());
+    recovered_packet_callback_->OnRecoveredPacket(packet->data, packet->length);
     crit_sect_.Enter();
   }