diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index c8196b68dd..ca6132fcd5 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -55,6 +55,68 @@ static const size_t kPathMTU = 1500; using webrtc_internal_rtp_video_sender::RtpStreamSender; +bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name) { + const VideoCodecType codecType = PayloadStringToCodecType(payload_name); + if (codecType == kVideoCodecVP8 || codecType == kVideoCodecVP9) { + return true; + } + if (codecType == kVideoCodecGeneric && + field_trial::IsEnabled("WebRTC-GenericPictureId")) { + return true; + } + return false; +} + +bool ShouldDisableRedAndUlpfec(bool flexfec_enabled, + const RtpConfig& rtp_config) { + // Consistency of NACK and RED+ULPFEC parameters is checked in this function. + const bool nack_enabled = rtp_config.nack.rtp_history_ms > 0; + + // Shorthands. + auto IsRedEnabled = [&]() { return rtp_config.ulpfec.red_payload_type >= 0; }; + auto IsUlpfecEnabled = [&]() { + return rtp_config.ulpfec.ulpfec_payload_type >= 0; + }; + + bool should_disable_red_and_ulpfec = false; + + if (webrtc::field_trial::IsEnabled("WebRTC-DisableUlpFecExperiment")) { + RTC_LOG(LS_INFO) << "Experiment to disable sending ULPFEC is enabled."; + should_disable_red_and_ulpfec = true; + } + + // If enabled, FlexFEC takes priority over RED+ULPFEC. + if (flexfec_enabled) { + if (IsUlpfecEnabled()) { + RTC_LOG(LS_INFO) + << "Both FlexFEC and ULPFEC are configured. Disabling ULPFEC."; + } + should_disable_red_and_ulpfec = true; + } + + // Payload types without picture ID cannot determine that a stream is complete + // without retransmitting FEC, so using ULPFEC + NACK for H.264 (for instance) + // is a waste of bandwidth since FEC packets still have to be transmitted. + // Note that this is not the case with FlexFEC. + if (nack_enabled && IsUlpfecEnabled() && + !PayloadTypeSupportsSkippingFecPackets(rtp_config.payload_name)) { + RTC_LOG(LS_WARNING) + << "Transmitting payload type without picture ID using " + "NACK+ULPFEC is a waste of bandwidth since ULPFEC packets " + "also have to be retransmitted. Disabling ULPFEC."; + should_disable_red_and_ulpfec = true; + } + + // Verify payload types. + if (IsUlpfecEnabled() ^ IsRedEnabled()) { + RTC_LOG(LS_WARNING) + << "Only RED or only ULPFEC enabled, but not both. Disabling both."; + should_disable_red_and_ulpfec = true; + } + + return should_disable_red_and_ulpfec; +} + std::vector CreateRtpStreamSenders( Clock* clock, const RtpConfig& rtp_config, @@ -129,31 +191,38 @@ std::vector CreateRtpStreamSenders( rtp_rtcp->SetSendingStatus(false); rtp_rtcp->SetSendingMediaStatus(false); rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound); + // Set NACK. + rtp_rtcp->SetStorePacketsStatus(true, kMinSendSidePacketHistorySize); - auto sender_video = std::make_unique( - configuration.clock, rtp_rtcp->RtpSender(), - configuration.flexfec_sender, playout_delay_oracle.get(), - frame_encryptor, crypto_options.sframe.require_frame_encryption, - rtp_config.lntf.enabled, /*enable_retransmit_all_layers*/ false, - FieldTrialBasedConfig()); + 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.playout_delay_oracle = playout_delay_oracle.get(); + video_config.frame_encryptor = frame_encryptor; + video_config.require_frame_encryption = + crypto_options.sframe.require_frame_encryption; + video_config.need_rtp_packet_infos = rtp_config.lntf.enabled; + video_config.enable_retransmit_all_layers = false; + video_config.field_trials = &field_trial_config; + const bool should_disable_red_and_ulpfec = + ShouldDisableRedAndUlpfec(enable_flexfec, rtp_config); + if (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(video_config); rtp_streams.emplace_back(std::move(playout_delay_oracle), std::move(rtp_rtcp), std::move(sender_video)); } return rtp_streams; } -bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name) { - const VideoCodecType codecType = PayloadStringToCodecType(payload_name); - if (codecType == kVideoCodecVP8 || codecType == kVideoCodecVP9) { - return true; - } - if (codecType == kVideoCodecGeneric && - field_trial::IsEnabled("WebRTC-GenericPictureId")) { - return true; - } - return false; -} - // TODO(brandtr): Update this function when we support multistream protection. std::unique_ptr MaybeCreateFlexfecSender( Clock* clock, @@ -316,7 +385,6 @@ RtpVideoSender::RtpVideoSender( } } - ConfigureProtection(); ConfigureSsrcs(); ConfigureRids(); @@ -497,65 +565,6 @@ void RtpVideoSender::OnBitrateAllocationUpdated( } } -void RtpVideoSender::ConfigureProtection() { - // Consistency of FlexFEC parameters is checked in MaybeCreateFlexfecSender. - const bool flexfec_enabled = (flexfec_sender_ != nullptr); - - // Consistency of NACK and RED+ULPFEC parameters is checked in this function. - const bool nack_enabled = rtp_config_.nack.rtp_history_ms > 0; - int red_payload_type = rtp_config_.ulpfec.red_payload_type; - int ulpfec_payload_type = rtp_config_.ulpfec.ulpfec_payload_type; - - // Shorthands. - auto IsRedEnabled = [&]() { return red_payload_type >= 0; }; - auto IsUlpfecEnabled = [&]() { return ulpfec_payload_type >= 0; }; - auto DisableRedAndUlpfec = [&]() { - red_payload_type = -1; - ulpfec_payload_type = -1; - }; - - if (webrtc::field_trial::IsEnabled("WebRTC-DisableUlpFecExperiment")) { - RTC_LOG(LS_INFO) << "Experiment to disable sending ULPFEC is enabled."; - DisableRedAndUlpfec(); - } - - // If enabled, FlexFEC takes priority over RED+ULPFEC. - if (flexfec_enabled) { - if (IsUlpfecEnabled()) { - RTC_LOG(LS_INFO) - << "Both FlexFEC and ULPFEC are configured. Disabling ULPFEC."; - } - DisableRedAndUlpfec(); - } - - // Payload types without picture ID cannot determine that a stream is complete - // without retransmitting FEC, so using ULPFEC + NACK for H.264 (for instance) - // is a waste of bandwidth since FEC packets still have to be transmitted. - // Note that this is not the case with FlexFEC. - if (nack_enabled && IsUlpfecEnabled() && - !PayloadTypeSupportsSkippingFecPackets(rtp_config_.payload_name)) { - RTC_LOG(LS_WARNING) - << "Transmitting payload type without picture ID using " - "NACK+ULPFEC is a waste of bandwidth since ULPFEC packets " - "also have to be retransmitted. Disabling ULPFEC."; - DisableRedAndUlpfec(); - } - - // Verify payload types. - if (IsUlpfecEnabled() ^ IsRedEnabled()) { - RTC_LOG(LS_WARNING) - << "Only RED or only ULPFEC enabled, but not both. Disabling both."; - DisableRedAndUlpfec(); - } - - for (const RtpStreamSender& stream : rtp_streams_) { - // Set NACK. - stream.rtp_rtcp->SetStorePacketsStatus(true, kMinSendSidePacketHistorySize); - // Set RED/ULPFEC information. - stream.sender_video->SetUlpfecConfig(red_payload_type, ulpfec_payload_type); - } -} - bool RtpVideoSender::FecEnabled() const { const bool flexfec_enabled = (flexfec_sender_ != nullptr); const bool ulpfec_enabled = diff --git a/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/modules/rtp_rtcp/source/nack_rtx_unittest.cc index aa30005980..62d5f98c34 100644 --- a/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -136,10 +136,13 @@ class RtpRtcpRtxNackTest : public ::testing::Test { configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; configuration.local_media_ssrc = kTestSsrc; rtp_rtcp_module_ = RtpRtcp::Create(configuration); - rtp_sender_video_ = std::make_unique( - &fake_clock, rtp_rtcp_module_->RtpSender(), nullptr, - &playout_delay_oracle_, nullptr, false, false, false, - FieldTrialBasedConfig()); + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &fake_clock; + video_config.rtp_sender = rtp_rtcp_module_->RtpSender(); + video_config.playout_delay_oracle = &playout_delay_oracle_; + video_config.field_trials = &field_trials; + rtp_sender_video_ = std::make_unique(video_config); rtp_rtcp_module_->SetRTCPStatus(RtcpMode::kCompound); rtp_rtcp_module_->SetStorePacketsStatus(true, 600); EXPECT_EQ(0, rtp_rtcp_module_->SetSendingStatus(true)); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 705e53ca46..34944bc13e 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -178,9 +178,13 @@ class RtpRtcpImplTest : public ::testing::Test { sender_.impl_->SetSequenceNumber(kSequenceNumber); sender_.impl_->SetStorePacketsStatus(true, 100); - sender_video_ = std::make_unique( - &clock_, sender_.impl_->RtpSender(), nullptr, &playout_delay_oracle_, - nullptr, false, false, false, FieldTrialBasedConfig()); + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &clock_; + video_config.rtp_sender = sender_.impl_->RtpSender(); + video_config.playout_delay_oracle = &playout_delay_oracle_; + video_config.field_trials = &field_trials; + sender_video_ = std::make_unique(video_config); memset(&codec_, 0, sizeof(VideoCodec)); codec_.plType = 100; diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 1138591b1d..90b92b3c3e 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -588,9 +588,13 @@ TEST_P(RtpSenderTestWithoutPacer, OnSendSideDelayUpdated) { rtp_sender_ = std::make_unique(config); PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - &playout_delay_oracle, nullptr, false, false, - false, FieldTrialBasedConfig()); + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &fake_clock_; + video_config.rtp_sender = rtp_sender_.get(); + video_config.playout_delay_oracle = &playout_delay_oracle; + video_config.field_trials = &field_trials; + RTPSenderVideo rtp_sender_video(video_config); const uint8_t kPayloadType = 127; const absl::optional kCodecType = @@ -1078,9 +1082,13 @@ TEST_P(RtpSenderTestWithoutPacer, SendGenericVideo) { const uint8_t kPayloadType = 127; const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric; PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - &playout_delay_oracle, nullptr, false, false, - false, FieldTrialBasedConfig()); + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &fake_clock_; + video_config.rtp_sender = rtp_sender_.get(); + video_config.playout_delay_oracle = &playout_delay_oracle; + video_config.field_trials = &field_trials; + RTPSenderVideo rtp_sender_video(video_config); uint8_t payload[] = {47, 11, 32, 93, 89}; // Send keyframe @@ -1118,9 +1126,13 @@ TEST_P(RtpSenderTestWithoutPacer, SendRawVideo) { const uint8_t payload[] = {11, 22, 33, 44, 55}; PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - &playout_delay_oracle, nullptr, false, false, - false, FieldTrialBasedConfig()); + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &fake_clock_; + video_config.rtp_sender = rtp_sender_.get(); + video_config.playout_delay_oracle = &playout_delay_oracle; + video_config.field_trials = &field_trials; + RTPSenderVideo rtp_sender_video(video_config); // Send a frame. RTPVideoHeader video_header; @@ -1160,9 +1172,14 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { rtp_sender_->SetStorePacketsStatus(true, 10); PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video( - &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle, - nullptr, false, false, false, FieldTrialBasedConfig()); + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &fake_clock_; + video_config.rtp_sender = rtp_sender_.get(); + video_config.flexfec_sender = &flexfec_sender; + video_config.playout_delay_oracle = &playout_delay_oracle; + video_config.field_trials = &field_trials; + RTPSenderVideo rtp_sender_video(video_config); // Parameters selected to generate a single FEC packet per media packet. FecProtectionParams params; @@ -1248,9 +1265,14 @@ TEST_P(RtpSenderTest, NoFlexfecForTimingFrames) { rtp_sender_->SetStorePacketsStatus(true, 10); PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video( - &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle, - nullptr, false, false, false, FieldTrialBasedConfig()); + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &fake_clock_; + video_config.rtp_sender = rtp_sender_.get(); + video_config.flexfec_sender = &flexfec_sender; + video_config.playout_delay_oracle = &playout_delay_oracle; + video_config.field_trials = &field_trials; + RTPSenderVideo rtp_sender_video(video_config); // Need extension to be registered for timing frames to be sent. ASSERT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( @@ -1376,9 +1398,14 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) { rtp_sender_->SetSequenceNumber(kSeqNum); PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video( - &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle, - nullptr, false, false, false, FieldTrialBasedConfig()); + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &fake_clock_; + video_config.rtp_sender = rtp_sender_.get(); + video_config.flexfec_sender = &flexfec_sender; + video_config.playout_delay_oracle = &playout_delay_oracle; + video_config.field_trials = &field_trials; + RTPSenderVideo rtp_sender_video(video_config); // Parameters selected to generate a single FEC packet per media packet. FecProtectionParams params; @@ -1643,9 +1670,14 @@ TEST_P(RtpSenderTest, FecOverheadRate) { rtp_sender_->SetSequenceNumber(kSeqNum); PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video( - &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle, - nullptr, false, false, false, FieldTrialBasedConfig()); + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &fake_clock_; + video_config.rtp_sender = rtp_sender_.get(); + video_config.flexfec_sender = &flexfec_sender; + video_config.playout_delay_oracle = &playout_delay_oracle; + video_config.field_trials = &field_trials; + RTPSenderVideo rtp_sender_video(video_config); // Parameters selected to generate a single FEC packet per media packet. FecProtectionParams params; params.fec_rate = 15; @@ -1715,9 +1747,13 @@ TEST_P(RtpSenderTest, BitrateCallbacks) { rtp_sender_ = std::make_unique(config); PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - &playout_delay_oracle, nullptr, false, false, - false, FieldTrialBasedConfig()); + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &fake_clock_; + video_config.rtp_sender = rtp_sender_.get(); + video_config.playout_delay_oracle = &playout_delay_oracle; + video_config.field_trials = &field_trials; + RTPSenderVideo rtp_sender_video(video_config); const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric; const uint8_t kPayloadType = 127; @@ -1766,45 +1802,50 @@ TEST_P(RtpSenderTest, BitrateCallbacks) { rtp_sender_.reset(); } +class StreamDataTestCallback : public StreamDataCountersCallback { + public: + StreamDataTestCallback() + : StreamDataCountersCallback(), ssrc_(0), counters_() {} + ~StreamDataTestCallback() override = default; + + void DataCountersUpdated(const StreamDataCounters& counters, + uint32_t ssrc) override { + ssrc_ = ssrc; + counters_ = counters; + } + + uint32_t ssrc_; + StreamDataCounters counters_; + + void MatchPacketCounter(const RtpPacketCounter& expected, + const RtpPacketCounter& actual) { + EXPECT_EQ(expected.payload_bytes, actual.payload_bytes); + EXPECT_EQ(expected.header_bytes, actual.header_bytes); + EXPECT_EQ(expected.padding_bytes, actual.padding_bytes); + EXPECT_EQ(expected.packets, actual.packets); + } + + void Matches(uint32_t ssrc, const StreamDataCounters& counters) { + EXPECT_EQ(ssrc, ssrc_); + MatchPacketCounter(counters.transmitted, counters_.transmitted); + MatchPacketCounter(counters.retransmitted, counters_.retransmitted); + EXPECT_EQ(counters.fec.packets, counters_.fec.packets); + } +}; + TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { - class TestCallback : public StreamDataCountersCallback { - public: - TestCallback() : StreamDataCountersCallback(), ssrc_(0), counters_() {} - ~TestCallback() override = default; + StreamDataTestCallback callback; - void DataCountersUpdated(const StreamDataCounters& counters, - uint32_t ssrc) override { - ssrc_ = ssrc; - counters_ = counters; - } - - uint32_t ssrc_; - StreamDataCounters counters_; - - void MatchPacketCounter(const RtpPacketCounter& expected, - const RtpPacketCounter& actual) { - EXPECT_EQ(expected.payload_bytes, actual.payload_bytes); - EXPECT_EQ(expected.header_bytes, actual.header_bytes); - EXPECT_EQ(expected.padding_bytes, actual.padding_bytes); - EXPECT_EQ(expected.packets, actual.packets); - } - - void Matches(uint32_t ssrc, const StreamDataCounters& counters) { - EXPECT_EQ(ssrc, ssrc_); - MatchPacketCounter(counters.transmitted, counters_.transmitted); - MatchPacketCounter(counters.retransmitted, counters_.retransmitted); - EXPECT_EQ(counters.fec.packets, counters_.fec.packets); - } - } callback; - - const uint8_t kRedPayloadType = 96; - const uint8_t kUlpfecPayloadType = 97; const uint8_t kPayloadType = 127; const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric; PlayoutDelayOracle playout_delay_oracle; - RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - &playout_delay_oracle, nullptr, false, false, - false, FieldTrialBasedConfig()); + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &fake_clock_; + video_config.rtp_sender = rtp_sender_.get(); + video_config.playout_delay_oracle = &playout_delay_oracle; + video_config.field_trials = &field_trials; + RTPSenderVideo rtp_sender_video(video_config); uint8_t payload[] = {47, 11, 32, 93, 89}; rtp_sender_->SetStorePacketsStatus(true, 1); uint32_t ssrc = rtp_sender_->SSRC(); @@ -1849,8 +1890,36 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { expected.transmitted.packets = 3; callback.Matches(ssrc, expected); + rtp_sender_->RegisterRtpStatisticsCallback(nullptr); +} + +TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) { + StreamDataTestCallback callback; + + const uint8_t kRedPayloadType = 96; + const uint8_t kUlpfecPayloadType = 97; + const uint8_t kPayloadType = 127; + const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric; + PlayoutDelayOracle playout_delay_oracle; + FieldTrialBasedConfig field_trials; + RTPSenderVideo::Config video_config; + video_config.clock = &fake_clock_; + video_config.rtp_sender = rtp_sender_.get(); + video_config.playout_delay_oracle = &playout_delay_oracle; + video_config.field_trials = &field_trials; + video_config.red_payload_type = kRedPayloadType; + video_config.ulpfec_payload_type = kUlpfecPayloadType; + RTPSenderVideo rtp_sender_video(video_config); + uint8_t payload[] = {47, 11, 32, 93, 89}; + rtp_sender_->SetStorePacketsStatus(true, 1); + uint32_t ssrc = rtp_sender_->SSRC(); + + rtp_sender_->RegisterRtpStatisticsCallback(&callback); + + RTPVideoHeader video_header; + StreamDataCounters expected; + // Send ULPFEC. - rtp_sender_video.SetUlpfecConfig(kRedPayloadType, kUlpfecPayloadType); FecProtectionParams fec_params; fec_params.fec_mask_type = kFecMaskRandom; fec_params.fec_rate = 1; @@ -1860,9 +1929,9 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { VideoFrameType::kVideoFrameDelta, kPayloadType, kCodecType, 1234, 4321, payload, sizeof(payload), nullptr, &video_header, kDefaultExpectedRetransmissionTimeMs)); - expected.transmitted.payload_bytes = 40; - expected.transmitted.header_bytes = 60; - expected.transmitted.packets = 5; + expected.transmitted.payload_bytes = 28; + expected.transmitted.header_bytes = 24; + expected.transmitted.packets = 2; expected.fec.packets = 1; callback.Matches(ssrc, expected); diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 6a6ef97c69..fc40a9723f 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -205,34 +205,50 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock, bool need_rtp_packet_infos, bool enable_retransmit_all_layers, const WebRtcKeyValueConfig& field_trials) - : rtp_sender_(rtp_sender), - clock_(clock), + : RTPSenderVideo([&] { + Config config; + config.clock = clock; + config.rtp_sender = rtp_sender; + config.flexfec_sender = flexfec_sender; + config.playout_delay_oracle = playout_delay_oracle; + config.frame_encryptor = frame_encryptor; + config.require_frame_encryption = require_frame_encryption; + config.need_rtp_packet_infos = need_rtp_packet_infos; + config.enable_retransmit_all_layers = enable_retransmit_all_layers; + config.field_trials = &field_trials; + return config; + }()) {} + +RTPSenderVideo::RTPSenderVideo(const Config& config) + : rtp_sender_(config.rtp_sender), + clock_(config.clock), retransmission_settings_( - enable_retransmit_all_layers + config.enable_retransmit_all_layers ? kRetransmitAllLayers : (kRetransmitBaseLayer | kConditionallyRetransmitHigherLayers)), last_rotation_(kVideoRotation_0), transmit_color_space_next_frame_(false), - playout_delay_oracle_(playout_delay_oracle), - rtp_sequence_number_map_(need_rtp_packet_infos + playout_delay_oracle_(config.playout_delay_oracle), + rtp_sequence_number_map_(config.need_rtp_packet_infos ? std::make_unique( kRtpSequenceNumberMapMaxEntries) : nullptr), - red_payload_type_(-1), - ulpfec_payload_type_(-1), - flexfec_sender_(flexfec_sender), + red_payload_type_(config.red_payload_type), + 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_(frame_encryptor), - require_frame_encryption_(require_frame_encryption), + frame_encryptor_(config.frame_encryptor), + require_frame_encryption_(config.require_frame_encryption), generic_descriptor_auth_experiment_( - field_trials.Lookup("WebRTC-GenericDescriptorAuth").find("Enabled") == - 0), + config.field_trials->Lookup("WebRTC-GenericDescriptorAuth") + .find("Enabled") == 0), exclude_transport_sequence_number_from_fec_experiment_( - field_trials.Lookup(kExcludeTransportSequenceNumberFromFecFieldTrial) + config.field_trials + ->Lookup(kExcludeTransportSequenceNumberFromFecFieldTrial) .find("Enabled") == 0) { RTC_DCHECK(playout_delay_oracle_); } @@ -273,7 +289,7 @@ void RTPSenderVideo::AppendAsRedMaybeWithUlpfec( { // Only protect while creating RED and FEC packets, not when sending. rtc::CritScope cs(&crit_); - red_packet->SetPayloadType(red_payload_type_); + red_packet->SetPayloadType(*red_payload_type_); if (ulpfec_enabled()) { if (protect_media_packet) { if (exclude_transport_sequence_number_from_fec_experiment_) { @@ -302,7 +318,8 @@ void RTPSenderVideo::AppendAsRedMaybeWithUlpfec( 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); + *red_payload_type_, *ulpfec_payload_type_, + first_fec_sequence_number); RTC_DCHECK_EQ(num_fec_packets, fec_packets.size()); } } @@ -399,8 +416,12 @@ void RTPSenderVideo::SetUlpfecConfig(int red_payload_type, RTC_DCHECK_LE(ulpfec_payload_type, 127); rtc::CritScope cs(&crit_); - red_payload_type_ = red_payload_type; - ulpfec_payload_type_ = ulpfec_payload_type; + if (red_payload_type != -1) { + red_payload_type_ = red_payload_type; + } + if (ulpfec_payload_type != -1) { + ulpfec_payload_type_ = ulpfec_payload_type; + } // Must not enable ULPFEC without RED. RTC_DCHECK(!(red_enabled() ^ ulpfec_enabled())); diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index 9ef95763f1..1b956fd9f0 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h @@ -56,6 +56,29 @@ class RTPSenderVideo { public: static constexpr int64_t kTLRateWindowSizeMs = 2500; + struct Config { + Config() = default; + Config(const Config&) = delete; + Config(Config&&) = default; + + // All members of this struct, with the exception of |field_trials|, are + // expected to outlive the RTPSenderVideo object they are passed to. + Clock* clock = nullptr; + RTPSender* rtp_sender = nullptr; + FlexfecSender* flexfec_sender = nullptr; + PlayoutDelayOracle* playout_delay_oracle = nullptr; + FrameEncryptorInterface* frame_encryptor = nullptr; + bool require_frame_encryption = false; + bool need_rtp_packet_infos = false; + bool enable_retransmit_all_layers = false; + absl::optional red_payload_type; + absl::optional ulpfec_payload_type; + const WebRtcKeyValueConfig* field_trials = nullptr; + }; + + explicit RTPSenderVideo(const Config& config); + + // TODO(bugs.webrtc.org/10809): Remove when downstream usage is gone. RTPSenderVideo(Clock* clock, RTPSender* rtpSender, FlexfecSender* flexfec_sender, @@ -100,6 +123,7 @@ class RTPSenderVideo { // corresponding feature is turned off. Note that we DO NOT support enabling // ULPFEC without enabling RED, and RED is only ever used when ULPFEC is // enabled. + // TODO(bugs.webrtc.org/10809): Remove when downstream usage is gone. void SetUlpfecConfig(int red_payload_type, int ulpfec_payload_type); // FlexFEC/ULPFEC. @@ -162,11 +186,11 @@ class RTPSenderVideo { size_t unpacketized_payload_size); bool red_enabled() const RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_) { - return red_payload_type_ >= 0; + return red_payload_type_.has_value(); } bool ulpfec_enabled() const RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_) { - return ulpfec_payload_type_ >= 0; + return ulpfec_payload_type_.has_value(); } bool flexfec_enabled() const { return flexfec_sender_ != nullptr; } @@ -209,8 +233,8 @@ class RTPSenderVideo { RTC_PT_GUARDED_BY(crit_); // RED/ULPFEC. - int red_payload_type_ RTC_GUARDED_BY(crit_); - int ulpfec_payload_type_ RTC_GUARDED_BY(crit_); + absl::optional red_payload_type_ RTC_GUARDED_BY(crit_); + absl::optional ulpfec_payload_type_ RTC_GUARDED_BY(crit_); UlpfecGenerator ulpfec_generator_ RTC_GUARDED_BY(crit_); // FlexFEC. diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index 4bd80d3147..856d2395db 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc @@ -99,15 +99,15 @@ class TestRtpSenderVideo : public RTPSenderVideo { RTPSender* rtp_sender, FlexfecSender* flexfec_sender, const WebRtcKeyValueConfig& field_trials) - : RTPSenderVideo(clock, - rtp_sender, - flexfec_sender, - &playout_delay_oracle_, - nullptr, - false, - false, - false, - field_trials) {} + : RTPSenderVideo([&] { + Config config; + config.clock = clock; + config.rtp_sender = rtp_sender; + config.flexfec_sender = flexfec_sender; + config.playout_delay_oracle = &playout_delay_oracle_; + config.field_trials = &field_trials; + return config; + }()) {} ~TestRtpSenderVideo() override {} bool AllowRetransmission(const RTPVideoHeader& header,