Style updates for ForwardErrorCorrection and related classes

webrtc/modules/rtp_rtcp/source/forward_error_correction.h

Index: webrtc/modules/rtp_rtcp/source/forward_error_correction.h
diff --git a/webrtc/modules/rtp_rtcp/source/forward_error_correction.h b/webrtc/modules/rtp_rtcp/source/forward_error_correction.h
index cbeb97e7b1d2876212ca88baae5b6513752e5d96..d328dc6f9247965ed9a1cc059725ff604497ac0b 100644
--- a/webrtc/modules/rtp_rtcp/source/forward_error_correction.h
+++ b/webrtc/modules/rtp_rtcp/source/forward_error_correction.h
@@ -31,7 +31,7 @@ class FecPacket;
 class ForwardErrorCorrection {
  public:
   // Maximum number of media packets we can protect
-  static const unsigned int kMaxMediaPackets = 48u;
+  static const uint8_t kMaxMediaPackets = 48u;

[danilchap, 2016/06/29 10:31:53]

using small int types because value is small is discouraged.
prefer size_t for constants describing size (even if it is small).
Might as well use constexpr instead of const.

[brandtr, 2016/06/29 14:24:11]

Acknowledged.

 
   // TODO(holmer): As a next step all these struct-like packet classes should be
   // refactored into proper classes, and their members should be made private.
@@ -49,7 +49,7 @@ class ForwardErrorCorrection {
     // reaches zero.
     virtual int32_t Release();
 
-    size_t length;               // Length of packet in bytes.
+    size_t length;                 // Length of packet in bytes.
     uint8_t data[IP_PACKET_SIZE];  // Packet data.
 
    private:
@@ -70,13 +70,13 @@ class ForwardErrorCorrection {
   // type. The last_media_pkt_in_frame is not required to be used for correct
   // recovery, but will reduce delay by allowing #DecodeFEC() to pre-emptively
   // determine frame completion. If set, we assume a FEC stream, and the
-  // following assumptions must hold:\n
+  // following assumptions must hold:
   //
   // 1. The media packets in a frame have contiguous sequence numbers, i.e. the
   //    frame's FEC packets have sequence numbers either lower than the first
-  //    media packet or higher than the last media packet.\n
+  //    media packet or higher than the last media packet.
   // 2. All FEC packets have a sequence number base equal to the first media
-  //    packet in the corresponding frame.\n
+  //    packet in the corresponding frame.
   //
   // The ssrc member is needed to ensure we can restore the SSRC field of
   // recovered packets. In most situations this could be retrieved from other
@@ -118,7 +118,6 @@ class ForwardErrorCorrection {
   typedef std::list<RecoveredPacket*> RecoveredPacketList;
 
   ForwardErrorCorrection();
-
   virtual ~ForwardErrorCorrection();
 
   /**
@@ -169,13 +168,13 @@ class ForwardErrorCorrection {
    *  ascending sequence number and have duplicates removed. The function
    *  should be called as new packets arrive, with the recovered list being
    *  progressively assembled with each call. The received packet list will be
-   *  empty at output.\n
+   *  empty at output.
    *
    *  The user will allocate packets submitted through the received list. The
    *  function will handle allocation of recovered packets and optionally
    *  deleting of all packet memory. The user may delete the recovered list
    *  packets, in which case they must remove deleted packets from the
-   *  recovered list.\n
+   *  recovered list.
    *
    * \param[in]  receivedPacketList  List of new received packets, of type
    *                                 #ReceivedPacket, belonging to a single
@@ -209,10 +208,6 @@ class ForwardErrorCorrection {
  private:
   typedef std::list<FecPacket*> FecPacketList;
 
-  void GenerateFecUlpHeaders(const PacketList& media_packet_list,
-                             uint8_t* packet_mask, bool l_bit,
-                             int num_fec_packets);
-
   // Analyzes |media_packets| for holes in the sequence and inserts zero columns
   // into the |packet_mask| where those holes are found. Zero columns means that
   // those packets will have no protection.
@@ -244,6 +239,10 @@ class ForwardErrorCorrection {
                          int num_fec_packets, int new_bit_index,
                          int old_bit_index);
 
+  void GenerateFecUlpHeaders(const PacketList& media_packet_list,
+                             uint8_t* packet_mask, int num_fec_packets,
+                             bool l_bit);
+
   void GenerateFecBitStrings(const PacketList& media_packet_list,
                              uint8_t* packet_mask, int num_fec_packets,
                              bool l_bit);
@@ -279,15 +278,15 @@ class ForwardErrorCorrection {
   void AttemptRecover(RecoveredPacketList* recovered_packet_list);
 
   // Initializes the packet recovery using the FEC packet.
-  static bool InitRecovery(const FecPacket* fec_packet,
-                           RecoveredPacket* recovered);
+  static bool InitRecoveryOfPacket(const FecPacket* fec_packet,

[danilchap, 2016/06/29 10:31:53]

maybe InitPacketRecovery or StartPacketRecovery (since word 'Start' better match
word 'Finish' in the next function)

[brandtr, 2016/06/29 14:24:11]

I like that. Done!

+                                   RecoveredPacket* recovered);
 
   // Performs XOR between |src_packet| and |dst_packet| and stores the result
   // in |dst_packet|.
   static void XorPackets(const Packet* src_packet, RecoveredPacket* dst_packet);
 
   // Finish up the recovery of a packet.
-  static bool FinishRecovery(RecoveredPacket* recovered);
+  static bool FinishRecoveryOfPacket(RecoveredPacket* recovered);

[danilchap, 2016/06/29 10:31:53]

may be FinishPacketRecovery?
just a suggestion, feel free to disagree if you prefer your name.

[brandtr, 2016/06/29 14:24:11]

Done.

 
   // Recover a missing packet.
   bool RecoverPacket(const FecPacket* fec_packet,
@@ -305,7 +304,6 @@ class ForwardErrorCorrection {
 
   std::vector<Packet> generated_fec_packets_;
   FecPacketList fec_packet_list_;
-  bool fec_packet_received_;
 };
 }  // namespace webrtc
 #endif  // WEBRTC_MODULES_RTP_RTCP_SOURCE_FORWARD_ERROR_CORRECTION_H_