diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 079ea711b4..31c73856d6 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -278,6 +278,10 @@ std::vector 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(video_config); rtp_streams.emplace_back(std::move(rtp_rtcp), std::move(sender_video), @@ -301,8 +305,8 @@ absl::optional GetVideoCodecType(const RtpConfig& config) { } return PayloadStringToCodecType(config.payload_name); } -bool TransportSeqNumExtensionConfigured(const RtpConfig& config_config) { - return absl::c_any_of(config_config.extensions, [](const RtpExtension& ext) { +bool TransportSeqNumExtensionConfigured(const RtpConfig& config) { + return absl::c_any_of(config.extensions, [](const RtpExtension& ext) { return ext.uri == RtpExtension::kTransportSequenceNumberUri; }); } @@ -685,12 +689,15 @@ std::map 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(); - if (fec_generator && - fec_generator->GetFecType() == VideoFecGenerator::FecType::kFlexFec) { - auto* flexfec_sender = static_cast(fec_generator); - uint32_t ssrc = rtp_config_.flexfec.ssrc; - rtp_states[ssrc] = flexfec_sender->GetRtpState(); + // Only happens during shutdown, when RTP module is already inactive, + // so OK to call fec generator here. + if (rtp_streams_[i].fec_generator) { + absl::optional fec_state = + rtp_streams_[i].fec_generator->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(¬_used, /*video_rate=*/nullptr, /*fec_rate=*/nullptr, &module_nack_rate); *sent_nack_rate_bps += module_nack_rate; diff --git a/modules/rtp_rtcp/include/flexfec_sender.h b/modules/rtp_rtcp/include/flexfec_sender.h index 4cc8f99ce6..7fe20181af 100644 --- a/modules/rtp_rtcp/include/flexfec_sender.h +++ b/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 GetRtpState() override; private: // Utility. diff --git a/modules/rtp_rtcp/source/flexfec_sender.cc b/modules/rtp_rtcp/source/flexfec_sender.cc index 4ff0893ee7..16a6f2603c 100644 --- a/modules/rtp_rtcp/source/flexfec_sender.cc +++ b/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 FlexfecSender::GetRtpState() { RtpState rtp_state; rtp_state.sequence_number = seq_num_; rtp_state.start_timestamp = timestamp_offset_; diff --git a/modules/rtp_rtcp/source/flexfec_sender_unittest.cc b/modules/rtp_rtcp/source/flexfec_sender_unittest.cc index e4501c2c1d..3ff657476b 100644 --- a/modules/rtp_rtcp/source/flexfec_sender_unittest.cc +++ b/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, diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 7dd9fdf420..eb1a48ba86 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/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(), 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, diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 4441c765f6..e073315cd7 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/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 (packet->packet_type() == RtpPacketMediaType::kForwardErrorCorrection) { - const uint32_t ssrc = packet->Ssrc(); - BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoFecBitrate_kbps", now_ms, - FecOverheadRate() / 1000, ssrc); + if (fec_generator_) { + uint32_t fec_rate_kbps = fec_generator_->CurrentFecRate().kbps(); + for (const auto& packet : packets) { + if (packet->packet_type() == + 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_generator_ && - fec_generator_->GetFecType() == VideoFecGenerator::FecType::kUlpFec) { + if (fec_type_ == 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_) { - fec_generator_->AddPacketAndGenerateFec(*packet); + // No FEC protection for upper temporal layers, if used. + 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() : 0u; -} - uint32_t RTPSenderVideo::PacketizationOverheadBps() const { rtc::CritScope cs(&stats_crit_); return packetization_overhead_bitrate_.Rate(clock_->TimeInMilliseconds()) diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index 21648168af..bf5f181823 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/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 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 red_payload_type_; VideoFecGenerator* const fec_generator_; + absl::optional 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. diff --git a/modules/rtp_rtcp/source/ulpfec_generator.h b/modules/rtp_rtcp/source/ulpfec_generator.h index 6c65f5f91e..be59e4c9ea 100644 --- a/modules/rtp_rtcp/source/ulpfec_generator.h +++ b/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 GetRtpState() override { return absl::nullopt; } + private: struct Params { Params(); diff --git a/modules/rtp_rtcp/source/video_fec_generator.h b/modules/rtp_rtcp/source/video_fec_generator.h index 3731449b5c..38e4103cb6 100644 --- a/modules/rtp_rtcp/source/video_fec_generator.h +++ b/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> 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 GetRtpState() = 0; }; } // namespace webrtc