diff --git a/call/BUILD.gn b/call/BUILD.gn index 808aa73afc..999ff21c3a 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -155,6 +155,7 @@ rtc_library("rtp_sender") { "../api/transport:field_trial_based_config", "../api/transport:goog_cc", "../api/transport:network_control", + "../api/transport:webrtc_key_value_config", "../api/units:data_rate", "../api/units:time_delta", "../api/units:timestamp", @@ -179,7 +180,6 @@ rtc_library("rtp_sender") { "../rtc_base:rtc_base_approved", "../rtc_base:rtc_task_queue", "../rtc_base/task_utils:repeating_task", - "../system_wrappers:field_trial", "//third_party/abseil-cpp/absl/algorithm:container", "//third_party/abseil-cpp/absl/container:inlined_vector", "//third_party/abseil-cpp/absl/strings:strings", diff --git a/call/rtp_payload_params.cc b/call/rtp_payload_params.cc index 31cb743d9d..bce3c13055 100644 --- a/call/rtp_payload_params.cc +++ b/call/rtp_payload_params.cc @@ -16,6 +16,7 @@ #include "absl/algorithm/container.h" #include "absl/container/inlined_vector.h" +#include "absl/strings/match.h" #include "absl/types/variant.h" #include "api/video/video_timing.h" #include "modules/video_coding/codecs/h264/include/h264_globals.h" @@ -28,7 +29,6 @@ #include "rtc_base/logging.h" #include "rtc_base/random.h" #include "rtc_base/time_utils.h" -#include "system_wrappers/include/field_trial.h" namespace webrtc { @@ -135,12 +135,15 @@ void SetVideoTiming(const EncodedImage& image, VideoSendTiming* timing) { } // namespace RtpPayloadParams::RtpPayloadParams(const uint32_t ssrc, - const RtpPayloadState* state) + const RtpPayloadState* state, + const WebRtcKeyValueConfig& trials) : ssrc_(ssrc), generic_picture_id_experiment_( - field_trial::IsEnabled("WebRTC-GenericPictureId")), + absl::StartsWith(trials.Lookup("WebRTC-GenericPictureId"), + "Enabled")), generic_descriptor_experiment_( - !field_trial::IsDisabled("WebRTC-GenericDescriptor")) { + !absl::StartsWith(trials.Lookup("WebRTC-GenericDescriptor"), + "Disabled")) { for (auto& spatial_layer : last_shared_frame_id_) spatial_layer.fill(-1); diff --git a/call/rtp_payload_params.h b/call/rtp_payload_params.h index 95a9cb762a..13b1050378 100644 --- a/call/rtp_payload_params.h +++ b/call/rtp_payload_params.h @@ -14,6 +14,7 @@ #include #include "absl/types/optional.h" +#include "api/transport/webrtc_key_value_config.h" #include "api/video_codecs/video_encoder.h" #include "call/rtp_config.h" #include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h" @@ -29,7 +30,9 @@ class RtpRtcp; // TODO(nisse): Make these properties not codec specific. class RtpPayloadParams final { public: - RtpPayloadParams(const uint32_t ssrc, const RtpPayloadState* state); + RtpPayloadParams(const uint32_t ssrc, + const RtpPayloadState* state, + const WebRtcKeyValueConfig& trials); RtpPayloadParams(const RtpPayloadParams& other); ~RtpPayloadParams(); diff --git a/call/rtp_payload_params_unittest.cc b/call/rtp_payload_params_unittest.cc index 54b4025ceb..1d6a70c1db 100644 --- a/call/rtp_payload_params_unittest.cc +++ b/call/rtp_payload_params_unittest.cc @@ -18,6 +18,7 @@ #include "absl/container/inlined_vector.h" #include "absl/types/optional.h" #include "absl/types/variant.h" +#include "api/transport/field_trial_based_config.h" #include "api/video/video_content_type.h" #include "api/video/video_rotation.h" #include "modules/video_coding/codecs/h264/include/h264_globals.h" @@ -50,7 +51,7 @@ TEST(RtpPayloadParamsTest, InfoMappedToRtpVideoHeader_Vp8) { state2.tl0_pic_idx = kTl0PicIdx; std::map states = {{kSsrc2, state2}}; - RtpPayloadParams params(kSsrc2, &state2); + RtpPayloadParams params(kSsrc2, &state2, FieldTrialBasedConfig()); EncodedImage encoded_image; encoded_image.rotation_ = kVideoRotation_90; encoded_image.content_type_ = VideoContentType::SCREENSHARE; @@ -90,7 +91,7 @@ TEST(RtpPayloadParamsTest, InfoMappedToRtpVideoHeader_Vp9) { RtpPayloadState state; state.picture_id = kPictureId; state.tl0_pic_idx = kTl0PicIdx; - RtpPayloadParams params(kSsrc1, &state); + RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig()); EncodedImage encoded_image; encoded_image.rotation_ = kVideoRotation_90; @@ -150,7 +151,7 @@ TEST(RtpPayloadParamsTest, InfoMappedToRtpVideoHeader_H264) { RtpPayloadState state; state.picture_id = kPictureId; state.tl0_pic_idx = kInitialTl0PicIdx1; - RtpPayloadParams params(kSsrc1, &state); + RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig()); EncodedImage encoded_image; CodecSpecificInfo codec_info; @@ -203,7 +204,7 @@ TEST(RtpPayloadParamsTest, PictureIdIsSetForVp8) { CodecSpecificInfo codec_info; codec_info.codecType = kVideoCodecVP8; - RtpPayloadParams params(kSsrc1, &state); + RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig()); RTPVideoHeader header = params.GetRtpVideoHeader(encoded_image, &codec_info, kDontCare); EXPECT_EQ(kVideoCodecVP8, header.codec); @@ -226,7 +227,7 @@ TEST(RtpPayloadParamsTest, PictureIdWraps) { codec_info.codecType = kVideoCodecVP8; codec_info.codecSpecific.VP8.temporalIdx = kNoTemporalIdx; - RtpPayloadParams params(kSsrc1, &state); + RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig()); RTPVideoHeader header = params.GetRtpVideoHeader(encoded_image, &codec_info, kDontCare); EXPECT_EQ(kVideoCodecVP8, header.codec); @@ -250,7 +251,7 @@ TEST(RtpPayloadParamsTest, Tl0PicIdxUpdatedForVp8) { codec_info.codecType = kVideoCodecVP8; codec_info.codecSpecific.VP8.temporalIdx = 1; - RtpPayloadParams params(kSsrc1, &state); + RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig()); RTPVideoHeader header = params.GetRtpVideoHeader(encoded_image, &codec_info, kDontCare); @@ -286,7 +287,7 @@ TEST(RtpPayloadParamsTest, Tl0PicIdxUpdatedForVp9) { codec_info.codecSpecific.VP9.temporal_idx = 1; codec_info.codecSpecific.VP9.first_frame_in_picture = true; - RtpPayloadParams params(kSsrc1, &state); + RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig()); RTPVideoHeader header = params.GetRtpVideoHeader(encoded_image, &codec_info, kDontCare); @@ -329,7 +330,7 @@ TEST(RtpPayloadParamsTest, PictureIdForOldGenericFormat) { codec_info.codecType = kVideoCodecGeneric; encoded_image._frameType = VideoFrameType::kVideoFrameKey; - RtpPayloadParams params(kSsrc1, &state); + RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig()); RTPVideoHeader header = params.GetRtpVideoHeader(encoded_image, &codec_info, 10); @@ -357,7 +358,7 @@ TEST(RtpPayloadParamsTest, GenericDescriptorForGenericCodec) { CodecSpecificInfo codec_info; codec_info.codecType = kVideoCodecGeneric; - RtpPayloadParams params(kSsrc1, &state); + RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig()); RTPVideoHeader header = params.GetRtpVideoHeader(encoded_image, &codec_info, 0); @@ -380,7 +381,7 @@ TEST(RtpPayloadParamsTest, SetsGenericFromGenericFrameInfo) { EncodedImage encoded_image; CodecSpecificInfo codec_info; - RtpPayloadParams params(kSsrc1, &state); + RtpPayloadParams params(kSsrc1, &state, FieldTrialBasedConfig()); encoded_image._frameType = VideoFrameType::kVideoFrameKey; codec_info.generic_frame_info = @@ -424,7 +425,7 @@ class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test { RtpPayloadParamsVp8ToGenericTest() : generic_descriptor_field_trial_("WebRTC-GenericDescriptor/Enabled/"), state_(), - params_(123, &state_) {} + params_(123, &state_, trials_config_) {} void ConvertAndCheck(int temporal_index, int64_t shared_frame_id, @@ -461,6 +462,7 @@ class RtpPayloadParamsVp8ToGenericTest : public ::testing::Test { protected: test::ScopedFieldTrials generic_descriptor_field_trial_; + FieldTrialBasedConfig trials_config_; RtpPayloadState state_; RtpPayloadParams params_; }; @@ -520,7 +522,7 @@ class RtpPayloadParamsH264ToGenericTest : public ::testing::Test { RtpPayloadParamsH264ToGenericTest() : generic_descriptor_field_trial_("WebRTC-GenericDescriptor/Enabled/"), state_(), - params_(123, &state_) {} + params_(123, &state_, trials_config_) {} void ConvertAndCheck(int temporal_index, int64_t shared_frame_id, @@ -557,6 +559,7 @@ class RtpPayloadParamsH264ToGenericTest : public ::testing::Test { protected: test::ScopedFieldTrials generic_descriptor_field_trial_; + FieldTrialBasedConfig trials_config_; RtpPayloadState state_; RtpPayloadParams params_; }; diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 3a6a27cc7a..079ea711b4 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -30,7 +30,6 @@ #include "rtc_base/checks.h" #include "rtc_base/location.h" #include "rtc_base/logging.h" -#include "system_wrappers/include/field_trial.h" namespace webrtc { @@ -55,20 +54,22 @@ static const size_t kPathMTU = 1500; using webrtc_internal_rtp_video_sender::RtpStreamSender; -bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name) { +bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name, + const WebRtcKeyValueConfig& trials) { const VideoCodecType codecType = PayloadStringToCodecType(payload_name); if (codecType == kVideoCodecVP8 || codecType == kVideoCodecVP9) { return true; } if (codecType == kVideoCodecGeneric && - field_trial::IsEnabled("WebRTC-GenericPictureId")) { + absl::StartsWith(trials.Lookup("WebRTC-GenericPictureId"), "Enabled")) { return true; } return false; } bool ShouldDisableRedAndUlpfec(bool flexfec_enabled, - const RtpConfig& rtp_config) { + const RtpConfig& rtp_config, + const WebRtcKeyValueConfig& trials) { // Consistency of NACK and RED+ULPFEC parameters is checked in this function. const bool nack_enabled = rtp_config.nack.rtp_history_ms > 0; @@ -80,7 +81,8 @@ bool ShouldDisableRedAndUlpfec(bool flexfec_enabled, bool should_disable_red_and_ulpfec = false; - if (webrtc::field_trial::IsEnabled("WebRTC-DisableUlpFecExperiment")) { + if (absl::StartsWith(trials.Lookup("WebRTC-DisableUlpFecExperiment"), + "Enabled")) { RTC_LOG(LS_INFO) << "Experiment to disable sending ULPFEC is enabled."; should_disable_red_and_ulpfec = true; } @@ -99,7 +101,7 @@ bool ShouldDisableRedAndUlpfec(bool flexfec_enabled, // 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)) { + !PayloadTypeSupportsSkippingFecPackets(rtp_config.payload_name, trials)) { RTC_LOG(LS_WARNING) << "Transmitting payload type without picture ID using " "NACK+ULPFEC is a waste of bandwidth since ULPFEC packets " @@ -122,7 +124,8 @@ std::unique_ptr MaybeCreateFecGenerator( Clock* clock, const RtpConfig& rtp, const std::map& suspended_ssrcs, - int simulcast_index) { + int simulcast_index, + const WebRtcKeyValueConfig& trials) { // If flexfec is configured that takes priority. if (rtp.flexfec.payload_type >= 0) { RTC_DCHECK_GE(rtp.flexfec.payload_type, 0); @@ -168,7 +171,8 @@ std::unique_ptr MaybeCreateFecGenerator( RTPSender::FecExtensionSizes(), rtp_state, clock); } else if (rtp.ulpfec.red_payload_type >= 0 && rtp.ulpfec.ulpfec_payload_type >= 0 && - !ShouldDisableRedAndUlpfec(/*flexfec_enabled=*/false, rtp)) { + !ShouldDisableRedAndUlpfec(/*flexfec_enabled=*/false, rtp, + trials)) { // Flexfec not configured, but ulpfec is and is not disabled. return std::make_unique( rtp.ulpfec.red_payload_type, rtp.ulpfec.ulpfec_payload_type, clock); @@ -192,7 +196,8 @@ std::vector CreateRtpStreamSenders( OverheadObserver* overhead_observer, FrameEncryptorInterface* frame_encryptor, const CryptoOptions& crypto_options, - rtc::scoped_refptr frame_transformer) { + rtc::scoped_refptr frame_transformer, + const WebRtcKeyValueConfig& trials) { RTC_DCHECK_GT(rtp_config.ssrcs.size(), 0); RtpRtcp::Configuration configuration; @@ -227,6 +232,7 @@ std::vector CreateRtpStreamSenders( crypto_options.sframe.require_frame_encryption; configuration.extmap_allow_mixed = rtp_config.extmap_allow_mixed; configuration.rtcp_report_interval_ms = rtcp_report_interval_ms; + configuration.field_trials = &trials; std::vector rtp_streams; @@ -237,7 +243,7 @@ std::vector CreateRtpStreamSenders( configuration.local_media_ssrc = rtp_config.ssrcs[i]; std::unique_ptr fec_generator = - MaybeCreateFecGenerator(clock, rtp_config, suspended_ssrcs, i); + MaybeCreateFecGenerator(clock, rtp_config, suspended_ssrcs, i, trials); configuration.fec_generator = fec_generator.get(); video_config.fec_generator = fec_generator.get(); @@ -255,20 +261,19 @@ std::vector CreateRtpStreamSenders( // Set NACK. rtp_rtcp->SetStorePacketsStatus(true, kMinSendSidePacketHistorySize); - FieldTrialBasedConfig field_trial_config; video_config.clock = configuration.clock; video_config.rtp_sender = rtp_rtcp->RtpSender(); video_config.frame_encryptor = frame_encryptor; video_config.require_frame_encryption = crypto_options.sframe.require_frame_encryption; video_config.enable_retransmit_all_layers = false; - video_config.field_trials = &field_trial_config; + video_config.field_trials = &trials; const bool using_flexfec = fec_generator && fec_generator->GetFecType() == VideoFecGenerator::FecType::kFlexFec; const bool should_disable_red_and_ulpfec = - ShouldDisableRedAndUlpfec(using_flexfec, rtp_config); + ShouldDisableRedAndUlpfec(using_flexfec, rtp_config, trials); if (!should_disable_red_and_ulpfec && rtp_config.ulpfec.red_payload_type != -1) { video_config.red_payload_type = rtp_config.ulpfec.red_payload_type; @@ -318,12 +323,15 @@ RtpVideoSender::RtpVideoSender( FrameEncryptorInterface* frame_encryptor, const CryptoOptions& crypto_options, rtc::scoped_refptr frame_transformer) - : send_side_bwe_with_overhead_( - webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")), - account_for_packetization_overhead_(!webrtc::field_trial::IsDisabled( - "WebRTC-SubtractPacketizationOverhead")), - use_early_loss_detection_( - !webrtc::field_trial::IsDisabled("WebRTC-UseEarlyLossDetection")), + : send_side_bwe_with_overhead_(absl::StartsWith( + field_trials_.Lookup("WebRTC-SendSideBwe-WithOverhead"), + "Enabled")), + account_for_packetization_overhead_(!absl::StartsWith( + field_trials_.Lookup("WebRTC-SubtractPacketizationOverhead"), + "Disabled")), + use_early_loss_detection_(!absl::StartsWith( + field_trials_.Lookup("WebRTC-UseEarlyLossDetection"), + "Disabled")), has_packet_feedback_(TransportSeqNumExtensionConfigured(rtp_config)), active_(false), module_process_thread_(nullptr), @@ -343,7 +351,8 @@ RtpVideoSender::RtpVideoSender( this, frame_encryptor, crypto_options, - std::move(frame_transformer))), + std::move(frame_transformer), + field_trials_)), rtp_config_(rtp_config), codec_type_(GetVideoCodecType(rtp_config)), transport_(transport), @@ -365,7 +374,7 @@ RtpVideoSender::RtpVideoSender( state = &it->second; shared_frame_id_ = std::max(shared_frame_id_, state->shared_frame_id); } - params_.push_back(RtpPayloadParams(ssrc, state)); + params_.push_back(RtpPayloadParams(ssrc, state, field_trials_)); } // RTP/RTCP initialization. diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index f7ebefcbb3..d2a20a95c9 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -22,6 +22,7 @@ #include "api/fec_controller.h" #include "api/fec_controller_override.h" #include "api/rtc_event_log/rtc_event_log.h" +#include "api/transport/field_trial_based_config.h" #include "api/video_codecs/video_encoder.h" #include "call/rtp_config.h" #include "call/rtp_payload_params.h" @@ -160,6 +161,7 @@ class RtpVideoSender : public RtpVideoSenderInterface, bool NackEnabled() const; uint32_t GetPacketizationOverheadRate() const; + const FieldTrialBasedConfig field_trials_; const bool send_side_bwe_with_overhead_; const bool account_for_packetization_overhead_; const bool use_early_loss_detection_;