Style updates to ProducerFec/FecReceiver.

webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc

Index: webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc
diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc b/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc
index c09f0dc6e0bdeed88ccefa2f17b08ac761cece68..28b1f40ef2870ba3043dd4712bf23d3bd3fb861a 100644
--- a/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc
+++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc
@@ -17,7 +17,6 @@
 #include "webrtc/modules/rtp_rtcp/source/byte_io.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_receiver_video.h"
 
-// RFC 5109
 namespace webrtc {
 
 FecReceiver* FecReceiver::Create(RtpData* callback) {
@@ -25,15 +24,11 @@ FecReceiver* FecReceiver::Create(RtpData* callback) {
 }
 
 FecReceiverImpl::FecReceiverImpl(RtpData* callback)
-    : recovered_packet_callback_(callback),
-      fec_(new ForwardErrorCorrection()) {}
+    : recovered_packet_callback_(callback) {}
 
 FecReceiverImpl::~FecReceiverImpl() {
-  received_packet_list_.clear();
-  if (fec_ != NULL) {
-    fec_->ResetState(&recovered_packet_list_);
-    delete fec_;
-  }
+  received_packets_.clear();
+  fec_.ResetState(&recovered_packets_);
 }
 
 FecPacketCounter FecReceiverImpl::GetPacketCounter() const {
@@ -73,7 +68,8 @@ int32_t FecReceiverImpl::AddReceivedRedPacket(
     const RTPHeader& header, const uint8_t* incoming_rtp_packet,
     size_t packet_length, uint8_t ulpfec_payload_type) {
   rtc::CritScope cs(&crit_sect_);
-  uint8_t REDHeaderLength = 1;
+
+  uint8_t RED_header_length = 1;

[philipel, 2016/07/04 15:35:03]

red_header_length

[brandtr, 2016/07/05 11:46:05]

Done.

   size_t payload_data_length = packet_length - header.headerLength;
 
   if (payload_data_length == 0) {
@@ -81,31 +77,27 @@ int32_t FecReceiverImpl::AddReceivedRedPacket(
     return -1;
   }
 
-  // Add to list without RED header, aka a virtual RTP packet
-  // we remove the RED header
-
+  // Remove RED header of incoming packet and store as a virtual RTP packet.
   std::unique_ptr<ForwardErrorCorrection::ReceivedPacket> received_packet(
       new ForwardErrorCorrection::ReceivedPacket());
   received_packet->pkt = new ForwardErrorCorrection::Packet();
 
-  // get payload type from RED header
-  uint8_t payload_type =
-      incoming_rtp_packet[header.headerLength] & 0x7f;
-
+  // Get payload type from RED header and sequence number from RTP header.
+  uint8_t payload_type = incoming_rtp_packet[header.headerLength] & 0x7f;
   received_packet->is_fec = payload_type == ulpfec_payload_type;
   received_packet->seq_num = header.sequenceNumber;
 
-  uint16_t blockLength = 0;
+  uint16_t block_length = 0;
   if (incoming_rtp_packet[header.headerLength] & 0x80) {
-    // f bit set in RED header
-    REDHeaderLength = 4;
-    if (payload_data_length < REDHeaderLength + 1u) {
+    // f bit set in RED header, i.e. there are more than one RED header blocks.
+    RED_header_length = 4;
+    if (payload_data_length < RED_header_length + 1u) {
       LOG(LS_WARNING) << "Corrupt/truncated FEC packet.";
       return -1;
     }
 
     uint16_t timestamp_offset =
-        (incoming_rtp_packet[header.headerLength + 1]) << 8;
+        incoming_rtp_packet[header.headerLength + 1] << 8;
     timestamp_offset +=
         incoming_rtp_packet[header.headerLength + 2];
     timestamp_offset = timestamp_offset >> 2;
@@ -114,112 +106,107 @@ int32_t FecReceiverImpl::AddReceivedRedPacket(
       return -1;
     }
 
-    blockLength =
-        (0x03 & incoming_rtp_packet[header.headerLength + 2]) << 8;
-    blockLength += (incoming_rtp_packet[header.headerLength + 3]);
+    block_length  = (0x03 & incoming_rtp_packet[header.headerLength + 2]) << 8;

[philipel, 2016/07/04 15:35:03]

0x3

[brandtr, 2016/07/05 11:46:05]

Done.

+    block_length +=  incoming_rtp_packet[header.headerLength + 3];
 
-    // check next RED header
+    // Check next RED header block.
     if (incoming_rtp_packet[header.headerLength + 4] & 0x80) {
       LOG(LS_WARNING) << "More than 2 blocks in packet not supported.";
       return -1;
     }
     // Check that the packet is long enough to contain data in the following
     // block.
-    if (blockLength > payload_data_length - (REDHeaderLength + 1)) {
+    if (block_length > payload_data_length - (RED_header_length + 1)) {
       LOG(LS_WARNING) << "Block length longer than packet.";
       return -1;
     }
   }
   ++packet_counter_.num_packets;
 
-  std::unique_ptr<ForwardErrorCorrection::ReceivedPacket>
-      second_received_packet;
-  if (blockLength > 0) {
-    // handle block length, split into 2 packets
-    REDHeaderLength = 5;
+  std::unique_ptr<ForwardErrorCorrection::ReceivedPacket> received_packet2;

[philipel, 2016/07/04 15:35:04]

Not 100% sure what this variable is suppose to hold, but I prefer the old name,
or do you think the old name misleading?

[brandtr, 2016/07/05 11:46:05]

IIUC, this variable holds a second virtual RTP packet which is embedded after
the first virtual RTP packet in the RED packet. The previous name is actually
more descriptive, so I changed it back.

+  if (block_length > 0) {
+    // Handle block length, split into two packets.
+    RED_header_length = 5;
 
-    // copy the RTP header
+    // Copy RTP header.
     memcpy(received_packet->pkt->data, incoming_rtp_packet,
            header.headerLength);
 
-    // replace the RED payload type
-    received_packet->pkt->data[1] &= 0x80;  // reset the payload
-    received_packet->pkt->data[1] +=
-        payload_type;                       // set the media payload type
+    // 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 the payload data
+    // Copy payload data.
     memcpy(
         received_packet->pkt->data + header.headerLength,
-        incoming_rtp_packet + header.headerLength + REDHeaderLength,
-        blockLength);
+        incoming_rtp_packet + header.headerLength + RED_header_length,
+        block_length);
+    received_packet->pkt->length = block_length;
 
-    received_packet->pkt->length = blockLength;
+    received_packet2.reset(new ForwardErrorCorrection::ReceivedPacket);
+    received_packet2->pkt = new ForwardErrorCorrection::Packet;
 
-    second_received_packet.reset(new ForwardErrorCorrection::ReceivedPacket());
-    second_received_packet->pkt = new ForwardErrorCorrection::Packet();
-
-    second_received_packet->is_fec = true;
-    second_received_packet->seq_num = header.sequenceNumber;
+    received_packet2->is_fec = true;
+    received_packet2->seq_num = header.sequenceNumber;
     ++packet_counter_.num_fec_packets;
 
-    // copy the FEC payload data
-    memcpy(second_received_packet->pkt->data,
+    // Copy FEC payload data.
+    memcpy(received_packet2->pkt->data,
            incoming_rtp_packet + header.headerLength +
-               REDHeaderLength + blockLength,
-           payload_data_length - REDHeaderLength - blockLength);
+               RED_header_length + block_length,
+           payload_data_length - RED_header_length - block_length);
 
-    second_received_packet->pkt->length =
-        payload_data_length - REDHeaderLength - blockLength;
+    received_packet2->pkt->length =
+        payload_data_length - RED_header_length - block_length;
 
   } else if (received_packet->is_fec) {
     ++packet_counter_.num_fec_packets;
     // everything behind the RED header
     memcpy(
         received_packet->pkt->data,
-        incoming_rtp_packet + header.headerLength + REDHeaderLength,
-        payload_data_length - REDHeaderLength);
-    received_packet->pkt->length = payload_data_length - REDHeaderLength;
+        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 {
-    // copy the RTP header
-    memcpy(received_packet->pkt->data, incoming_rtp_packet,
+    // Copy RTP header.
+    memcpy(received_packet->pkt->data,
+           incoming_rtp_packet,
            header.headerLength);
 
-    // replace the RED payload type
-    received_packet->pkt->data[1] &= 0x80;  // reset the payload
-    received_packet->pkt->data[1] +=
-        payload_type;                       // set the media payload type
+    // 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 the media payload data
+    // Copy payload data.
     memcpy(
         received_packet->pkt->data + header.headerLength,
-        incoming_rtp_packet + header.headerLength + REDHeaderLength,
-        payload_data_length - REDHeaderLength);
-
+        incoming_rtp_packet + header.headerLength + RED_header_length,
+        payload_data_length - RED_header_length);
     received_packet->pkt->length =
-        header.headerLength + payload_data_length - REDHeaderLength;
+        header.headerLength + payload_data_length - RED_header_length;
   }
 
   if (received_packet->pkt->length == 0) {
     return 0;
   }
 
-  received_packet_list_.push_back(std::move(received_packet));
-  if (second_received_packet) {
-    received_packet_list_.push_back(std::move(second_received_packet));
+  received_packets_.push_back(std::move(received_packet));
+  if (received_packet2) {
+    received_packets_.push_back(std::move(received_packet));
   }
   return 0;
 }
 
 int32_t FecReceiverImpl::ProcessReceivedFec() {
   crit_sect_.Enter();
-  if (!received_packet_list_.empty()) {
+  if (!received_packets_.empty()) {
     // Send received media packet to VCM.
-    if (!received_packet_list_.front()->is_fec) {
+    if (!received_packets_.front()->is_fec) {
       ForwardErrorCorrection::Packet* packet =
-          received_packet_list_.front()->pkt;
+          received_packets_.front()->pkt;
       crit_sect_.Leave();
       if (!recovered_packet_callback_->OnRecoveredPacket(packet->data,
                                                          packet->length)) {
@@ -227,14 +214,14 @@ int32_t FecReceiverImpl::ProcessReceivedFec() {
       }
       crit_sect_.Enter();
     }
-    if (fec_->DecodeFec(&received_packet_list_, &recovered_packet_list_) != 0) {
+    if (fec_.DecodeFec(&received_packets_, &recovered_packets_) != 0) {
       crit_sect_.Leave();
       return -1;
     }
-    RTC_DCHECK(received_packet_list_.empty());
+    RTC_DCHECK(received_packets_.empty());
   }
   // Send any recovered media packets to VCM.
-  for(auto& recovered_packet : recovered_packet_list_) {
+  for (auto& recovered_packet : recovered_packets_) {
     if (recovered_packet->returned) {
       // Already sent to the VCM and the jitter buffer.
       continue;