From e11b7d2e8087262bc29d41ae3782ed94824cbe3f Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Thu, 21 Feb 2019 07:55:59 +0100 Subject: [PATCH] Replace field trials with WebRtcKeyValueConfig in RtpRtcpModule Replaces use of field trials in RtpSender and RtpVideoSender with injectable WebRtcKeyValueConfig. Implementation still defaults to field trials. BUG: webrtc:10335 Change-Id: I5fc6d327ee5d011ccc41385734b38344df172627 Reviewed-on: https://webrtc-review.googlesource.com/c/123447 Reviewed-by: Danil Chapovalov Reviewed-by: Sebastian Jansson Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/master@{#26795} --- modules/rtp_rtcp/BUILD.gn | 4 +- modules/rtp_rtcp/DEPS | 2 + modules/rtp_rtcp/include/rtp_rtcp.h | 5 + modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 11 +- modules/rtp_rtcp/source/rtp_sender.cc | 7 +- modules/rtp_rtcp/source/rtp_sender.h | 4 +- .../source/rtp_sender_audio_unittest.cc | 4 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 107 ++++++++++-------- modules/rtp_rtcp/source/rtp_sender_video.cc | 7 +- modules/rtp_rtcp/source/rtp_sender_video.h | 3 +- .../source/rtp_sender_video_unittest.cc | 38 +++++-- 11 files changed, 125 insertions(+), 67 deletions(-) diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index 6f641eba77..bf8e69f635 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -202,6 +202,8 @@ rtc_static_library("rtp_rtcp") { "../../api:scoped_refptr", "../../api:transport_api", "../../api/audio_codecs:audio_codecs_api", + "../../api/transport:field_trial_based_config", + "../../api/transport:webrtc_key_value_config", "../../api/video:video_bitrate_allocation", "../../api/video:video_bitrate_allocator", "../../api/video:video_frame", @@ -222,7 +224,6 @@ rtc_static_library("rtp_rtcp") { "../../rtc_base/system:fallthrough", "../../rtc_base/time:timestamp_extrapolator", "../../system_wrappers", - "../../system_wrappers:field_trial", "../../system_wrappers:metrics", "../remote_bitrate_estimator", "../video_coding:codec_globals_headers", @@ -432,6 +433,7 @@ if (rtc_include_tests) { "../../api:libjingle_peerconnection_api", "../../api:scoped_refptr", "../../api:transport_api", + "../../api/transport:field_trial_based_config", "../../api/video:video_bitrate_allocation", "../../api/video:video_bitrate_allocator", "../../api/video:video_codec_constants", diff --git a/modules/rtp_rtcp/DEPS b/modules/rtp_rtcp/DEPS index 9d9f33cbaf..dac95dd23a 100644 --- a/modules/rtp_rtcp/DEPS +++ b/modules/rtp_rtcp/DEPS @@ -3,4 +3,6 @@ include_rules = [ "+common_video", "+logging/rtc_event_log", "+system_wrappers", + # Avoid directly using field_trial. Instead use WebRtcKeyValueConfig. + "-system_wrappers/include/field_trial.h", ] diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index 0ad8635171..5f7e0d901a 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -18,6 +18,7 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" +#include "api/transport/webrtc_key_value_config.h" #include "api/video/video_bitrate_allocation.h" #include "modules/include/module.h" #include "modules/rtp_rtcp/include/flexfec_sender.h" @@ -107,6 +108,10 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { // Corresponds to extmap-allow-mixed in SDP negotiation. bool extmap_allow_mixed = false; + // If set, field trials are read from |field_trials|, otherwise + // defaults to webrtc::FieldTrialBasedConfig. + WebRtcKeyValueConfig* field_trials = nullptr; + private: RTC_DISALLOW_COPY_AND_ASSIGN(Configuration); }; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 3589d6a890..3ed38d7312 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -18,6 +18,7 @@ #include #include "absl/memory/memory.h" +#include "api/transport/field_trial_based_config.h" #include "modules/rtp_rtcp/source/rtcp_packet/dlrr.h" #include "modules/rtp_rtcp/source/rtp_rtcp_config.h" #include "rtc_base/checks.h" @@ -97,6 +98,7 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) remote_bitrate_(configuration.remote_bitrate_estimator), rtt_stats_(configuration.rtt_stats), rtt_ms_(0) { + FieldTrialBasedConfig default_trials; if (!configuration.receiver_only) { rtp_sender_.reset(new RTPSender( configuration.audio, configuration.clock, @@ -113,14 +115,17 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) configuration.overhead_observer, configuration.populate_network2_timestamp, configuration.frame_encryptor, configuration.require_frame_encryption, - configuration.extmap_allow_mixed)); + configuration.extmap_allow_mixed, + configuration.field_trials ? *configuration.field_trials + : default_trials)); if (configuration.audio) { audio_ = absl::make_unique(clock_, rtp_sender_.get()); } else { video_ = absl::make_unique( clock_, rtp_sender_.get(), configuration.flexfec_sender, - configuration.frame_encryptor, - configuration.require_frame_encryption); + configuration.frame_encryptor, configuration.require_frame_encryption, + configuration.field_trials ? *configuration.field_trials + : default_trials); } // Make sure rtcp sender use same timestamp offset as rtp sender. rtcp_sender_.SetTimestampOffset(rtp_sender_->TimestampOffset()); diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 9be7058294..197f52ba11 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -32,7 +32,6 @@ #include "rtc_base/numerics/safe_minmax.h" #include "rtc_base/rate_limiter.h" #include "rtc_base/time_utils.h" -#include "system_wrappers/include/field_trial.h" namespace webrtc { @@ -104,7 +103,8 @@ RTPSender::RTPSender( bool populate_network2_timestamp, FrameEncryptorInterface* frame_encryptor, bool require_frame_encryption, - bool extmap_allow_mixed) + bool extmap_allow_mixed, + const WebRtcKeyValueConfig& field_trials) : clock_(clock), // TODO(holmer): Remove this conversion? clock_delta_ms_(clock_->TimeInMilliseconds() - rtc::TimeMillis()), @@ -148,7 +148,8 @@ RTPSender::RTPSender( overhead_observer_(overhead_observer), populate_network2_timestamp_(populate_network2_timestamp), send_side_bwe_with_overhead_( - webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")) { + field_trials.Lookup("WebRTC-SendSideBwe-WithOverhead") + .find("Enabled") == 0) { // This random initialization is not intended to be cryptographic strong. timestamp_offset_ = random_.Rand(); // Random start, 16 bits. Can't be 0. diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 622f41a08d..f08eb896b2 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -21,6 +21,7 @@ #include "absl/types/optional.h" #include "api/array_view.h" #include "api/call/transport.h" +#include "api/transport/webrtc_key_value_config.h" #include "common_types.h" // NOLINT(build/include) #include "modules/rtp_rtcp/include/flexfec_sender.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" @@ -60,7 +61,8 @@ class RTPSender { bool populate_network2_timestamp, FrameEncryptorInterface* frame_encryptor, bool require_frame_encryption, - bool extmap_allow_mixed); + bool extmap_allow_mixed, + const WebRtcKeyValueConfig& field_trials); ~RTPSender(); diff --git a/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc index c5ca7f01be..31b0048247 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_audio_unittest.cc @@ -10,6 +10,7 @@ #include +#include "api/transport/field_trial_based_config.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtp_header_extensions.h" @@ -79,7 +80,8 @@ class RtpSenderAudioTest : public ::testing::Test { false, nullptr, false, - false), + false, + FieldTrialBasedConfig()), rtp_sender_audio_(&fake_clock_, &rtp_sender_) { rtp_sender_.SetSSRC(kSsrc); rtp_sender_.SetSequenceNumber(kSeqNum); diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 85566c4178..2bd3c949ee 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -12,6 +12,7 @@ #include #include "absl/memory/memory.h" +#include "api/transport/field_trial_based_config.h" #include "api/video/video_codec_constants.h" #include "api/video/video_timing.h" #include "logging/rtc_event_log/events/rtc_event.h" @@ -193,7 +194,7 @@ class RtpSenderTest : public ::testing::TestWithParam { absl::nullopt, &seq_num_allocator_, nullptr, nullptr, nullptr, &mock_rtc_event_log_, &send_packet_observer_, &retransmission_rate_limiter_, nullptr, populate_network2, nullptr, - false, false)); + false, false, FieldTrialBasedConfig())); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetTimestampOffset(0); rtp_sender_->SetSSRC(kSsrc); @@ -333,7 +334,7 @@ TEST_P(RtpSenderTest, AssignSequenceNumberAllowsPaddingOnAudio) { kEnableAudio, &fake_clock_, &transport, &mock_paced_sender_, absl::nullopt, nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr, &retransmission_rate_limiter_, nullptr, false, nullptr, false, - false)); + false, FieldTrialBasedConfig())); rtp_sender_->SetTimestampOffset(0); rtp_sender_->SetSSRC(kSsrc); @@ -376,11 +377,12 @@ TEST_P(RtpSenderTestWithoutPacer, TransportFeedbackObserverGetsCorrectByteCount) { constexpr int kRtpOverheadBytesPerPacket = 12 + 8; testing::NiceMock mock_overhead_observer; - rtp_sender_.reset(new RTPSender( - false, &fake_clock_, &transport_, nullptr, absl::nullopt, - &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, - &mock_rtc_event_log_, nullptr, &retransmission_rate_limiter_, - &mock_overhead_observer, false, nullptr, false, false)); + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, + &mock_rtc_event_log_, nullptr, + &retransmission_rate_limiter_, &mock_overhead_observer, + false, nullptr, false, false, FieldTrialBasedConfig())); rtp_sender_->SetSSRC(kSsrc); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, @@ -403,11 +405,12 @@ TEST_P(RtpSenderTestWithoutPacer, } TEST_P(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) { - rtp_sender_.reset(new RTPSender( - false, &fake_clock_, &transport_, nullptr, absl::nullopt, - &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, - &mock_rtc_event_log_, &send_packet_observer_, - &retransmission_rate_limiter_, nullptr, false, nullptr, false, false)); + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, + &mock_rtc_event_log_, &send_packet_observer_, + &retransmission_rate_limiter_, nullptr, false, nullptr, + false, false, FieldTrialBasedConfig())); rtp_sender_->SetSSRC(kSsrc); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, @@ -435,11 +438,12 @@ TEST_P(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) { } TEST_P(RtpSenderTestWithoutPacer, PacketOptionsNoRetransmission) { - rtp_sender_.reset(new RTPSender( - false, &fake_clock_, &transport_, nullptr, absl::nullopt, - &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, - &mock_rtc_event_log_, &send_packet_observer_, - &retransmission_rate_limiter_, nullptr, false, nullptr, false, false)); + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, + &mock_rtc_event_log_, &send_packet_observer_, + &retransmission_rate_limiter_, nullptr, false, nullptr, + false, false, FieldTrialBasedConfig())); rtp_sender_->SetSSRC(kSsrc); SendGenericPacket(); @@ -490,13 +494,14 @@ TEST_P(RtpSenderTestWithoutPacer, DoesnSetIncludedInAllocationByDefault) { TEST_P(RtpSenderTestWithoutPacer, OnSendSideDelayUpdated) { testing::StrictMock send_side_delay_observer_; - rtp_sender_.reset(new RTPSender( - false, &fake_clock_, &transport_, nullptr, absl::nullopt, nullptr, - nullptr, nullptr, &send_side_delay_observer_, &mock_rtc_event_log_, - nullptr, nullptr, nullptr, false, nullptr, false, false)); + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + nullptr, nullptr, nullptr, &send_side_delay_observer_, + &mock_rtc_event_log_, nullptr, nullptr, nullptr, false, + nullptr, false, false, FieldTrialBasedConfig())); rtp_sender_->SetSSRC(kSsrc); RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - nullptr, false); + nullptr, false, FieldTrialBasedConfig()); const uint8_t kPayloadType = 127; const char payload_name[] = "GENERIC"; @@ -573,7 +578,8 @@ TEST_P(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) { false, &fake_clock_, &transport_, &mock_paced_sender_, absl::nullopt, &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, &mock_rtc_event_log_, &send_packet_observer_, - &retransmission_rate_limiter_, nullptr, false, nullptr, false, false)); + &retransmission_rate_limiter_, nullptr, false, nullptr, false, false, + FieldTrialBasedConfig())); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetSSRC(kSsrc); rtp_sender_->SetStorePacketsStatus(true, 10); @@ -958,7 +964,7 @@ TEST_P(RtpSenderTest, OnSendPacketNotUpdatedWithoutSeqNumAllocator) { false, &fake_clock_, &transport_, &mock_paced_sender_, absl::nullopt, nullptr /* TransportSequenceNumberAllocator */, nullptr, nullptr, nullptr, nullptr, &send_packet_observer_, &retransmission_rate_limiter_, nullptr, - false, nullptr, false, false)); + false, nullptr, false, false, FieldTrialBasedConfig())); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetSSRC(kSsrc); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( @@ -984,7 +990,8 @@ TEST_P(RtpSenderTest, SendRedundantPayloads) { rtp_sender_.reset(new RTPSender( false, &fake_clock_, &transport, &mock_paced_sender_, absl::nullopt, nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr, - &retransmission_rate_limiter_, nullptr, false, nullptr, false, false)); + &retransmission_rate_limiter_, nullptr, false, nullptr, false, false, + FieldTrialBasedConfig())); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetSSRC(kSsrc); rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); @@ -1060,7 +1067,7 @@ TEST_P(RtpSenderTestWithoutPacer, SendGenericVideo) { const char payload_name[] = "GENERIC"; const uint8_t payload_type = 127; RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - nullptr, false); + nullptr, false, FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(payload_type, payload_name); uint8_t payload[] = {47, 11, 32, 93, 89}; @@ -1109,13 +1116,14 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) { false, &fake_clock_, &transport_, &mock_paced_sender_, kFlexfecSsrc, &seq_num_allocator_, nullptr, nullptr, nullptr, &mock_rtc_event_log_, &send_packet_observer_, &retransmission_rate_limiter_, nullptr, false, - nullptr, false, false)); + nullptr, false, false, FieldTrialBasedConfig())); rtp_sender_->SetSSRC(kMediaSsrc); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetStorePacketsStatus(true, 10); RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), - &flexfec_sender, nullptr, false); + &flexfec_sender, nullptr, false, + FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC"); // Parameters selected to generate a single FEC packet per media packet. @@ -1180,13 +1188,15 @@ TEST_P(RtpSenderTest, NoFlexfecForTimingFrames) { false, &fake_clock_, &transport_, &mock_paced_sender_, flexfec_sender.ssrc(), &seq_num_allocator_, nullptr, nullptr, nullptr, &mock_rtc_event_log_, &send_packet_observer_, - &retransmission_rate_limiter_, nullptr, false, nullptr, false, false)); + &retransmission_rate_limiter_, nullptr, false, nullptr, false, false, + FieldTrialBasedConfig())); rtp_sender_->SetSSRC(kMediaSsrc); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetStorePacketsStatus(true, 10); RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), - &flexfec_sender, nullptr, false); + &flexfec_sender, nullptr, false, + FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC"); // Need extension to be registered for timing frames to be sent. @@ -1277,12 +1287,13 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) { false, &fake_clock_, &transport_, nullptr, flexfec_sender.ssrc(), &seq_num_allocator_, nullptr, nullptr, nullptr, &mock_rtc_event_log_, &send_packet_observer_, &retransmission_rate_limiter_, nullptr, false, - nullptr, false, false)); + nullptr, false, false, FieldTrialBasedConfig())); rtp_sender_->SetSSRC(kMediaSsrc); rtp_sender_->SetSequenceNumber(kSeqNum); RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), - &flexfec_sender, nullptr, false); + &flexfec_sender, nullptr, false, + FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC"); // Parameters selected to generate a single FEC packet per media packet. @@ -1404,12 +1415,14 @@ TEST_P(RtpSenderTest, FecOverheadRate) { false, &fake_clock_, &transport_, &mock_paced_sender_, flexfec_sender.ssrc(), &seq_num_allocator_, nullptr, nullptr, nullptr, &mock_rtc_event_log_, &send_packet_observer_, - &retransmission_rate_limiter_, nullptr, false, nullptr, false, false)); + &retransmission_rate_limiter_, nullptr, false, nullptr, false, false, + FieldTrialBasedConfig())); rtp_sender_->SetSSRC(kMediaSsrc); rtp_sender_->SetSequenceNumber(kSeqNum); RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), - &flexfec_sender, nullptr, false); + &flexfec_sender, nullptr, false, + FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC"); // Parameters selected to generate a single FEC packet per media packet. FecProtectionParams params; @@ -1469,14 +1482,15 @@ TEST_P(RtpSenderTest, BitrateCallbacks) { uint32_t total_bitrate_; uint32_t retransmit_bitrate_; } callback; - rtp_sender_.reset(new RTPSender( - false, &fake_clock_, &transport_, nullptr, absl::nullopt, nullptr, - nullptr, &callback, nullptr, nullptr, nullptr, - &retransmission_rate_limiter_, nullptr, false, nullptr, false, false)); + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, + nullptr, nullptr, &callback, nullptr, nullptr, nullptr, + &retransmission_rate_limiter_, nullptr, false, nullptr, + false, false, FieldTrialBasedConfig())); rtp_sender_->SetSSRC(kSsrc); RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - nullptr, false); + nullptr, false, FieldTrialBasedConfig()); const char payload_name[] = "GENERIC"; const uint8_t payload_type = 127; rtp_sender_video.RegisterPayloadType(payload_type, payload_name); @@ -1561,7 +1575,7 @@ TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { const char payload_name[] = "GENERIC"; const uint8_t payload_type = 127; RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr, - nullptr, false); + nullptr, false, FieldTrialBasedConfig()); rtp_sender_video.RegisterPayloadType(payload_type, payload_name); uint8_t payload[] = {47, 11, 32, 93, 89}; rtp_sender_->SetStorePacketsStatus(true, 1); @@ -1703,7 +1717,7 @@ TEST_P(RtpSenderTest, OnOverheadChanged) { new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_, &mock_overhead_observer, - false, nullptr, false, false)); + false, nullptr, false, false, FieldTrialBasedConfig())); rtp_sender_->SetSSRC(kSsrc); // RTP overhead is 12B. @@ -1725,7 +1739,7 @@ TEST_P(RtpSenderTest, DoesNotUpdateOverheadOnEqualSize) { new RTPSender(false, &fake_clock_, &transport_, nullptr, absl::nullopt, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_, &mock_overhead_observer, - false, nullptr, false, false)); + false, nullptr, false, false, FieldTrialBasedConfig())); rtp_sender_->SetSSRC(kSsrc); EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(_)).Times(1); @@ -1735,10 +1749,11 @@ TEST_P(RtpSenderTest, DoesNotUpdateOverheadOnEqualSize) { TEST_P(RtpSenderTest, SendsKeepAlive) { MockTransport transport; - rtp_sender_.reset(new RTPSender( - false, &fake_clock_, &transport, nullptr, absl::nullopt, nullptr, nullptr, - nullptr, nullptr, &mock_rtc_event_log_, nullptr, - &retransmission_rate_limiter_, nullptr, false, nullptr, false, false)); + rtp_sender_.reset( + new RTPSender(false, &fake_clock_, &transport, nullptr, absl::nullopt, + nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_, + nullptr, &retransmission_rate_limiter_, nullptr, false, + nullptr, false, false, FieldTrialBasedConfig())); rtp_sender_->SetSequenceNumber(kSeqNum); rtp_sender_->SetTimestampOffset(0); rtp_sender_->SetSSRC(kSsrc); diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index cc1c58b8ad..32a3621248 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -33,7 +33,6 @@ #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "rtc_base/trace_event.h" -#include "system_wrappers/include/field_trial.h" namespace webrtc { @@ -188,7 +187,8 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock, RTPSender* rtp_sender, FlexfecSender* flexfec_sender, FrameEncryptorInterface* frame_encryptor, - bool require_frame_encryption) + bool require_frame_encryption, + const WebRtcKeyValueConfig& field_trials) : rtp_sender_(rtp_sender), clock_(clock), retransmission_settings_(kRetransmitBaseLayer | @@ -206,7 +206,8 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock, frame_encryptor_(frame_encryptor), require_frame_encryption_(require_frame_encryption), generic_descriptor_auth_experiment_( - field_trial::IsEnabled("WebRTC-GenericDescriptorAuth")) {} + field_trials.Lookup("WebRTC-GenericDescriptorAuth").find("Enabled") == + 0) {} RTPSenderVideo::~RTPSenderVideo() {} diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index d29934f92d..b3c7f305c7 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h @@ -54,7 +54,8 @@ class RTPSenderVideo { RTPSender* rtpSender, FlexfecSender* flexfec_sender, FrameEncryptorInterface* frame_encryptor, - bool require_frame_encryption); + bool require_frame_encryption, + const WebRtcKeyValueConfig& field_trials); virtual ~RTPSenderVideo(); bool SendVideo(FrameType frame_type, diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index 6dd5353c3d..47fabd1272 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc @@ -8,6 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include #include #include "api/video/video_codec_constants.h" @@ -24,7 +25,6 @@ #include "modules/rtp_rtcp/source/rtp_sender_video.h" #include "rtc_base/arraysize.h" #include "rtc_base/rate_limiter.h" -#include "test/field_trial.h" #include "test/gmock.h" #include "test/gtest.h" @@ -96,8 +96,14 @@ class TestRtpSenderVideo : public RTPSenderVideo { public: TestRtpSenderVideo(Clock* clock, RTPSender* rtp_sender, - FlexfecSender* flexfec_sender) - : RTPSenderVideo(clock, rtp_sender, flexfec_sender, nullptr, false) {} + FlexfecSender* flexfec_sender, + const WebRtcKeyValueConfig& field_trials) + : RTPSenderVideo(clock, + rtp_sender, + flexfec_sender, + nullptr, + false, + field_trials) {} ~TestRtpSenderVideo() override {} StorageType GetStorageType(const RTPVideoHeader& header, @@ -109,11 +115,26 @@ class TestRtpSenderVideo : public RTPSenderVideo { } }; +class FieldTrials : public WebRtcKeyValueConfig { + public: + explicit FieldTrials(bool use_send_side_bwe_with_overhead) + : use_send_side_bwe_with_overhead_(use_send_side_bwe_with_overhead) {} + + std::string Lookup(absl::string_view key) const override { + return key == "WebRTC-SendSideBwe-WithOverhead" && + use_send_side_bwe_with_overhead_ + ? "Enabled" + : ""; + } + + private: + bool use_send_side_bwe_with_overhead_; +}; + class RtpSenderVideoTest : public ::testing::TestWithParam { public: RtpSenderVideoTest() - : field_trials_(GetParam() ? "WebRTC-SendSideBwe-WithOverhead/Enabled/" - : ""), + : field_trials_(GetParam()), fake_clock_(kStartTime), retransmission_rate_limiter_(&fake_clock_, 1000), // TODO(pbos): Set up to use pacer. @@ -133,8 +154,9 @@ class RtpSenderVideoTest : public ::testing::TestWithParam { false, nullptr, false, - false), - rtp_sender_video_(&fake_clock_, &rtp_sender_, nullptr) { + false, + field_trials_), + rtp_sender_video_(&fake_clock_, &rtp_sender_, nullptr, field_trials_) { rtp_sender_.SetSequenceNumber(kSeqNum); rtp_sender_.SetTimestampOffset(0); rtp_sender_.SetSSRC(kSsrc); @@ -148,7 +170,7 @@ class RtpSenderVideoTest : public ::testing::TestWithParam { int version); protected: - test::ScopedFieldTrials field_trials_; + FieldTrials field_trials_; SimulatedClock fake_clock_; LoopbackTransportTest transport_; RateLimiter retransmission_rate_limiter_;