From 3efe1a3a90c14ce339e22c451d48f4f7e831c75b Mon Sep 17 00:00:00 2001
From: Danil Chapovalov <danilchap@webrtc.org>
Date: Tue, 13 Oct 2026 11:00:54 +0200
Subject: [PATCH] Spanify and thus harden forward error correction code

Bug: chromium:570927027
Change-Id: I36cc3771838fc6e0524a4c8a1e37528eb30d769e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/508300
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#48878}
---
diff --git a/modules/rtp_rtcp/source/fec_private_tables_bursty.cc b/modules/rtp_rtcp/source/fec_private_tables_bursty.cc
index 84a4857..52ddbef 100644
--- a/modules/rtp_rtcp/source/fec_private_tables_bursty.cc
+++ b/modules/rtp_rtcp/source/fec_private_tables_bursty.cc
@@ -10,6 +10,7 @@
 
 #include "modules/rtp_rtcp/source/fec_private_tables_bursty.h"
 
+#include <array>
 #include <cstdint>
 
 namespace webrtc {
@@ -643,7 +644,7 @@
 
 namespace fec_private_tables {
 
-const uint8_t kPacketMaskBurstyTbl[] = {
+constexpr std::array<uint8_t, kPacketMaskBurstyTblSize> kPacketMaskBurstyTbl = {
     12,
     kPacketMaskBursty1,
     kPacketMaskBursty2,
@@ -659,5 +660,11 @@
     kPacketMaskBursty12,
 };
 
+// Check size is set correctly. If size is too small, compiler would warn there
+// are excess elements in struct initializer. If size is too large, compiler
+// would initialize unspecfied values with default value 0. Check size is not
+// too large by checking last element wasn't default initialized to 0.
+static_assert(kPacketMaskBurstyTbl.back() != 0);
+
 }  // namespace fec_private_tables
 }  // namespace webrtc
diff --git a/modules/rtp_rtcp/source/fec_private_tables_bursty.h b/modules/rtp_rtcp/source/fec_private_tables_bursty.h
index 217d950..99729ea 100644
--- a/modules/rtp_rtcp/source/fec_private_tables_bursty.h
+++ b/modules/rtp_rtcp/source/fec_private_tables_bursty.h
@@ -25,12 +25,23 @@
 // (i.e., more packets/symbols in the code, so larger (k,m), i.e.,  k > 4,
 // m > 3).
 
-#include <stdint.h>
+#include <array>
+#include <cstddef>
+#include <cstdint>
 
 namespace webrtc {
 namespace fec_private_tables {
 
-extern const uint8_t kPacketMaskBurstyTbl[];
+// Size of the flattened table for up to N=12 media packets.
+// The flattened table keeps 12 square tables sizes from 1 to 12.
+// Sum of all squared numbers N^2 from 1 to 12: 650.
+// Each line in each table is prepended by size.
+// Sum of all numbers N from 1 to 12: 78
+// Each 2d table is prepended by its size, adding 12 more entries.
+// Table itself is prepended by the number of subtables, adding 1 more entry.
+// Total number of entries = 1 + 12 + 78 + 650 = 741.
+inline constexpr size_t kPacketMaskBurstyTblSize = 741;
+extern const std::array<uint8_t, kPacketMaskBurstyTblSize> kPacketMaskBurstyTbl;
 
 }  // namespace fec_private_tables
 }  // namespace webrtc
diff --git a/modules/rtp_rtcp/source/fec_private_tables_random.cc b/modules/rtp_rtcp/source/fec_private_tables_random.cc
index b5cd8bc..88bcb93 100644
--- a/modules/rtp_rtcp/source/fec_private_tables_random.cc
+++ b/modules/rtp_rtcp/source/fec_private_tables_random.cc
@@ -10,6 +10,7 @@
 
 #include "modules/rtp_rtcp/source/fec_private_tables_random.h"
 
+#include <array>
 #include <cstdint>
 
 namespace webrtc {
@@ -643,7 +644,7 @@
 
 namespace fec_private_tables {
 
-const uint8_t kPacketMaskRandomTbl[] = {
+constexpr std::array<uint8_t, kPacketMaskRandomTblSize> kPacketMaskRandomTbl = {
     12,
     kPacketMaskRandom1,  // 2 byte entries.
     kPacketMaskRandom2,
@@ -659,5 +660,11 @@
     kPacketMaskRandom12,
 };
 
+// Check size is set correctly. If size is too small, compiler would warn there
+// are excess elements in struct initializer. If size is too large, compiler
+// would initialize unspecfied values with default value 0. Check size is not
+// too large by checking last element wasn't default initialized to 0.
+static_assert(kPacketMaskRandomTbl.back() != 0);
+
 }  // namespace fec_private_tables
 }  // namespace webrtc
diff --git a/modules/rtp_rtcp/source/fec_private_tables_random.h b/modules/rtp_rtcp/source/fec_private_tables_random.h
index cc7b929..674ff06 100644
--- a/modules/rtp_rtcp/source/fec_private_tables_random.h
+++ b/modules/rtp_rtcp/source/fec_private_tables_random.h
@@ -15,12 +15,23 @@
 // this table are specifically designed to favor recovery to random loss.
 // These packet masks are defined to protect up to maximum of 48 media packets.
 
-#include <stdint.h>
+#include <array>
+#include <cstddef>
+#include <cstdint>
 
 namespace webrtc {
 namespace fec_private_tables {
 
-extern const uint8_t kPacketMaskRandomTbl[];
+// Size of the flattened table for up to N=12 media packets.
+// The flattened table keeps 12 square tables sizes from 1 to 12.
+// Sum of all squared numbers N^2 from 1 to 12: 650.
+// Each line in each table is prepended by its size.
+// Sum of all numbers N from 1 to 12: 78
+// Each 2d table is prepended by its size, adding 12 more entries.
+// Table itself is prepended by the number of subtables, adding 1 more entry.
+// Total number of entries = 1 + 12 + 78 + 650 = 741.
+inline constexpr size_t kPacketMaskRandomTblSize = 741;
+extern const std::array<uint8_t, kPacketMaskRandomTblSize> kPacketMaskRandomTbl;
 
 }  // namespace fec_private_tables
 }  // namespace webrtc
diff --git a/modules/rtp_rtcp/source/forward_error_correction.cc b/modules/rtp_rtcp/source/forward_error_correction.cc
index 0bb81a8..0e31bf1 100644
--- a/modules/rtp_rtcp/source/forward_error_correction.cc
+++ b/modules/rtp_rtcp/source/forward_error_correction.cc
@@ -15,6 +15,7 @@
 #include <cstring>
 #include <list>
 #include <memory>
+#include <span>
 #include <utility>
 #include <vector>
 
@@ -26,6 +27,7 @@
 #include "modules/rtp_rtcp/source/flexfec_03_header_reader_writer.h"
 #include "modules/rtp_rtcp/source/forward_error_correction_internal.h"
 #include "modules/rtp_rtcp/source/ulpfec_header_reader_writer.h"
+#include "rtc_base/byte_order.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/numerics/mod_ops.h"
@@ -167,7 +169,9 @@
 
   internal::PacketMaskTable mask_table(fec_mask_type, num_media_packets);
   packet_mask_size_ = internal::PacketMaskSize(num_media_packets);
-  memset(packet_masks_, 0, num_fec_packets * packet_mask_size_);
+  std::ranges::fill(
+      std::span(packet_masks_).first(num_fec_packets * packet_mask_size_), 0);
+
   internal::GeneratePacketMasks(num_media_packets, num_fec_packets,
                                 num_important_packets, use_unequal_protection,
                                 &mask_table, packet_masks_);
@@ -186,9 +190,9 @@
   GenerateFecPayloads(media_packets, num_fec_packets);
   // TODO(brandtr): Generalize this when multistream protection support is
   // added.
-  const uint32_t media_ssrc = ParseSsrc(media_packets.front()->data.data());
+  const uint32_t media_ssrc = ParseSsrc(media_packets.front()->data);
   const uint16_t seq_num_base =
-      ParseSequenceNumber(media_packets.front()->data.data());
+      ParseSequenceNumber(media_packets.front()->data);
   FinalizeFecHeaders(num_fec_packets, media_ssrc, seq_num_base);
 
   return 0;
@@ -220,8 +224,7 @@
 
     size_t media_pkt_idx = 0;
     auto media_packets_it = media_packets.cbegin();
-    uint16_t prev_seq_num =
-        ParseSequenceNumber((*media_packets_it)->data.data());
+    uint16_t prev_seq_num = ParseSequenceNumber((*media_packets_it)->data);
     while (media_packets_it != media_packets.end()) {
       Packet* const media_packet = media_packets_it->get();
       // Should `media_packet` be protected by `fec_packet`?
@@ -233,8 +236,8 @@
         if (fec_packet_length > fec_packet->data.size()) {
           size_t old_size = fec_packet->data.size();
           fec_packet->data.SetSize(fec_packet_length);
-          memset(fec_packet->data.MutableData() + old_size, 0,
-                 fec_packet_length - old_size);
+          std::ranges::fill(fec_packet->data.AsMutableSpan().subspan(old_size),
+                            0);
         }
         XorHeaders(*media_packet, fec_packet);
         XorPayloads(*media_packet, media_payload_length, fec_header_size,
@@ -242,8 +245,7 @@
       }
       media_packets_it++;
       if (media_packets_it != media_packets.end()) {
-        uint16_t seq_num =
-            ParseSequenceNumber((*media_packets_it)->data.data());
+        uint16_t seq_num = ParseSequenceNumber((*media_packets_it)->data);
         media_pkt_idx += static_cast<uint16_t>(seq_num - prev_seq_num);
         prev_seq_num = seq_num;
       }
@@ -262,10 +264,8 @@
   if (num_media_packets <= 1) {
     return num_media_packets;
   }
-  uint16_t last_seq_num =
-      ParseSequenceNumber(media_packets.back()->data.data());
-  uint16_t first_seq_num =
-      ParseSequenceNumber(media_packets.front()->data.data());
+  uint16_t last_seq_num = ParseSequenceNumber(media_packets.back()->data);
+  uint16_t first_seq_num = ParseSequenceNumber(media_packets.front()->data);
   size_t total_missing_seq_nums =
       static_cast<uint16_t>(last_seq_num - first_seq_num) - num_media_packets +
       1;
@@ -281,7 +281,9 @@
   // Allocate the new mask.
   size_t tmp_packet_mask_size =
       internal::PacketMaskSize(total_missing_seq_nums + num_media_packets);
-  memset(tmp_packet_masks_, 0, num_fec_packets * tmp_packet_mask_size);
+  std::ranges::fill(std::span(tmp_packet_masks_)
+                        .first(num_fec_packets * tmp_packet_mask_size),
+                    0);
 
   auto media_packets_it = media_packets.cbegin();
   uint16_t prev_seq_num = first_seq_num;
@@ -298,7 +300,7 @@
       // We can only cover up to 48 packets.
       break;
     }
-    uint16_t seq_num = ParseSequenceNumber((*media_packets_it)->data.data());
+    uint16_t seq_num = ParseSequenceNumber((*media_packets_it)->data);
     const int num_zeros_to_insert =
         static_cast<uint16_t>(seq_num - prev_seq_num - 1);
     if (num_zeros_to_insert > 0) {
@@ -323,8 +325,9 @@
     }
   }
   // Replace the old mask with the new.
-  memcpy(packet_masks_, tmp_packet_masks_,
-         num_fec_packets * tmp_packet_mask_size);
+  std::ranges::copy(std::span(tmp_packet_masks_)
+                        .first(num_fec_packets * tmp_packet_mask_size),
+                    packet_masks_.begin());
   return new_bit_index;
 }
 
@@ -565,20 +568,19 @@
   }
   // Initialize recovered packet data.
   recovered_packet->pkt->data.EnsureCapacity(IP_PACKET_SIZE);
-  recovered_packet->pkt->data.SetSize(fec_packet.protection_length +
-                                      kRtpHeaderSize);
+
   recovered_packet->returned = false;
   recovered_packet->was_recovered = true;
   // Copy bytes corresponding to minimum RTP header size.
   // Note that the sequence number and SSRC fields will be overwritten
   // at the end of packet recovery.
-  memcpy(recovered_packet->pkt->data.MutableData(),
-         fec_packet.pkt->data.cdata(), kRtpHeaderSize);
+  recovered_packet->pkt->data.Set(
+      std::span(fec_packet.pkt->data).first(kRtpHeaderSize));
   // Copy remaining FEC payload.
   if (fec_packet.protection_length > 0) {
-    memcpy(recovered_packet->pkt->data.MutableData() + kRtpHeaderSize,
-           fec_packet.pkt->data.cdata() + fec_packet.fec_header_size,
-           fec_packet.protection_length);
+    recovered_packet->pkt->data.Append(
+        std::span(fec_packet.pkt->data)
+            .subspan(fec_packet.fec_header_size, fec_packet.protection_length));
   }
   return true;
 }
@@ -586,13 +588,12 @@
 bool ForwardErrorCorrection::FinishPacketRecovery(
     const ReceivedFecPacket& /* fec_packet */,
     RecoveredPacket* recovered_packet) {
-  uint8_t* data = recovered_packet->pkt->data.MutableData();
+  std::span<uint8_t> data = recovered_packet->pkt->data.AsMutableSpan();
   // Set the RTP version to 2.
   data[0] |= 0x80;  // Set the 1st bit.
   data[0] &= 0xbf;  // Clear the 2nd bit.
   // Recover the packet length, from temporary location.
-  const size_t new_size =
-      ByteReader<uint16_t>::ReadBigEndian(&data[2]) + kRtpHeaderSize;
+  const size_t new_size = GetBE16(data.subspan(2)) + kRtpHeaderSize;
   if (new_size > size_t{IP_PACKET_SIZE - kRtpHeaderSize}) {
     RTC_LOG(LS_WARNING) << "The recovered packet had a length larger than a "
                            "typical IP packet, and is thus dropped.";
@@ -600,21 +601,21 @@
   }
   size_t old_size = recovered_packet->pkt->data.size();
   recovered_packet->pkt->data.SetSize(new_size);
-  data = recovered_packet->pkt->data.MutableData();
+  data = recovered_packet->pkt->data.AsMutableSpan();
   if (new_size > old_size) {
-    memset(data + old_size, 0, new_size - old_size);
+    std::ranges::fill(data.subspan(old_size), 0);
   }
 
   // Set the SN field.
-  ByteWriter<uint16_t>::WriteBigEndian(&data[2], recovered_packet->seq_num);
+  SetBE16(data.subspan(2), recovered_packet->seq_num);
   // Set the SSRC field.
-  ByteWriter<uint32_t>::WriteBigEndian(&data[8], recovered_packet->ssrc);
+  SetBE32(data.subspan(8), recovered_packet->ssrc);
   return true;
 }
 
 void ForwardErrorCorrection::XorHeaders(const Packet& src, Packet* dst) {
-  uint8_t* dst_data = dst->data.MutableData();
-  const uint8_t* src_data = src.data.cdata();
+  std::span<uint8_t> dst_data = dst->data.AsMutableSpan();
+  std::span<const uint8_t> src_data = src.data;
   // XOR the first 2 bytes of the header: V, P, X, CC, M, PT fields.
   dst_data[0] ^= src_data[0];
   dst_data[1] ^= src_data[1];
@@ -644,12 +645,11 @@
   RTC_DCHECK_LE(dst_offset + payload_length, dst->data.capacity());
   if (dst_offset + payload_length > dst->data.size()) {
     size_t old_size = dst->data.size();
-    size_t new_size = dst_offset + payload_length;
-    dst->data.SetSize(new_size);
-    memset(dst->data.MutableData() + old_size, 0, new_size - old_size);
+    dst->data.SetSize(dst_offset + payload_length);
+    std::ranges::fill(dst->data.AsMutableSpan().subspan(old_size), 0);
   }
-  uint8_t* dst_data = dst->data.MutableData();
-  const uint8_t* src_data = src.data.cdata();
+  std::span<uint8_t> dst_data = dst->data.AsMutableSpan();
+  std::span<const uint8_t> src_data = src.data;
   for (size_t i = 0; i < payload_length; ++i) {
     dst_data[dst_offset + i] ^= src_data[kRtpHeaderSize + i];
   }
@@ -774,12 +774,13 @@
           kOldSequenceThreshold);
 }
 
-uint16_t ForwardErrorCorrection::ParseSequenceNumber(const uint8_t* packet) {
-  return (packet[2] << 8) + packet[3];
+uint16_t ForwardErrorCorrection::ParseSequenceNumber(
+    std::span<const uint8_t> packet) {
+  return GetBE16(packet.subspan(2));
 }
 
-uint32_t ForwardErrorCorrection::ParseSsrc(const uint8_t* packet) {
-  return (packet[8] << 24) + (packet[9] << 16) + (packet[10] << 8) + packet[11];
+uint32_t ForwardErrorCorrection::ParseSsrc(std::span<const uint8_t> packet) {
+  return GetBE32(packet.subspan(8));
 }
 
 ForwardErrorCorrection::DecodeFecResult ForwardErrorCorrection::DecodeFec(
diff --git a/modules/rtp_rtcp/source/forward_error_correction.h b/modules/rtp_rtcp/source/forward_error_correction.h
index fcfa641..a50064bc 100644
--- a/modules/rtp_rtcp/source/forward_error_correction.h
+++ b/modules/rtp_rtcp/source/forward_error_correction.h
@@ -14,6 +14,7 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include <array>
 #include <list>
 #include <memory>
 #include <span>
@@ -250,8 +251,8 @@
 
   // TODO(brandtr): Remove these functions when the Packet classes
   // have been refactored.
-  static uint16_t ParseSequenceNumber(const uint8_t* packet);
-  static uint32_t ParseSsrc(const uint8_t* packet);
+  static uint16_t ParseSequenceNumber(std::span<const uint8_t> packet);
+  static uint32_t ParseSsrc(std::span<const uint8_t> packet);
 
  protected:
   ForwardErrorCorrection(std::unique_ptr<FecHeaderReader> fec_header_reader,
@@ -363,8 +364,10 @@
   // Arrays used to avoid dynamically allocating memory when generating
   // the packet masks.
   // (There are never more than `kUlpfecMaxMediaPackets` FEC packets generated.)
-  uint8_t packet_masks_[kUlpfecMaxMediaPackets * kUlpfecMaxPacketMaskSize];
-  uint8_t tmp_packet_masks_[kUlpfecMaxMediaPackets * kUlpfecMaxPacketMaskSize];
+  std::array<uint8_t, kUlpfecMaxMediaPackets * kUlpfecMaxPacketMaskSize>
+      packet_masks_;
+  std::array<uint8_t, kUlpfecMaxMediaPackets * kUlpfecMaxPacketMaskSize>
+      tmp_packet_masks_;
   size_t packet_mask_size_;
 };
 

diff --git a/modules/rtp_rtcp/source/fec_private_tables_bursty_unittest.cc b/modules/rtp_rtcp/source/fec_private_tables_bursty_unittest.cc
index b14e2fb..bda185c 100644
--- a/modules/rtp_rtcp/source/fec_private_tables_bursty_unittest.cc
+++ b/modules/rtp_rtcp/source/fec_private_tables_bursty_unittest.cc
@@ -10,6 +10,7 @@
 
 #include "modules/rtp_rtcp/source/fec_private_tables_bursty.h"
 
+#include <array>
 #include <cstddef>
 #include <cstdint>
 #include <span>
@@ -21,8 +22,8 @@
 
 namespace webrtc {
 namespace {
-constexpr uint8_t kMaskRandom15_6[] = {0x82, 0x08, 0x41, 0x04, 0x20, 0x82,
-                                       0x10, 0x40, 0x08, 0x20, 0x04, 0x10};
+constexpr std::array<uint8_t, 12> kMaskRandom15_6 = {
+    0x82, 0x08, 0x41, 0x04, 0x20, 0x82, 0x10, 0x40, 0x08, 0x20, 0x04, 0x10};
 }  // namespace
 
 namespace fec_private_tables {
@@ -31,24 +32,24 @@
 
 TEST(FecTable, TestBurstyLookup) {
   std::span<const uint8_t> result;
-  result = LookUpInFecTable(&kPacketMaskBurstyTbl[0], 0, 0);
+  result = LookUpInFecTable(kPacketMaskBurstyTbl, 0, 0);
   // Should match kMaskBursty1_1.
   EXPECT_EQ(2u, result.size());
   EXPECT_EQ(0x80u, result[0]);
 
-  result = LookUpInFecTable(&kPacketMaskBurstyTbl[0], 3, 0);
+  result = LookUpInFecTable(kPacketMaskBurstyTbl, 3, 0);
   // Should match kMaskBursty4_1.
   EXPECT_EQ(2u, result.size());
   EXPECT_EQ(0xf0u, result[0]);
   EXPECT_EQ(0x00u, result[1]);
 
-  result = LookUpInFecTable(&kPacketMaskBurstyTbl[0], 1, 1);
+  result = LookUpInFecTable(kPacketMaskBurstyTbl, 1, 1);
   // Should match kMaskBursty2_2.
   EXPECT_EQ(4u, result.size());
   EXPECT_EQ(0x80u, result[0]);
   EXPECT_EQ(0xc0u, result[2]);
 
-  result = LookUpInFecTable(&kPacketMaskBurstyTbl[0], 11, 11);
+  result = LookUpInFecTable(kPacketMaskBurstyTbl, 11, 11);
   // Should match kMaskBursty12_12.
   EXPECT_EQ(24u, result.size());
   EXPECT_EQ(0x80u, result[0]);
@@ -57,12 +58,12 @@
 
 TEST(FecTable, TestRandomLookup) {
   std::span<const uint8_t> result;
-  result = LookUpInFecTable(&kPacketMaskRandomTbl[0], 0, 0);
+  result = LookUpInFecTable(kPacketMaskRandomTbl, 0, 0);
   EXPECT_EQ(2u, result.size());
   EXPECT_EQ(0x80u, result[0]);
   EXPECT_EQ(0x00u, result[1]);
 
-  result = LookUpInFecTable(&kPacketMaskRandomTbl[0], 4, 1);
+  result = LookUpInFecTable(kPacketMaskRandomTbl, 4, 1);
   // kMaskRandom5_2.
   EXPECT_EQ(4u, result.size());
   EXPECT_EQ(0xa8u, result[0]);
diff --git a/modules/rtp_rtcp/test/testFec/test_fec.cc b/modules/rtp_rtcp/test/testFec/test_fec.cc
index 067fa0c..2f66abf 100644
--- a/modules/rtp_rtcp/test/testFec/test_fec.cc
+++ b/modules/rtp_rtcp/test/testFec/test_fec.cc
@@ -13,12 +13,14 @@
  * functions in ForwardErrorCorrection directly.
  */
 
+#include <algorithm>
 #include <cstdint>
 #include <cstdio>
 #include <cstring>
 #include <ctime>
 #include <list>
 #include <memory>
+#include <span>
 #include <string>
 #include <utility>
 #include <vector>
@@ -168,8 +170,7 @@
              mask_type_idx);
 
       const uint32_t packet_mask_max = kMaxMediaPackets[mask_type_idx];
-      std::unique_ptr<uint8_t[]> packet_mask(
-          new uint8_t[packet_mask_max * kNumMaskBytesL1]);
+      std::vector<uint8_t> packet_mask(packet_mask_max * kNumMaskBytesL1);
 
       FecMaskType fec_mask_type = kMaskTypes[mask_type_idx];
 
@@ -194,13 +195,15 @@
             const uint32_t mask_bytes_per_fec_packet =
                 (num_media_packets > 16) ? kNumMaskBytesL1 : kNumMaskBytesL0;
 
-            memset(packet_mask.get(), 0,
-                   num_media_packets * mask_bytes_per_fec_packet);
+            std::ranges::fill(
+                std::span(packet_mask)
+                    .first(num_media_packets * mask_bytes_per_fec_packet),
+                0);
 
             // Transfer packet masks from bit-mask to byte-mask.
             internal::GeneratePacketMasks(
                 num_media_packets, num_fec_packets, num_imp_packets,
-                kUseUnequalProtection, &mask_table, packet_mask.get());
+                kUseUnequalProtection, &mask_table, packet_mask);
 
 #ifdef VERBOSE_OUTPUT
             printf(