Reland "Reland "Allows FEC generation after pacer step.""
This is a reland of 19df870d924662e3b6efb86078d31a8e086b38b5
Patchset 1 is the original.
Subsequent patchset changes threadchecker that crashed with downstream
code.
Original change's description:
> Reland "Allows FEC generation after pacer step."
>
> This is a reland of 75fd127640bdf1729af6b4a25875e6d01f1570e0
>
> Patchset 2 contains a fix. Old code can in factor call
> RtpRtcpImpl::FetchFec(). It should only be a noop since deferred fec
> is not supported there - we shouldn't crash.
>
> Original change's description:
> > Allows FEC generation after pacer step.
> >
> > Split out from https://webrtc-review.googlesource.com/c/src/+/173708
> > This CL enables FEC packets to be generated as media packets are sent,
> > rather than generated, i.e. media packets are inserted into the fec
> > generator after the pacing stage rather than at packetization time.
> >
> > This may have some small impact of performance. FEC packets are
> > typically only generated when a new packet with a marker bit is added,
> > which means FEC packets protecting a frame will now be sent after all
> > of the media packets, rather than (potentially) interleaved with them.
> > Therefore this feature is currently behind a flag so we can examine the
> > impact. Once we are comfortable with the behavior we'll make it default
> > and remove the old code.
> >
> > Note that this change does not include the "protect all header
> > extensions" part of the original CL - that will be a follow-up.
> >
> > Bug: webrtc:11340
> > Change-Id: I3fe139c5d53968579b75b91e2612075451ff0f5d
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177760
> > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#31558}
>
> Bug: webrtc:11340
> Change-Id: I2ea49ee87ee9ff409044e34a777a7dd0ae0a077f
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177984
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31613}
Bug: webrtc:11340
Change-Id: Ib741c8c284f523c959f8aca454088d9eee7b17f8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178600
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31619}
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 5f8d2df..854a18a 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -197,6 +197,7 @@
FrameEncryptorInterface* frame_encryptor,
const CryptoOptions& crypto_options,
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
+ bool use_deferred_fec,
const WebRtcKeyValueConfig& trials) {
RTC_DCHECK_GT(rtp_config.ssrcs.size(), 0);
@@ -244,7 +245,9 @@
std::unique_ptr<VideoFecGenerator> fec_generator =
MaybeCreateFecGenerator(clock, rtp_config, suspended_ssrcs, i, trials);
configuration.fec_generator = fec_generator.get();
- video_config.fec_generator = fec_generator.get();
+ if (!use_deferred_fec) {
+ video_config.fec_generator = fec_generator.get();
+ }
configuration.rtx_send_ssrc =
rtp_config.GetRtxSsrcAssociatedWithMediaSsrc(rtp_config.ssrcs[i]);
@@ -338,6 +341,9 @@
field_trials_.Lookup("WebRTC-UseEarlyLossDetection"),
"Disabled")),
has_packet_feedback_(TransportSeqNumExtensionConfigured(rtp_config)),
+ use_deferred_fec_(
+ absl::StartsWith(field_trials_.Lookup("WebRTC-DeferredFecGeneration"),
+ "Enabled")),
active_(false),
module_process_thread_(nullptr),
suspended_ssrcs_(std::move(suspended_ssrcs)),
@@ -356,6 +362,7 @@
frame_encryptor,
crypto_options,
std::move(frame_transformer),
+ use_deferred_fec_,
field_trials_)),
rtp_config_(rtp_config),
codec_type_(GetVideoCodecType(rtp_config)),
@@ -848,14 +855,26 @@
*sent_nack_rate_bps = 0;
*sent_fec_rate_bps = 0;
for (const RtpStreamSender& stream : rtp_streams_) {
- if (stream.fec_generator) {
- stream.fec_generator->SetProtectionParameters(*delta_params, *key_params);
- *sent_fec_rate_bps += stream.fec_generator->CurrentFecRate().bps();
+ if (use_deferred_fec_) {
+ stream.rtp_rtcp->SetFecProtectionParams(*delta_params, *key_params);
+
+ auto send_bitrate = stream.rtp_rtcp->GetSendRates();
+ *sent_video_rate_bps += send_bitrate[RtpPacketMediaType::kVideo].bps();
+ *sent_fec_rate_bps +=
+ send_bitrate[RtpPacketMediaType::kForwardErrorCorrection].bps();
+ *sent_nack_rate_bps +=
+ send_bitrate[RtpPacketMediaType::kRetransmission].bps();
+ } else {
+ 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_nack_rate_bps +=
+ stream.rtp_rtcp->GetSendRates()[RtpPacketMediaType::kRetransmission]
+ .bps<uint32_t>();
}
- *sent_video_rate_bps += stream.sender_video->VideoBitrateSent();
- *sent_nack_rate_bps +=
- stream.rtp_rtcp->GetSendRates()[RtpPacketMediaType::kRetransmission]
- .bps<uint32_t>();
}
return 0;
}
