Revert "Reland "Refactors UlpFec and FlexFec to use a common interface.""

This reverts commit 49734dc0faa69616a58a1a95c7fc61a4610793cf.

Reason for revert: Still something wrong with ulpfec fuzzer setup.

Original change's description:
> Reland "Refactors UlpFec and FlexFec to use a common interface."
> 
> This is a reland of 11af1d7444fd7438766b7bc52cbd64752d72e32e
> 
> Original change's description:
> > Refactors UlpFec and FlexFec to use a common interface.
> >
> > The new VideoFecGenerator is now injected into RtpSenderVideo,
> > and generalizes the usage.
> > This also prepares for being able to genera FEC in the RTP egress
> > module.
> >
> > Bug: webrtc:11340
> > Change-Id: I8aa873129b2fb4131eb3399ee88f6ea2747155a3
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168347
> > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> > Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
> > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#30515}
> 
> Bug: webrtc:11340, chromium:1052323
> Change-Id: Id646047365f1c46cca9e6f3e8eefa5151207b4a0
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168608
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#30593}

TBR=sprang@webrtc.org,stefan@webrtc.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:11340, chromium:1052323
Change-Id: I920ce0a48a08768d7a98a563e2b66bd6eb8602b7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/169121
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30616}

call/rtp_video_sender.cc

@@ -36,13 +36,9 @@ namespace webrtc {
 
 namespace webrtc_internal_rtp_video_sender {
 
-RtpStreamSender::RtpStreamSender(
+RtpStreamSender::RtpStreamSender(std::unique_ptr<RtpRtcp> rtp_rtcp,
-    std::unique_ptr<RtpRtcp> rtp_rtcp,
+                                 std::unique_ptr<RTPSenderVideo> sender_video)
-    std::unique_ptr<RTPSenderVideo> sender_video,
+    : rtp_rtcp(std::move(rtp_rtcp)), sender_video(std::move(sender_video)) {}
-    std::unique_ptr<VideoFecGenerator> fec_generator)
-    : rtp_rtcp(std::move(rtp_rtcp)),
-      sender_video(std::move(sender_video)),
-      fec_generator(std::move(fec_generator)) {}
 
 RtpStreamSender::~RtpStreamSender() = default;
 
@@ -117,67 +113,6 @@ bool ShouldDisableRedAndUlpfec(bool flexfec_enabled,
   return should_disable_red_and_ulpfec;
 }
 
-// TODO(brandtr): Update this function when we support multistream protection.
-std::unique_ptr<VideoFecGenerator> MaybeCreateFecGenerator(
-    Clock* clock,
-    const RtpConfig& rtp,
-    const std::map<uint32_t, RtpState>& suspended_ssrcs,
-    int simulcast_index) {
-  // If flexfec is configured that takes priority.
-  if (rtp.flexfec.payload_type >= 0) {
-    RTC_DCHECK_GE(rtp.flexfec.payload_type, 0);
-    RTC_DCHECK_LE(rtp.flexfec.payload_type, 127);
-    if (rtp.flexfec.ssrc == 0) {
-      RTC_LOG(LS_WARNING) << "FlexFEC is enabled, but no FlexFEC SSRC given. "
-                             "Therefore disabling FlexFEC.";
-      return nullptr;
-    }
-    if (rtp.flexfec.protected_media_ssrcs.empty()) {
-      RTC_LOG(LS_WARNING)
-          << "FlexFEC is enabled, but no protected media SSRC given. "
-             "Therefore disabling FlexFEC.";
-      return nullptr;
-    }
-
-    if (rtp.flexfec.protected_media_ssrcs.size() > 1) {
-      RTC_LOG(LS_WARNING)
-          << "The supplied FlexfecConfig contained multiple protected "
-             "media streams, but our implementation currently only "
-             "supports protecting a single media stream. "
-             "To avoid confusion, disabling FlexFEC completely.";
-      return nullptr;
-    }
-
-    if (absl::c_find(rtp.flexfec.protected_media_ssrcs,
-                     rtp.ssrcs[simulcast_index]) ==
-        rtp.flexfec.protected_media_ssrcs.end()) {
-      // Media SSRC not among flexfec protected SSRCs.
-      return nullptr;
-    }
-
-    const RtpState* rtp_state = nullptr;
-    auto it = suspended_ssrcs.find(rtp.flexfec.ssrc);
-    if (it != suspended_ssrcs.end()) {
-      rtp_state = &it->second;
-    }
-
-    RTC_DCHECK_EQ(1U, rtp.flexfec.protected_media_ssrcs.size());
-    return std::make_unique<FlexfecSender>(
-        rtp.flexfec.payload_type, rtp.flexfec.ssrc,
-        rtp.flexfec.protected_media_ssrcs[0], rtp.mid, rtp.extensions,
-        RTPSender::FecExtensionSizes(), rtp_state, clock);
-  } else if (rtp.ulpfec.red_payload_type >= 0 &&
-             rtp.ulpfec.ulpfec_payload_type >= 0 &&
-             !ShouldDisableRedAndUlpfec(/*flexfec_enabled=*/false, rtp)) {
-    // Flexfec not configured, but ulpfec is and is not disabled.
-    return std::make_unique<UlpfecGenerator>(
-        rtp.ulpfec.red_payload_type, rtp.ulpfec.ulpfec_payload_type, clock);
-  }
-
-  // Not a single FEC is given.
-  return nullptr;
-}
-
 std::vector<RtpStreamSender> CreateRtpStreamSenders(
     Clock* clock,
     const RtpConfig& rtp_config,
@@ -186,7 +121,7 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
     Transport* send_transport,
     RtcpBandwidthObserver* bandwidth_callback,
     RtpTransportControllerSendInterface* transport,
-    const std::map<uint32_t, RtpState>& suspended_ssrcs,
+    FlexfecSender* flexfec_sender,
     RtcEventLog* event_log,
     RateLimiter* retransmission_rate_limiter,
     OverheadObserver* overhead_observer,
@@ -225,17 +160,18 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
   configuration.rtcp_report_interval_ms = rtcp_report_interval_ms;
 
   std::vector<RtpStreamSender> rtp_streams;
-
+  const std::vector<uint32_t>& flexfec_protected_ssrcs =
+      rtp_config.flexfec.protected_media_ssrcs;
   RTC_DCHECK(rtp_config.rtx.ssrcs.empty() ||
              rtp_config.rtx.ssrcs.size() == rtp_config.rtx.ssrcs.size());
   for (size_t i = 0; i < rtp_config.ssrcs.size(); ++i) {
-    RTPSenderVideo::Config video_config;
     configuration.local_media_ssrc = rtp_config.ssrcs[i];
-
+    bool enable_flexfec = flexfec_sender != nullptr &&
-    std::unique_ptr<VideoFecGenerator> fec_generator =
+                          std::find(flexfec_protected_ssrcs.begin(),
-        MaybeCreateFecGenerator(clock, rtp_config, suspended_ssrcs, i);
+                                    flexfec_protected_ssrcs.end(),
-    configuration.fec_generator = fec_generator.get();
+                                    configuration.local_media_ssrc) !=
-    video_config.fec_generator = fec_generator.get();
+                              flexfec_protected_ssrcs.end();
+    configuration.flexfec_sender = enable_flexfec ? flexfec_sender : nullptr;
 
     if (rtp_config.rtx.ssrcs.size() > i) {
       configuration.rtx_send_ssrc = rtp_config.rtx.ssrcs[i];
@@ -251,31 +187,75 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
     rtp_rtcp->SetStorePacketsStatus(true, kMinSendSidePacketHistorySize);
 
     FieldTrialBasedConfig field_trial_config;
+    RTPSenderVideo::Config video_config;
     video_config.clock = configuration.clock;
     video_config.rtp_sender = rtp_rtcp->RtpSender();
+    video_config.flexfec_sender = configuration.flexfec_sender;
     video_config.frame_encryptor = frame_encryptor;
     video_config.require_frame_encryption =
         crypto_options.sframe.require_frame_encryption;
     video_config.enable_retransmit_all_layers = false;
     video_config.field_trials = &field_trial_config;
-
-    const bool using_flexfec =
-        fec_generator &&
-        fec_generator->GetFecType() == VideoFecGenerator::FecType::kFlexFec;
     const bool should_disable_red_and_ulpfec =
-        ShouldDisableRedAndUlpfec(using_flexfec, rtp_config);
+        ShouldDisableRedAndUlpfec(enable_flexfec, rtp_config);
-    if (!should_disable_red_and_ulpfec &&
+    if (rtp_config.ulpfec.red_payload_type != -1 &&
-        rtp_config.ulpfec.red_payload_type != -1) {
+        !should_disable_red_and_ulpfec) {
       video_config.red_payload_type = rtp_config.ulpfec.red_payload_type;
     }
-
+    if (rtp_config.ulpfec.ulpfec_payload_type != -1 &&
+        !should_disable_red_and_ulpfec) {
+      video_config.ulpfec_payload_type = rtp_config.ulpfec.ulpfec_payload_type;
+    }
     auto sender_video = std::make_unique<RTPSenderVideo>(video_config);
-    rtp_streams.emplace_back(std::move(rtp_rtcp), std::move(sender_video),
+    rtp_streams.emplace_back(std::move(rtp_rtcp), std::move(sender_video));
-                             std::move(fec_generator));
   }
   return rtp_streams;
 }
 
+// TODO(brandtr): Update this function when we support multistream protection.
+std::unique_ptr<FlexfecSender> MaybeCreateFlexfecSender(
+    Clock* clock,
+    const RtpConfig& rtp,
+    const std::map<uint32_t, RtpState>& suspended_ssrcs) {
+  if (rtp.flexfec.payload_type < 0) {
+    return nullptr;
+  }
+  RTC_DCHECK_GE(rtp.flexfec.payload_type, 0);
+  RTC_DCHECK_LE(rtp.flexfec.payload_type, 127);
+  if (rtp.flexfec.ssrc == 0) {
+    RTC_LOG(LS_WARNING) << "FlexFEC is enabled, but no FlexFEC SSRC given. "
+                           "Therefore disabling FlexFEC.";
+    return nullptr;
+  }
+  if (rtp.flexfec.protected_media_ssrcs.empty()) {
+    RTC_LOG(LS_WARNING)
+        << "FlexFEC is enabled, but no protected media SSRC given. "
+           "Therefore disabling FlexFEC.";
+    return nullptr;
+  }
+
+  if (rtp.flexfec.protected_media_ssrcs.size() > 1) {
+    RTC_LOG(LS_WARNING)
+        << "The supplied FlexfecConfig contained multiple protected "
+           "media streams, but our implementation currently only "
+           "supports protecting a single media stream. "
+           "To avoid confusion, disabling FlexFEC completely.";
+    return nullptr;
+  }
+
+  const RtpState* rtp_state = nullptr;
+  auto it = suspended_ssrcs.find(rtp.flexfec.ssrc);
+  if (it != suspended_ssrcs.end()) {
+    rtp_state = &it->second;
+  }
+
+  RTC_DCHECK_EQ(1U, rtp.flexfec.protected_media_ssrcs.size());
+  return std::make_unique<FlexfecSender>(
+      rtp.flexfec.payload_type, rtp.flexfec.ssrc,
+      rtp.flexfec.protected_media_ssrcs[0], rtp.mid, rtp.extensions,
+      RTPSender::FecExtensionSizes(), rtp_state, clock);
+}
+
 DataRate CalculateOverheadRate(DataRate data_rate,
                                DataSize packet_size,
                                DataSize overhead_per_packet) {
@@ -322,6 +302,8 @@ RtpVideoSender::RtpVideoSender(
       active_(false),
       module_process_thread_(nullptr),
       suspended_ssrcs_(std::move(suspended_ssrcs)),
+      flexfec_sender_(
+          MaybeCreateFlexfecSender(clock, rtp_config, suspended_ssrcs_)),
       fec_controller_(std::move(fec_controller)),
       fec_allowed_(true),
       rtp_streams_(CreateRtpStreamSenders(clock,
@@ -331,7 +313,7 @@ RtpVideoSender::RtpVideoSender(
                                           send_transport,
                                           transport->GetBandwidthObserver(),
                                           transport,
-                                          suspended_ssrcs_,
+                                          flexfec_sender_.get(),
                                           event_log,
                                           retransmission_limiter,
                                           this,
@@ -393,7 +375,6 @@ RtpVideoSender::RtpVideoSender(
     }
   }
 
-  bool fec_enabled = false;
   for (const RtpStreamSender& stream : rtp_streams_) {
     // Simulcast has one module for each layer. Set the CNAME on all modules.
     stream.rtp_rtcp->SetCNAME(rtp_config_.c_name.c_str());
@@ -403,13 +384,10 @@ RtpVideoSender::RtpVideoSender(
     stream.rtp_rtcp->SetMaxRtpPacketSize(rtp_config_.max_packet_size);
     stream.rtp_rtcp->RegisterSendPayloadFrequency(rtp_config_.payload_type,
                                                   kVideoPayloadTypeFrequency);
-    if (stream.fec_generator != nullptr) {
-      fec_enabled = true;
-    }
   }
   // Currently, both ULPFEC and FlexFEC use the same FEC rate calculation logic,
   // so enable that logic if either of those FEC schemes are enabled.
-  fec_controller_->SetProtectionMethod(fec_enabled, NackEnabled());
+  fec_controller_->SetProtectionMethod(FecEnabled(), NackEnabled());
 
   fec_controller_->SetProtectionCallback(this);
   // Signal congestion controller this object is ready for OnPacket* callbacks.
@@ -577,6 +555,14 @@ void RtpVideoSender::OnBitrateAllocationUpdated(
   }
 }
 
+bool RtpVideoSender::FecEnabled() const {
+  const bool flexfec_enabled = (flexfec_sender_ != nullptr);
+  const bool ulpfec_enabled =
+      !webrtc::field_trial::IsEnabled("WebRTC-DisableUlpFecExperiment") &&
+      (rtp_config_.ulpfec.ulpfec_payload_type >= 0);
+  return flexfec_enabled || ulpfec_enabled;
+}
+
 bool RtpVideoSender::NackEnabled() const {
   const bool nack_enabled = rtp_config_.nack.rtp_history_ms > 0;
   return nack_enabled;
@@ -671,14 +657,6 @@ std::map<uint32_t, RtpState> RtpVideoSender::GetRtpStates() const {
     uint32_t ssrc = rtp_config_.ssrcs[i];
     RTC_DCHECK_EQ(ssrc, rtp_streams_[i].rtp_rtcp->SSRC());
     rtp_states[ssrc] = rtp_streams_[i].rtp_rtcp->GetRtpState();
-
-    VideoFecGenerator* fec_generator = rtp_streams_[i].fec_generator.get();
-    if (fec_generator &&
-        fec_generator->GetFecType() == VideoFecGenerator::FecType::kFlexFec) {
-      auto* flexfec_sender = static_cast<FlexfecSender*>(fec_generator);
-      uint32_t ssrc = rtp_config_.flexfec.ssrc;
-      rtp_states[ssrc] = flexfec_sender->GetRtpState();
-    }
   }
 
   for (size_t i = 0; i < rtp_config_.rtx.ssrcs.size(); ++i) {
@@ -686,6 +664,11 @@ std::map<uint32_t, RtpState> RtpVideoSender::GetRtpStates() const {
     rtp_states[ssrc] = rtp_streams_[i].rtp_rtcp->GetRtxState();
   }
 
+  if (flexfec_sender_) {
+    uint32_t ssrc = rtp_config_.flexfec.ssrc;
+    rtp_states[ssrc] = flexfec_sender_->GetRtpState();
+  }
+
   return rtp_states;
 }
 

call/rtp_video_sender.h

@@ -51,8 +51,7 @@ namespace webrtc_internal_rtp_video_sender {
 // RtpVideoSender.
 struct RtpStreamSender {
   RtpStreamSender(std::unique_ptr<RtpRtcp> rtp_rtcp,
-                  std::unique_ptr<RTPSenderVideo> sender_video,
+                  std::unique_ptr<RTPSenderVideo> sender_video);
-                  std::unique_ptr<VideoFecGenerator> fec_generator);
   ~RtpStreamSender();
 
   RtpStreamSender(RtpStreamSender&&) = default;
@@ -61,7 +60,6 @@ struct RtpStreamSender {
   // Note: Needs pointer stability.
   std::unique_ptr<RtpRtcp> rtp_rtcp;
   std::unique_ptr<RTPSenderVideo> sender_video;
-  std::unique_ptr<VideoFecGenerator> fec_generator;
 };
 
 }  // namespace webrtc_internal_rtp_video_sender
@@ -156,6 +154,7 @@ class RtpVideoSender : public RtpVideoSenderInterface,
   void ConfigureProtection();
   void ConfigureSsrcs();
   void ConfigureRids();
+  bool FecEnabled() const;
   bool NackEnabled() const;
   uint32_t GetPacketizationOverheadRate() const;
 
@@ -173,6 +172,8 @@ class RtpVideoSender : public RtpVideoSenderInterface,
   rtc::ThreadChecker module_process_thread_checker_;
   std::map<uint32_t, RtpState> suspended_ssrcs_;
 
+  std::unique_ptr<FlexfecSender> flexfec_sender_;
+
   const std::unique_ptr<FecController> fec_controller_;
   bool fec_allowed_ RTC_GUARDED_BY(crit_);
 

modules/include/module_fec_types.h

@@ -24,9 +24,9 @@ enum FecMaskType {
 
 // Struct containing forward error correction settings.
 struct FecProtectionParams {
-  int fec_rate = 0;
+  int fec_rate;
-  int max_fec_frames = 0;
+  int max_fec_frames;
-  FecMaskType fec_mask_type = FecMaskType::kFecMaskRandom;
+  FecMaskType fec_mask_type;
 };
 
 }  // namespace webrtc

modules/rtp_rtcp/BUILD.gn

@@ -210,7 +210,6 @@ rtc_library("rtp_rtcp") {
     "source/ulpfec_header_reader_writer.h",
     "source/ulpfec_receiver_impl.cc",
     "source/ulpfec_receiver_impl.h",
-    "source/video_fec_generator.h",
     "source/video_rtp_depacketizer.h",
     "source/video_rtp_depacketizer_av1.cc",
     "source/video_rtp_depacketizer_av1.h",

modules/rtp_rtcp/include/flexfec_sender.h

@@ -21,9 +21,7 @@
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtp_header_extension_size.h"
 #include "modules/rtp_rtcp/source/ulpfec_generator.h"
-#include "modules/rtp_rtcp/source/video_fec_generator.h"
 #include "rtc_base/random.h"
-#include "rtc_base/rate_statistics.h"
 
 namespace webrtc {
 
@@ -33,7 +31,7 @@ class RtpPacketToSend;
 // Note that this class is not thread safe, and thus requires external
 // synchronization. Currently, this is done using the lock in PayloadRouter.
 
-class FlexfecSender : public VideoFecGenerator {
+class FlexfecSender {
  public:
   FlexfecSender(int payload_type,
                 uint32_t ssrc,
@@ -45,28 +43,26 @@ class FlexfecSender : public VideoFecGenerator {
                 Clock* clock);
   ~FlexfecSender();
 
-  FecType GetFecType() const override {
+  uint32_t ssrc() const { return ssrc_; }
-    return VideoFecGenerator::FecType::kFlexFec;
-  }
-  absl::optional<uint32_t> FecSsrc() override { return ssrc_; }
 
   // Sets the FEC rate, max frames sent before FEC packets are sent,
   // and what type of generator matrices are used.
-  void SetProtectionParameters(const FecProtectionParams& delta_params,
+  void SetFecParameters(const FecProtectionParams& params);
-                               const FecProtectionParams& key_params) override;
 
   // Adds a media packet to the internal buffer. When enough media packets
   // have been added, the FEC packets are generated and stored internally.
   // These FEC packets are then obtained by calling GetFecPackets().
-  void AddPacketAndGenerateFec(const RtpPacketToSend& packet) override;
+  // Returns true if the media packet was successfully added.
+  bool AddRtpPacketAndGenerateFec(const RtpPacketToSend& packet);
+
+  // Returns true if there are generated FEC packets available.
+  bool FecAvailable() const;
 
   // Returns generated FlexFEC packets.
-  std::vector<std::unique_ptr<RtpPacketToSend>> GetFecPackets() override;
+  std::vector<std::unique_ptr<RtpPacketToSend>> GetFecPackets();
 
   // Returns the overhead, per packet, for FlexFEC.
-  size_t MaxPacketOverhead() const override;
+  size_t MaxPacketOverhead() const;
-
-  DataRate CurrentFecRate() const override;
 
   // Only called on the VideoSendStream queue, after operation has shut down.
   RtpState GetRtpState();
@@ -91,9 +87,6 @@ class FlexfecSender : public VideoFecGenerator {
   UlpfecGenerator ulpfec_generator_;
   const RtpHeaderExtensionMap rtp_header_extension_map_;
   const size_t header_extensions_size_;
-
-  rtc::CriticalSection crit_;
-  RateStatistics fec_bitrate_ RTC_GUARDED_BY(crit_);
 };
 
 }  // namespace webrtc

modules/rtp_rtcp/include/rtp_rtcp.h

@@ -22,13 +22,13 @@
 #include "api/transport/webrtc_key_value_config.h"
 #include "api/video/video_bitrate_allocation.h"
 #include "modules/include/module.h"
+#include "modules/rtp_rtcp/include/flexfec_sender.h"
 #include "modules/rtp_rtcp/include/receive_statistics.h"
 #include "modules/rtp_rtcp/include/report_block_data.h"
 #include "modules/rtp_rtcp/include/rtp_packet_sender.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
 #include "modules/rtp_rtcp/source/rtp_sequence_number_map.h"
-#include "modules/rtp_rtcp/source/video_fec_generator.h"
 #include "rtc_base/constructor_magic.h"
 #include "rtc_base/deprecation.h"
 
@@ -92,9 +92,9 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface {
     // Spread any bursts of packets into smaller bursts to minimize packet loss.
     RtpPacketSender* paced_sender = nullptr;
 
-    // Generates FEC packets.
+    // Generate FlexFEC packets.
-    // TODO(sprang): Wire up to RtpSenderEgress.
+    // TODO(brandtr): Remove when FlexfecSender is wired up to PacedSender.
-    VideoFecGenerator* fec_generator = nullptr;
+    FlexfecSender* flexfec_sender = nullptr;
 
     BitrateStatisticsObserver* send_bitrate_observer = nullptr;
     SendSideDelayObserver* send_side_delay_observer = nullptr;

modules/rtp_rtcp/source/flexfec_sender.cc

@@ -91,13 +91,11 @@ FlexfecSender::FlexfecSender(
       seq_num_(rtp_state ? rtp_state->sequence_number
                          : random_.Rand(1, kMaxInitRtpSeqNumber)),
       ulpfec_generator_(
-          ForwardErrorCorrection::CreateFlexfec(ssrc, protected_media_ssrc),
+          ForwardErrorCorrection::CreateFlexfec(ssrc, protected_media_ssrc)),
-          clock_),
       rtp_header_extension_map_(
           RegisterSupportedExtensions(rtp_header_extensions)),
       header_extensions_size_(
-          RtpHeaderExtensionSize(extension_sizes, rtp_header_extension_map_)),
+          RtpHeaderExtensionSize(extension_sizes, rtp_header_extension_map_)) {
-      fec_bitrate_(/*max_window_size_ms=*/1000, RateStatistics::kBpsScale) {
   // This object should not have been instantiated if FlexFEC is disabled.
   RTC_DCHECK_GE(payload_type, 0);
   RTC_DCHECK_LE(payload_type, 127);
@@ -107,30 +105,30 @@ FlexfecSender::~FlexfecSender() = default;
 
 // We are reusing the implementation from UlpfecGenerator for SetFecParameters,
 // AddRtpPacketAndGenerateFec, and FecAvailable.
-void FlexfecSender::SetProtectionParameters(
+void FlexfecSender::SetFecParameters(const FecProtectionParams& params) {
-    const FecProtectionParams& delta_params,
+  ulpfec_generator_.SetFecParameters(params);
-    const FecProtectionParams& key_params) {
-  ulpfec_generator_.SetProtectionParameters(delta_params, key_params);
 }
 
-void FlexfecSender::AddPacketAndGenerateFec(const RtpPacketToSend& packet) {
+bool FlexfecSender::AddRtpPacketAndGenerateFec(const RtpPacketToSend& packet) {
   // TODO(brandtr): Generalize this SSRC check when we support multistream
   // protection.
   RTC_DCHECK_EQ(packet.Ssrc(), protected_media_ssrc_);
-  ulpfec_generator_.AddPacketAndGenerateFec(packet);
+  return ulpfec_generator_.AddRtpPacketAndGenerateFec(
+             packet.Buffer(), packet.headers_size()) == 0;
+}
+
+bool FlexfecSender::FecAvailable() const {
+  return ulpfec_generator_.FecAvailable();
 }
 
 std::vector<std::unique_ptr<RtpPacketToSend>> FlexfecSender::GetFecPackets() {
-  RTC_CHECK_RUNS_SERIALIZED(&ulpfec_generator_.race_checker_);
   std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets_to_send;
   fec_packets_to_send.reserve(ulpfec_generator_.generated_fec_packets_.size());
-  size_t total_fec_data_bytes = 0;
   for (const auto* fec_packet : ulpfec_generator_.generated_fec_packets_) {
     std::unique_ptr<RtpPacketToSend> fec_packet_to_send(
         new RtpPacketToSend(&rtp_header_extension_map_));
     fec_packet_to_send->set_packet_type(
         RtpPacketMediaType::kForwardErrorCorrection);
-    fec_packet_to_send->set_allow_retransmission(false);
 
     // RTP header.
     fec_packet_to_send->SetMarker(false);
@@ -159,13 +157,9 @@ std::vector<std::unique_ptr<RtpPacketToSend>> FlexfecSender::GetFecPackets() {
         fec_packet_to_send->AllocatePayload(fec_packet->data.size());
     memcpy(payload, fec_packet->data.cdata(), fec_packet->data.size());
 
-    total_fec_data_bytes += fec_packet_to_send->size();
     fec_packets_to_send.push_back(std::move(fec_packet_to_send));
   }
-
-  if (!fec_packets_to_send.empty()) {
   ulpfec_generator_.ResetState();
-  }
 
   int64_t now_ms = clock_->TimeInMilliseconds();
   if (!fec_packets_to_send.empty() &&
@@ -176,9 +170,6 @@ std::vector<std::unique_ptr<RtpPacketToSend>> FlexfecSender::GetFecPackets() {
     last_generated_packet_ms_ = now_ms;
   }
 
-  rtc::CritScope cs(&crit_);
-  fec_bitrate_.Update(total_fec_data_bytes, now_ms);
-
   return fec_packets_to_send;
 }
 
@@ -187,12 +178,6 @@ size_t FlexfecSender::MaxPacketOverhead() const {
   return header_extensions_size_ + kFlexfecMaxHeaderSize;
 }
 
-DataRate FlexfecSender::CurrentFecRate() const {
-  rtc::CritScope cs(&crit_);
-  return DataRate::BitsPerSec(
-      fec_bitrate_.Rate(clock_->TimeInMilliseconds()).value_or(0));
-}
-
 RtpState FlexfecSender::GetRtpState() {
   RtpState rtp_state;
   rtp_state.sequence_number = seq_num_;

modules/rtp_rtcp/source/flexfec_sender_unittest.cc

@@ -55,7 +55,7 @@ std::unique_ptr<RtpPacketToSend> GenerateSingleFlexfecPacket(
   params.fec_mask_type = kFecMaskRandom;
   constexpr size_t kNumPackets = 4;
 
-  sender->SetProtectionParameters(params, params);
+  sender->SetFecParameters(params);
   AugmentedPacketGenerator packet_generator(kMediaSsrc);
   packet_generator.NewFrame(kNumPackets);
   for (size_t i = 0; i < kNumPackets; ++i) {
@@ -63,12 +63,13 @@ std::unique_ptr<RtpPacketToSend> GenerateSingleFlexfecPacket(
         packet_generator.NextPacket(i, kPayloadLength);
     RtpPacketToSend rtp_packet(nullptr);  // No header extensions.
     rtp_packet.Parse(packet->data);
-    sender->AddPacketAndGenerateFec(rtp_packet);
+    EXPECT_TRUE(sender->AddRtpPacketAndGenerateFec(rtp_packet));
   }
+  EXPECT_TRUE(sender->FecAvailable());
   std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets =
       sender->GetFecPackets();
+  EXPECT_FALSE(sender->FecAvailable());
   EXPECT_EQ(1U, fec_packets.size());
-  EXPECT_TRUE(sender->GetFecPackets().empty());
 
   return std::move(fec_packets.front());
 }
@@ -81,7 +82,7 @@ TEST(FlexfecSenderTest, Ssrc) {
                        kNoRtpHeaderExtensions, kNoRtpHeaderExtensionSizes,
                        nullptr /* rtp_state */, &clock);
 
-  EXPECT_EQ(kFlexfecSsrc, sender.FecSsrc());
+  EXPECT_EQ(kFlexfecSsrc, sender.ssrc());
 }
 
 TEST(FlexfecSenderTest, NoFecAvailableBeforeMediaAdded) {
@@ -90,7 +91,9 @@ TEST(FlexfecSenderTest, NoFecAvailableBeforeMediaAdded) {
                        kNoRtpHeaderExtensions, kNoRtpHeaderExtensionSizes,
                        nullptr /* rtp_state */, &clock);
 
-  EXPECT_TRUE(sender.GetFecPackets().empty());
+  EXPECT_FALSE(sender.FecAvailable());
+  auto fec_packets = sender.GetFecPackets();
+  EXPECT_EQ(0U, fec_packets.size());
 }
 
 TEST(FlexfecSenderTest, ProtectOneFrameWithOneFecPacket) {
@@ -121,7 +124,7 @@ TEST(FlexfecSenderTest, ProtectTwoFramesWithOneFecPacket) {
   FlexfecSender sender(kFlexfecPayloadType, kFlexfecSsrc, kMediaSsrc, kNoMid,
                        kNoRtpHeaderExtensions, kNoRtpHeaderExtensionSizes,
                        nullptr /* rtp_state */, &clock);
-  sender.SetProtectionParameters(params, params);
+  sender.SetFecParameters(params);
 
   AugmentedPacketGenerator packet_generator(kMediaSsrc);
   for (size_t i = 0; i < kNumFrames; ++i) {
@@ -131,13 +134,14 @@ TEST(FlexfecSenderTest, ProtectTwoFramesWithOneFecPacket) {
           packet_generator.NextPacket(i, kPayloadLength);
       RtpPacketToSend rtp_packet(nullptr);
       rtp_packet.Parse(packet->data);
-      sender.AddPacketAndGenerateFec(rtp_packet);
+      EXPECT_TRUE(sender.AddRtpPacketAndGenerateFec(rtp_packet));
     }
   }
+  EXPECT_TRUE(sender.FecAvailable());
   std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets =
       sender.GetFecPackets();
+  EXPECT_FALSE(sender.FecAvailable());
   ASSERT_EQ(1U, fec_packets.size());
-  EXPECT_TRUE(sender.GetFecPackets().empty());
 
   RtpPacketToSend* fec_packet = fec_packets.front().get();
   EXPECT_EQ(kRtpHeaderSize, fec_packet->headers_size());
@@ -160,7 +164,7 @@ TEST(FlexfecSenderTest, ProtectTwoFramesWithTwoFecPackets) {
   FlexfecSender sender(kFlexfecPayloadType, kFlexfecSsrc, kMediaSsrc, kNoMid,
                        kNoRtpHeaderExtensions, kNoRtpHeaderExtensionSizes,
                        nullptr /* rtp_state */, &clock);
-  sender.SetProtectionParameters(params, params);
+  sender.SetFecParameters(params);
 
   AugmentedPacketGenerator packet_generator(kMediaSsrc);
   for (size_t i = 0; i < kNumFrames; ++i) {
@@ -170,12 +174,13 @@ TEST(FlexfecSenderTest, ProtectTwoFramesWithTwoFecPackets) {
           packet_generator.NextPacket(i, kPayloadLength);
       RtpPacketToSend rtp_packet(nullptr);
       rtp_packet.Parse(packet->data);
-      sender.AddPacketAndGenerateFec(rtp_packet);
+      EXPECT_TRUE(sender.AddRtpPacketAndGenerateFec(rtp_packet));
     }
+    EXPECT_TRUE(sender.FecAvailable());
     std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets =
         sender.GetFecPackets();
+    EXPECT_FALSE(sender.FecAvailable());
     ASSERT_EQ(1U, fec_packets.size());
-    EXPECT_TRUE(sender.GetFecPackets().empty());
 
     RtpPacketToSend* fec_packet = fec_packets.front().get();
     EXPECT_EQ(kRtpHeaderSize, fec_packet->headers_size());

modules/rtp_rtcp/source/rtcp_receiver.cc

@@ -72,11 +72,8 @@ std::set<uint32_t> GetRegisteredSsrcs(const RtpRtcp::Configuration& config) {
   if (config.rtx_send_ssrc) {
     ssrcs.insert(*config.rtx_send_ssrc);
   }
-  if (config.fec_generator) {
+  if (config.flexfec_sender) {
-    absl::optional<uint32_t> flexfec_ssrc = config.fec_generator->FecSsrc();
+    ssrcs.insert(config.flexfec_sender->ssrc());
-    if (flexfec_ssrc) {
-      ssrcs.insert(*flexfec_ssrc);
-    }
   }
   return ssrcs;
 }

modules/rtp_rtcp/source/rtp_packet_to_send.h

@@ -98,15 +98,11 @@ class RtpPacketToSend : public RtpPacket {
         VideoTimingExtension::kNetwork2TimestampDeltaOffset);
   }
 
-  // Indicates if packet is the first packet of a video frame.
   void set_first_packet_of_frame(bool is_first_packet) {
     is_first_packet_of_frame_ = is_first_packet;
   }
-  bool is_first_packet_of_frame() const { return is_first_packet_of_frame_; }
 
-  // Indicates if packet contains payload for a video key-frame.
+  bool is_first_packet_of_frame() const { return is_first_packet_of_frame_; }
-  void set_is_key_frame(bool is_key_frame) { is_key_frame_ = is_key_frame; }
-  bool is_key_frame() const { return is_key_frame_; }
 
  private:
   int64_t capture_time_ms_ = 0;
@@ -115,7 +111,6 @@ class RtpPacketToSend : public RtpPacket {
   absl::optional<uint16_t> retransmitted_sequence_number_;
   std::vector<uint8_t> application_data_;
   bool is_first_packet_of_frame_ = false;
-  bool is_key_frame_ = false;
 };
 
 }  // namespace webrtc

modules/rtp_rtcp/source/rtp_sender.cc

@@ -102,7 +102,8 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config,
       audio_configured_(config.audio),
       ssrc_(config.local_media_ssrc),
       rtx_ssrc_(config.rtx_send_ssrc),
-      flexfec_ssrc_(config.fec_generator ? config.fec_generator->FecSsrc()
+      flexfec_ssrc_(config.flexfec_sender
+                        ? absl::make_optional(config.flexfec_sender->ssrc())
                         : absl::nullopt),
       packet_history_(packet_history),
       paced_sender_(packet_sender),

modules/rtp_rtcp/source/rtp_sender_egress.cc

@@ -57,7 +57,8 @@ RtpSenderEgress::RtpSenderEgress(const RtpRtcp::Configuration& config,
                                  RtpPacketHistory* packet_history)
     : ssrc_(config.local_media_ssrc),
       rtx_ssrc_(config.rtx_send_ssrc),
-      flexfec_ssrc_(config.fec_generator ? config.fec_generator->FecSsrc()
+      flexfec_ssrc_(config.flexfec_sender
+                        ? absl::make_optional(config.flexfec_sender->ssrc())
                         : absl::nullopt),
       populate_network2_timestamp_(config.populate_network2_timestamp),
       send_side_bwe_with_overhead_(

modules/rtp_rtcp/source/rtp_sender_unittest.cc

@@ -272,7 +272,7 @@ class RtpSenderTest : public ::testing::TestWithParam<TestConfig> {
     config.outgoing_transport = &transport_;
     config.local_media_ssrc = kSsrc;
     config.rtx_send_ssrc = kRtxSsrc;
-    config.fec_generator = &flexfec_sender_;
+    config.flexfec_sender = &flexfec_sender_;
     config.event_log = &mock_rtc_event_log_;
     config.send_packet_observer = &send_packet_observer_;
     config.retransmission_rate_limiter = &retransmission_rate_limiter_;
@@ -1225,7 +1225,7 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) {
   config.outgoing_transport = &transport_;
   config.paced_sender = &mock_paced_sender_;
   config.local_media_ssrc = kSsrc;
-  config.fec_generator = &flexfec_sender_;
+  config.flexfec_sender = &flexfec_sender_;
   config.event_log = &mock_rtc_event_log_;
   config.send_packet_observer = &send_packet_observer_;
   config.retransmission_rate_limiter = &retransmission_rate_limiter_;
@@ -1239,7 +1239,7 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) {
   RTPSenderVideo::Config video_config;
   video_config.clock = &fake_clock_;
   video_config.rtp_sender = rtp_sender();
-  video_config.fec_generator = &flexfec_sender;
+  video_config.flexfec_sender = &flexfec_sender;
   video_config.field_trials = &field_trials;
   RTPSenderVideo rtp_sender_video(video_config);
 
@@ -1311,7 +1311,7 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) {
   config.clock = &fake_clock_;
   config.outgoing_transport = &transport_;
   config.local_media_ssrc = kSsrc;
-  config.fec_generator = &flexfec_sender;
+  config.flexfec_sender = &flexfec_sender;
   config.event_log = &mock_rtc_event_log_;
   config.send_packet_observer = &send_packet_observer_;
   config.retransmission_rate_limiter = &retransmission_rate_limiter_;
@@ -1323,7 +1323,7 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) {
   RTPSenderVideo::Config video_config;
   video_config.clock = &fake_clock_;
   video_config.rtp_sender = rtp_sender();
-  video_config.fec_generator = &flexfec_sender;
+  video_config.flexfec_sender = &flexfec_sender;
   video_config.field_trials = &field_trials;
   RTPSenderVideo rtp_sender_video(video_config);
 
@@ -1583,7 +1583,7 @@ TEST_P(RtpSenderTest, FecOverheadRate) {
   config.outgoing_transport = &transport_;
   config.paced_sender = &mock_paced_sender_;
   config.local_media_ssrc = kSsrc;
-  config.fec_generator = &flexfec_sender;
+  config.flexfec_sender = &flexfec_sender;
   config.event_log = &mock_rtc_event_log_;
   config.send_packet_observer = &send_packet_observer_;
   config.retransmission_rate_limiter = &retransmission_rate_limiter_;
@@ -1595,7 +1595,7 @@ TEST_P(RtpSenderTest, FecOverheadRate) {
   RTPSenderVideo::Config video_config;
   video_config.clock = &fake_clock_;
   video_config.rtp_sender = rtp_sender();
-  video_config.fec_generator = &flexfec_sender;
+  video_config.flexfec_sender = &flexfec_sender;
   video_config.field_trials = &field_trials;
   RTPSenderVideo rtp_sender_video(video_config);
   // Parameters selected to generate a single FEC packet per media packet.
@@ -1777,14 +1777,12 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) {
   const uint8_t kPayloadType = 127;
   const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric;
   FieldTrialBasedConfig field_trials;
-  UlpfecGenerator ulpfec_generator(kRedPayloadType, kUlpfecPayloadType,
-                                   &fake_clock_);
   RTPSenderVideo::Config video_config;
   video_config.clock = &fake_clock_;
   video_config.rtp_sender = rtp_sender();
   video_config.field_trials = &field_trials;
   video_config.red_payload_type = kRedPayloadType;
-  video_config.fec_generator = &ulpfec_generator;
+  video_config.ulpfec_payload_type = kUlpfecPayloadType;
   RTPSenderVideo rtp_sender_video(video_config);
   uint8_t payload[] = {47, 11, 32, 93, 89};
   rtp_sender_context_->packet_history_.SetStorePacketsStatus(
@@ -2120,7 +2118,7 @@ TEST_P(RtpSenderTest, SendPacketUpdatesStats) {
   config.outgoing_transport = &transport_;
   config.local_media_ssrc = kSsrc;
   config.rtx_send_ssrc = kRtxSsrc;
-  config.fec_generator = &flexfec_sender_;
+  config.flexfec_sender = &flexfec_sender_;
   config.send_side_delay_observer = &send_side_delay_observer;
   config.event_log = &mock_rtc_event_log_;
   config.send_packet_observer = &send_packet_observer_;

modules/rtp_rtcp/source/rtp_sender_video.cc

@@ -259,7 +259,11 @@ RTPSenderVideo::RTPSenderVideo(const Config& config)
       current_playout_delay_{-1, -1},
       playout_delay_pending_(false),
       red_payload_type_(config.red_payload_type),
-      fec_generator_(config.fec_generator),
+      ulpfec_payload_type_(config.ulpfec_payload_type),
+      flexfec_sender_(config.flexfec_sender),
+      delta_fec_params_{0, 1, kFecMaskRandom},
+      key_fec_params_{0, 1, kFecMaskRandom},
+      fec_bitrate_(1000, RateStatistics::kBpsScale),
       video_bitrate_(1000, RateStatistics::kBpsScale),
       packetization_overhead_bitrate_(1000, RateStatistics::kBpsScale),
       frame_encryptor_(config.frame_encryptor),
@@ -275,6 +279,83 @@ RTPSenderVideo::RTPSenderVideo(const Config& config)
 
 RTPSenderVideo::~RTPSenderVideo() {}
 
+void RTPSenderVideo::AppendAsRedMaybeWithUlpfec(
+    std::unique_ptr<RtpPacketToSend> media_packet,
+    bool protect_media_packet,
+    std::vector<std::unique_ptr<RtpPacketToSend>>* packets) {
+  std::unique_ptr<RtpPacketToSend> red_packet(
+      new RtpPacketToSend(*media_packet));
+  BuildRedPayload(*media_packet, red_packet.get());
+  red_packet->SetPayloadType(*red_payload_type_);
+
+  std::vector<std::unique_ptr<RedPacket>> fec_packets;
+  if (ulpfec_enabled()) {
+    if (protect_media_packet) {
+      if (exclude_transport_sequence_number_from_fec_experiment_) {
+        // See comments at the top of the file why experiment
+        // "WebRTC-kExcludeTransportSequenceNumberFromFec" is needed in
+        // conjunction with datagram transport.
+        // TODO(sukhanov): We may also need to implement it for flexfec_sender
+        // if we decide to keep this approach in the future.
+        uint16_t transport_senquence_number;
+        if (media_packet->GetExtension<webrtc::TransportSequenceNumber>(
+                &transport_senquence_number)) {
+          if (!media_packet->RemoveExtension(
+                  webrtc::TransportSequenceNumber::kId)) {
+            RTC_NOTREACHED()
+                << "Failed to remove transport sequence number, packet="
+                << media_packet->ToString();
+          }
+        }
+      }
+
+      ulpfec_generator_.AddRtpPacketAndGenerateFec(
+          media_packet->Buffer(), media_packet->headers_size());
+    }
+    uint16_t num_fec_packets = ulpfec_generator_.NumAvailableFecPackets();
+    if (num_fec_packets > 0) {
+      uint16_t first_fec_sequence_number =
+          rtp_sender_->AllocateSequenceNumber(num_fec_packets);
+      fec_packets = ulpfec_generator_.GetUlpfecPacketsAsRed(
+          *red_payload_type_, *ulpfec_payload_type_, first_fec_sequence_number);
+      RTC_DCHECK_EQ(num_fec_packets, fec_packets.size());
+    }
+  }
+
+  // Send |red_packet| instead of |packet| for allocated sequence number.
+  red_packet->set_packet_type(RtpPacketMediaType::kVideo);
+  red_packet->set_allow_retransmission(media_packet->allow_retransmission());
+  packets->emplace_back(std::move(red_packet));
+
+  for (const auto& fec_packet : fec_packets) {
+    // TODO(danilchap): Make ulpfec_generator_ generate RtpPacketToSend to avoid
+    // reparsing them.
+    std::unique_ptr<RtpPacketToSend> rtp_packet(
+        new RtpPacketToSend(*media_packet));
+    RTC_CHECK(rtp_packet->Parse(fec_packet->data(), fec_packet->length()));
+    rtp_packet->set_capture_time_ms(media_packet->capture_time_ms());
+    rtp_packet->set_packet_type(RtpPacketMediaType::kForwardErrorCorrection);
+    rtp_packet->set_allow_retransmission(false);
+    RTC_DCHECK_EQ(fec_packet->length(), rtp_packet->size());
+    packets->emplace_back(std::move(rtp_packet));
+  }
+}
+
+void RTPSenderVideo::GenerateAndAppendFlexfec(
+    std::vector<std::unique_ptr<RtpPacketToSend>>* packets) {
+  RTC_DCHECK(flexfec_sender_);
+
+  if (flexfec_sender_->FecAvailable()) {
+    std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets =
+        flexfec_sender_->GetFecPackets();
+    for (auto& fec_packet : fec_packets) {
+      fec_packet->set_packet_type(RtpPacketMediaType::kForwardErrorCorrection);
+      fec_packet->set_allow_retransmission(false);
+      packets->emplace_back(std::move(fec_packet));
+    }
+  }
+}
+
 void RTPSenderVideo::LogAndSendToNetwork(
     std::vector<std::unique_ptr<RtpPacketToSend>> packets,
     size_t unpacketized_payload_size) {
@@ -293,9 +374,16 @@ void RTPSenderVideo::LogAndSendToNetwork(
     rtc::CritScope cs(&stats_crit_);
     size_t packetized_payload_size = 0;
     for (const auto& packet : packets) {
-      if (*packet->packet_type() == RtpPacketMediaType::kVideo) {
+      switch (*packet->packet_type()) {
+        case RtpPacketMediaType::kVideo:
           video_bitrate_.Update(packet->size(), now_ms);
           packetized_payload_size += packet->payload_size();
+          break;
+        case RtpPacketMediaType::kForwardErrorCorrection:
+          fec_bitrate_.Update(packet->size(), clock_->TimeInMilliseconds());
+          break;
+        default:
+          continue;
       }
     }
     // AV1 packetizer may produce less packetized bytes than unpacketized.
@@ -310,31 +398,39 @@ void RTPSenderVideo::LogAndSendToNetwork(
 }
 
 size_t RTPSenderVideo::FecPacketOverhead() const {
-  size_t overhead = fec_generator_ ? fec_generator_->MaxPacketOverhead() : 0u;
+  if (flexfec_enabled())
+    return flexfec_sender_->MaxPacketOverhead();
+
+  size_t overhead = 0;
   if (red_enabled()) {
     // The RED overhead is due to a small header.
     overhead += kRedForFecHeaderLength;
-
+  }
-    // TODO(bugs.webrtc.org/11340): Move this into UlpfecGenerator.
+  if (ulpfec_enabled()) {
-    if (fec_generator_ &&
-        fec_generator_->GetFecType() == VideoFecGenerator::FecType::kUlpFec) {
     // For ULPFEC, the overhead is the FEC headers plus RED for FEC header
     // (see above) plus anything in RTP header beyond the 12 bytes base header
     // (CSRC list, extensions...)
     // This reason for the header extensions to be included here is that
     // from an FEC viewpoint, they are part of the payload to be protected.
     // (The base RTP header is already protected by the FEC header.)
-      overhead += rtp_sender_->RtpHeaderLength() - kRtpHeaderSize;
+    overhead += ulpfec_generator_.MaxPacketOverhead() +
-    }
+                (rtp_sender_->RtpHeaderLength() - kRtpHeaderSize);
   }
   return overhead;
 }
 
 void RTPSenderVideo::SetFecParameters(const FecProtectionParams& delta_params,
                                       const FecProtectionParams& key_params) {
-  if (fec_generator_) {
+  rtc::CritScope cs(&crit_);
-    fec_generator_->SetProtectionParameters(delta_params, key_params);
+  delta_fec_params_ = delta_params;
+  key_fec_params_ = key_params;
+}
+
+absl::optional<uint32_t> RTPSenderVideo::FlexfecSsrc() const {
+  if (flexfec_sender_) {
+    return flexfec_sender_->ssrc();
   }
+  return absl::nullopt;
 }
 
 void RTPSenderVideo::SetVideoStructure(
@@ -445,6 +541,19 @@ bool RTPSenderVideo::SendVideo(
         transmit_color_space_next_frame_ ? !IsBaseLayer(video_header) : false;
   }
 
+  if (flexfec_enabled() || ulpfec_enabled()) {
+    rtc::CritScope cs(&crit_);
+    // FEC settings.
+    const FecProtectionParams& fec_params =
+        video_header.frame_type == VideoFrameType::kVideoFrameKey
+            ? key_fec_params_
+            : delta_fec_params_;
+    if (flexfec_enabled())
+      flexfec_sender_->SetFecParameters(fec_params);
+    if (ulpfec_enabled())
+      ulpfec_generator_.SetFecParameters(fec_params);
+  }
+
   // Maximum size of packet including rtp headers.
   // Extra space left in case packet will be resent using fec or rtx.
   int packet_capacity = rtp_sender_->MaxRtpPacketSize() - FecPacketOverhead() -
@@ -636,40 +745,21 @@ bool RTPSenderVideo::SendVideo(
       packet->set_packetization_finish_time_ms(clock_->TimeInMilliseconds());
     }
 
-    if (protect_packet && fec_generator_) {
-      if (red_enabled() &&
-          exclude_transport_sequence_number_from_fec_experiment_) {
-        // See comments at the top of the file why experiment
-        // "WebRTC-kExcludeTransportSequenceNumberFromFec" is needed in
-        // conjunction with datagram transport.
-        // TODO(sukhanov): We may also need to implement it for flexfec_sender
-        // if we decide to keep this approach in the future.
-        uint16_t transport_senquence_number;
-        if (packet->GetExtension<webrtc::TransportSequenceNumber>(
-                &transport_senquence_number)) {
-          if (!packet->RemoveExtension(webrtc::TransportSequenceNumber::kId)) {
-            RTC_NOTREACHED()
-                << "Failed to remove transport sequence number, packet="
-                << packet->ToString();
-          }
-        }
-      }
-
-      fec_generator_->AddPacketAndGenerateFec(*packet);
-    }
-
     if (red_enabled()) {
-      std::unique_ptr<RtpPacketToSend> red_packet(new RtpPacketToSend(*packet));
+      AppendAsRedMaybeWithUlpfec(std::move(packet), protect_packet,
-      BuildRedPayload(*packet, red_packet.get());
+                                 &rtp_packets);
-      red_packet->SetPayloadType(*red_payload_type_);
-
-      // Send |red_packet| instead of |packet| for allocated sequence number.
-      red_packet->set_packet_type(RtpPacketMediaType::kVideo);
-      red_packet->set_allow_retransmission(packet->allow_retransmission());
-      rtp_packets.emplace_back(std::move(red_packet));
     } else {
       packet->set_packet_type(RtpPacketMediaType::kVideo);
+      const RtpPacketToSend& media_packet = *packet;
       rtp_packets.emplace_back(std::move(packet));
+      if (flexfec_enabled()) {
+        // TODO(brandtr): Remove the FlexFEC code path when FlexfecSender
+        // is wired up to PacedSender instead.
+        if (protect_packet) {
+          flexfec_sender_->AddRtpPacketAndGenerateFec(media_packet);
+        }
+        GenerateAndAppendFlexfec(&rtp_packets);
+      }
     }
 
     if (first_frame) {
@@ -684,22 +774,6 @@ bool RTPSenderVideo::SendVideo(
     }
   }
 
-  if (fec_generator_) {
-    // Fetch any FEC packets generated from the media frame and add them to
-    // the list of packets to send.
-    auto fec_packets = fec_generator_->GetFecPackets();
-
-    // TODO(bugs.webrtc.org/11340): Move sequence number assignment into
-    // UlpfecGenerator.
-    const bool generate_sequence_numbers = !fec_generator_->FecSsrc();
-    for (auto& fec_packet : fec_packets) {
-      if (generate_sequence_numbers) {
-        rtp_sender_->AssignSequenceNumber(fec_packet.get());
-      }
-      rtp_packets.emplace_back(std::move(fec_packet));
-    }
-  }
-
   LogAndSendToNetwork(std::move(rtp_packets), unpacketized_payload_size);
 
   TRACE_EVENT_ASYNC_END1("webrtc", "Video", capture_time_ms, "timestamp",
@@ -713,7 +787,8 @@ uint32_t RTPSenderVideo::VideoBitrateSent() const {
 }
 
 uint32_t RTPSenderVideo::FecOverheadRate() const {
-  return fec_generator_ ? fec_generator_->CurrentFecRate().bps<uint32_t>() : 0u;
+  rtc::CritScope cs(&stats_crit_);
+  return fec_bitrate_.Rate(clock_->TimeInMilliseconds()).value_or(0);
 }
 
 uint32_t RTPSenderVideo::PacketizationOverheadBps() const {

modules/rtp_rtcp/source/rtp_sender_video.h

@@ -22,12 +22,13 @@
 #include "api/video/video_codec_type.h"
 #include "api/video/video_frame_type.h"
 #include "modules/include/module_common_types.h"
+#include "modules/rtp_rtcp/include/flexfec_sender.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/absolute_capture_time_sender.h"
 #include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
 #include "modules/rtp_rtcp/source/rtp_sender.h"
 #include "modules/rtp_rtcp/source/rtp_video_header.h"
-#include "modules/rtp_rtcp/source/video_fec_generator.h"
+#include "modules/rtp_rtcp/source/ulpfec_generator.h"
 #include "rtc_base/critical_section.h"
 #include "rtc_base/one_time_event.h"
 #include "rtc_base/race_checker.h"
@@ -67,11 +68,11 @@ class RTPSenderVideo {
     Clock* clock = nullptr;
     RTPSender* rtp_sender = nullptr;
     FlexfecSender* flexfec_sender = nullptr;
-    VideoFecGenerator* fec_generator = nullptr;
     FrameEncryptorInterface* frame_encryptor = nullptr;
     bool require_frame_encryption = false;
     bool enable_retransmit_all_layers = false;
     absl::optional<int> red_payload_type;
+    absl::optional<int> ulpfec_payload_type;
     const WebRtcKeyValueConfig* field_trials = nullptr;
   };
 
@@ -98,9 +99,13 @@ class RTPSenderVideo {
 
   // FlexFEC/ULPFEC.
   // Set FEC rates, max frames before FEC is sent, and type of FEC masks.
+  // Returns false on failure.
   void SetFecParameters(const FecProtectionParams& delta_params,
                         const FecProtectionParams& key_params);
 
+  // FlexFEC.
+  absl::optional<uint32_t> FlexfecSsrc() const;
+
   uint32_t VideoBitrateSent() const;
   uint32_t FecOverheadRate() const;
 
@@ -129,12 +134,27 @@ class RTPSenderVideo {
 
   size_t FecPacketOverhead() const RTC_EXCLUSIVE_LOCKS_REQUIRED(send_checker_);
 
+  void AppendAsRedMaybeWithUlpfec(
+      std::unique_ptr<RtpPacketToSend> media_packet,
+      bool protect_media_packet,
+      std::vector<std::unique_ptr<RtpPacketToSend>>* packets)
+      RTC_EXCLUSIVE_LOCKS_REQUIRED(send_checker_);
+
+  // TODO(brandtr): Remove the FlexFEC functions when FlexfecSender has been
+  // moved to PacedSender.
+  void GenerateAndAppendFlexfec(
+      std::vector<std::unique_ptr<RtpPacketToSend>>* packets);
+
   void LogAndSendToNetwork(
       std::vector<std::unique_ptr<RtpPacketToSend>> packets,
       size_t unpacketized_payload_size);
 
   bool red_enabled() const { return red_payload_type_.has_value(); }
 
+  bool ulpfec_enabled() const { return ulpfec_payload_type_.has_value(); }
+
+  bool flexfec_enabled() const { return flexfec_sender_ != nullptr; }
+
   bool UpdateConditionalRetransmit(uint8_t temporal_id,
                                    int64_t expected_retransmission_time_ms)
       RTC_EXCLUSIVE_LOCKS_REQUIRED(stats_crit_);
@@ -165,10 +185,22 @@ class RTPSenderVideo {
   // Should never be held when calling out of this class.
   rtc::CriticalSection crit_;
 
+  // RED/ULPFEC.
   const absl::optional<int> red_payload_type_;
-  VideoFecGenerator* const fec_generator_;
+  const absl::optional<int> ulpfec_payload_type_;
+  UlpfecGenerator ulpfec_generator_ RTC_GUARDED_BY(send_checker_);
+
+  // FlexFEC.
+  FlexfecSender* const flexfec_sender_;
+
+  // FEC parameters, applicable to either ULPFEC or FlexFEC.
+  FecProtectionParams delta_fec_params_ RTC_GUARDED_BY(crit_);
+  FecProtectionParams key_fec_params_ RTC_GUARDED_BY(crit_);
 
   rtc::CriticalSection stats_crit_;
+  // Bitrate used for FEC payload, RED headers, RTP headers for FEC packets
+  // and any padding overhead.
+  RateStatistics fec_bitrate_ RTC_GUARDED_BY(stats_crit_);
   // Bitrate used for video payload and RTP headers.
   RateStatistics video_bitrate_ RTC_GUARDED_BY(stats_crit_);
   RateStatistics packetization_overhead_bitrate_ RTC_GUARDED_BY(stats_crit_);

modules/rtp_rtcp/source/rtp_sender_video_unittest.cc

@@ -130,7 +130,7 @@ class TestRtpSenderVideo : public RTPSenderVideo {
           Config config;
           config.clock = clock;
           config.rtp_sender = rtp_sender;
-          config.fec_generator = flexfec_sender;
+          config.flexfec_sender = flexfec_sender;
           config.field_trials = &field_trials;
           return config;
         }()) {}

modules/rtp_rtcp/source/ulpfec_generator.cc

@@ -22,7 +22,6 @@
 #include "modules/rtp_rtcp/source/forward_error_correction_internal.h"
 #include "modules/rtp_rtcp/source/rtp_utility.h"
 #include "rtc_base/checks.h"
-#include "rtc_base/critical_section.h"
 
 namespace webrtc {
 
@@ -63,119 +62,128 @@ constexpr uint32_t kUnknownSsrc = 0;
 
 }  // namespace
 
-UlpfecGenerator::Params::Params() = default;
+RedPacket::RedPacket(size_t length)
-UlpfecGenerator::Params::Params(FecProtectionParams delta_params,
+    : data_(new uint8_t[length]), length_(length), header_length_(0) {}
-                                FecProtectionParams keyframe_params)
-    : delta_params(delta_params), keyframe_params(keyframe_params) {}
 
-UlpfecGenerator::UlpfecGenerator(int red_payload_type,
+RedPacket::~RedPacket() = default;
-                                 int ulpfec_payload_type,
-                                 Clock* clock)
-    : red_payload_type_(red_payload_type),
-      ulpfec_payload_type_(ulpfec_payload_type),
-      clock_(clock),
-      fec_(ForwardErrorCorrection::CreateUlpfec(kUnknownSsrc)),
-      num_protected_frames_(0),
-      min_num_media_packets_(1),
-      keyframe_in_process_(false),
-      fec_bitrate_(/*max_window_size_ms=*/1000, RateStatistics::kBpsScale) {}
 
-// Used by FlexFecSender, payload types are unused.
+void RedPacket::CreateHeader(const uint8_t* rtp_header,
-UlpfecGenerator::UlpfecGenerator(std::unique_ptr<ForwardErrorCorrection> fec,
+                             size_t header_length,
-                                 Clock* clock)
+                             int red_payload_type,
-    : red_payload_type_(0),
+                             int payload_type) {
-      ulpfec_payload_type_(0),
+  RTC_DCHECK_LE(header_length + kRedForFecHeaderLength, length_);
-      clock_(clock),
+  memcpy(data_.get(), rtp_header, header_length);
-      fec_(std::move(fec)),
+  // Replace payload type.
+  data_[1] &= 0x80;
+  data_[1] += red_payload_type;
+  // Add RED header
+  // f-bit always 0
+  data_[header_length] = static_cast<uint8_t>(payload_type);
+  header_length_ = header_length + kRedForFecHeaderLength;
+}
+
+void RedPacket::SetSeqNum(int seq_num) {
+  RTC_DCHECK_GE(seq_num, 0);
+  RTC_DCHECK_LT(seq_num, 1 << 16);
+
+  ByteWriter<uint16_t>::WriteBigEndian(&data_[2], seq_num);
+}
+
+void RedPacket::AssignPayload(const uint8_t* payload, size_t length) {
+  RTC_DCHECK_LE(header_length_ + length, length_);
+  memcpy(data_.get() + header_length_, payload, length);
+}
+
+void RedPacket::ClearMarkerBit() {
+  data_[1] &= 0x7F;
+}
+
+uint8_t* RedPacket::data() const {
+  return data_.get();
+}
+
+size_t RedPacket::length() const {
+  return length_;
+}
+
+UlpfecGenerator::UlpfecGenerator()
+    : UlpfecGenerator(ForwardErrorCorrection::CreateUlpfec(kUnknownSsrc)) {}
+
+UlpfecGenerator::UlpfecGenerator(std::unique_ptr<ForwardErrorCorrection> fec)
+    : fec_(std::move(fec)),
+      last_media_packet_rtp_header_length_(0),
       num_protected_frames_(0),
-      min_num_media_packets_(1),
+      min_num_media_packets_(1) {
-      keyframe_in_process_(false),
+  memset(&params_, 0, sizeof(params_));
-      fec_bitrate_(/*max_window_size_ms=*/1000, RateStatistics::kBpsScale) {}
+  memset(&new_params_, 0, sizeof(new_params_));
+}
 
 UlpfecGenerator::~UlpfecGenerator() = default;
 
-void UlpfecGenerator::SetProtectionParameters(
+void UlpfecGenerator::SetFecParameters(const FecProtectionParams& params) {
-    const FecProtectionParams& delta_params,
+  RTC_DCHECK_GE(params.fec_rate, 0);
-    const FecProtectionParams& key_params) {
+  RTC_DCHECK_LE(params.fec_rate, 255);
-  RTC_DCHECK_GE(delta_params.fec_rate, 0);
-  RTC_DCHECK_LE(delta_params.fec_rate, 255);
-  RTC_DCHECK_GE(key_params.fec_rate, 0);
-  RTC_DCHECK_LE(key_params.fec_rate, 255);
   // Store the new params and apply them for the next set of FEC packets being
   // produced.
-  rtc::CritScope cs(&crit_);
+  new_params_ = params;
-  pending_params_.emplace(delta_params, key_params);
+  if (params.fec_rate > kHighProtectionThreshold) {
-}
-
-void UlpfecGenerator::AddPacketAndGenerateFec(const RtpPacketToSend& packet) {
-  RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
-  RTC_DCHECK(generated_fec_packets_.empty());
-
-  if (media_packets_.empty()) {
-    rtc::CritScope cs(&crit_);
-    if (pending_params_) {
-      current_params_ = *pending_params_;
-      pending_params_.reset();
-
-      if (CurrentParams().fec_rate > kHighProtectionThreshold) {
     min_num_media_packets_ = kMinMediaPackets;
   } else {
     min_num_media_packets_ = 1;
   }
-    }
+}
 
-    keyframe_in_process_ = packet.is_key_frame();
+int UlpfecGenerator::AddRtpPacketAndGenerateFec(
+    const rtc::CopyOnWriteBuffer& data_buffer,
+    size_t rtp_header_length) {
+  RTC_DCHECK(generated_fec_packets_.empty());
+  if (media_packets_.empty()) {
+    params_ = new_params_;
   }
-  RTC_DCHECK_EQ(packet.is_key_frame(), keyframe_in_process_);
-
   bool complete_frame = false;
-  const bool marker_bit = packet.Marker();
+  const bool marker_bit = (data_buffer[1] & kRtpMarkerBitMask) ? true : false;
   if (media_packets_.size() < kUlpfecMaxMediaPackets) {
     // Our packet masks can only protect up to |kUlpfecMaxMediaPackets| packets.
-    auto fec_packet = std::make_unique<ForwardErrorCorrection::Packet>();
+    std::unique_ptr<ForwardErrorCorrection::Packet> packet(
-    fec_packet->data = packet.Buffer();
+        new ForwardErrorCorrection::Packet());
-    media_packets_.push_back(std::move(fec_packet));
+    RTC_DCHECK_GE(data_buffer.size(), rtp_header_length);
-
+    packet->data = data_buffer;
-    // Keep a copy of the last RTP packet, so we can copy the RTP header
+    media_packets_.push_back(std::move(packet));
-    // from it when creating newly generated ULPFEC+RED packets.
+    // Keep track of the RTP header length, so we can copy the RTP header
-    RTC_DCHECK_GE(packet.headers_size(), kRtpHeaderSize);
+    // from |packet| to newly generated ULPFEC+RED packets.
-    last_media_packet_ = packet;
+    RTC_DCHECK_GE(rtp_header_length, kRtpHeaderSize);
+    last_media_packet_rtp_header_length_ = rtp_header_length;
   }
-
   if (marker_bit) {
     ++num_protected_frames_;
     complete_frame = true;
   }
-
-  auto params = CurrentParams();
-
   // Produce FEC over at most |params_.max_fec_frames| frames, or as soon as:
   // (1) the excess overhead (actual overhead - requested/target overhead) is
   // less than |kMaxExcessOverhead|, and
   // (2) at least |min_num_media_packets_| media packets is reached.
   if (complete_frame &&
-      (num_protected_frames_ == params.max_fec_frames ||
+      (num_protected_frames_ == params_.max_fec_frames ||
        (ExcessOverheadBelowMax() && MinimumMediaPacketsReached()))) {
     // We are not using Unequal Protection feature of the parity erasure code.
     constexpr int kNumImportantPackets = 0;
     constexpr bool kUseUnequalProtection = false;
-    fec_->EncodeFec(media_packets_, params.fec_rate, kNumImportantPackets,
+    int ret = fec_->EncodeFec(media_packets_, params_.fec_rate,
-                    kUseUnequalProtection, params.fec_mask_type,
+                              kNumImportantPackets, kUseUnequalProtection,
-                    &generated_fec_packets_);
+                              params_.fec_mask_type, &generated_fec_packets_);
     if (generated_fec_packets_.empty()) {
       ResetState();
     }
+    return ret;
   }
+  return 0;
 }
 
 bool UlpfecGenerator::ExcessOverheadBelowMax() const {
-  RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
+  return ((Overhead() - params_.fec_rate) < kMaxExcessOverhead);
-
-  return ((Overhead() - CurrentParams().fec_rate) < kMaxExcessOverhead);
 }
 
 bool UlpfecGenerator::MinimumMediaPacketsReached() const {
-  RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
   float average_num_packets_per_frame =
       static_cast<float>(media_packets_.size()) / num_protected_frames_;
   int num_media_packets = static_cast<int>(media_packets_.size());
@@ -188,79 +196,61 @@ bool UlpfecGenerator::MinimumMediaPacketsReached() const {
   }
 }
 
-const FecProtectionParams& UlpfecGenerator::CurrentParams() const {
+bool UlpfecGenerator::FecAvailable() const {
-  RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
+  return !generated_fec_packets_.empty();
-  return keyframe_in_process_ ? current_params_.keyframe_params
+}
-                              : current_params_.delta_params;
+
+size_t UlpfecGenerator::NumAvailableFecPackets() const {
+  return generated_fec_packets_.size();
 }
 
 size_t UlpfecGenerator::MaxPacketOverhead() const {
-  RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
   return fec_->MaxPacketOverhead();
 }
 
-std::vector<std::unique_ptr<RtpPacketToSend>> UlpfecGenerator::GetFecPackets() {
+std::vector<std::unique_ptr<RedPacket>> UlpfecGenerator::GetUlpfecPacketsAsRed(
-  RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
+    int red_payload_type,
-  if (generated_fec_packets_.empty()) {
+    int ulpfec_payload_type,
-    return std::vector<std::unique_ptr<RtpPacketToSend>>();
+    uint16_t first_seq_num) {
-  }
+  std::vector<std::unique_ptr<RedPacket>> red_packets;
-
+  red_packets.reserve(generated_fec_packets_.size());
+  RTC_DCHECK(!media_packets_.empty());
+  ForwardErrorCorrection::Packet* last_media_packet =
+      media_packets_.back().get();
+  uint16_t seq_num = first_seq_num;
+  for (const auto* fec_packet : generated_fec_packets_) {
     // Wrap FEC packet (including FEC headers) in a RED packet. Since the
     // FEC packets in |generated_fec_packets_| don't have RTP headers, we
     // reuse the header from the last media packet.
-  RTC_CHECK(last_media_packet_.has_value());
+    RTC_DCHECK_GT(last_media_packet_rtp_header_length_, 0);
-  last_media_packet_->SetPayloadSize(0);
+    std::unique_ptr<RedPacket> red_packet(
-
+        new RedPacket(last_media_packet_rtp_header_length_ +
-  std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets;
+                      kRedForFecHeaderLength + fec_packet->data.size()));
-  fec_packets.reserve(generated_fec_packets_.size());
+    red_packet->CreateHeader(last_media_packet->data.data(),
-
+                             last_media_packet_rtp_header_length_,
-  size_t total_fec_size_bytes = 0;
+                             red_payload_type, ulpfec_payload_type);
-  for (const auto* fec_packet : generated_fec_packets_) {
+    red_packet->SetSeqNum(seq_num++);
-    std::unique_ptr<RtpPacketToSend> red_packet =
+    red_packet->ClearMarkerBit();
-        std::make_unique<RtpPacketToSend>(*last_media_packet_);
+    red_packet->AssignPayload(fec_packet->data.data(), fec_packet->data.size());
-    red_packet->SetPayloadType(red_payload_type_);
+    red_packets.push_back(std::move(red_packet));
-    red_packet->SetMarker(false);
-    uint8_t* payload_buffer = red_packet->SetPayloadSize(
-        kRedForFecHeaderLength + fec_packet->data.size());
-    // Primary RED header with F bit unset.
-    // See https://tools.ietf.org/html/rfc2198#section-3
-    payload_buffer[0] = ulpfec_payload_type_;  // RED header.
-    memcpy(&payload_buffer[1], fec_packet->data.data(),
-           fec_packet->data.size());
-    total_fec_size_bytes += red_packet->size();
-    red_packet->set_packet_type(RtpPacketMediaType::kForwardErrorCorrection);
-    red_packet->set_allow_retransmission(false);
-    fec_packets.push_back(std::move(red_packet));
   }
 
   ResetState();
 
-  rtc::CritScope cs(&crit_);
+  return red_packets;
-  fec_bitrate_.Update(total_fec_size_bytes, clock_->TimeInMilliseconds());
-
-  return fec_packets;
-}
-
-DataRate UlpfecGenerator::CurrentFecRate() const {
-  rtc::CritScope cs(&crit_);
-  return DataRate::BitsPerSec(
-      fec_bitrate_.Rate(clock_->TimeInMilliseconds()).value_or(0));
 }
 
 int UlpfecGenerator::Overhead() const {
-  RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
   RTC_DCHECK(!media_packets_.empty());
   int num_fec_packets =
-      fec_->NumFecPackets(media_packets_.size(), CurrentParams().fec_rate);
+      fec_->NumFecPackets(media_packets_.size(), params_.fec_rate);
-
   // Return the overhead in Q8.
   return (num_fec_packets << 8) / media_packets_.size();
 }
 
 void UlpfecGenerator::ResetState() {
-  RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
   media_packets_.clear();
-  last_media_packet_.reset();
+  last_media_packet_rtp_header_length_ = 0;
   generated_fec_packets_.clear();
   num_protected_frames_ = 0;
 }

modules/rtp_rtcp/source/ulpfec_generator.h

@@ -20,54 +20,63 @@
 
 #include "modules/include/module_fec_types.h"
 #include "modules/rtp_rtcp/source/forward_error_correction.h"
-#include "modules/rtp_rtcp/source/video_fec_generator.h"
-#include "rtc_base/critical_section.h"
-#include "rtc_base/race_checker.h"
-#include "rtc_base/rate_statistics.h"
 
 namespace webrtc {
 
 class FlexfecSender;
 
-class UlpfecGenerator : public VideoFecGenerator {
+class RedPacket {
+ public:
+  explicit RedPacket(size_t length);
+  ~RedPacket();
+
+  void CreateHeader(const uint8_t* rtp_header,
+                    size_t header_length,
+                    int red_payload_type,
+                    int payload_type);
+  void SetSeqNum(int seq_num);
+  void AssignPayload(const uint8_t* payload, size_t length);
+  void ClearMarkerBit();
+  uint8_t* data() const;
+  size_t length() const;
+
+ private:
+  std::unique_ptr<uint8_t[]> data_;
+  size_t length_;
+  size_t header_length_;
+};
+
+class UlpfecGenerator {
   friend class FlexfecSender;
 
  public:
-  UlpfecGenerator(int red_payload_type, int ulpfec_payload_type, Clock* clock);
+  UlpfecGenerator();
   ~UlpfecGenerator();
 
-  FecType GetFecType() const override {
+  void SetFecParameters(const FecProtectionParams& params);
-    return VideoFecGenerator::FecType::kUlpFec;
-  }
-  absl::optional<uint32_t> FecSsrc() override { return absl::nullopt; }
-
-  void SetProtectionParameters(const FecProtectionParams& delta_params,
-                               const FecProtectionParams& key_params) override;
 
   // Adds a media packet to the internal buffer. When enough media packets
   // have been added, the FEC packets are generated and stored internally.
   // These FEC packets are then obtained by calling GetFecPacketsAsRed().
-  void AddPacketAndGenerateFec(const RtpPacketToSend& packet) override;
+  int AddRtpPacketAndGenerateFec(const rtc::CopyOnWriteBuffer& data_buffer,
+                                 size_t rtp_header_length);
+
+  // Returns true if there are generated FEC packets available.
+  bool FecAvailable() const;
+
+  size_t NumAvailableFecPackets() const;
 
   // Returns the overhead, per packet, for FEC (and possibly RED).
-  size_t MaxPacketOverhead() const override;
+  size_t MaxPacketOverhead() const;
 
-  std::vector<std::unique_ptr<RtpPacketToSend>> GetFecPackets() override;
+  // Returns generated FEC packets with RED headers added.
-
+  std::vector<std::unique_ptr<RedPacket>> GetUlpfecPacketsAsRed(
-  // Current rate of FEC packets generated, including all RTP-level headers.
+      int red_payload_type,
-  DataRate CurrentFecRate() const override;
+      int ulpfec_payload_type,
+      uint16_t first_seq_num);
 
  private:
-  struct Params {
+  explicit UlpfecGenerator(std::unique_ptr<ForwardErrorCorrection> fec);
-    Params();
-    Params(FecProtectionParams delta_params,
-           FecProtectionParams keyframe_params);
-
-    FecProtectionParams delta_params;
-    FecProtectionParams keyframe_params;
-  };
-
-  UlpfecGenerator(std::unique_ptr<ForwardErrorCorrection> fec, Clock* clock);
 
   // Overhead is defined as relative to the number of media packets, and not
   // relative to total number of packets. This definition is inherited from the
@@ -88,31 +97,16 @@ class UlpfecGenerator : public VideoFecGenerator {
   // (e.g. (2k,2m) vs (k,m)) are generally more effective at recovering losses.
   bool MinimumMediaPacketsReached() const;
 
-  const FecProtectionParams& CurrentParams() const;
-
   void ResetState();
 
-  const int red_payload_type_;
+  std::unique_ptr<ForwardErrorCorrection> fec_;
-  const int ulpfec_payload_type_;
+  ForwardErrorCorrection::PacketList media_packets_;
-  Clock* const clock_;
+  size_t last_media_packet_rtp_header_length_;
-
+  std::list<ForwardErrorCorrection::Packet*> generated_fec_packets_;
-  rtc::RaceChecker race_checker_;
+  int num_protected_frames_;
-  const std::unique_ptr<ForwardErrorCorrection> fec_
+  int min_num_media_packets_;
-      RTC_GUARDED_BY(race_checker_);
+  FecProtectionParams params_;
-  ForwardErrorCorrection::PacketList media_packets_
+  FecProtectionParams new_params_;
-      RTC_GUARDED_BY(race_checker_);
-  absl::optional<RtpPacketToSend> last_media_packet_
-      RTC_GUARDED_BY(race_checker_);
-  std::list<ForwardErrorCorrection::Packet*> generated_fec_packets_
-      RTC_GUARDED_BY(race_checker_);
-  int num_protected_frames_ RTC_GUARDED_BY(race_checker_);
-  int min_num_media_packets_ RTC_GUARDED_BY(race_checker_);
-  Params current_params_ RTC_GUARDED_BY(race_checker_);
-  bool keyframe_in_process_ RTC_GUARDED_BY(race_checker_);
-
-  rtc::CriticalSection crit_;
-  absl::optional<Params> pending_params_ RTC_GUARDED_BY(crit_);
-  RateStatistics fec_bitrate_ RTC_GUARDED_BY(crit_);
 };
 
 }  // namespace webrtc

modules/rtp_rtcp/source/ulpfec_generator_unittest.cc

@@ -35,8 +35,11 @@ void VerifyHeader(uint16_t seq_num,
                   uint32_t timestamp,
                   int red_payload_type,
                   int fec_payload_type,
-                  bool marker_bit,
+                  RedPacket* packet,
-                  const rtc::CopyOnWriteBuffer& data) {
+                  bool marker_bit) {
+  EXPECT_GT(packet->length(), kRtpHeaderSize);
+  EXPECT_TRUE(packet->data() != NULL);
+  uint8_t* data = packet->data();
   // Marker bit not set.
   EXPECT_EQ(marker_bit ? 0x80 : 0, data[1] & 0x80);
   EXPECT_EQ(red_payload_type, data[1] & 0x7F);
@@ -49,12 +52,8 @@ void VerifyHeader(uint16_t seq_num,
 
 class UlpfecGeneratorTest : public ::testing::Test {
  protected:
-  UlpfecGeneratorTest()
+  UlpfecGeneratorTest() : packet_generator_(kMediaSsrc) {}
-      : fake_clock_(1),
-        ulpfec_generator_(kRedPayloadType, kFecPayloadType, &fake_clock_),
-        packet_generator_(kMediaSsrc) {}
 
-  SimulatedClock fake_clock_;
   UlpfecGenerator ulpfec_generator_;
   AugmentedPacketGenerator packet_generator_;
 };
@@ -82,22 +81,24 @@ TEST_F(UlpfecGeneratorTest, NoEmptyFecWithSeqNumGaps) {
   protected_packets.push_back({21, 0, 55, 0});
   protected_packets.push_back({13, 3, 57, 1});
   FecProtectionParams params = {117, 3, kFecMaskBursty};
-  ulpfec_generator_.SetProtectionParameters(params, params);
+  ulpfec_generator_.SetFecParameters(params);
+  uint8_t packet[28] = {0};
   for (Packet p : protected_packets) {
-    RtpPacketToSend packet(nullptr);
+    if (p.marker_bit) {
-    packet.SetMarker(p.marker_bit);
+      packet[1] |= 0x80;
-    packet.AllocateExtension(RTPExtensionType::kRtpExtensionMid,
-                             p.header_size - packet.headers_size());
-    packet.SetSequenceNumber(p.seq_num);
-    packet.AllocatePayload(p.payload_size);
-    ulpfec_generator_.AddPacketAndGenerateFec(packet);
-
-    std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets =
-        ulpfec_generator_.GetFecPackets();
-    if (!p.marker_bit) {
-      EXPECT_TRUE(fec_packets.empty());
     } else {
-      EXPECT_FALSE(fec_packets.empty());
+      packet[1] &= ~0x80;
+    }
+    ByteWriter<uint16_t>::WriteBigEndian(&packet[2], p.seq_num);
+    ulpfec_generator_.AddRtpPacketAndGenerateFec(
+        rtc::CopyOnWriteBuffer(packet, p.payload_size + p.header_size),
+        p.header_size);
+    size_t num_fec_packets = ulpfec_generator_.NumAvailableFecPackets();
+    if (num_fec_packets > 0) {
+      std::vector<std::unique_ptr<RedPacket>> fec_packets =
+          ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType,
+                                                  kFecPayloadType, 100);
+      EXPECT_EQ(num_fec_packets, fec_packets.size());
     }
   }
 }
@@ -112,28 +113,24 @@ TEST_F(UlpfecGeneratorTest, OneFrameFec) {
   constexpr size_t kNumPackets = 4;
   FecProtectionParams params = {15, 3, kFecMaskRandom};
   packet_generator_.NewFrame(kNumPackets);
-  // Expecting one FEC packet.
+  ulpfec_generator_.SetFecParameters(params);  // Expecting one FEC packet.
-  ulpfec_generator_.SetProtectionParameters(params, params);
   uint32_t last_timestamp = 0;
   for (size_t i = 0; i < kNumPackets; ++i) {
     std::unique_ptr<AugmentedPacket> packet =
         packet_generator_.NextPacket(i, 10);
-    RtpPacketToSend rtp_packet(nullptr);
+    EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec(packet->data,
-    EXPECT_TRUE(rtp_packet.Parse(packet->data.data(), packet->data.size()));
+                                                              kRtpHeaderSize));
-    ulpfec_generator_.AddPacketAndGenerateFec(rtp_packet);
     last_timestamp = packet->header.timestamp;
   }
-  std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets =
+  EXPECT_TRUE(ulpfec_generator_.FecAvailable());
-      ulpfec_generator_.GetFecPackets();
+  const uint16_t seq_num = packet_generator_.NextPacketSeqNum();
-  EXPECT_EQ(fec_packets.size(), 1u);
+  std::vector<std::unique_ptr<RedPacket>> red_packets =
-  uint16_t seq_num = packet_generator_.NextPacketSeqNum();
+      ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType,
-  fec_packets[0]->SetSequenceNumber(seq_num);
+                                              seq_num);
-  EXPECT_TRUE(ulpfec_generator_.GetFecPackets().empty());
+  EXPECT_FALSE(ulpfec_generator_.FecAvailable());
-
+  ASSERT_EQ(1u, red_packets.size());
-  EXPECT_EQ(fec_packets[0]->headers_size(), kRtpHeaderSize);
+  VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType,
-
+               red_packets.front().get(), false);
-  VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType, false,
-               fec_packets[0]->Buffer());
 }
 
 TEST_F(UlpfecGeneratorTest, TwoFrameFec) {
@@ -148,27 +145,27 @@ TEST_F(UlpfecGeneratorTest, TwoFrameFec) {
   constexpr size_t kNumFrames = 2;
 
   FecProtectionParams params = {15, 3, kFecMaskRandom};
-  // Expecting one FEC packet.
+  ulpfec_generator_.SetFecParameters(params);  // Expecting one FEC packet.
-  ulpfec_generator_.SetProtectionParameters(params, params);
   uint32_t last_timestamp = 0;
   for (size_t i = 0; i < kNumFrames; ++i) {
     packet_generator_.NewFrame(kNumPackets);
     for (size_t j = 0; j < kNumPackets; ++j) {
       std::unique_ptr<AugmentedPacket> packet =
           packet_generator_.NextPacket(i * kNumPackets + j, 10);
-      RtpPacketToSend rtp_packet(nullptr);
+      EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec(
-      EXPECT_TRUE(rtp_packet.Parse(packet->data.data(), packet->data.size()));
+                       packet->data, kRtpHeaderSize));
-      ulpfec_generator_.AddPacketAndGenerateFec(rtp_packet);
       last_timestamp = packet->header.timestamp;
     }
   }
-  std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets =
+  EXPECT_TRUE(ulpfec_generator_.FecAvailable());
-      ulpfec_generator_.GetFecPackets();
-  EXPECT_EQ(fec_packets.size(), 1u);
   const uint16_t seq_num = packet_generator_.NextPacketSeqNum();
-  fec_packets[0]->SetSequenceNumber(seq_num);
+  std::vector<std::unique_ptr<RedPacket>> red_packets =
-  VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType, false,
+      ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType,
-               fec_packets[0]->Buffer());
+                                              seq_num);
+  EXPECT_FALSE(ulpfec_generator_.FecAvailable());
+  ASSERT_EQ(1u, red_packets.size());
+  VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType,
+               red_packets.front().get(), false);
 }
 
 TEST_F(UlpfecGeneratorTest, MixedMediaRtpHeaderLengths) {
@@ -177,43 +174,34 @@ TEST_F(UlpfecGeneratorTest, MixedMediaRtpHeaderLengths) {
 
   // Only one frame required to generate FEC.
   FecProtectionParams params = {127, 1, kFecMaskRandom};
-  ulpfec_generator_.SetProtectionParameters(params, params);
+  ulpfec_generator_.SetFecParameters(params);
 
   // Fill up internal buffer with media packets with short RTP header length.
   packet_generator_.NewFrame(kUlpfecMaxMediaPackets + 1);
   for (size_t i = 0; i < kUlpfecMaxMediaPackets; ++i) {
     std::unique_ptr<AugmentedPacket> packet =
         packet_generator_.NextPacket(i, 10);
-    RtpPacketToSend rtp_packet(nullptr);
+    EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec(
-    EXPECT_TRUE(rtp_packet.Parse(packet->data.data(), packet->data.size()));
+                     packet->data, kShortRtpHeaderLength));
-    EXPECT_EQ(rtp_packet.headers_size(), kShortRtpHeaderLength);
+    EXPECT_FALSE(ulpfec_generator_.FecAvailable());
-    ulpfec_generator_.AddPacketAndGenerateFec(rtp_packet);
-    EXPECT_TRUE(ulpfec_generator_.GetFecPackets().empty());
   }
 
   // Kick off FEC generation with media packet with long RTP header length.
   // Since the internal buffer is full, this packet will not be protected.
   std::unique_ptr<AugmentedPacket> packet =
       packet_generator_.NextPacket(kUlpfecMaxMediaPackets, 10);
-  RtpPacketToSend rtp_packet(nullptr);
+  EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec(
-  EXPECT_TRUE(rtp_packet.Parse(packet->data.data(), packet->data.size()));
+                   packet->data, kLongRtpHeaderLength));
-  EXPECT_TRUE(rtp_packet.SetPayloadSize(0) != nullptr);
+  EXPECT_TRUE(ulpfec_generator_.FecAvailable());
-  const uint32_t csrcs[]{1};
-  rtp_packet.SetCsrcs(csrcs);
-
-  EXPECT_EQ(rtp_packet.headers_size(), kLongRtpHeaderLength);
-
-  ulpfec_generator_.AddPacketAndGenerateFec(rtp_packet);
-  std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets =
-      ulpfec_generator_.GetFecPackets();
-  EXPECT_FALSE(fec_packets.empty());
 
   // Ensure that the RED header is placed correctly, i.e. the correct
   // RTP header length was used in the RED packet creation.
-  uint16_t seq_num = packet_generator_.NextPacketSeqNum();
+  const uint16_t seq_num = packet_generator_.NextPacketSeqNum();
-  for (const auto& fec_packet : fec_packets) {
+  std::vector<std::unique_ptr<RedPacket>> red_packets =
-    fec_packet->SetSequenceNumber(seq_num++);
+      ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType,
-    EXPECT_EQ(kFecPayloadType, fec_packet->data()[kShortRtpHeaderLength]);
+                                              seq_num);
+  for (const auto& red_packet : red_packets) {
+    EXPECT_EQ(kFecPayloadType, red_packet->data()[kShortRtpHeaderLength]);
   }
 }
 

modules/rtp_rtcp/source/video_fec_generator.h

@@ -1,51 +0,0 @@
-/*
- *  Copyright (c) 2019 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.
- */
-
-#ifndef MODULES_RTP_RTCP_SOURCE_VIDEO_FEC_GENERATOR_H_
-#define MODULES_RTP_RTCP_SOURCE_VIDEO_FEC_GENERATOR_H_
-
-#include <memory>
-#include <vector>
-
-#include "api/units/data_rate.h"
-#include "modules/include/module_fec_types.h"
-#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
-
-namespace webrtc {
-
-class VideoFecGenerator {
- public:
-  VideoFecGenerator() = default;
-  virtual ~VideoFecGenerator() = default;
-
-  enum class FecType { kFlexFec, kUlpFec };
-  virtual FecType GetFecType() const = 0;
-  // Returns the SSRC used for FEC packets (i.e. FlexFec SSRC).
-  virtual absl::optional<uint32_t> FecSsrc() = 0;
-  // Returns the overhead, in bytes per packet, for FEC (and possibly RED).
-  virtual size_t MaxPacketOverhead() const = 0;
-  // Current rate of FEC packets generated, including all RTP-level headers.
-  virtual DataRate CurrentFecRate() const = 0;
-  // Set FEC rates, max frames before FEC is sent, and type of FEC masks.
-  virtual void SetProtectionParameters(
-      const FecProtectionParams& delta_params,
-      const FecProtectionParams& key_params) = 0;
-  // Called on new media packet to be protected. The generator may choose
-  // to generate FEC packets at this time, if so they will be stored in an
-  // internal buffer.
-  virtual void AddPacketAndGenerateFec(const RtpPacketToSend& packet) = 0;
-  // Get (and remove) and FEC packets pending in the generator. These packets
-  // will lack sequence numbers, that needs to be set externally.
-  // TODO(bugs.webrtc.org/11340): Actually FlexFec sets seq#, fix that!
-  virtual std::vector<std::unique_ptr<RtpPacketToSend>> GetFecPackets() = 0;
-};
-
-}  // namespace webrtc
-#endif  // MODULES_RTP_RTCP_SOURCE_VIDEO_FEC_GENERATOR_H_

test/fuzzers/BUILD.gn

@@ -148,7 +148,6 @@ webrtc_fuzzer_test("ulpfec_generator_fuzzer") {
     "../../modules/rtp_rtcp:rtp_rtcp_format",
     "../../rtc_base:checks",
     "../../rtc_base:rtc_base_approved",
-    "../../system_wrappers",
   ]
 }
 

test/fuzzers/flexfec_sender_fuzzer.cc

@@ -41,7 +41,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
   FecProtectionParams params = {
       data[i++], static_cast<int>(data[i++] % 100),
       data[i++] <= 127 ? kFecMaskRandom : kFecMaskBursty};
-  sender.SetProtectionParameters(params, params);
+  sender.SetFecParameters(params);
   uint16_t seq_num = data[i++];
 
   while (i + 1 < size) {
@@ -59,9 +59,12 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
     RtpPacketToSend rtp_packet(nullptr);
     if (!rtp_packet.Parse(packet.get(), kRtpHeaderSize + payload_size))
       break;
-    sender.AddPacketAndGenerateFec(rtp_packet);
+    sender.AddRtpPacketAndGenerateFec(rtp_packet);
+    if (sender.FecAvailable()) {
+      std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets =
           sender.GetFecPackets();
     }
+  }
 }
 
 }  // namespace webrtc

test/fuzzers/ulpfec_generator_fuzzer.cc

@@ -16,7 +16,6 @@
 #include "modules/rtp_rtcp/source/ulpfec_generator.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/copy_on_write_buffer.h"
-#include "system_wrappers/include/clock.h"
 
 namespace webrtc {
 
@@ -26,14 +25,13 @@ constexpr uint8_t kRedPayloadType = 97;
 }  // namespace
 
 void FuzzOneInput(const uint8_t* data, size_t size) {
-  SimulatedClock clock(1);
+  UlpfecGenerator generator;
-  UlpfecGenerator generator(kRedPayloadType, kFecPayloadType, &clock);
   size_t i = 0;
   if (size < 4)
     return;
   FecProtectionParams params = {
       data[i++] % 128, static_cast<int>(data[i++] % 10), kFecMaskBursty};
-  generator.SetProtectionParameters(params, params);
+  generator.SetFecParameters(params);
   uint16_t seq_num = data[i++];
   uint16_t prev_seq_num = 0;
   while (i + 3 < size) {
@@ -53,13 +51,16 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
     // number became out of order.
     if (protect && IsNewerSequenceNumber(seq_num, prev_seq_num) &&
         seq_num < prev_seq_num + kUlpfecMaxMediaPackets) {
-      RtpPacketToSend rtp_packet(nullptr);
+      generator.AddRtpPacketAndGenerateFec(packet, rtp_header_length);
-      rtp_packet.Parse(packet);
-      generator.AddPacketAndGenerateFec(rtp_packet);
       prev_seq_num = seq_num;
     }
-
+    const size_t num_fec_packets = generator.NumAvailableFecPackets();
-    generator.GetFecPackets();
+    if (num_fec_packets > 0) {
+      std::vector<std::unique_ptr<RedPacket>> fec_packets =
+          generator.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType,
+                                          100);
+      RTC_CHECK_EQ(num_fec_packets, fec_packets.size());
+    }
   }
 }
 }  // namespace webrtc