Refactors FEC in preparation for deferred packet generation.

RtpVideoSender now stores fec type and overhead instead of querying the
generator all the time. Setting of protection parameters and asking for
current bitrate is also now handled just by the VideoFecGenerator
instance, instead of going via RtpVideoSender.
Finally, adds method to query for RtpState in VideoFecGenerator
interface. This avoids an ugly cast that would have been even more
trouble after moving fec generation.

For context, see https://webrtc-review.googlesource.com/c/src/+/173708

Bug: webrtc:11340
Change-Id: Ia5e6cd919e71850c9cc5ed5a4f4417338d577162
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174203
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31166}

call/rtp_video_sender.cc

@@ -278,6 +278,10 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
         rtp_config.ulpfec.red_payload_type != -1) {
       video_config.red_payload_type = rtp_config.ulpfec.red_payload_type;
     }
+    if (fec_generator) {
+      video_config.fec_type = fec_generator->GetFecType();
+      video_config.fec_overhead_bytes = fec_generator->MaxPacketOverhead();
+    }
     video_config.frame_transformer = frame_transformer;
     auto sender_video = std::make_unique<RTPSenderVideo>(video_config);
     rtp_streams.emplace_back(std::move(rtp_rtcp), std::move(sender_video),
@@ -301,8 +305,8 @@ absl::optional<VideoCodecType> GetVideoCodecType(const RtpConfig& config) {
   }
   return PayloadStringToCodecType(config.payload_name);
 }
-bool TransportSeqNumExtensionConfigured(const RtpConfig& config_config) {
+bool TransportSeqNumExtensionConfigured(const RtpConfig& config) {
-  return absl::c_any_of(config_config.extensions, [](const RtpExtension& ext) {
+  return absl::c_any_of(config.extensions, [](const RtpExtension& ext) {
     return ext.uri == RtpExtension::kTransportSequenceNumberUri;
   });
 }
@@ -685,12 +689,15 @@ std::map<uint32_t, RtpState> RtpVideoSender::GetRtpStates() const {
     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();
+    // Only happens during shutdown, when RTP module is already inactive,
-    if (fec_generator &&
+    // so OK to call fec generator here.
-        fec_generator->GetFecType() == VideoFecGenerator::FecType::kFlexFec) {
+    if (rtp_streams_[i].fec_generator) {
-      auto* flexfec_sender = static_cast<FlexfecSender*>(fec_generator);
+      absl::optional<RtpState> fec_state =
-      uint32_t ssrc = rtp_config_.flexfec.ssrc;
+          rtp_streams_[i].fec_generator->GetRtpState();
-      rtp_states[ssrc] = flexfec_sender->GetRtpState();
+      if (fec_state) {
+        uint32_t ssrc = rtp_config_.flexfec.ssrc;
+        rtp_states[ssrc] = *fec_state;
+      }
     }
   }
 
@@ -828,9 +835,11 @@ int RtpVideoSender::ProtectionRequest(const FecProtectionParams* delta_params,
   for (const RtpStreamSender& stream : rtp_streams_) {
     uint32_t not_used = 0;
     uint32_t module_nack_rate = 0;
-    stream.sender_video->SetFecParameters(*delta_params, *key_params);
+    if (stream.fec_generator) {
+      stream.fec_generator->SetProtectionParameters(*delta_params, *key_params);
+      *sent_fec_rate_bps += stream.fec_generator->CurrentFecRate().bps();
+    }
     *sent_video_rate_bps += stream.sender_video->VideoBitrateSent();
-    *sent_fec_rate_bps += stream.sender_video->FecOverheadRate();
     stream.rtp_rtcp->BitrateSent(&not_used, /*video_rate=*/nullptr,
                                  /*fec_rate=*/nullptr, &module_nack_rate);
     *sent_nack_rate_bps += module_nack_rate;

modules/rtp_rtcp/include/flexfec_sender.h

@@ -69,7 +69,7 @@ class FlexfecSender : public VideoFecGenerator {
   DataRate CurrentFecRate() const override;
 
   // Only called on the VideoSendStream queue, after operation has shut down.
-  RtpState GetRtpState();
+  absl::optional<RtpState> GetRtpState() override;
 
  private:
   // Utility.

modules/rtp_rtcp/source/flexfec_sender.cc

@@ -193,7 +193,7 @@ DataRate FlexfecSender::CurrentFecRate() const {
       fec_bitrate_.Rate(clock_->TimeInMilliseconds()).value_or(0));
 }
 
-RtpState FlexfecSender::GetRtpState() {
+absl::optional<RtpState> FlexfecSender::GetRtpState() {
   RtpState rtp_state;
   rtp_state.sequence_number = seq_num_;
   rtp_state.start_timestamp = timestamp_offset_;

modules/rtp_rtcp/source/flexfec_sender_unittest.cc

@@ -326,7 +326,7 @@ TEST(FlexfecSenderTest, SetsAndGetsRtpState) {
   EXPECT_EQ(initial_rtp_state.start_timestamp + 1 * kVideoPayloadTypeFrequency,
             fec_packet->Timestamp());
 
-  RtpState updated_rtp_state = sender.GetRtpState();
+  RtpState updated_rtp_state = sender.GetRtpState().value();
   EXPECT_EQ(initial_rtp_state.sequence_number + 2,
             updated_rtp_state.sequence_number);
   EXPECT_EQ(initial_rtp_state.start_timestamp,

modules/rtp_rtcp/source/rtp_sender_unittest.cc

@@ -1268,6 +1268,8 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) {
   video_config.clock = &fake_clock_;
   video_config.rtp_sender = rtp_sender();
   video_config.fec_generator = &flexfec_sender;
+  video_config.fec_type = flexfec_sender.GetFecType();
+  video_config.fec_overhead_bytes = flexfec_sender.MaxPacketOverhead();
   video_config.field_trials = &field_trials;
   RTPSenderVideo rtp_sender_video(video_config);
 
@@ -1276,7 +1278,7 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) {
   params.fec_rate = 15;
   params.max_fec_frames = 1;
   params.fec_mask_type = kFecMaskRandom;
-  rtp_sender_video.SetFecParameters(params, params);
+  flexfec_sender.SetProtectionParameters(params, params);
 
   uint16_t flexfec_seq_num;
   RTPVideoHeader video_header;
@@ -1352,6 +1354,8 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) {
   video_config.clock = &fake_clock_;
   video_config.rtp_sender = rtp_sender();
   video_config.fec_generator = &flexfec_sender;
+  video_config.fec_type = flexfec_sender.GetFecType();
+  video_config.fec_overhead_bytes = flexfec_sender_.MaxPacketOverhead();
   video_config.field_trials = &field_trials;
   RTPSenderVideo rtp_sender_video(video_config);
 
@@ -1360,7 +1364,7 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) {
   params.fec_rate = 15;
   params.max_fec_frames = 1;
   params.fec_mask_type = kFecMaskRandom;
-  rtp_sender_video.SetFecParameters(params, params);
+  flexfec_sender.SetProtectionParameters(params, params);
 
   EXPECT_CALL(mock_rtc_event_log_,
               LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing)))
@@ -1684,6 +1688,8 @@ TEST_P(RtpSenderTest, FecOverheadRate) {
   video_config.clock = &fake_clock_;
   video_config.rtp_sender = rtp_sender();
   video_config.fec_generator = &flexfec_sender;
+  video_config.fec_type = flexfec_sender.GetFecType();
+  video_config.fec_overhead_bytes = flexfec_sender.MaxPacketOverhead();
   video_config.field_trials = &field_trials;
   RTPSenderVideo rtp_sender_video(video_config);
   // Parameters selected to generate a single FEC packet per media packet.
@@ -1691,7 +1697,7 @@ TEST_P(RtpSenderTest, FecOverheadRate) {
   params.fec_rate = 15;
   params.max_fec_frames = 1;
   params.fec_mask_type = kFecMaskRandom;
-  rtp_sender_video.SetFecParameters(params, params);
+  flexfec_sender.SetProtectionParameters(params, params);
 
   constexpr size_t kNumMediaPackets = 10;
   constexpr size_t kNumFecPackets = kNumMediaPackets;
@@ -1716,7 +1722,7 @@ TEST_P(RtpSenderTest, FecOverheadRate) {
                                    kGenericCodecHeaderLength + kPayloadLength;
   EXPECT_NEAR(kNumFecPackets * kPacketLength * 8 /
                   (kNumFecPackets * kTimeBetweenPacketsMs / 1000.0f),
-              rtp_sender_video.FecOverheadRate(), 500);
+              flexfec_sender.CurrentFecRate().bps<double>(), 500);
 }
 
 TEST_P(RtpSenderTest, BitrateCallbacks) {
@@ -1873,6 +1879,8 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) {
   video_config.field_trials = &field_trials;
   video_config.red_payload_type = kRedPayloadType;
   video_config.fec_generator = &ulpfec_generator;
+  video_config.fec_type = ulpfec_generator.GetFecType();
+  video_config.fec_overhead_bytes = ulpfec_generator.MaxPacketOverhead();
   RTPSenderVideo rtp_sender_video(video_config);
   uint8_t payload[] = {47, 11, 32, 93, 89};
   rtp_sender_context_->packet_history_.SetStorePacketsStatus(
@@ -1887,7 +1895,7 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) {
   fec_params.fec_mask_type = kFecMaskRandom;
   fec_params.fec_rate = 1;
   fec_params.max_fec_frames = 1;
-  rtp_sender_video.SetFecParameters(fec_params, fec_params);
+  ulpfec_generator.SetProtectionParameters(fec_params, fec_params);
   video_header.frame_type = VideoFrameType::kVideoFrameDelta;
   ASSERT_TRUE(rtp_sender_video.SendVideo(kPayloadType, kCodecType, 1234, 4321,
                                          payload, nullptr, video_header,

modules/rtp_rtcp/source/rtp_sender_video.cc

@@ -129,6 +129,8 @@ RTPSenderVideo::RTPSenderVideo(const Config& config)
       playout_delay_pending_(false),
       red_payload_type_(config.red_payload_type),
       fec_generator_(config.fec_generator),
+      fec_type_(config.fec_type),
+      fec_overhead_bytes_(config.fec_overhead_bytes),
       video_bitrate_(1000, RateStatistics::kBpsScale),
       packetization_overhead_bitrate_(1000, RateStatistics::kBpsScale),
       frame_encryptor_(config.frame_encryptor),
@@ -159,11 +161,15 @@ void RTPSenderVideo::LogAndSendToNetwork(
     size_t unpacketized_payload_size) {
   int64_t now_ms = clock_->TimeInMilliseconds();
 #if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE
-  for (const auto& packet : packets) {
+  if (fec_generator_) {
-    if (packet->packet_type() == RtpPacketMediaType::kForwardErrorCorrection) {
+    uint32_t fec_rate_kbps = fec_generator_->CurrentFecRate().kbps();
-      const uint32_t ssrc = packet->Ssrc();
+    for (const auto& packet : packets) {
-      BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoFecBitrate_kbps", now_ms,
+      if (packet->packet_type() ==
-                                      FecOverheadRate() / 1000, ssrc);
+          RtpPacketMediaType::kForwardErrorCorrection) {
+        const uint32_t ssrc = packet->Ssrc();
+        BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoFecBitrate_kbps", now_ms,
+                                        fec_rate_kbps, ssrc);
+      }
     }
   }
 #endif
@@ -189,14 +195,12 @@ void RTPSenderVideo::LogAndSendToNetwork(
 }
 
 size_t RTPSenderVideo::FecPacketOverhead() const {
-  size_t overhead = fec_generator_ ? fec_generator_->MaxPacketOverhead() : 0u;
+  size_t overhead = fec_overhead_bytes_;
   if (red_enabled()) {
     // The RED overhead is due to a small header.
     overhead += kRedForFecHeaderLength;
 
-    // TODO(bugs.webrtc.org/11340): Move this into UlpfecGenerator.
+    if (fec_type_ == VideoFecGenerator::FecType::kUlpFec) {
-    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...)
@@ -209,13 +213,6 @@ size_t RTPSenderVideo::FecPacketOverhead() const {
   return overhead;
 }
 
-void RTPSenderVideo::SetFecParameters(const FecProtectionParams& delta_params,
-                                      const FecProtectionParams& key_params) {
-  if (fec_generator_) {
-    fec_generator_->SetProtectionParameters(delta_params, key_params);
-  }
-}
-
 void RTPSenderVideo::SetVideoStructure(
     const FrameDependencyStructure* video_structure) {
   if (frame_transformer_delegate_) {
@@ -582,9 +579,6 @@ bool RTPSenderVideo::SendVideo(
     if (!rtp_sender_->AssignSequenceNumber(packet.get()))
       return false;
 
-    // No FEC protection for upper temporal layers, if used.
-    bool protect_packet = temporal_id == 0 || temporal_id == kNoTemporalIdx;
-
     packet->set_allow_retransmission(allow_retransmission);
 
     // Put packetization finish timestamp into extension.
@@ -592,8 +586,15 @@ bool RTPSenderVideo::SendVideo(
       packet->set_packetization_finish_time_ms(clock_->TimeInMilliseconds());
     }
 
-    if (protect_packet && fec_generator_) {
+    // No FEC protection for upper temporal layers, if used.
-      fec_generator_->AddPacketAndGenerateFec(*packet);
+    if (fec_type_.has_value() &&
+        (temporal_id == 0 || temporal_id == kNoTemporalIdx)) {
+      if (fec_generator_) {
+        fec_generator_->AddPacketAndGenerateFec(*packet);
+      } else {
+        // TODO(sprang): When deferred FEC generation is enabled, just mark the
+        // packet as protected here.
+      }
     }
 
     if (red_enabled()) {
@@ -626,9 +627,6 @@ bool RTPSenderVideo::SendVideo(
     // 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) {
@@ -692,10 +690,6 @@ uint32_t RTPSenderVideo::VideoBitrateSent() const {
   return video_bitrate_.Rate(clock_->TimeInMilliseconds()).value_or(0);
 }
 
-uint32_t RTPSenderVideo::FecOverheadRate() const {
-  return fec_generator_ ? fec_generator_->CurrentFecRate().bps<uint32_t>() : 0u;
-}
-
 uint32_t RTPSenderVideo::PacketizationOverheadBps() const {
   rtc::CritScope cs(&stats_crit_);
   return packetization_overhead_bitrate_.Rate(clock_->TimeInMilliseconds())

modules/rtp_rtcp/source/rtp_sender_video.h

@@ -71,6 +71,10 @@ class RTPSenderVideo {
     RTPSender* rtp_sender = nullptr;
     FlexfecSender* flexfec_sender = nullptr;
     VideoFecGenerator* fec_generator = nullptr;
+    // Some FEC data is duplicated here in preparation of moving FEC to
+    // the egress stage.
+    absl::optional<VideoFecGenerator::FecType> fec_type;
+    size_t fec_overhead_bytes = 0;  // Per packet max FEC overhead.
     FrameEncryptorInterface* frame_encryptor = nullptr;
     bool require_frame_encryption = false;
     bool enable_retransmit_all_layers = false;
@@ -112,13 +116,7 @@ class RTPSenderVideo {
   void SetVideoStructureUnderLock(
       const FrameDependencyStructure* video_structure);
 
-  // FlexFEC/ULPFEC.
-  // Set FEC rates, max frames before FEC is sent, and type of FEC masks.
-  void SetFecParameters(const FecProtectionParams& delta_params,
-                        const FecProtectionParams& key_params);
-
   uint32_t VideoBitrateSent() const;
-  uint32_t FecOverheadRate() const;
 
   // Returns the current packetization overhead rate, in bps. Note that this is
   // the payload overhead, eg the VP8 payload headers, not the RTP headers
@@ -191,6 +189,8 @@ class RTPSenderVideo {
 
   const absl::optional<int> red_payload_type_;
   VideoFecGenerator* const fec_generator_;
+  absl::optional<VideoFecGenerator::FecType> fec_type_;
+  const size_t fec_overhead_bytes_;  // Per packet max FEC overhead.
 
   rtc::CriticalSection stats_crit_;
   // Bitrate used for video payload and RTP headers.

modules/rtp_rtcp/source/ulpfec_generator.h

@@ -57,6 +57,8 @@ class UlpfecGenerator : public VideoFecGenerator {
   // Current rate of FEC packets generated, including all RTP-level headers.
   DataRate CurrentFecRate() const override;
 
+  absl::optional<RtpState> GetRtpState() override { return absl::nullopt; }
+
  private:
   struct Params {
     Params();

modules/rtp_rtcp/source/video_fec_generator.h

@@ -45,6 +45,9 @@ class VideoFecGenerator {
   // 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;
+  // Only called on the VideoSendStream queue, after operation has shut down,
+  // and only populated if there is an RtpState (e.g. FlexFec).
+  virtual absl::optional<RtpState> GetRtpState() = 0;
 };
 
 }  // namespace webrtc