Move InsertZeroColumns and CopyColumn to ::internal. (pt. 1)

webrtc/modules/rtp_rtcp/source/forward_error_correction.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 288 matching lines @@
       total_missing_seq_nums > 8 * kMaskSizeLBitClear) {
     new_mask_bytes = kMaskSizeLBitSet;
   }
   memset(tmp_packet_mask_, 0, num_fec_packets * kMaskSizeLBitSet);
 
   auto media_packets_it = media_packets.cbegin();
   uint16_t prev_seq_num = first_seq_num;
   ++media_packets_it;
 
   // Insert the first column.
-  CopyColumn(tmp_packet_mask_, new_mask_bytes, packet_mask, num_mask_bytes,
-             num_fec_packets, 0, 0);
-  int new_bit_index = 1;
-  int old_bit_index = 1;
+  internal::CopyColumn(tmp_packet_mask_, new_mask_bytes, packet_mask_,
+                       num_mask_bytes, num_fec_packets, 0, 0);
+  size_t new_bit_index = 1;
+  size_t old_bit_index = 1;
   // Insert zeros in the bit mask for every hole in the sequence.
   while (media_packets_it != media_packets.end()) {
     if (new_bit_index == 8 * kMaskSizeLBitSet) {
       // We can only cover up to 48 packets.
       break;
     }
     uint16_t seq_num = ParseSequenceNumber((*media_packets_it)->data);
-    const int zeros_to_insert =
+    const uint16_t num_zeros_to_insert =

[danilchap, 2016/08/23 08:43:00]

probably better to leave this type as int:
variable represent count that is better to type as size_t or as int.
InsertZeroColumns accepts it as int, so int look like better choice.
cast next line is done for proper wrapping, not for proper typing.
Alternatively change it to size_t (like with _bit_indexes) if you plan to change
InsertZeroColumns's signature to take size_t as paremeters.

[brandtr, 2016/08/23 09:26:02]

Good call, I think I misunderstood line 321. Changing back to int for now.
Changing the types (and other general cleanup) in _internal is on my todo list,
but will have to wait for later on.

         static_cast<uint16_t>(seq_num - prev_seq_num - 1);
-    if (zeros_to_insert > 0) {
-      InsertZeroColumns(zeros_to_insert, tmp_packet_mask_, new_mask_bytes,
-                        num_fec_packets, new_bit_index);
+    if (num_zeros_to_insert > 0) {
+      internal::InsertZeroColumns(num_zeros_to_insert, tmp_packet_mask_,
+                                  new_mask_bytes, num_fec_packets,
+                                  new_bit_index);
     }
-    new_bit_index += zeros_to_insert;
-    CopyColumn(tmp_packet_mask_, new_mask_bytes, packet_mask, num_mask_bytes,
-               num_fec_packets, new_bit_index, old_bit_index);
+    new_bit_index += num_zeros_to_insert;
+    internal::CopyColumn(tmp_packet_mask_, new_mask_bytes, packet_mask_,
+                         num_mask_bytes, num_fec_packets, new_bit_index,
+                         old_bit_index);
     ++new_bit_index;
     ++old_bit_index;
     prev_seq_num = seq_num;
     ++media_packets_it;
   }
   if (new_bit_index % 8 != 0) {
     // We didn't fill the last byte. Shift bits to correct position.
     for (uint16_t row = 0; row < num_fec_packets; ++row) {
       int new_byte_index = row * new_mask_bytes + new_bit_index / 8;
       tmp_packet_mask_[new_byte_index] <<= (7 - (new_bit_index % 8));
     }
   }
   // Replace the old mask with the new.
   memcpy(packet_mask, tmp_packet_mask_, kMaskSizeLBitSet * num_fec_packets);
   return new_bit_index;
 }
 
-void ForwardErrorCorrection::InsertZeroColumns(int num_zeros,
-                                               uint8_t* new_mask,
-                                               int new_mask_bytes,
-                                               int num_fec_packets,
-                                               int new_bit_index) {
-  for (uint16_t row = 0; row < num_fec_packets; ++row) {
-    const int new_byte_index = row * new_mask_bytes + new_bit_index / 8;
-    const int max_shifts = (7 - (new_bit_index % 8));
-    new_mask[new_byte_index] <<= std::min(num_zeros, max_shifts);
-  }
-}
-
-void ForwardErrorCorrection::CopyColumn(uint8_t* new_mask,
-                                        int new_mask_bytes,
-                                        uint8_t* old_mask,
-                                        int old_mask_bytes,
-                                        int num_fec_packets,
-                                        int new_bit_index,
-                                        int old_bit_index) {
-  // Copy column from the old mask to the beginning of the new mask and shift it
-  // out from the old mask.
-  for (uint16_t row = 0; row < num_fec_packets; ++row) {
-    int new_byte_index = row * new_mask_bytes + new_bit_index / 8;
-    int old_byte_index = row * old_mask_bytes + old_bit_index / 8;
-    new_mask[new_byte_index] |= ((old_mask[old_byte_index] & 0x80) >> 7);
-    if (new_bit_index % 8 != 7) {
-      new_mask[new_byte_index] <<= 1;
-    }
-    old_mask[old_byte_index] <<= 1;
-  }
-}
-
 void ForwardErrorCorrection::GenerateFecUlpHeaders(
     const PacketList& media_packets,
     uint8_t* packet_mask,
     int num_fec_packets,
     bool l_bit) {
   // -- Generate FEC and ULP headers --
   //
   // FEC Header, 10 bytes
   //    0                   1                   2                   3
   //    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
@@ skipping 415 matching lines @@
   }
   InsertPackets(received_packets, recovered_packets);
   AttemptRecover(recovered_packets);
   return 0;
 }
 
 size_t ForwardErrorCorrection::PacketOverhead() {
   return kFecHeaderSize + kUlpHeaderSizeLBitSet;
 }
 }  // namespace webrtc