Generalize FEC header formatting. (pt. 4)

webrtc/modules/rtp_rtcp/source/forward_error_correction_internal.cc

Index: webrtc/modules/rtp_rtcp/source/forward_error_correction_internal.cc
diff --git a/webrtc/modules/rtp_rtcp/source/forward_error_correction_internal.cc b/webrtc/modules/rtp_rtcp/source/forward_error_correction_internal.cc
index 790e705bc4aa4e9505496e84d8e9e7cf556fdead..ec8d0913313dea17d343d1778e48aaf6ff1aaa27 100644
--- a/webrtc/modules/rtp_rtcp/source/forward_error_correction_internal.cc
+++ b/webrtc/modules/rtp_rtcp/source/forward_error_correction_internal.cc
@@ -15,6 +15,7 @@
 
 #include <algorithm>
 
+#include "webrtc/base/checks.h"
 #include "webrtc/modules/rtp_rtcp/source/fec_private_tables_bursty.h"
 #include "webrtc/modules/rtp_rtcp/source/fec_private_tables_random.h"
 
@@ -205,8 +206,8 @@ void RemainingPacketProtection(int num_media_packets,
     const int l_bit =
         (num_media_packets - num_fec_for_imp_packets) > 16 ? 1 : 0;
 
-    const int res_mask_bytes =
-        (l_bit == 1) ? kMaskSizeLBitSet : kMaskSizeLBitClear;
+    const int res_mask_bytes = (l_bit == 1) ? kUlpfecPacketMaskSizeLBitSet
+                                            : kUlpfecPacketMaskSizeLBitClear;
 
     const uint8_t* packet_mask_sub_21 =
         mask_table.fec_packet_mask_table()[num_media_packets -
@@ -246,8 +247,8 @@ void ImportantPacketProtection(int num_fec_for_imp_packets,
                                uint8_t* packet_mask,
                                const PacketMaskTable& mask_table) {
   const int l_bit = num_imp_packets > 16 ? 1 : 0;
-  const int num_imp_mask_bytes =
-      (l_bit == 1) ? kMaskSizeLBitSet : kMaskSizeLBitClear;
+  const int num_imp_mask_bytes = (l_bit == 1) ? kUlpfecPacketMaskSizeLBitSet
+                                              : kUlpfecPacketMaskSizeLBitClear;
 
   // Get sub_mask1 from table
   const uint8_t* packet_mask_sub_1 =
@@ -376,8 +377,8 @@ void GeneratePacketMasks(int num_media_packets,
   assert(num_imp_packets <= num_media_packets && num_imp_packets >= 0);
 
   int l_bit = num_media_packets > 16 ? 1 : 0;
-  const int num_mask_bytes =
-      (l_bit == 1) ? kMaskSizeLBitSet : kMaskSizeLBitClear;
+  const int num_mask_bytes = (l_bit == 1) ? kUlpfecPacketMaskSizeLBitSet
+                                          : kUlpfecPacketMaskSizeLBitClear;
 
   // Equal-protection for these cases.
   if (!use_unequal_protection || num_imp_packets == 0) {
@@ -394,6 +395,15 @@ void GeneratePacketMasks(int num_media_packets,
   }  // End of UEP modification
 }  // End of GetPacketMasks
 
+size_t PacketMaskSize(size_t seq_num_spread) {
+  RTC_DCHECK_LE(seq_num_spread, 8 * kUlpfecPacketMaskSizeLBitSet);

[danilchap, 2016/08/25 17:49:15]

that is kUlpfecMaxMediaPackets?

may be add semipublic constants
constexpr size_t kUlpfecMaxPacketMaskSize =
   std::max(kUlpFecPacketMaskSizeLBitSet, kUlpFecPacketMaskSizeLBitClear);
constexpr size_t kUlpfecMaxMediaPackets = 8 * kUlpfecPacketMaskSizeMax;  // 48.
and make 
kUlpFecPacketMaskSizeLBitSet, kUlpFecPacketMaskSizeLBitClear more internal.
(does normal reader need to know what LBit is?)

[brandtr, 2016/08/26 11:22:22]

This is a really nice idea, but seems that std::max is only constexpr in
C++14... :(
http://en.cppreference.com/w/cpp/algorithm/min

When it comes to reading the packet mask code, I think the normal reader will
need to check the header format, and thus know what the L bit is.

+  if (seq_num_spread > 8 * kUlpfecPacketMaskSizeLBitClear) {
+    return kUlpfecPacketMaskSizeLBitSet;
+  } else {

[danilchap, 2016/08/25 17:49:15]

do not use else after return:
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

[brandtr, 2016/08/26 11:22:22]

Done.

+    return kUlpfecPacketMaskSizeLBitClear;
+  }
+}
+
 void InsertZeroColumns(int num_zeros,
                        uint8_t* new_mask,
                        int new_mask_bytes,