From 1d50cb61d87bdc5d5d29c009da1abcc06e68b4c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Thu, 2 Jul 2020 17:41:32 +0200 Subject: [PATCH] Reland "Reland "Allows FEC generation after pacer step."" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 > > Reviewed-by: Sebastian Jansson > > 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 > Reviewed-by: Sebastian Jansson > 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 Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#31619} --- call/rtp_video_sender.cc | 35 +++-- call/rtp_video_sender.h | 1 + modules/pacing/pacing_controller.cc | 8 +- modules/pacing/pacing_controller.h | 2 + modules/pacing/pacing_controller_unittest.cc | 51 ++++++- modules/pacing/packet_router.cc | 12 ++ modules/pacing/packet_router.h | 6 + .../task_queue_paced_sender_unittest.cc | 4 + modules/rtp_rtcp/include/rtp_rtcp_defines.h | 11 ++ modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 9 ++ modules/rtp_rtcp/source/rtp_packet_to_send.h | 11 ++ modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 11 ++ modules/rtp_rtcp/source/rtp_rtcp_impl.h | 5 + modules/rtp_rtcp/source/rtp_rtcp_impl2.cc | 31 +++- modules/rtp_rtcp/source/rtp_rtcp_impl2.h | 8 +- modules/rtp_rtcp/source/rtp_rtcp_interface.h | 11 ++ modules/rtp_rtcp/source/rtp_sender_egress.cc | 105 ++++++++++++-- modules/rtp_rtcp/source/rtp_sender_egress.h | 13 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 133 +++++++++++++----- modules/rtp_rtcp/source/rtp_sender_video.cc | 5 +- modules/rtp_rtcp/source/ulpfec_generator.cc | 2 + test/scenario/video_stream_unittest.cc | 20 +++ 22 files changed, 434 insertions(+), 60 deletions(-) diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 5f8d2df965..854a18aa54 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -197,6 +197,7 @@ std::vector CreateRtpStreamSenders( FrameEncryptorInterface* frame_encryptor, const CryptoOptions& crypto_options, rtc::scoped_refptr frame_transformer, + bool use_deferred_fec, const WebRtcKeyValueConfig& trials) { RTC_DCHECK_GT(rtp_config.ssrcs.size(), 0); @@ -244,7 +245,9 @@ std::vector CreateRtpStreamSenders( std::unique_ptr 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 @@ RtpVideoSender::RtpVideoSender( 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 @@ RtpVideoSender::RtpVideoSender( 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 @@ int RtpVideoSender::ProtectionRequest(const FecProtectionParams* delta_params, *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(); } - *sent_video_rate_bps += stream.sender_video->VideoBitrateSent(); - *sent_nack_rate_bps += - stream.rtp_rtcp->GetSendRates()[RtpPacketMediaType::kRetransmission] - .bps(); } return 0; } diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index 0c277d6aa7..e364729236 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -176,6 +176,7 @@ class RtpVideoSender : public RtpVideoSenderInterface, const bool account_for_packetization_overhead_; const bool use_early_loss_detection_; const bool has_packet_feedback_; + const bool use_deferred_fec_; // TODO(holmer): Remove crit_ once RtpVideoSender runs on the // transport task queue. diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index 07e265b0da..7e7a01b628 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -441,6 +441,9 @@ void PacingController::ProcessPackets() { keepalive_data_sent += DataSize::Bytes(packet->payload_size() + packet->padding_size()); packet_sender_->SendPacket(std::move(packet), PacedPacketInfo()); + for (auto& packet : packet_sender_->FetchFec()) { + EnqueuePacket(std::move(packet)); + } } OnPaddingSent(keepalive_data_sent); } @@ -559,8 +562,11 @@ void PacingController::ProcessPackets() { packet_size += DataSize::Bytes(rtp_packet->headers_size()) + transport_overhead_per_packet_; } - packet_sender_->SendPacket(std::move(rtp_packet), pacing_info); + packet_sender_->SendPacket(std::move(rtp_packet), pacing_info); + for (auto& packet : packet_sender_->FetchFec()) { + EnqueuePacket(std::move(packet)); + } data_sent += packet_size; // Send done, update send/process time to the target send time. diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h index 6e361aebb4..775fdc9557 100644 --- a/modules/pacing/pacing_controller.h +++ b/modules/pacing/pacing_controller.h @@ -57,6 +57,8 @@ class PacingController { virtual ~PacketSender() = default; virtual void SendPacket(std::unique_ptr packet, const PacedPacketInfo& cluster_info) = 0; + // Should be called after each call to SendPacket(). + virtual std::vector> FetchFec() = 0; virtual std::vector> GeneratePadding( DataSize size) = 0; }; diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc index bc4d47333e..9194d079a9 100644 --- a/modules/pacing/pacing_controller_unittest.cc +++ b/modules/pacing/pacing_controller_unittest.cc @@ -97,6 +97,10 @@ class MockPacingControllerCallback : public PacingController::PacketSender { int64_t capture_timestamp, bool retransmission, bool padding)); + MOCK_METHOD(std::vector>, + FetchFec, + (), + (override)); MOCK_METHOD(size_t, SendPadding, (size_t target_size)); }; @@ -108,6 +112,11 @@ class MockPacketSender : public PacingController::PacketSender { (std::unique_ptr packet, const PacedPacketInfo& cluster_info), (override)); + MOCK_METHOD(std::vector>, + FetchFec, + (), + (override)); + MOCK_METHOD(std::vector>, GeneratePadding, (DataSize target_size), @@ -125,6 +134,10 @@ class PacingControllerPadding : public PacingController::PacketSender { total_bytes_sent_ += packet->payload_size(); } + std::vector> FetchFec() override { + return {}; + } + std::vector> GeneratePadding( DataSize target_size) override { size_t num_packets = @@ -158,6 +171,10 @@ class PacingControllerProbing : public PacingController::PacketSender { } } + std::vector> FetchFec() override { + return {}; + } + std::vector> GeneratePadding( DataSize target_size) override { // From RTPSender: @@ -299,7 +316,7 @@ class PacingControllerTest } SimulatedClock clock_; - MockPacingControllerCallback callback_; + ::testing::NiceMock callback_; std::unique_ptr pacer_; }; @@ -2029,6 +2046,38 @@ TEST_P(PacingControllerTest, PaddingTargetAccountsForPaddingRate) { AdvanceTimeAndProcess(); } +TEST_P(PacingControllerTest, SendsDeferredFecPackets) { + ScopedFieldTrials trial("WebRTC-DeferredFecGeneration/Enabled/"); + SetUp(); + + const uint32_t kSsrc = 12345; + const uint32_t kFlexSsrc = 54321; + uint16_t sequence_number = 1234; + uint16_t flexfec_sequence_number = 4321; + const size_t kPacketSize = 123; + + // Set pacing rate to 1000 packet/s, no padding. + pacer_->SetPacingRates( + DataSize::Bytes(1000 * kPacketSize) / TimeDelta::Seconds(1), + DataRate::Zero()); + + int64_t now = clock_.TimeInMilliseconds(); + Send(RtpPacketMediaType::kVideo, kSsrc, sequence_number, now, kPacketSize); + EXPECT_CALL(callback_, SendPacket(kSsrc, sequence_number, now, false, false)); + EXPECT_CALL(callback_, FetchFec).WillOnce([&]() { + EXPECT_CALL(callback_, SendPacket(kFlexSsrc, flexfec_sequence_number, now, + false, false)); + EXPECT_CALL(callback_, FetchFec); + std::vector> fec_packets; + fec_packets.push_back( + BuildPacket(RtpPacketMediaType::kForwardErrorCorrection, kFlexSsrc, + flexfec_sequence_number, now, kPacketSize)); + return fec_packets; + }); + AdvanceTimeAndProcess(); + AdvanceTimeAndProcess(); +} + INSTANTIATE_TEST_SUITE_P( WithAndWithoutIntervalBudget, PacingControllerTest, diff --git a/modules/pacing/packet_router.cc b/modules/pacing/packet_router.cc index e75b5a337a..5317f510c9 100644 --- a/modules/pacing/packet_router.cc +++ b/modules/pacing/packet_router.cc @@ -171,6 +171,18 @@ void PacketRouter::SendPacket(std::unique_ptr packet, // properties needed for payload based padding. Cache it for later use. last_send_module_ = rtp_module; } + + for (auto& packet : rtp_module->FetchFecPackets()) { + pending_fec_packets_.push_back(std::move(packet)); + } +} + +std::vector> PacketRouter::FetchFec() { + MutexLock lock(&modules_mutex_); + std::vector> fec_packets = + std::move(pending_fec_packets_); + pending_fec_packets_.clear(); + return fec_packets; } std::vector> PacketRouter::GeneratePadding( diff --git a/modules/pacing/packet_router.h b/modules/pacing/packet_router.h index 73837f2ffe..38a48b584e 100644 --- a/modules/pacing/packet_router.h +++ b/modules/pacing/packet_router.h @@ -57,6 +57,7 @@ class PacketRouter : public RemoteBitrateObserver, void SendPacket(std::unique_ptr packet, const PacedPacketInfo& cluster_info) override; + std::vector> FetchFec() override; std::vector> GeneratePadding( DataSize size) override; @@ -128,6 +129,11 @@ class PacketRouter : public RemoteBitrateObserver, uint64_t transport_seq_ RTC_GUARDED_BY(modules_mutex_); + // TODO(bugs.webrtc.org/10809): Replace lock with a sequence checker once the + // process thread is gone. + std::vector> pending_fec_packets_ + RTC_GUARDED_BY(modules_mutex_); + RTC_DISALLOW_COPY_AND_ASSIGN(PacketRouter); }; } // namespace webrtc diff --git a/modules/pacing/task_queue_paced_sender_unittest.cc b/modules/pacing/task_queue_paced_sender_unittest.cc index 876cd96cfd..b02f387768 100644 --- a/modules/pacing/task_queue_paced_sender_unittest.cc +++ b/modules/pacing/task_queue_paced_sender_unittest.cc @@ -43,6 +43,10 @@ class MockPacketRouter : public PacketRouter { (std::unique_ptr packet, const PacedPacketInfo& cluster_info), (override)); + MOCK_METHOD(std::vector>, + FetchFec, + (), + (override)); MOCK_METHOD(std::vector>, GeneratePadding, (DataSize target_size), diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 869f3d218a..46c310e276 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -33,6 +33,7 @@ namespace webrtc { class RtpPacket; +class RtpPacketToSend; namespace rtcp { class TransportFeedback; } @@ -466,5 +467,15 @@ class SendPacketObserver { int64_t capture_time_ms, uint32_t ssrc) = 0; }; + +// Interface for a class that can assign RTP sequence numbers for a packet +// to be sent. +class SequenceNumberAssigner { + public: + SequenceNumberAssigner() = default; + virtual ~SequenceNumberAssigner() = default; + + virtual void AssignSequenceNumber(RtpPacketToSend* packet) = 0; +}; } // namespace webrtc #endif // MODULES_RTP_RTCP_INCLUDE_RTP_RTCP_DEFINES_H_ diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 08b38eee7b..d597b1e289 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -98,6 +98,15 @@ class MockRtpRtcpInterface : public RtpRtcpInterface { TrySendPacket, (RtpPacketToSend * packet, const PacedPacketInfo& pacing_info), (override)); + MOCK_METHOD(void, + SetFecProtectionParams, + (const FecProtectionParams& delta_params, + const FecProtectionParams& key_params), + (override)); + MOCK_METHOD(std::vector>, + FetchFecPackets, + (), + (override)); MOCK_METHOD(void, OnPacketsAcknowledged, (rtc::ArrayView), diff --git a/modules/rtp_rtcp/source/rtp_packet_to_send.h b/modules/rtp_rtcp/source/rtp_packet_to_send.h index 8997bce0d2..9aaf9a52e6 100644 --- a/modules/rtp_rtcp/source/rtp_packet_to_send.h +++ b/modules/rtp_rtcp/source/rtp_packet_to_send.h @@ -108,6 +108,15 @@ class RtpPacketToSend : public RtpPacket { void set_is_key_frame(bool is_key_frame) { is_key_frame_ = is_key_frame; } bool is_key_frame() const { return is_key_frame_; } + // Indicates if packets should be protected by FEC (Forward Error Correction). + void set_fec_protect_packet(bool protect) { fec_protect_packet_ = protect; } + bool fec_protect_packet() const { return fec_protect_packet_; } + + // Indicates if packet is using RED encapsulation, in accordance with + // https://tools.ietf.org/html/rfc2198 + void set_is_red(bool is_red) { is_red_ = is_red; } + bool is_red() const { return is_red_; } + private: int64_t capture_time_ms_ = 0; absl::optional packet_type_; @@ -116,6 +125,8 @@ class RtpPacketToSend : public RtpPacket { std::vector application_data_; bool is_first_packet_of_frame_ = false; bool is_key_frame_ = false; + bool fec_protect_packet_ = false; + bool is_red_ = false; }; } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 7b9586ad78..9f8bac54f4 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -389,6 +389,17 @@ bool ModuleRtpRtcpImpl::TrySendPacket(RtpPacketToSend* packet, return true; } +void ModuleRtpRtcpImpl::SetFecProtectionParams(const FecProtectionParams&, + const FecProtectionParams&) { + // Deferred FEC not supported in deprecated RTP module. +} + +std::vector> +ModuleRtpRtcpImpl::FetchFecPackets() { + // Deferred FEC not supported in deprecated RTP module. + return {}; +} + void ModuleRtpRtcpImpl::OnPacketsAcknowledged( rtc::ArrayView sequence_numbers) { RTC_DCHECK(rtp_sender_); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 989b8d3717..1fa57db372 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -139,6 +139,11 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { bool TrySendPacket(RtpPacketToSend* packet, const PacedPacketInfo& pacing_info) override; + void SetFecProtectionParams(const FecProtectionParams& delta_params, + const FecProtectionParams& key_params) override; + + std::vector> FetchFecPackets() override; + void OnPacketsAcknowledged( rtc::ArrayView sequence_numbers) override; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc index 70f05d7085..67cb70e1cb 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc @@ -42,11 +42,15 @@ ModuleRtpRtcpImpl2::RtpSenderContext::RtpSenderContext( const RtpRtcpInterface::Configuration& config) : packet_history(config.clock, config.enable_rtx_padding_prioritization), packet_sender(config, &packet_history), - non_paced_sender(&packet_sender), + non_paced_sender(&packet_sender, this), packet_generator( config, &packet_history, config.paced_sender ? config.paced_sender : &non_paced_sender) {} +void ModuleRtpRtcpImpl2::RtpSenderContext::AssignSequenceNumber( + RtpPacketToSend* packet) { + packet_generator.AssignSequenceNumber(packet); +} ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration) : worker_queue_(TaskQueueBase::Current()), @@ -333,6 +337,31 @@ bool ModuleRtpRtcpImpl2::TrySendPacket(RtpPacketToSend* packet, return true; } +void ModuleRtpRtcpImpl2::SetFecProtectionParams( + const FecProtectionParams& delta_params, + const FecProtectionParams& key_params) { + RTC_DCHECK(rtp_sender_); + rtp_sender_->packet_sender.SetFecProtectionParameters(delta_params, + key_params); +} + +std::vector> +ModuleRtpRtcpImpl2::FetchFecPackets() { + RTC_DCHECK(rtp_sender_); + auto fec_packets = rtp_sender_->packet_sender.FetchFecPackets(); + if (!fec_packets.empty()) { + // Don't assign sequence numbers for FlexFEC packets. + const bool generate_sequence_numbers = + !rtp_sender_->packet_sender.FlexFecSsrc().has_value(); + if (generate_sequence_numbers) { + for (auto& fec_packet : fec_packets) { + rtp_sender_->packet_generator.AssignSequenceNumber(fec_packet.get()); + } + } + } + return fec_packets; +} + void ModuleRtpRtcpImpl2::OnPacketsAcknowledged( rtc::ArrayView sequence_numbers) { RTC_DCHECK(rtp_sender_); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h index c04edfcb7f..932c43656d 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.h @@ -148,6 +148,11 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, bool TrySendPacket(RtpPacketToSend* packet, const PacedPacketInfo& pacing_info) override; + void SetFecProtectionParams(const FecProtectionParams& delta_params, + const FecProtectionParams& key_params) override; + + std::vector> FetchFecPackets() override; + void OnPacketsAcknowledged( rtc::ArrayView sequence_numbers) override; @@ -268,8 +273,9 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcpInterface, FRIEND_TEST_ALL_PREFIXES(RtpRtcpImpl2Test, Rtt); FRIEND_TEST_ALL_PREFIXES(RtpRtcpImpl2Test, RttForReceiverOnly); - struct RtpSenderContext { + struct RtpSenderContext : public SequenceNumberAssigner { explicit RtpSenderContext(const RtpRtcpInterface::Configuration& config); + void AssignSequenceNumber(RtpPacketToSend* packet) override; // Storage of packets, for retransmissions and padding, if applicable. RtpPacketHistory packet_history; // Handles final time timestamping/stats/etc and handover to Transport. diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index 440837fc5d..f763da244c 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h @@ -293,6 +293,17 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { virtual bool TrySendPacket(RtpPacketToSend* packet, const PacedPacketInfo& pacing_info) = 0; + // Update the FEC protection parameters to use for delta- and key-frames. + // Only used when deferred FEC is active. + virtual void SetFecProtectionParams( + const FecProtectionParams& delta_params, + const FecProtectionParams& key_params) = 0; + + // If deferred FEC generation is enabled, this method should be called after + // calling TrySendPacket(). Any generated FEC packets will be removed and + // returned from the FEC generator. + virtual std::vector> FetchFecPackets() = 0; + virtual void OnPacketsAcknowledged( rtc::ArrayView sequence_numbers) = 0; diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc index 7196dcd659..c8f10ee1f2 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc @@ -10,6 +10,7 @@ #include "modules/rtp_rtcp/source/rtp_sender_egress.h" +#include #include #include #include @@ -38,21 +39,45 @@ bool IsEnabled(absl::string_view name, } // namespace RtpSenderEgress::NonPacedPacketSender::NonPacedPacketSender( - RtpSenderEgress* sender) - : transport_sequence_number_(0), sender_(sender) {} + RtpSenderEgress* sender, + SequenceNumberAssigner* sequence_number_assigner) + : transport_sequence_number_(0), + sender_(sender), + sequence_number_assigner_(sequence_number_assigner) { + RTC_DCHECK(sequence_number_assigner_); +} RtpSenderEgress::NonPacedPacketSender::~NonPacedPacketSender() = default; void RtpSenderEgress::NonPacedPacketSender::EnqueuePackets( std::vector> packets) { for (auto& packet : packets) { - if (!packet->SetExtension( - ++transport_sequence_number_)) { - --transport_sequence_number_; - } - packet->ReserveExtension(); - packet->ReserveExtension(); + PrepareForSend(packet.get()); sender_->SendPacket(packet.get(), PacedPacketInfo()); } + auto fec_packets = sender_->FetchFecPackets(); + if (!fec_packets.empty()) { + // Don't generate sequence numbers for flexfec, they are already running on + // an internally maintained sequence. + const bool generate_sequence_numbers = !sender_->FlexFecSsrc().has_value(); + + for (auto& packet : fec_packets) { + if (generate_sequence_numbers) { + sequence_number_assigner_->AssignSequenceNumber(packet.get()); + } + PrepareForSend(packet.get()); + } + EnqueuePackets(std::move(fec_packets)); + } +} + +void RtpSenderEgress::NonPacedPacketSender::PrepareForSend( + RtpPacketToSend* packet) { + if (!packet->SetExtension( + ++transport_sequence_number_)) { + --transport_sequence_number_; + } + packet->ReserveExtension(); + packet->ReserveExtension(); } RtpSenderEgress::RtpSenderEgress(const RtpRtcpInterface::Configuration& config, @@ -73,6 +98,10 @@ RtpSenderEgress::RtpSenderEgress(const RtpRtcpInterface::Configuration& config, is_audio_(config.audio), #endif need_rtp_packet_infos_(config.need_rtp_packet_infos), + fec_generator_( + IsEnabled("WebRTC-DeferredFecGeneration", config.field_trials) + ? config.fec_generator + : nullptr), transport_feedback_observer_(config.transport_feedback_callback), send_side_delay_observer_(config.send_side_delay_observer), send_packet_observer_(config.send_packet_observer), @@ -142,6 +171,41 @@ void RtpSenderEgress::SendPacket(RtpPacketToSend* packet, })); } + if (fec_generator_ && packet->fec_protect_packet()) { + // Deferred fec generation is used, add packet to generator. + RTC_DCHECK(fec_generator_); + RTC_DCHECK(packet->packet_type() == RtpPacketMediaType::kVideo); + absl::optional> + new_fec_params; + { + rtc::CritScope lock(&lock_); + new_fec_params.swap(pending_fec_params_); + } + if (new_fec_params) { + fec_generator_->SetProtectionParameters(new_fec_params->first, + new_fec_params->second); + } + if (packet->is_red()) { + RtpPacketToSend unpacked_packet(*packet); + + const rtc::CopyOnWriteBuffer buffer = packet->Buffer(); + // Grab media payload type from RED header. + const size_t headers_size = packet->headers_size(); + unpacked_packet.SetPayloadType(buffer[headers_size]); + + // Copy the media payload into the unpacked buffer. + uint8_t* payload_buffer = + unpacked_packet.SetPayloadSize(packet->payload_size() - 1); + std::copy(&packet->payload()[0] + 1, + &packet->payload()[0] + packet->payload_size(), payload_buffer); + + fec_generator_->AddPacketAndGenerateFec(unpacked_packet); + } else { + // If not RED encapsulated - we can just insert packet directly. + fec_generator_->AddPacketAndGenerateFec(*packet); + } + } + // Bug webrtc:7859. While FEC is invoked from rtp_sender_video, and not after // the pacer, these modifications of the header below are happening after the // FEC protection packets are calculated. This will corrupt recovered packets @@ -208,9 +272,14 @@ void RtpSenderEgress::SendPacket(RtpPacketToSend* packet, } if (send_success) { - // TODO(tommi): Is this assuming is_media is true? + // |media_has_been_sent_| is used by RTPSender to figure out if it can send + // padding in the absence of transport-cc or abs-send-time. + // In those cases media must be sent first to set a reference timestamp. media_has_been_sent_ = true; + // TODO(sprang): Add support for FEC protecting all header extensions, add + // media packet to generator here instead. + RTC_DCHECK(packet->packet_type().has_value()); RtpPacketMediaType packet_type = *packet->packet_type(); RtpPacketCounter counter(*packet); @@ -295,6 +364,24 @@ std::vector RtpSenderEgress::GetSentRtpPacketInfos( return results; } +void RtpSenderEgress::SetFecProtectionParameters( + const FecProtectionParams& delta_params, + const FecProtectionParams& key_params) { + // TODO(sprang): Post task to pacer queue instead, one pacer is fully + // migrated to a task queue. + rtc::CritScope lock(&lock_); + pending_fec_params_.emplace(delta_params, key_params); +} + +std::vector> +RtpSenderEgress::FetchFecPackets() { + RTC_DCHECK_RUN_ON(&pacer_checker_); + if (fec_generator_) { + return fec_generator_->GetFecPackets(); + } + return {}; +} + bool RtpSenderEgress::HasCorrectSsrc(const RtpPacketToSend& packet) const { switch (*packet.packet_type()) { case RtpPacketMediaType::kAudio: diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.h b/modules/rtp_rtcp/source/rtp_sender_egress.h index 5b50ddfb93..e2cc00ff24 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.h +++ b/modules/rtp_rtcp/source/rtp_sender_egress.h @@ -13,6 +13,7 @@ #include #include +#include #include #include "absl/types/optional.h" @@ -41,15 +42,18 @@ class RtpSenderEgress { // without passing through an actual paced sender. class NonPacedPacketSender : public RtpPacketSender { public: - explicit NonPacedPacketSender(RtpSenderEgress* sender); + NonPacedPacketSender(RtpSenderEgress* sender, + SequenceNumberAssigner* sequence_number_assigner); virtual ~NonPacedPacketSender(); void EnqueuePackets( std::vector> packets) override; private: + void PrepareForSend(RtpPacketToSend* packet); uint16_t transport_sequence_number_; RtpSenderEgress* const sender_; + SequenceNumberAssigner* sequence_number_assigner_; }; RtpSenderEgress(const RtpRtcpInterface::Configuration& config, @@ -82,6 +86,10 @@ class RtpSenderEgress { rtc::ArrayView sequence_numbers) const RTC_LOCKS_EXCLUDED(lock_); + void SetFecProtectionParameters(const FecProtectionParams& delta_params, + const FecProtectionParams& key_params); + std::vector> FetchFecPackets(); + private: // Maps capture time in milliseconds to send-side delay in milliseconds. // Send-side delay is the difference between transmission time and capture @@ -133,6 +141,7 @@ class RtpSenderEgress { const bool is_audio_; #endif const bool need_rtp_packet_infos_; + VideoFecGenerator* const fec_generator_ RTC_GUARDED_BY(pacer_checker_); TransportFeedbackObserver* const transport_feedback_observer_; SendSideDelayObserver* const send_side_delay_observer_; @@ -154,6 +163,8 @@ class RtpSenderEgress { StreamDataCounters rtx_rtp_stats_ RTC_GUARDED_BY(lock_); // One element per value in RtpPacketMediaType, with index matching value. std::vector send_rates_ RTC_GUARDED_BY(lock_); + absl::optional> + pending_fec_params_ RTC_GUARDED_BY(lock_); // Maps sent packets' sequence numbers to a tuple consisting of: // 1. The timestamp, without the randomizing offset mandated by the RFC. diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index dbd474cbe1..9e1aefa6f9 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -32,6 +32,7 @@ #include "modules/rtp_rtcp/source/rtp_sender_egress.h" #include "modules/rtp_rtcp/source/rtp_sender_video.h" #include "modules/rtp_rtcp/source/rtp_utility.h" +#include "modules/rtp_rtcp/source/video_fec_generator.h" #include "rtc_base/arraysize.h" #include "rtc_base/rate_limiter.h" #include "rtc_base/strings/string_builder.h" @@ -141,8 +142,10 @@ MATCHER_P(SameRtcEventTypeAs, value, "") { } struct TestConfig { - explicit TestConfig(bool with_overhead) : with_overhead(with_overhead) {} + TestConfig(bool with_overhead, bool deferred_fec) + : with_overhead(with_overhead), deferred_fec(deferred_fec) {} bool with_overhead = false; + bool deferred_fec = false; }; class MockRtpPacketPacer : public RtpPacketSender { @@ -212,15 +215,18 @@ class StreamDataTestCallback : public StreamDataCountersCallback { // Mimics ModuleRtpRtcp::RtpSenderContext. // TODO(sprang): Split up unit tests and test these components individually // wherever possible. -struct RtpSenderContext { +struct RtpSenderContext : public SequenceNumberAssigner { explicit RtpSenderContext(const RtpRtcpInterface::Configuration& config) : packet_history_(config.clock, config.enable_rtx_padding_prioritization), packet_sender_(config, &packet_history_), - non_paced_sender_(&packet_sender_), + non_paced_sender_(&packet_sender_, this), packet_generator_( config, &packet_history_, config.paced_sender ? config.paced_sender : &non_paced_sender_) {} + void AssignSequenceNumber(RtpPacketToSend* packet) override { + packet_generator_.AssignSequenceNumber(packet); + } RtpPacketHistory packet_history_; RtpSenderEgress packet_sender_; RtpSenderEgress::NonPacedPacketSender non_paced_sender_; @@ -229,10 +235,14 @@ struct RtpSenderContext { class FieldTrialConfig : public WebRtcKeyValueConfig { public: - FieldTrialConfig() : overhead_enabled_(false), max_padding_factor_(1200) {} + FieldTrialConfig() + : overhead_enabled_(false), + deferred_fec_(false), + max_padding_factor_(1200) {} ~FieldTrialConfig() override {} void SetOverHeadEnabled(bool enabled) { overhead_enabled_ = enabled; } + void UseDeferredFec(bool enabled) { deferred_fec_ = enabled; } void SetMaxPaddingFactor(double factor) { max_padding_factor_ = factor; } std::string Lookup(absl::string_view key) const override { @@ -243,12 +253,15 @@ class FieldTrialConfig : public WebRtcKeyValueConfig { return ssb.str(); } else if (key == "WebRTC-SendSideBwe-WithOverhead") { return overhead_enabled_ ? "Enabled" : "Disabled"; + } else if (key == "WebRTC-DeferredFecGeneration") { + return deferred_fec_ ? "Enabled" : "Disabled"; } return ""; } private: bool overhead_enabled_; + bool deferred_fec_; double max_padding_factor_; }; @@ -269,6 +282,7 @@ class RtpSenderTest : public ::testing::TestWithParam { &fake_clock_), kMarkerBit(true) { field_trials_.SetOverHeadEnabled(GetParam().with_overhead); + field_trials_.UseDeferredFec(GetParam().deferred_fec); } void SetUp() override { SetUpRtpSender(true, false, false); } @@ -286,12 +300,20 @@ class RtpSenderTest : public ::testing::TestWithParam { void SetUpRtpSender(bool pacer, bool populate_network2, bool always_send_mid_and_rid) { + SetUpRtpSender(pacer, populate_network2, always_send_mid_and_rid, + &flexfec_sender_); + } + + void SetUpRtpSender(bool pacer, + bool populate_network2, + bool always_send_mid_and_rid, + VideoFecGenerator* fec_generator) { RtpRtcpInterface::Configuration config; config.clock = &fake_clock_; config.outgoing_transport = &transport_; config.local_media_ssrc = kSsrc; config.rtx_send_ssrc = kRtxSsrc; - config.fec_generator = &flexfec_sender_; + config.fec_generator = fec_generator; config.event_log = &mock_rtc_event_log_; config.send_packet_observer = &send_packet_observer_; config.retransmission_rate_limiter = &retransmission_rate_limiter_; @@ -1250,6 +1272,7 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { config.event_log = &mock_rtc_event_log_; config.send_packet_observer = &send_packet_observer_; config.retransmission_rate_limiter = &retransmission_rate_limiter_; + config.field_trials = &field_trials_; rtp_sender_context_ = std::make_unique(config); rtp_sender()->SetSequenceNumber(kSeqNum); @@ -1260,7 +1283,11 @@ 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; + if (!GetParam().deferred_fec) { + video_config.fec_generator = &flexfec_sender; + } + video_config.fec_type = flexfec_sender.GetFecType(); + video_config.fec_overhead_bytes = flexfec_sender.MaxPacketOverhead(); video_config.fec_type = flexfec_sender.GetFecType(); video_config.fec_overhead_bytes = flexfec_sender.MaxPacketOverhead(); video_config.field_trials = &field_trials; @@ -1286,11 +1313,21 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { EXPECT_EQ(packet->Ssrc(), kSsrc); EXPECT_EQ(packet->SequenceNumber(), kSeqNum); media_packet = std::move(packet); + if (GetParam().deferred_fec) { + // Simulate RtpSenderEgress adding packet to fec generator. + flexfec_sender.AddPacketAndGenerateFec(*media_packet); + auto fec_packets = flexfec_sender.GetFecPackets(); + EXPECT_EQ(fec_packets.size(), 1u); + fec_packet = std::move(fec_packets[0]); + EXPECT_EQ(fec_packet->packet_type(), + RtpPacketMediaType::kForwardErrorCorrection); + EXPECT_EQ(fec_packet->Ssrc(), kFlexFecSsrc); + } } else { EXPECT_EQ(packet->packet_type(), RtpPacketMediaType::kForwardErrorCorrection); - EXPECT_EQ(packet->Ssrc(), kFlexFecSsrc); fec_packet = std::move(packet); + EXPECT_EQ(fec_packet->Ssrc(), kFlexFecSsrc); } } }); @@ -1338,6 +1375,7 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) { config.event_log = &mock_rtc_event_log_; config.send_packet_observer = &send_packet_observer_; config.retransmission_rate_limiter = &retransmission_rate_limiter_; + config.field_trials = &field_trials_; rtp_sender_context_ = std::make_unique(config); rtp_sender()->SetSequenceNumber(kSeqNum); @@ -1346,7 +1384,9 @@ 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; + if (!GetParam().deferred_fec) { + 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; @@ -1357,7 +1397,11 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) { params.fec_rate = 15; params.max_fec_frames = 1; params.fec_mask_type = kFecMaskRandom; - flexfec_sender.SetProtectionParameters(params, params); + if (GetParam().deferred_fec) { + rtp_egress()->SetFecProtectionParameters(params, params); + } else { + flexfec_sender.SetProtectionParameters(params, params); + } EXPECT_CALL(mock_rtc_event_log_, LogProxy(SameRtcEventTypeAs(RtcEvent::Type::RtpPacketOutgoing))) @@ -1662,25 +1706,16 @@ TEST_P(RtpSenderTest, FecOverheadRate) { kNoRtpExtensions, kNoRtpExtensionSizes, nullptr /* rtp_state */, &fake_clock_); - // Reset |rtp_sender_| to use FlexFEC. - RtpRtcpInterface::Configuration config; - config.clock = &fake_clock_; - config.outgoing_transport = &transport_; - config.paced_sender = &mock_paced_sender_; - config.local_media_ssrc = kSsrc; - config.fec_generator = &flexfec_sender; - config.event_log = &mock_rtc_event_log_; - config.send_packet_observer = &send_packet_observer_; - config.retransmission_rate_limiter = &retransmission_rate_limiter_; - rtp_sender_context_ = std::make_unique(config); - - rtp_sender()->SetSequenceNumber(kSeqNum); + // Reset |rtp_sender_| to use this FlexFEC instance. + SetUpRtpSender(false, false, false, &flexfec_sender); FieldTrialBasedConfig field_trials; RTPSenderVideo::Config video_config; video_config.clock = &fake_clock_; video_config.rtp_sender = rtp_sender(); - video_config.fec_generator = &flexfec_sender; + if (!GetParam().deferred_fec) { + 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; @@ -1690,12 +1725,15 @@ TEST_P(RtpSenderTest, FecOverheadRate) { params.fec_rate = 15; params.max_fec_frames = 1; params.fec_mask_type = kFecMaskRandom; - flexfec_sender.SetProtectionParameters(params, params); + if (GetParam().deferred_fec) { + rtp_egress()->SetFecProtectionParameters(params, params); + } else { + flexfec_sender.SetProtectionParameters(params, params); + } constexpr size_t kNumMediaPackets = 10; constexpr size_t kNumFecPackets = kNumMediaPackets; constexpr int64_t kTimeBetweenPacketsMs = 10; - EXPECT_CALL(mock_paced_sender_, EnqueuePackets).Times(kNumMediaPackets); for (size_t i = 0; i < kNumMediaPackets; ++i) { RTPVideoHeader video_header; @@ -1713,9 +1751,21 @@ TEST_P(RtpSenderTest, FecOverheadRate) { constexpr size_t kPayloadLength = sizeof(kPayloadData); constexpr size_t kPacketLength = kRtpHeaderLength + kFlexfecHeaderLength + kGenericCodecHeaderLength + kPayloadLength; - EXPECT_NEAR(kNumFecPackets * kPacketLength * 8 / - (kNumFecPackets * kTimeBetweenPacketsMs / 1000.0f), - flexfec_sender.CurrentFecRate().bps(), 500); + + loop_.Flush(); + if (GetParam().deferred_fec) { + EXPECT_NEAR( + kNumFecPackets * kPacketLength * 8 / + (kNumFecPackets * kTimeBetweenPacketsMs / 1000.0f), + rtp_egress() + ->GetSendRates()[RtpPacketMediaType::kForwardErrorCorrection] + .bps(), + 500); + } else { + EXPECT_NEAR(kNumFecPackets * kPacketLength * 8 / + (kNumFecPackets * kTimeBetweenPacketsMs / 1000.0f), + flexfec_sender.CurrentFecRate().bps(), 500); + } } TEST_P(RtpSenderTest, BitrateCallbacks) { @@ -1862,15 +1912,18 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) { const uint8_t kUlpfecPayloadType = 97; const uint8_t kPayloadType = 127; const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric; - FieldTrialBasedConfig field_trials; + UlpfecGenerator ulpfec_generator(kRedPayloadType, kUlpfecPayloadType, &fake_clock_); + SetUpRtpSender(false, false, false, &ulpfec_generator); RTPSenderVideo::Config video_config; video_config.clock = &fake_clock_; video_config.rtp_sender = rtp_sender(); - video_config.field_trials = &field_trials; + video_config.field_trials = &field_trials_; video_config.red_payload_type = kRedPayloadType; - video_config.fec_generator = &ulpfec_generator; + if (!GetParam().deferred_fec) { + 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); @@ -1887,7 +1940,11 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) { fec_params.fec_mask_type = kFecMaskRandom; fec_params.fec_rate = 1; fec_params.max_fec_frames = 1; - ulpfec_generator.SetProtectionParameters(fec_params, fec_params); + if (GetParam().deferred_fec) { + rtp_egress()->SetFecProtectionParameters(fec_params, fec_params); + } else { + 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, @@ -2713,12 +2770,16 @@ TEST_P(RtpSenderTest, IgnoresNackAfterDisablingMedia) { INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderTest, - ::testing::Values(TestConfig{false}, - TestConfig{true})); + ::testing::Values(TestConfig{false, false}, + TestConfig{false, true}, + TestConfig{true, false}, + TestConfig{false, false})); INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderTestWithoutPacer, - ::testing::Values(TestConfig{false}, - TestConfig{true})); + ::testing::Values(TestConfig{false, false}, + TestConfig{false, true}, + TestConfig{true, false}, + TestConfig{false, false})); } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 9ebfa77b8a..7d870d25a8 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -598,8 +598,8 @@ bool RTPSenderVideo::SendVideo( if (fec_generator_) { fec_generator_->AddPacketAndGenerateFec(*packet); } else { - // TODO(sprang): When deferred FEC generation is enabled, just mark the - // packet as protected here. + // Deferred FEC generation, just mark packet. + packet->set_fec_protect_packet(true); } } @@ -607,6 +607,7 @@ bool RTPSenderVideo::SendVideo( std::unique_ptr red_packet(new RtpPacketToSend(*packet)); BuildRedPayload(*packet, red_packet.get()); red_packet->SetPayloadType(*red_payload_type_); + red_packet->set_is_red(true); // Send |red_packet| instead of |packet| for allocated sequence number. red_packet->set_packet_type(RtpPacketMediaType::kVideo); diff --git a/modules/rtp_rtcp/source/ulpfec_generator.cc b/modules/rtp_rtcp/source/ulpfec_generator.cc index 265fa4d1ac..04cb59dc7d 100644 --- a/modules/rtp_rtcp/source/ulpfec_generator.cc +++ b/modules/rtp_rtcp/source/ulpfec_generator.cc @@ -230,6 +230,8 @@ std::vector> UlpfecGenerator::GetFecPackets() { total_fec_size_bytes += red_packet->size(); red_packet->set_packet_type(RtpPacketMediaType::kForwardErrorCorrection); red_packet->set_allow_retransmission(false); + red_packet->set_is_red(true); + red_packet->set_fec_protect_packet(false); fec_packets.push_back(std::move(red_packet)); } diff --git a/test/scenario/video_stream_unittest.cc b/test/scenario/video_stream_unittest.cc index 37afe1b1e7..873ef639ba 100644 --- a/test/scenario/video_stream_unittest.cc +++ b/test/scenario/video_stream_unittest.cc @@ -9,6 +9,7 @@ */ #include +#include "test/field_trial.h" #include "test/gtest.h" #include "test/scenario/scenario.h" @@ -170,6 +171,25 @@ TEST(VideoStreamTest, SendsFecWithFlexFec) { EXPECT_GT(video_stats.substreams.begin()->second.rtp_stats.fec.packets, 0u); } +TEST(VideoStreamTest, SendsFecWithDeferredFlexFec) { + ScopedFieldTrials trial("WebRTC-DeferredFecGeneration/Enabled/"); + Scenario s; + auto route = + s.CreateRoutes(s.CreateClient("caller", CallClientConfig()), + {s.CreateSimulationNode([](NetworkSimulationConfig* c) { + c->loss_rate = 0.1; + c->delay = TimeDelta::Millis(100); + })}, + s.CreateClient("callee", CallClientConfig()), + {s.CreateSimulationNode(NetworkSimulationConfig())}); + auto video = s.CreateVideoStream(route->forward(), [&](VideoStreamConfig* c) { + c->stream.use_flexfec = true; + }); + s.RunFor(TimeDelta::Seconds(5)); + VideoSendStream::Stats video_stats = video->send()->GetStats(); + EXPECT_GT(video_stats.substreams.begin()->second.rtp_stats.fec.packets, 0u); +} + TEST(VideoStreamTest, ResolutionAdaptsToAvailableBandwidth) { // Declared before scenario to avoid use after free. std::atomic num_qvga_frames_(0);