diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 8bc0f4b540..7d6ec794d4 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -152,8 +152,6 @@ AudioSendStream::AudioSendStream( field_trials_.IsEnabled("WebRTC-Audio-ABWENoTWCC")), enable_audio_alr_probing_( !field_trials_.IsDisabled("WebRTC-Audio-AlrProbing")), - send_side_bwe_with_overhead_( - !field_trials_.IsDisabled("WebRTC-SendSideBwe-WithOverhead")), allocation_settings_(field_trials_), config_(Config(/*send_transport=*/nullptr)), audio_state_(audio_state), @@ -372,8 +370,7 @@ void AudioSendStream::Start() { config_.max_bitrate_bps != -1 && (allocate_audio_without_feedback_ || TransportSeqNumId(config_) != 0)) { rtp_transport_->AccountForAudioPacketsInPacedSender(true); - if (send_side_bwe_with_overhead_) - rtp_transport_->IncludeOverheadInPacedSender(); + rtp_transport_->IncludeOverheadInPacedSender(); rtp_rtcp_module_->SetAsPartOfAllocation(true); ConfigureBitrateObserver(); } else { @@ -811,8 +808,7 @@ void AudioSendStream::ReconfigureBitrateObserver( if (!new_config.has_dscp && new_config.min_bitrate_bps != -1 && new_config.max_bitrate_bps != -1 && TransportSeqNumId(new_config) != 0) { rtp_transport_->AccountForAudioPacketsInPacedSender(true); - if (send_side_bwe_with_overhead_) - rtp_transport_->IncludeOverheadInPacedSender(); + rtp_transport_->IncludeOverheadInPacedSender(); // We may get a callback immediately as the observer is registered, so // make sure the bitrate limits in config_ are up-to-date. config_.min_bitrate_bps = new_config.min_bitrate_bps; @@ -835,22 +831,21 @@ void AudioSendStream::ConfigureBitrateObserver() { RTC_DCHECK(constraints.has_value()); DataRate priority_bitrate = allocation_settings_.priority_bitrate; - if (send_side_bwe_with_overhead_) { - if (use_legacy_overhead_calculation_) { - // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12) - constexpr int kOverheadPerPacket = 20 + 8 + 10 + 12; - const TimeDelta kMinPacketDuration = TimeDelta::Millis(20); - DataRate max_overhead = - DataSize::Bytes(kOverheadPerPacket) / kMinPacketDuration; - priority_bitrate += max_overhead; - } else { - RTC_DCHECK(frame_length_range_); - const DataSize overhead_per_packet = - DataSize::Bytes(total_packet_overhead_bytes_); - DataRate min_overhead = overhead_per_packet / frame_length_range_->second; - priority_bitrate += min_overhead; - } + if (use_legacy_overhead_calculation_) { + // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12) + constexpr int kOverheadPerPacket = 20 + 8 + 10 + 12; + const TimeDelta kMinPacketDuration = TimeDelta::Millis(20); + DataRate max_overhead = + DataSize::Bytes(kOverheadPerPacket) / kMinPacketDuration; + priority_bitrate += max_overhead; + } else { + RTC_DCHECK(frame_length_range_); + const DataSize overhead_per_packet = + DataSize::Bytes(total_packet_overhead_bytes_); + DataRate min_overhead = overhead_per_packet / frame_length_range_->second; + priority_bitrate += min_overhead; } + if (allocation_settings_.priority_bitrate_raw) priority_bitrate = *allocation_settings_.priority_bitrate_raw; @@ -903,25 +898,23 @@ AudioSendStream::GetMinMaxBitrateConstraints() const { << "TargetAudioBitrateConstraints::min"; return absl::nullopt; } - if (send_side_bwe_with_overhead_) { - if (use_legacy_overhead_calculation_) { - // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12) - const DataSize kOverheadPerPacket = DataSize::Bytes(20 + 8 + 10 + 12); - const TimeDelta kMaxFrameLength = - TimeDelta::Millis(60); // Based on Opus spec - const DataRate kMinOverhead = kOverheadPerPacket / kMaxFrameLength; - constraints.min += kMinOverhead; - constraints.max += kMinOverhead; - } else { - if (!frame_length_range_.has_value()) { - RTC_LOG(LS_WARNING) << "frame_length_range_ is not set"; - return absl::nullopt; - } - const DataSize kOverheadPerPacket = - DataSize::Bytes(total_packet_overhead_bytes_); - constraints.min += kOverheadPerPacket / frame_length_range_->second; - constraints.max += kOverheadPerPacket / frame_length_range_->first; + if (use_legacy_overhead_calculation_) { + // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12) + const DataSize kOverheadPerPacket = DataSize::Bytes(20 + 8 + 10 + 12); + const TimeDelta kMaxFrameLength = + TimeDelta::Millis(60); // Based on Opus spec + const DataRate kMinOverhead = kOverheadPerPacket / kMaxFrameLength; + constraints.min += kMinOverhead; + constraints.max += kMinOverhead; + } else { + if (!frame_length_range_.has_value()) { + RTC_LOG(LS_WARNING) << "frame_length_range_ is not set"; + return absl::nullopt; } + const DataSize kOverheadPerPacket = + DataSize::Bytes(total_packet_overhead_bytes_); + constraints.min += kOverheadPerPacket / frame_length_range_->second; + constraints.max += kOverheadPerPacket / frame_length_range_->first; } return constraints; } diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h index cdb7472b37..42be43afb9 100644 --- a/audio/audio_send_stream.h +++ b/audio/audio_send_stream.h @@ -178,7 +178,6 @@ class AudioSendStream final : public webrtc::AudioSendStream, const bool allocate_audio_without_feedback_; const bool force_no_audio_feedback_ = allocate_audio_without_feedback_; const bool enable_audio_alr_probing_; - const bool send_side_bwe_with_overhead_; const AudioAllocationConfig allocation_settings_; webrtc::AudioSendStream::Config config_ diff --git a/call/call_perf_tests.cc b/call/call_perf_tests.cc index ac139af5e2..d59b70418f 100644 --- a/call/call_perf_tests.cc +++ b/call/call_perf_tests.cc @@ -778,13 +778,9 @@ TEST_F(CallPerfTest, Bitrate_Kbps_NoPadWithoutMinTransmitBitrate) { #endif TEST_F(CallPerfTest, MAYBE_KeepsHighBitrateWhenReconfiguringSender) { static const uint32_t kInitialBitrateKbps = 400; + static const uint32_t kInitialBitrateOverheadKpbs = 6; static const uint32_t kReconfigureThresholdKbps = 600; - // We get lower bitrate than expected by this test if the following field - // trial is enabled. - test::ScopedKeyValueConfig field_trials( - field_trials_, "WebRTC-SendSideBwe-WithOverhead/Disabled/"); - class VideoStreamFactory : public VideoEncoderConfig::VideoStreamFactoryInterface { public: @@ -824,9 +820,10 @@ TEST_F(CallPerfTest, MAYBE_KeepsHighBitrateWhenReconfiguringSender) { // First time initialization. Frame size is known. // `expected_bitrate` is affected by bandwidth estimation before the // first frame arrives to the encoder. - uint32_t expected_bitrate = last_set_bitrate_kbps_ > 0 - ? last_set_bitrate_kbps_ - : kInitialBitrateKbps; + uint32_t expected_bitrate = + last_set_bitrate_kbps_ > 0 + ? last_set_bitrate_kbps_ + : kInitialBitrateKbps - kInitialBitrateOverheadKpbs; EXPECT_EQ(expected_bitrate, config->startBitrate) << "Encoder not initialized at expected bitrate."; EXPECT_EQ(kDefaultWidth, config->width); diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 18c75df4e5..940dff7894 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -65,10 +65,6 @@ bool IsEnabled(const FieldTrialsView& trials, absl::string_view key) { return absl::StartsWith(trials.Lookup(key), "Enabled"); } -bool IsDisabled(const FieldTrialsView& trials, absl::string_view key) { - return absl::StartsWith(trials.Lookup(key), "Disabled"); -} - bool IsRelayed(const rtc::NetworkRoute& route) { return route.local.uses_turn() || route.remote.uses_turn(); } @@ -107,8 +103,6 @@ RtpTransportControllerSend::RtpTransportControllerSend( last_report_block_time_(Timestamp::Millis(clock_->TimeInMilliseconds())), reset_feedback_on_route_change_( !IsEnabled(*config.trials, "WebRTC-Bwe-NoFeedbackReset")), - send_side_bwe_with_overhead_( - !IsDisabled(*config.trials, "WebRTC-SendSideBwe-WithOverhead")), add_pacing_to_cwin_( IsEnabled(*config.trials, "WebRTC-AddPacingToCongestionWindowPushback")), @@ -554,9 +548,7 @@ void RtpTransportControllerSend::OnAddPacket( RTC_DCHECK_RUN_ON(&task_queue_); feedback_demuxer_.AddPacket(packet_info); transport_feedback_adapter_.AddPacket( - packet_info, - send_side_bwe_with_overhead_ ? transport_overhead_bytes_per_packet_ : 0, - creation_time); + packet_info, transport_overhead_bytes_per_packet_, creation_time); }); } diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index 2ba0fda9ba..51bda73445 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -189,7 +189,6 @@ class RtpTransportControllerSend final StreamsConfig streams_config_ RTC_GUARDED_BY(task_queue_); const bool reset_feedback_on_route_change_; - const bool send_side_bwe_with_overhead_; const bool add_pacing_to_cwin_; FieldTrialParameter relay_bandwidth_cap_; diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 5d2d1f1288..02155bc14a 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -375,9 +375,6 @@ RtpVideoSender::RtpVideoSender( const FieldTrialsView& field_trials, TaskQueueFactory* task_queue_factory) : field_trials_(field_trials), - send_side_bwe_with_overhead_(!absl::StartsWith( - field_trials_.Lookup("WebRTC-SendSideBwe-WithOverhead"), - "Disabled")), use_frame_rate_for_overhead_(absl::StartsWith( field_trials_.Lookup("WebRTC-Video-UseFrameRateForOverhead"), "Enabled")), @@ -409,7 +406,7 @@ RtpVideoSender::RtpVideoSender( frame_count_observer_(observers.frame_count_observer) { transport_checker_.Detach(); RTC_DCHECK_EQ(rtp_config_.ssrcs.size(), rtp_streams_.size()); - if (send_side_bwe_with_overhead_ && has_packet_feedback_) + if (has_packet_feedback_) transport_->IncludeOverheadInPacedSender(); // SSRCs are assumed to be sorted in the same order as `rtp_modules`. for (uint32_t ssrc : rtp_config_.ssrcs) { @@ -835,7 +832,7 @@ void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update, DataSize max_total_packet_size = DataSize::Bytes( rtp_config_.max_packet_size + transport_overhead_bytes_per_packet_); uint32_t payload_bitrate_bps = update.target_bitrate.bps(); - if (send_side_bwe_with_overhead_ && has_packet_feedback_) { + if (has_packet_feedback_) { DataRate overhead_rate = CalculateOverheadRate(update.target_bitrate, max_total_packet_size, packet_overhead, Frequency::Hertz(framerate)); @@ -869,7 +866,7 @@ void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update, loss_mask_vector_.clear(); uint32_t encoder_overhead_rate_bps = 0; - if (send_side_bwe_with_overhead_ && has_packet_feedback_) { + if (has_packet_feedback_) { // TODO(srte): The packet size should probably be the same as in the // CalculateOverheadRate call above (just max_total_packet_size), it doesn't // make sense to use different packet rates for different overhead @@ -882,12 +879,11 @@ void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update, encoder_overhead_rate.bps(), update.target_bitrate.bps() - encoder_target_rate_bps_); } - // When the field trial "WebRTC-SendSideBwe-WithOverhead" is enabled - // protection_bitrate includes overhead. const uint32_t media_rate = encoder_target_rate_bps_ + encoder_overhead_rate_bps + packetization_rate_bps; RTC_DCHECK_GE(update.target_bitrate, DataRate::BitsPerSec(media_rate)); + // `protection_bitrate_bps_` includes overhead. protection_bitrate_bps_ = update.target_bitrate.bps() - media_rate; } diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index 9804bd8630..88e55f8b13 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -170,7 +170,6 @@ class RtpVideoSender : public RtpVideoSenderInterface, Frequency framerate) const; const FieldTrialsView& field_trials_; - const bool send_side_bwe_with_overhead_; const bool use_frame_rate_for_overhead_; const bool has_packet_feedback_; diff --git a/modules/audio_coding/audio_network_adaptor/bitrate_controller_unittest.cc b/modules/audio_coding/audio_network_adaptor/bitrate_controller_unittest.cc index 3155f198a4..9c593b818b 100644 --- a/modules/audio_coding/audio_network_adaptor/bitrate_controller_unittest.cc +++ b/modules/audio_coding/audio_network_adaptor/bitrate_controller_unittest.cc @@ -74,8 +74,6 @@ TEST(AnaBitrateControllerTest, OutputInitValueWhenOverheadUnknown) { } TEST(AnaBitrateControllerTest, ChangeBitrateOnTargetBitrateChanged) { - test::ScopedFieldTrials override_field_trials( - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); constexpr int kInitialFrameLengthMs = 20; BitrateController controller( BitrateController::Config(32000, kInitialFrameLengthMs, 0, 0)); @@ -98,8 +96,6 @@ TEST(AnaBitrateControllerTest, UpdateMultipleNetworkMetricsAtOnce) { // BitrateController::UpdateNetworkMetrics(...) can handle multiple // network updates at once. This is, however, not a common use case in current // audio_network_adaptor_impl.cc. - test::ScopedFieldTrials override_field_trials( - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); constexpr int kInitialFrameLengthMs = 20; BitrateController controller( BitrateController::Config(32000, kInitialFrameLengthMs, 0, 0)); @@ -116,8 +112,6 @@ TEST(AnaBitrateControllerTest, UpdateMultipleNetworkMetricsAtOnce) { } TEST(AnaBitrateControllerTest, TreatUnknownFrameLengthAsFrameLengthUnchanged) { - test::ScopedFieldTrials override_field_trials( - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); constexpr int kInitialFrameLengthMs = 20; BitrateController controller( BitrateController::Config(32000, kInitialFrameLengthMs, 0, 0)); @@ -131,8 +125,6 @@ TEST(AnaBitrateControllerTest, TreatUnknownFrameLengthAsFrameLengthUnchanged) { } TEST(AnaBitrateControllerTest, IncreaseBitrateOnFrameLengthIncreased) { - test::ScopedFieldTrials override_field_trials( - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); constexpr int kInitialFrameLengthMs = 20; BitrateController controller( BitrateController::Config(32000, kInitialFrameLengthMs, 0, 0)); @@ -155,8 +147,6 @@ TEST(AnaBitrateControllerTest, IncreaseBitrateOnFrameLengthIncreased) { } TEST(AnaBitrateControllerTest, DecreaseBitrateOnFrameLengthDecreased) { - test::ScopedFieldTrials override_field_trials( - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); constexpr int kInitialFrameLengthMs = 60; BitrateController controller( BitrateController::Config(32000, kInitialFrameLengthMs, 0, 0)); @@ -179,8 +169,6 @@ TEST(AnaBitrateControllerTest, DecreaseBitrateOnFrameLengthDecreased) { } TEST(AnaBitrateControllerTest, BitrateNeverBecomesNegative) { - test::ScopedFieldTrials override_field_trials( - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); BitrateController controller(BitrateController::Config(32000, 20, 0, 0)); constexpr size_t kOverheadBytesPerPacket = 64; constexpr int kFrameLengthMs = 60; @@ -192,8 +180,6 @@ TEST(AnaBitrateControllerTest, BitrateNeverBecomesNegative) { } TEST(AnaBitrateControllerTest, CheckBehaviorOnChangingCondition) { - test::ScopedFieldTrials override_field_trials( - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); BitrateController controller(BitrateController::Config(32000, 20, 0, 0)); // Start from an arbitrary overall bitrate. diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc index 19afca1bdb..51b0fcd492 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc @@ -362,8 +362,6 @@ AudioEncoderOpusImpl::AudioEncoderOpusImpl( const AudioNetworkAdaptorCreator& audio_network_adaptor_creator, std::unique_ptr bitrate_smoother) : payload_type_(payload_type), - send_side_bwe_with_overhead_( - !webrtc::field_trial::IsDisabled("WebRTC-SendSideBwe-WithOverhead")), use_stable_target_for_adaptation_(!webrtc::field_trial::IsDisabled( "WebRTC-Audio-StableTargetAdaptation")), adjust_bandwidth_( @@ -521,7 +519,7 @@ void AudioEncoderOpusImpl::OnReceivedUplinkBandwidth( } ApplyAudioNetworkAdaptor(); - } else if (send_side_bwe_with_overhead_) { + } else { if (!overhead_bytes_per_packet_) { RTC_LOG(LS_INFO) << "AudioEncoderOpusImpl: Overhead unknown, target audio bitrate " @@ -534,8 +532,6 @@ void AudioEncoderOpusImpl::OnReceivedUplinkBandwidth( std::min(AudioEncoderOpusConfig::kMaxBitrateBps, std::max(AudioEncoderOpusConfig::kMinBitrateBps, target_audio_bitrate_bps - overhead_bps))); - } else { - SetTargetBitrate(target_audio_bitrate_bps); } } void AudioEncoderOpusImpl::OnReceivedUplinkBandwidth( diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.h b/modules/audio_coding/codecs/opus/audio_encoder_opus.h index a0c42af121..8c5c235016 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus.h +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.h @@ -154,7 +154,6 @@ class AudioEncoderOpusImpl final : public AudioEncoder { AudioEncoderOpusConfig config_; const int payload_type_; - const bool send_side_bwe_with_overhead_; const bool use_stable_target_for_adaptation_; const bool adjust_bandwidth_; bool bitrate_changed_; diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc index 673aad734b..a2ebe43bbe 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc @@ -373,9 +373,6 @@ TEST_P(AudioEncoderOpusTest, PacketLossRateUpperBounded) { } TEST_P(AudioEncoderOpusTest, DoNotInvokeSetTargetBitrateIfOverheadUnknown) { - test::ScopedFieldTrials override_field_trials( - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); - auto states = CreateCodec(sample_rate_hz_, 2); states->encoder->OnReceivedUplinkBandwidth(kDefaultOpusRate * 2, diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc index 44054f10db..d4cb827098 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc @@ -140,7 +140,6 @@ absl::optional PacketTransmissionAndFeedbackBlock( // Scenarios: void UpdatesTargetRateBasedOnLinkCapacity(absl::string_view test_name = "") { - ScopedFieldTrials trial("WebRTC-SendSideBwe-WithOverhead/Enabled/"); auto factory = CreateFeedbackOnlyFactory(); Scenario s("googcc_unit/target_capacity" + std::string(test_name), false); CallClientConfig config; @@ -769,9 +768,7 @@ TEST(GoogCcScenario, CutsHighRateInSafeResetTrial) { } TEST(GoogCcScenario, DetectsHighRateInSafeResetTrial) { - ScopedFieldTrials trial( - "WebRTC-Bwe-SafeResetOnRouteChange/Enabled,ack/" - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); + ScopedFieldTrials trial("WebRTC-Bwe-SafeResetOnRouteChange/Enabled,ack/"); const DataRate kInitialLinkCapacity = DataRate::KilobitsPerSec(200); const DataRate kNewLinkCapacity = DataRate::KilobitsPerSec(800); const DataRate kStartRate = DataRate::KilobitsPerSec(300); diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index 843d2f2de8..abcdb619f4 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -665,6 +665,7 @@ if (rtc_include_tests) { "../../rtc_base:threading", "../../rtc_base:timeutils", "../../system_wrappers", + "../../test:explicit_key_value_config", "../../test:field_trial", "../../test:mock_frame_transformer", "../../test:mock_transport", diff --git a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc index 3687669b2f..2e7e219f94 100644 --- a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc @@ -28,11 +28,6 @@ constexpr int kSendSideDelayWindowMs = 1000; constexpr int kBitrateStatisticsWindowMs = 1000; constexpr size_t kRtpSequenceNumberMapMaxEntries = 1 << 13; -bool IsDisabled(absl::string_view name, const FieldTrialsView* field_trials) { - FieldTrialBasedConfig default_trials; - auto& trials = field_trials ? *field_trials : default_trials; - return absl::StartsWith(trials.Lookup(name), "Disabled"); -} } // namespace DEPRECATED_RtpSenderEgress::NonPacedPacketSender::NonPacedPacketSender( @@ -72,8 +67,6 @@ DEPRECATED_RtpSenderEgress::DEPRECATED_RtpSenderEgress( flexfec_ssrc_(config.fec_generator ? config.fec_generator->FecSsrc() : absl::nullopt), populate_network2_timestamp_(config.populate_network2_timestamp), - send_side_bwe_with_overhead_( - !IsDisabled("WebRTC-SendSideBwe-WithOverhead", config.field_trials)), clock_(config.clock), packet_history_(packet_history), transport_(config.outgoing_transport), @@ -316,16 +309,11 @@ void DEPRECATED_RtpSenderEgress::AddPacketToTransportFeedback( const RtpPacketToSend& packet, const PacedPacketInfo& pacing_info) { if (transport_feedback_observer_) { - size_t packet_size = packet.payload_size() + packet.padding_size(); - if (send_side_bwe_with_overhead_) { - packet_size = packet.size(); - } - RtpPacketSendInfo packet_info; packet_info.media_ssrc = ssrc_; packet_info.transport_sequence_number = packet_id; packet_info.rtp_sequence_number = packet.SequenceNumber(); - packet_info.length = packet_size; + packet_info.length = packet.size(); packet_info.pacing_info = pacing_info; packet_info.packet_type = packet.packet_type(); transport_feedback_observer_->OnAddPacket(packet_info); diff --git a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.h b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.h index fd5dfddc02..da833f5355 100644 --- a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.h +++ b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.h @@ -110,7 +110,6 @@ class DEPRECATED_RtpSenderEgress { const absl::optional rtx_ssrc_; const absl::optional flexfec_ssrc_; const bool populate_network2_timestamp_; - const bool send_side_bwe_with_overhead_; Clock* const clock_; RtpPacketHistory* const packet_history_; Transport* const transport_; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc index f5c6311ded..918e075be8 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc @@ -28,6 +28,7 @@ #include "rtc_base/logging.h" #include "rtc_base/rate_limiter.h" #include "rtc_base/strings/string_builder.h" +#include "test/explicit_key_value_config.h" #include "test/gmock.h" #include "test/gtest.h" #include "test/rtcp_packet_parser.h" @@ -43,6 +44,8 @@ using ::testing::Not; using ::testing::Optional; using ::testing::SizeIs; +using webrtc::test::ExplicitKeyValueConfig; + namespace webrtc { namespace { constexpr uint32_t kSenderSsrc = 0x12345; @@ -151,36 +154,6 @@ class SendTransport : public Transport, std::deque rtcp_packets_; }; -struct TestConfig { - explicit TestConfig(bool with_overhead) : with_overhead(with_overhead) {} - - bool with_overhead = false; -}; - -class FieldTrialConfig : public FieldTrialsRegistry { - public: - static FieldTrialConfig GetFromTestConfig(const TestConfig& config) { - FieldTrialConfig trials; - trials.overhead_enabled_ = config.with_overhead; - return trials; - } - - FieldTrialConfig() : overhead_enabled_(false) {} - ~FieldTrialConfig() override {} - - void SetOverHeadEnabled(bool enabled) { overhead_enabled_ = enabled; } - - private: - std::string GetValue(absl::string_view key) const override { - if (key == "WebRTC-SendSideBwe-WithOverhead") { - return overhead_enabled_ ? "Enabled" : "Disabled"; - } - return ""; - } - - bool overhead_enabled_; -}; - class RtpRtcpModule : public RtcpPacketTypeCounterObserver, public SendPacketObserver { public: @@ -194,7 +167,7 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver, RtpRtcpModule(GlobalSimulatedTimeController* time_controller, bool is_sender, - const FieldTrialConfig& trials) + const FieldTrialsRegistry& trials) : time_controller_(time_controller), is_sender_(is_sender), trials_(trials), @@ -206,7 +179,7 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver, TimeController* const time_controller_; const bool is_sender_; - const FieldTrialConfig& trials_; + const FieldTrialsRegistry& trials_; RtcpPacketTypeCounter packets_sent_; RtcpPacketTypeCounter packets_received_; std::unique_ptr receive_statistics_; @@ -289,11 +262,11 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver, }; } // namespace -class RtpRtcpImpl2Test : public ::testing::TestWithParam { +class RtpRtcpImpl2Test : public ::testing::Test { protected: RtpRtcpImpl2Test() : time_controller_(Timestamp::Micros(133590000000000)), - field_trials_(FieldTrialConfig::GetFromTestConfig(GetParam())), + field_trials_(""), sender_(&time_controller_, /*is_sender=*/true, field_trials_), @@ -346,7 +319,7 @@ class RtpRtcpImpl2Test : public ::testing::TestWithParam { } GlobalSimulatedTimeController time_controller_; - FieldTrialConfig field_trials_; + test::ExplicitKeyValueConfig field_trials_; RtpRtcpModule sender_; std::unique_ptr sender_video_; RtpRtcpModule receiver_; @@ -403,7 +376,7 @@ class RtpRtcpImpl2Test : public ::testing::TestWithParam { } }; -TEST_P(RtpRtcpImpl2Test, RetransmitsAllLayers) { +TEST_F(RtpRtcpImpl2Test, RetransmitsAllLayers) { // Send frames. EXPECT_EQ(0, sender_.RtpSent()); EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), @@ -432,7 +405,7 @@ TEST_P(RtpRtcpImpl2Test, RetransmitsAllLayers) { EXPECT_EQ(kSequenceNumber + 2, sender_.LastRtpSequenceNumber()); } -TEST_P(RtpRtcpImpl2Test, Rtt) { +TEST_F(RtpRtcpImpl2Test, Rtt) { RtpPacketReceived packet; packet.SetTimestamp(1); packet.SetSequenceNumber(123); @@ -476,7 +449,7 @@ TEST_P(RtpRtcpImpl2Test, Rtt) { EXPECT_NEAR(2 * kOneWayNetworkDelay.ms(), sender_.impl_->rtt_ms(), 1); } -TEST_P(RtpRtcpImpl2Test, RttForReceiverOnly) { +TEST_F(RtpRtcpImpl2Test, RttForReceiverOnly) { // Receiver module should send a Receiver time reference report (RTRR). EXPECT_EQ(0, receiver_.impl_->SendRTCP(kRtcpReport)); @@ -495,7 +468,7 @@ TEST_P(RtpRtcpImpl2Test, RttForReceiverOnly) { EXPECT_NEAR(2 * kOneWayNetworkDelay.ms(), receiver_.impl_->rtt_ms(), 1); } -TEST_P(RtpRtcpImpl2Test, NoSrBeforeMedia) { +TEST_F(RtpRtcpImpl2Test, NoSrBeforeMedia) { // Ignore fake transport delays in this test. sender_.transport_.SimulateNetworkDelay(TimeDelta::Zero()); receiver_.transport_.SimulateNetworkDelay(TimeDelta::Zero()); @@ -512,7 +485,7 @@ TEST_P(RtpRtcpImpl2Test, NoSrBeforeMedia) { EXPECT_EQ(sender_.transport_.NumRtcpSent(), 1u); } -TEST_P(RtpRtcpImpl2Test, RtcpPacketTypeCounter_Nack) { +TEST_F(RtpRtcpImpl2Test, RtcpPacketTypeCounter_Nack) { EXPECT_EQ(0U, sender_.RtcpReceived().nack_packets); EXPECT_EQ(0U, receiver_.RtcpSent().nack_packets); @@ -527,7 +500,7 @@ TEST_P(RtpRtcpImpl2Test, RtcpPacketTypeCounter_Nack) { EXPECT_EQ(1U, sender_.RtcpReceived().nack_packets); } -TEST_P(RtpRtcpImpl2Test, AddStreamDataCounters) { +TEST_F(RtpRtcpImpl2Test, AddStreamDataCounters) { StreamDataCounters rtp; const int64_t kStartTimeMs = 1; rtp.first_packet_time_ms = kStartTimeMs; @@ -570,7 +543,7 @@ TEST_P(RtpRtcpImpl2Test, AddStreamDataCounters) { EXPECT_EQ(kStartTimeMs, sum.first_packet_time_ms); // Holds oldest time. } -TEST_P(RtpRtcpImpl2Test, SendsInitialNackList) { +TEST_F(RtpRtcpImpl2Test, SendsInitialNackList) { // Send module sends a NACK. const uint16_t kNackLength = 1; uint16_t nack_list[kNackLength] = {123}; @@ -582,7 +555,7 @@ TEST_P(RtpRtcpImpl2Test, SendsInitialNackList) { EXPECT_THAT(sender_.LastNackListSent(), ElementsAre(123)); } -TEST_P(RtpRtcpImpl2Test, SendsExtendedNackList) { +TEST_F(RtpRtcpImpl2Test, SendsExtendedNackList) { // Send module sends a NACK. const uint16_t kNackLength = 1; uint16_t nack_list[kNackLength] = {123}; @@ -606,7 +579,7 @@ TEST_P(RtpRtcpImpl2Test, SendsExtendedNackList) { EXPECT_THAT(sender_.LastNackListSent(), ElementsAre(124)); } -TEST_P(RtpRtcpImpl2Test, ReSendsNackListAfterRttMs) { +TEST_F(RtpRtcpImpl2Test, ReSendsNackListAfterRttMs) { sender_.transport_.SimulateNetworkDelay(TimeDelta::Zero()); // Send module sends a NACK. const uint16_t kNackLength = 2; @@ -631,7 +604,7 @@ TEST_P(RtpRtcpImpl2Test, ReSendsNackListAfterRttMs) { EXPECT_THAT(sender_.LastNackListSent(), ElementsAre(123, 125)); } -TEST_P(RtpRtcpImpl2Test, UniqueNackRequests) { +TEST_F(RtpRtcpImpl2Test, UniqueNackRequests) { receiver_.transport_.SimulateNetworkDelay(TimeDelta::Zero()); EXPECT_EQ(0U, receiver_.RtcpSent().nack_packets); EXPECT_EQ(0U, receiver_.RtcpSent().nack_requests); @@ -671,7 +644,7 @@ TEST_P(RtpRtcpImpl2Test, UniqueNackRequests) { EXPECT_EQ(75, sender_.RtcpReceived().UniqueNackRequestsInPercent()); } -TEST_P(RtpRtcpImpl2Test, ConfigurableRtcpReportInterval) { +TEST_F(RtpRtcpImpl2Test, ConfigurableRtcpReportInterval) { const TimeDelta kVideoReportInterval = TimeDelta::Millis(3000); // Recreate sender impl with new configuration, and redo setup. @@ -709,7 +682,7 @@ TEST_P(RtpRtcpImpl2Test, ConfigurableRtcpReportInterval) { EXPECT_EQ(sender_.transport_.NumRtcpSent(), 2u); } -TEST_P(RtpRtcpImpl2Test, RtpSenderEgressTimestampOffset) { +TEST_F(RtpRtcpImpl2Test, RtpSenderEgressTimestampOffset) { // RTP timestamp offset not explicitly set, default to random value. uint16_t seqno = sender_.impl_->GetRtpState().sequence_number; uint32_t media_rtp_ts = 1001; @@ -739,7 +712,7 @@ TEST_P(RtpRtcpImpl2Test, RtpSenderEgressTimestampOffset) { ElementsAre(Field(&RtpSequenceNumberMap::Info::timestamp, media_rtp_ts))); } -TEST_P(RtpRtcpImpl2Test, StoresPacketInfoForSentPackets) { +TEST_F(RtpRtcpImpl2Test, StoresPacketInfoForSentPackets) { const uint32_t kStartTimestamp = 1u; SetUp(); sender_.impl_->SetStartTimestamp(kStartTimestamp); @@ -798,12 +771,12 @@ TEST_P(RtpRtcpImpl2Test, StoresPacketInfoForSentPackets) { } // Checks that the sender report stats are not available if no RTCP SR was sent. -TEST_P(RtpRtcpImpl2Test, SenderReportStatsNotAvailable) { +TEST_F(RtpRtcpImpl2Test, SenderReportStatsNotAvailable) { EXPECT_THAT(receiver_.impl_->GetSenderReportStats(), Eq(absl::nullopt)); } // Checks that the sender report stats are available if an RTCP SR was sent. -TEST_P(RtpRtcpImpl2Test, SenderReportStatsAvailable) { +TEST_F(RtpRtcpImpl2Test, SenderReportStatsAvailable) { // Send a frame in order to send an SR. EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); // Send an SR. @@ -814,7 +787,7 @@ TEST_P(RtpRtcpImpl2Test, SenderReportStatsAvailable) { // Checks that the sender report stats are not available if an RTCP SR with an // unexpected SSRC is received. -TEST_P(RtpRtcpImpl2Test, SenderReportStatsNotUpdatedWithUnexpectedSsrc) { +TEST_F(RtpRtcpImpl2Test, SenderReportStatsNotUpdatedWithUnexpectedSsrc) { constexpr uint32_t kUnexpectedSenderSsrc = 0x87654321; static_assert(kUnexpectedSenderSsrc != kSenderSsrc, ""); // Forge a sender report and pass it to the receiver as if an RTCP SR were @@ -830,7 +803,7 @@ TEST_P(RtpRtcpImpl2Test, SenderReportStatsNotUpdatedWithUnexpectedSsrc) { } // Checks the stats derived from the last received RTCP SR are set correctly. -TEST_P(RtpRtcpImpl2Test, SenderReportStatsCheckStatsFromLastReport) { +TEST_F(RtpRtcpImpl2Test, SenderReportStatsCheckStatsFromLastReport) { using SenderReportStats = RtpRtcpInterface::SenderReportStats; const NtpTime ntp(/*seconds=*/1u, /*fractions=*/1u << 31); constexpr uint32_t kPacketCount = 123u; @@ -853,7 +826,7 @@ TEST_P(RtpRtcpImpl2Test, SenderReportStatsCheckStatsFromLastReport) { } // Checks that the sender report stats count equals the number of sent RTCP SRs. -TEST_P(RtpRtcpImpl2Test, SenderReportStatsCount) { +TEST_F(RtpRtcpImpl2Test, SenderReportStatsCount) { using SenderReportStats = RtpRtcpInterface::SenderReportStats; // Send a frame in order to send an SR. EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); @@ -871,7 +844,7 @@ TEST_P(RtpRtcpImpl2Test, SenderReportStatsCount) { // Checks that the sender report stats include a valid arrival time if an RTCP // SR was sent. -TEST_P(RtpRtcpImpl2Test, SenderReportStatsArrivalTimestampSet) { +TEST_F(RtpRtcpImpl2Test, SenderReportStatsArrivalTimestampSet) { // Send a frame in order to send an SR. EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); // Send an SR. @@ -884,7 +857,7 @@ TEST_P(RtpRtcpImpl2Test, SenderReportStatsArrivalTimestampSet) { // Checks that the packet and byte counters from an RTCP SR are not zero once // a frame is sent. -TEST_P(RtpRtcpImpl2Test, SenderReportStatsPacketByteCounters) { +TEST_F(RtpRtcpImpl2Test, SenderReportStatsPacketByteCounters) { using SenderReportStats = RtpRtcpInterface::SenderReportStats; // Send a frame in order to send an SR. EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); @@ -900,14 +873,14 @@ TEST_P(RtpRtcpImpl2Test, SenderReportStatsPacketByteCounters) { Field(&SenderReportStats::bytes_sent, Gt(0u))))); } -TEST_P(RtpRtcpImpl2Test, SendingVideoAdvancesSequenceNumber) { +TEST_F(RtpRtcpImpl2Test, SendingVideoAdvancesSequenceNumber) { const uint16_t sequence_number = sender_.impl_->SequenceNumber(); EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); ASSERT_THAT(sender_.transport_.rtp_packets_sent_, Gt(0)); EXPECT_EQ(sequence_number + 1, sender_.impl_->SequenceNumber()); } -TEST_P(RtpRtcpImpl2Test, SequenceNumberNotAdvancedWhenNotSending) { +TEST_F(RtpRtcpImpl2Test, SequenceNumberNotAdvancedWhenNotSending) { const uint16_t sequence_number = sender_.impl_->SequenceNumber(); sender_.impl_->SetSendingMediaStatus(false); EXPECT_FALSE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid)); @@ -915,7 +888,7 @@ TEST_P(RtpRtcpImpl2Test, SequenceNumberNotAdvancedWhenNotSending) { EXPECT_EQ(sequence_number, sender_.impl_->SequenceNumber()); } -TEST_P(RtpRtcpImpl2Test, PaddingNotAllowedInMiddleOfFrame) { +TEST_F(RtpRtcpImpl2Test, PaddingNotAllowedInMiddleOfFrame) { constexpr size_t kPaddingSize = 100; // Can't send padding before media. @@ -950,7 +923,7 @@ TEST_P(RtpRtcpImpl2Test, PaddingNotAllowedInMiddleOfFrame) { EXPECT_THAT(sender_.impl_->GeneratePadding(kPaddingSize), SizeIs(Gt(0u))); } -TEST_P(RtpRtcpImpl2Test, PaddingTimestampMatchesMedia) { +TEST_F(RtpRtcpImpl2Test, PaddingTimestampMatchesMedia) { constexpr size_t kPaddingSize = 100; const uint32_t kTimestamp = 123; @@ -971,7 +944,7 @@ TEST_P(RtpRtcpImpl2Test, PaddingTimestampMatchesMedia) { EXPECT_EQ(sender_.last_packet().Timestamp(), kTimestamp); } -TEST_P(RtpRtcpImpl2Test, AssignsTransportSequenceNumber) { +TEST_F(RtpRtcpImpl2Test, AssignsTransportSequenceNumber) { sender_.RegisterHeaderExtension(TransportSequenceNumber::Uri(), kTransportSequenceNumberExtensionId); @@ -988,7 +961,7 @@ TEST_P(RtpRtcpImpl2Test, AssignsTransportSequenceNumber) { EXPECT_EQ(first_transport_seq + 1, second_transport_seq); } -TEST_P(RtpRtcpImpl2Test, AssignsAbsoluteSendTime) { +TEST_F(RtpRtcpImpl2Test, AssignsAbsoluteSendTime) { sender_.RegisterHeaderExtension(AbsoluteSendTime::Uri(), kAbsoluteSendTimeExtensionId); @@ -996,7 +969,7 @@ TEST_P(RtpRtcpImpl2Test, AssignsAbsoluteSendTime) { EXPECT_NE(sender_.last_packet().GetExtension(), 0u); } -TEST_P(RtpRtcpImpl2Test, AssignsTransmissionTimeOffset) { +TEST_F(RtpRtcpImpl2Test, AssignsTransmissionTimeOffset) { sender_.RegisterHeaderExtension(TransmissionOffset::Uri(), kTransmissionOffsetExtensionId); @@ -1012,7 +985,7 @@ TEST_P(RtpRtcpImpl2Test, AssignsTransmissionTimeOffset) { kOffset.ms() * kCaptureTimeMsToRtpTimestamp); } -TEST_P(RtpRtcpImpl2Test, PropagatesSentPacketInfo) { +TEST_F(RtpRtcpImpl2Test, PropagatesSentPacketInfo) { sender_.RegisterHeaderExtension(TransportSequenceNumber::Uri(), kTransportSequenceNumberExtensionId); int64_t now_ms = time_controller_.GetClock()->TimeInMilliseconds(); @@ -1027,7 +1000,7 @@ TEST_P(RtpRtcpImpl2Test, PropagatesSentPacketInfo) { Field(&RtpRtcpModule::SentPacket::ssrc, Eq(kSenderSsrc))))); } -TEST_P(RtpRtcpImpl2Test, GeneratesFlexfec) { +TEST_F(RtpRtcpImpl2Test, GeneratesFlexfec) { constexpr int kFlexfecPayloadType = 118; constexpr uint32_t kFlexfecSsrc = 17; const char kNoMid[] = ""; @@ -1060,7 +1033,7 @@ TEST_P(RtpRtcpImpl2Test, GeneratesFlexfec) { EXPECT_EQ(fec_packet.PayloadType(), kFlexfecPayloadType); } -TEST_P(RtpRtcpImpl2Test, GeneratesUlpfec) { +TEST_F(RtpRtcpImpl2Test, GeneratesUlpfec) { constexpr int kUlpfecPayloadType = 118; constexpr int kRedPayloadType = 119; UlpfecGenerator ulpfec_sender(kRedPayloadType, kUlpfecPayloadType, @@ -1088,7 +1061,7 @@ TEST_P(RtpRtcpImpl2Test, GeneratesUlpfec) { EXPECT_EQ(fec_packet.payload()[0], kUlpfecPayloadType); } -TEST_P(RtpRtcpImpl2Test, RtpStateReflectsCurrentState) { +TEST_F(RtpRtcpImpl2Test, RtpStateReflectsCurrentState) { // Verify that that each of the field of GetRtpState actually reflects // the current state. @@ -1136,7 +1109,7 @@ TEST_P(RtpRtcpImpl2Test, RtpStateReflectsCurrentState) { EXPECT_EQ(state.ssrc_has_acked, true); } -TEST_P(RtpRtcpImpl2Test, RtxRtpStateReflectsCurrentState) { +TEST_F(RtpRtcpImpl2Test, RtxRtpStateReflectsCurrentState) { // Enable RTX. sender_.impl_->SetStorePacketsStatus(/*enable=*/true, /*number_to_store=*/10); sender_.impl_->SetRtxSendPayloadType(kRtxPayloadType, kPayloadType); @@ -1181,9 +1154,4 @@ TEST_P(RtpRtcpImpl2Test, RtxRtpStateReflectsCurrentState) { EXPECT_EQ(rtx_state.sequence_number, rtx_packet.SequenceNumber() + 1); } -INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, - RtpRtcpImpl2Test, - ::testing::Values(TestConfig{false}, - TestConfig{true})); - } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc index e81ea8da19..c211b5a1ec 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc @@ -28,14 +28,6 @@ constexpr int kBitrateStatisticsWindowMs = 1000; constexpr size_t kRtpSequenceNumberMapMaxEntries = 1 << 13; constexpr TimeDelta kUpdateInterval = TimeDelta::Millis(kBitrateStatisticsWindowMs); - -bool IsTrialSetTo(const FieldTrialsView* field_trials, - absl::string_view name, - absl::string_view value) { - FieldTrialBasedConfig default_trials; - auto& trials = field_trials ? *field_trials : default_trials; - return absl::StartsWith(trials.Lookup(name), value); -} } // namespace RtpSenderEgress::NonPacedPacketSender::NonPacedPacketSender( @@ -81,10 +73,6 @@ RtpSenderEgress::RtpSenderEgress(const RtpRtcpInterface::Configuration& config, flexfec_ssrc_(config.fec_generator ? config.fec_generator->FecSsrc() : absl::nullopt), populate_network2_timestamp_(config.populate_network2_timestamp), - send_side_bwe_with_overhead_( - !IsTrialSetTo(config.field_trials, - "WebRTC-SendSideBwe-WithOverhead", - "Disabled")), clock_(config.clock), packet_history_(packet_history), transport_(config.outgoing_transport), @@ -422,15 +410,10 @@ void RtpSenderEgress::AddPacketToTransportFeedback( const RtpPacketToSend& packet, const PacedPacketInfo& pacing_info) { if (transport_feedback_observer_) { - size_t packet_size = packet.payload_size() + packet.padding_size(); - if (send_side_bwe_with_overhead_) { - packet_size = packet.size(); - } - RtpPacketSendInfo packet_info; packet_info.transport_sequence_number = packet_id; packet_info.rtp_timestamp = packet.Timestamp(); - packet_info.length = packet_size; + packet_info.length = packet.size(); packet_info.pacing_info = pacing_info; packet_info.packet_type = packet.packet_type(); diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.h b/modules/rtp_rtcp/source/rtp_sender_egress.h index c46f6aeb40..ab62edd442 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.h +++ b/modules/rtp_rtcp/source/rtp_sender_egress.h @@ -137,7 +137,6 @@ class RtpSenderEgress { const absl::optional rtx_ssrc_; const absl::optional flexfec_ssrc_; const bool populate_network2_timestamp_; - const bool send_side_bwe_with_overhead_; Clock* const clock_; RtpPacketHistory* const packet_history_; Transport* const transport_; diff --git a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc index 30e0c64a30..cc1c8feb8d 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc @@ -26,6 +26,7 @@ #include "modules/rtp_rtcp/source/rtp_packet_history.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_packet_to_send.h" +#include "test/explicit_key_value_config.h" #include "test/gmock.h" #include "test/gtest.h" #include "test/time_controller/simulated_time_controller.h" @@ -53,11 +54,6 @@ enum : int { kVideoTimingExtensionId, }; -struct TestConfig { - explicit TestConfig(bool with_overhead) : with_overhead(with_overhead) {} - bool with_overhead = false; -}; - class MockSendPacketObserver : public SendPacketObserver { public: MOCK_METHOD(void, OnSendPacket, (uint16_t, int64_t, uint32_t), (override)); @@ -85,24 +81,6 @@ class MockSendSideDelayObserver : public SendSideDelayObserver { MOCK_METHOD(void, SendSideDelayUpdated, (int, int, uint32_t), (override)); }; -class FieldTrialConfig : public FieldTrialsRegistry { - public: - FieldTrialConfig() : overhead_enabled_(false) {} - ~FieldTrialConfig() override {} - - void SetOverHeadEnabled(bool enabled) { overhead_enabled_ = enabled; } - - private: - std::string GetValue(absl::string_view key) const override { - if (key == "WebRTC-SendSideBwe-WithOverhead") { - return overhead_enabled_ ? "Enabled" : "Disabled"; - } - return ""; - } - - bool overhead_enabled_; -}; - struct TransmittedPacket { TransmittedPacket(rtc::ArrayView data, const PacketOptions& packet_options, @@ -139,16 +117,15 @@ class TestTransport : public Transport { } // namespace -class RtpSenderEgressTest : public ::testing::TestWithParam { +class RtpSenderEgressTest : public ::testing::Test { protected: RtpSenderEgressTest() : time_controller_(kStartTime), clock_(time_controller_.GetClock()), transport_(&header_extensions_), packet_history_(clock_, /*enable_rtx_padding_prioritization=*/true), - sequence_number_(kStartSequenceNumber) { - trials_.SetOverHeadEnabled(GetParam().with_overhead); - } + trials_(""), + sequence_number_(kStartSequenceNumber) {} std::unique_ptr CreateRtpSenderEgress() { return std::make_unique(DefaultConfig(), &packet_history_); @@ -200,11 +177,11 @@ class RtpSenderEgressTest : public ::testing::TestWithParam { RtpHeaderExtensionMap header_extensions_; TestTransport transport_; RtpPacketHistory packet_history_; - FieldTrialConfig trials_; + test::ExplicitKeyValueConfig trials_; uint16_t sequence_number_; }; -TEST_P(RtpSenderEgressTest, TransportFeedbackObserverGetsCorrectByteCount) { +TEST_F(RtpSenderEgressTest, TransportFeedbackObserverGetsCorrectByteCount) { constexpr size_t kRtpOverheadBytesPerPacket = 12 + 8; constexpr size_t kPayloadSize = 1400; const uint16_t kTransportSequenceNumber = 17; @@ -212,9 +189,7 @@ TEST_P(RtpSenderEgressTest, TransportFeedbackObserverGetsCorrectByteCount) { header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, TransportSequenceNumber::Uri()); - const size_t expected_bytes = GetParam().with_overhead - ? kPayloadSize + kRtpOverheadBytesPerPacket - : kPayloadSize; + const size_t expected_bytes = kPayloadSize + kRtpOverheadBytesPerPacket; EXPECT_CALL( feedback_observer_, @@ -234,7 +209,7 @@ TEST_P(RtpSenderEgressTest, TransportFeedbackObserverGetsCorrectByteCount) { sender->SendPacket(packet.get(), PacedPacketInfo()); } -TEST_P(RtpSenderEgressTest, PacketOptionsIsRetransmitSetByPacketType) { +TEST_F(RtpSenderEgressTest, PacketOptionsIsRetransmitSetByPacketType) { std::unique_ptr sender = CreateRtpSenderEgress(); std::unique_ptr media_packet = BuildRtpPacket(); @@ -250,7 +225,7 @@ TEST_P(RtpSenderEgressTest, PacketOptionsIsRetransmitSetByPacketType) { EXPECT_TRUE(transport_.last_packet()->options.is_retransmit); } -TEST_P(RtpSenderEgressTest, DoesnSetIncludedInAllocationByDefault) { +TEST_F(RtpSenderEgressTest, DoesnSetIncludedInAllocationByDefault) { std::unique_ptr sender = CreateRtpSenderEgress(); std::unique_ptr packet = BuildRtpPacket(); @@ -259,7 +234,7 @@ TEST_P(RtpSenderEgressTest, DoesnSetIncludedInAllocationByDefault) { EXPECT_FALSE(transport_.last_packet()->options.included_in_allocation); } -TEST_P(RtpSenderEgressTest, +TEST_F(RtpSenderEgressTest, SetsIncludedInFeedbackWhenTransportSequenceNumberExtensionIsRegistered) { std::unique_ptr sender = CreateRtpSenderEgress(); @@ -270,7 +245,7 @@ TEST_P(RtpSenderEgressTest, EXPECT_TRUE(transport_.last_packet()->options.included_in_feedback); } -TEST_P( +TEST_F( RtpSenderEgressTest, SetsIncludedInAllocationWhenTransportSequenceNumberExtensionIsRegistered) { std::unique_ptr sender = CreateRtpSenderEgress(); @@ -282,7 +257,7 @@ TEST_P( EXPECT_TRUE(transport_.last_packet()->options.included_in_allocation); } -TEST_P(RtpSenderEgressTest, +TEST_F(RtpSenderEgressTest, SetsIncludedInAllocationWhenForcedAsPartOfAllocation) { std::unique_ptr sender = CreateRtpSenderEgress(); sender->ForceIncludeSendPacketsInAllocation(true); @@ -293,7 +268,7 @@ TEST_P(RtpSenderEgressTest, EXPECT_TRUE(transport_.last_packet()->options.included_in_allocation); } -TEST_P(RtpSenderEgressTest, OnSendSideDelayUpdated) { +TEST_F(RtpSenderEgressTest, OnSendSideDelayUpdated) { StrictMock send_side_delay_observer; RtpRtcpInterface::Configuration config = DefaultConfig(); config.send_side_delay_observer = &send_side_delay_observer; @@ -335,7 +310,7 @@ TEST_P(RtpSenderEgressTest, OnSendSideDelayUpdated) { PacedPacketInfo()); } -TEST_P(RtpSenderEgressTest, WritesPacerExitToTimingExtension) { +TEST_F(RtpSenderEgressTest, WritesPacerExitToTimingExtension) { std::unique_ptr sender = CreateRtpSenderEgress(); header_extensions_.RegisterByUri(kVideoTimingExtensionId, VideoTimingExtension::Uri()); @@ -355,7 +330,7 @@ TEST_P(RtpSenderEgressTest, WritesPacerExitToTimingExtension) { EXPECT_EQ(video_timing.pacer_exit_delta_ms, kStoredTimeInMs); } -TEST_P(RtpSenderEgressTest, WritesNetwork2ToTimingExtension) { +TEST_F(RtpSenderEgressTest, WritesNetwork2ToTimingExtension) { RtpRtcpInterface::Configuration rtp_config = DefaultConfig(); rtp_config.populate_network2_timestamp = true; auto sender = std::make_unique(rtp_config, &packet_history_); @@ -381,7 +356,7 @@ TEST_P(RtpSenderEgressTest, WritesNetwork2ToTimingExtension) { EXPECT_EQ(video_timing.pacer_exit_delta_ms, kPacerExitMs); } -TEST_P(RtpSenderEgressTest, OnSendPacketUpdated) { +TEST_F(RtpSenderEgressTest, OnSendPacketUpdated) { std::unique_ptr sender = CreateRtpSenderEgress(); header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, TransportSequenceNumber::Uri()); @@ -395,7 +370,7 @@ TEST_P(RtpSenderEgressTest, OnSendPacketUpdated) { sender->SendPacket(packet.get(), PacedPacketInfo()); } -TEST_P(RtpSenderEgressTest, OnSendPacketNotUpdatedForRetransmits) { +TEST_F(RtpSenderEgressTest, OnSendPacketNotUpdatedForRetransmits) { std::unique_ptr sender = CreateRtpSenderEgress(); header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, TransportSequenceNumber::Uri()); @@ -409,7 +384,7 @@ TEST_P(RtpSenderEgressTest, OnSendPacketNotUpdatedForRetransmits) { sender->SendPacket(packet.get(), PacedPacketInfo()); } -TEST_P(RtpSenderEgressTest, ReportsFecRate) { +TEST_F(RtpSenderEgressTest, ReportsFecRate) { constexpr int kNumPackets = 10; constexpr TimeDelta kTimeBetweenPackets = TimeDelta::Millis(33); @@ -437,7 +412,7 @@ TEST_P(RtpSenderEgressTest, ReportsFecRate) { (total_fec_data_sent / (kTimeBetweenPackets * kNumPackets)).bps(), 500); } -TEST_P(RtpSenderEgressTest, BitrateCallbacks) { +TEST_F(RtpSenderEgressTest, BitrateCallbacks) { class MockBitrateStaticsObserver : public BitrateStatisticsObserver { public: MOCK_METHOD(void, Notify, (uint32_t, uint32_t, uint32_t), (override)); @@ -484,7 +459,7 @@ TEST_P(RtpSenderEgressTest, BitrateCallbacks) { } } -TEST_P(RtpSenderEgressTest, DoesNotPutNotRetransmittablePacketsInHistory) { +TEST_F(RtpSenderEgressTest, DoesNotPutNotRetransmittablePacketsInHistory) { std::unique_ptr sender = CreateRtpSenderEgress(); packet_history_.SetStorePacketsStatus( RtpPacketHistory::StorageMode::kStoreAndCull, 10); @@ -495,7 +470,7 @@ TEST_P(RtpSenderEgressTest, DoesNotPutNotRetransmittablePacketsInHistory) { EXPECT_FALSE(packet_history_.GetPacketState(packet->SequenceNumber())); } -TEST_P(RtpSenderEgressTest, PutsRetransmittablePacketsInHistory) { +TEST_F(RtpSenderEgressTest, PutsRetransmittablePacketsInHistory) { std::unique_ptr sender = CreateRtpSenderEgress(); packet_history_.SetStorePacketsStatus( RtpPacketHistory::StorageMode::kStoreAndCull, 10); @@ -506,7 +481,7 @@ TEST_P(RtpSenderEgressTest, PutsRetransmittablePacketsInHistory) { EXPECT_TRUE(packet_history_.GetPacketState(packet->SequenceNumber())); } -TEST_P(RtpSenderEgressTest, DoesNotPutNonMediaInHistory) { +TEST_F(RtpSenderEgressTest, DoesNotPutNonMediaInHistory) { std::unique_ptr sender = CreateRtpSenderEgress(); packet_history_.SetStorePacketsStatus( RtpPacketHistory::StorageMode::kStoreAndCull, 10); @@ -535,7 +510,7 @@ TEST_P(RtpSenderEgressTest, DoesNotPutNonMediaInHistory) { EXPECT_FALSE(packet_history_.GetPacketState(padding->SequenceNumber())); } -TEST_P(RtpSenderEgressTest, UpdatesSendStatusOfRetransmittedPackets) { +TEST_F(RtpSenderEgressTest, UpdatesSendStatusOfRetransmittedPackets) { std::unique_ptr sender = CreateRtpSenderEgress(); packet_history_.SetStorePacketsStatus( RtpPacketHistory::StorageMode::kStoreAndCull, 10); @@ -559,7 +534,7 @@ TEST_P(RtpSenderEgressTest, UpdatesSendStatusOfRetransmittedPackets) { EXPECT_TRUE(packet_history_.GetPacketState(media_packet->SequenceNumber())); } -TEST_P(RtpSenderEgressTest, StreamDataCountersCallbacks) { +TEST_F(RtpSenderEgressTest, StreamDataCountersCallbacks) { std::unique_ptr sender = CreateRtpSenderEgress(); const RtpPacketCounter kEmptyCounter; @@ -644,7 +619,7 @@ TEST_P(RtpSenderEgressTest, StreamDataCountersCallbacks) { time_controller_.AdvanceTime(TimeDelta::Zero()); } -TEST_P(RtpSenderEgressTest, StreamDataCountersCallbacksFec) { +TEST_F(RtpSenderEgressTest, StreamDataCountersCallbacksFec) { std::unique_ptr sender = CreateRtpSenderEgress(); const RtpPacketCounter kEmptyCounter; @@ -694,7 +669,7 @@ TEST_P(RtpSenderEgressTest, StreamDataCountersCallbacksFec) { time_controller_.AdvanceTime(TimeDelta::Zero()); } -TEST_P(RtpSenderEgressTest, UpdatesDataCounters) { +TEST_F(RtpSenderEgressTest, UpdatesDataCounters) { std::unique_ptr sender = CreateRtpSenderEgress(); const RtpPacketCounter kEmptyCounter; @@ -735,7 +710,7 @@ TEST_P(RtpSenderEgressTest, UpdatesDataCounters) { EXPECT_EQ(rtx_stats.fec, kEmptyCounter); } -TEST_P(RtpSenderEgressTest, SendPacketUpdatesExtensions) { +TEST_F(RtpSenderEgressTest, SendPacketUpdatesExtensions) { header_extensions_.RegisterByUri(kVideoTimingExtensionId, VideoTimingExtension::Uri()); header_extensions_.RegisterByUri(kAbsoluteSendTimeExtensionId, @@ -764,7 +739,7 @@ TEST_P(RtpSenderEgressTest, SendPacketUpdatesExtensions) { EXPECT_EQ(timing.pacer_exit_delta_ms, kDiffMs); } -TEST_P(RtpSenderEgressTest, SendPacketSetsPacketOptions) { +TEST_F(RtpSenderEgressTest, SendPacketSetsPacketOptions) { const uint16_t kPacketId = 42; std::unique_ptr sender = CreateRtpSenderEgress(); header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, @@ -791,7 +766,7 @@ TEST_P(RtpSenderEgressTest, SendPacketSetsPacketOptions) { EXPECT_TRUE(transport_.last_packet()->options.is_retransmit); } -TEST_P(RtpSenderEgressTest, SendPacketUpdatesStats) { +TEST_F(RtpSenderEgressTest, SendPacketUpdatesStats) { const size_t kPayloadSize = 1000; StrictMock send_side_delay_observer; @@ -856,7 +831,7 @@ TEST_P(RtpSenderEgressTest, SendPacketUpdatesStats) { EXPECT_EQ(rtx_stats.retransmitted.packets, 1u); } -TEST_P(RtpSenderEgressTest, TransportFeedbackObserverWithRetransmission) { +TEST_F(RtpSenderEgressTest, TransportFeedbackObserverWithRetransmission) { const uint16_t kTransportSequenceNumber = 17; header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, TransportSequenceNumber::Uri()); @@ -878,7 +853,7 @@ TEST_P(RtpSenderEgressTest, TransportFeedbackObserverWithRetransmission) { sender->SendPacket(retransmission.get(), PacedPacketInfo()); } -TEST_P(RtpSenderEgressTest, TransportFeedbackObserverWithRtxRetransmission) { +TEST_F(RtpSenderEgressTest, TransportFeedbackObserverWithRtxRetransmission) { const uint16_t kTransportSequenceNumber = 17; header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, TransportSequenceNumber::Uri()); @@ -902,7 +877,7 @@ TEST_P(RtpSenderEgressTest, TransportFeedbackObserverWithRtxRetransmission) { sender->SendPacket(rtx_retransmission.get(), PacedPacketInfo()); } -TEST_P(RtpSenderEgressTest, TransportFeedbackObserverPadding) { +TEST_F(RtpSenderEgressTest, TransportFeedbackObserverPadding) { const uint16_t kTransportSequenceNumber = 17; header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, TransportSequenceNumber::Uri()); @@ -920,7 +895,7 @@ TEST_P(RtpSenderEgressTest, TransportFeedbackObserverPadding) { sender->SendPacket(padding.get(), PacedPacketInfo()); } -TEST_P(RtpSenderEgressTest, TransportFeedbackObserverRtxPadding) { +TEST_F(RtpSenderEgressTest, TransportFeedbackObserverRtxPadding) { const uint16_t kTransportSequenceNumber = 17; header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, TransportSequenceNumber::Uri()); @@ -940,7 +915,7 @@ TEST_P(RtpSenderEgressTest, TransportFeedbackObserverRtxPadding) { sender->SendPacket(rtx_padding.get(), PacedPacketInfo()); } -TEST_P(RtpSenderEgressTest, TransportFeedbackObserverFec) { +TEST_F(RtpSenderEgressTest, TransportFeedbackObserverFec) { const uint16_t kTransportSequenceNumber = 17; header_extensions_.RegisterByUri(kTransportSequenceNumberExtensionId, TransportSequenceNumber::Uri()); @@ -965,7 +940,7 @@ TEST_P(RtpSenderEgressTest, TransportFeedbackObserverFec) { sender->SendPacket(fec_packet.get(), PacedPacketInfo()); } -TEST_P(RtpSenderEgressTest, SupportsAbortingRetransmissions) { +TEST_F(RtpSenderEgressTest, SupportsAbortingRetransmissions) { std::unique_ptr sender = CreateRtpSenderEgress(); packet_history_.SetStorePacketsStatus( RtpPacketHistory::StorageMode::kStoreAndCull, 10); @@ -992,9 +967,4 @@ TEST_P(RtpSenderEgressTest, SupportsAbortingRetransmissions) { EXPECT_TRUE(packet_history_.GetPacketAndMarkAsPending(media_sequence_number)); } -INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, - RtpSenderEgressTest, - ::testing::Values(TestConfig(false), - TestConfig(true))); - } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index 3eacda86a2..8b41e5b4e7 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc @@ -153,9 +153,7 @@ class TestRtpSenderVideo : public RTPSenderVideo { class FieldTrials : public FieldTrialsRegistry { public: - explicit FieldTrials(bool use_send_side_bwe_with_overhead) - : use_send_side_bwe_with_overhead_(use_send_side_bwe_with_overhead), - include_capture_clock_offset_(false) {} + FieldTrials() : include_capture_clock_offset_(false) {} void set_include_capture_clock_offset(bool include_capture_clock_offset) { include_capture_clock_offset_ = include_capture_clock_offset; @@ -163,23 +161,19 @@ class FieldTrials : public FieldTrialsRegistry { private: std::string GetValue(absl::string_view key) const override { - if (key == "WebRTC-SendSideBwe-WithOverhead") { - return use_send_side_bwe_with_overhead_ ? "Enabled" : ""; - } else if (key == "WebRTC-IncludeCaptureClockOffset") { + if (key == "WebRTC-IncludeCaptureClockOffset") { return include_capture_clock_offset_ ? "" : "Disabled"; } return ""; } - bool use_send_side_bwe_with_overhead_; bool include_capture_clock_offset_; }; -class RtpSenderVideoTest : public ::testing::TestWithParam { +class RtpSenderVideoTest : public ::testing::Test { public: RtpSenderVideoTest() - : field_trials_(GetParam()), - fake_clock_(kStartTime), + : fake_clock_(kStartTime), retransmission_rate_limiter_(&fake_clock_, 1000), rtp_module_(ModuleRtpRtcpImpl2::Create([&] { RtpRtcpInterface::Configuration config; @@ -212,7 +206,7 @@ class RtpSenderVideoTest : public ::testing::TestWithParam { std::unique_ptr rtp_sender_video_; }; -TEST_P(RtpSenderVideoTest, KeyFrameHasCVO) { +TEST_F(RtpSenderVideoTest, KeyFrameHasCVO) { uint8_t kFrame[kMaxPacketLength]; rtp_module_->RegisterRtpHeaderExtension(VideoOrientation::Uri(), kVideoRotationExtensionId); @@ -229,7 +223,7 @@ TEST_P(RtpSenderVideoTest, KeyFrameHasCVO) { EXPECT_EQ(kVideoRotation_0, rotation); } -TEST_P(RtpSenderVideoTest, TimingFrameHasPacketizationTimstampSet) { +TEST_F(RtpSenderVideoTest, TimingFrameHasPacketizationTimstampSet) { uint8_t kFrame[kMaxPacketLength]; const int64_t kPacketizationTimeMs = 100; const int64_t kEncodeStartDeltaMs = 10; @@ -257,7 +251,7 @@ TEST_P(RtpSenderVideoTest, TimingFrameHasPacketizationTimstampSet) { EXPECT_EQ(kEncodeFinishDeltaMs, timing.encode_finish_delta_ms); } -TEST_P(RtpSenderVideoTest, DeltaFrameHasCVOWhenChanged) { +TEST_F(RtpSenderVideoTest, DeltaFrameHasCVOWhenChanged) { uint8_t kFrame[kMaxPacketLength]; rtp_module_->RegisterRtpHeaderExtension(VideoOrientation::Uri(), kVideoRotationExtensionId); @@ -281,7 +275,7 @@ TEST_P(RtpSenderVideoTest, DeltaFrameHasCVOWhenChanged) { EXPECT_EQ(kVideoRotation_0, rotation); } -TEST_P(RtpSenderVideoTest, DeltaFrameHasCVOWhenNonZero) { +TEST_F(RtpSenderVideoTest, DeltaFrameHasCVOWhenNonZero) { uint8_t kFrame[kMaxPacketLength]; rtp_module_->RegisterRtpHeaderExtension(VideoOrientation::Uri(), kVideoRotationExtensionId); @@ -306,7 +300,7 @@ TEST_P(RtpSenderVideoTest, DeltaFrameHasCVOWhenNonZero) { // Make sure rotation is parsed correctly when the Camera (C) and Flip (F) bits // are set in the CVO byte. -TEST_P(RtpSenderVideoTest, SendVideoWithCameraAndFlipCVO) { +TEST_F(RtpSenderVideoTest, SendVideoWithCameraAndFlipCVO) { // Test extracting rotation when Camera (C) and Flip (F) bits are zero. EXPECT_EQ(kVideoRotation_0, ConvertCVOByteToVideoRotation(0)); EXPECT_EQ(kVideoRotation_90, ConvertCVOByteToVideoRotation(1)); @@ -325,7 +319,7 @@ TEST_P(RtpSenderVideoTest, SendVideoWithCameraAndFlipCVO) { ConvertCVOByteToVideoRotation(flip_bit | camera_bit | 3)); } -TEST_P(RtpSenderVideoTest, RetransmissionTypesGeneric) { +TEST_F(RtpSenderVideoTest, RetransmissionTypesGeneric) { RTPVideoHeader header; header.codec = kVideoCodecGeneric; @@ -340,7 +334,7 @@ TEST_P(RtpSenderVideoTest, RetransmissionTypesGeneric) { kDefaultExpectedRetransmissionTimeMs)); } -TEST_P(RtpSenderVideoTest, RetransmissionTypesH264) { +TEST_F(RtpSenderVideoTest, RetransmissionTypesH264) { RTPVideoHeader header; header.video_type_header.emplace().packetization_mode = H264PacketizationMode::NonInterleaved; @@ -357,7 +351,7 @@ TEST_P(RtpSenderVideoTest, RetransmissionTypesH264) { kDefaultExpectedRetransmissionTimeMs)); } -TEST_P(RtpSenderVideoTest, RetransmissionTypesVP8BaseLayer) { +TEST_F(RtpSenderVideoTest, RetransmissionTypesVP8BaseLayer) { RTPVideoHeader header; header.codec = kVideoCodecVP8; auto& vp8_header = header.video_type_header.emplace(); @@ -380,7 +374,7 @@ TEST_P(RtpSenderVideoTest, RetransmissionTypesVP8BaseLayer) { kDefaultExpectedRetransmissionTimeMs)); } -TEST_P(RtpSenderVideoTest, RetransmissionTypesVP8HigherLayers) { +TEST_F(RtpSenderVideoTest, RetransmissionTypesVP8HigherLayers) { RTPVideoHeader header; header.codec = kVideoCodecVP8; @@ -400,7 +394,7 @@ TEST_P(RtpSenderVideoTest, RetransmissionTypesVP8HigherLayers) { } } -TEST_P(RtpSenderVideoTest, RetransmissionTypesVP9) { +TEST_F(RtpSenderVideoTest, RetransmissionTypesVP9) { RTPVideoHeader header; header.codec = kVideoCodecVP9; @@ -420,7 +414,7 @@ TEST_P(RtpSenderVideoTest, RetransmissionTypesVP9) { } } -TEST_P(RtpSenderVideoTest, ConditionalRetransmit) { +TEST_F(RtpSenderVideoTest, ConditionalRetransmit) { const int64_t kFrameIntervalMs = 33; const int64_t kRttMs = (kFrameIntervalMs * 3) / 2; const uint8_t kSettings = @@ -478,7 +472,7 @@ TEST_P(RtpSenderVideoTest, ConditionalRetransmit) { rtp_sender_video_->AllowRetransmission(header, kSettings, kRttMs)); } -TEST_P(RtpSenderVideoTest, ConditionalRetransmitLimit) { +TEST_F(RtpSenderVideoTest, ConditionalRetransmitLimit) { const int64_t kFrameIntervalMs = 200; const int64_t kRttMs = (kFrameIntervalMs * 3) / 2; const int32_t kSettings = @@ -511,7 +505,7 @@ TEST_P(RtpSenderVideoTest, ConditionalRetransmitLimit) { rtp_sender_video_->AllowRetransmission(header, kSettings, kRttMs)); } -TEST_P(RtpSenderVideoTest, SendsDependencyDescriptorWhenVideoStructureIsSet) { +TEST_F(RtpSenderVideoTest, SendsDependencyDescriptorWhenVideoStructureIsSet) { const int64_t kFrameId = 100000; uint8_t kFrame[100]; rtp_module_->RegisterRtpHeaderExtension( @@ -579,7 +573,7 @@ TEST_P(RtpSenderVideoTest, SendsDependencyDescriptorWhenVideoStructureIsSet) { ElementsAre(1, 501)); } -TEST_P(RtpSenderVideoTest, +TEST_F(RtpSenderVideoTest, SkipsDependencyDescriptorOnDeltaFrameWhenFailedToAttachToKeyFrame) { const int64_t kFrameId = 100000; uint8_t kFrame[100]; @@ -633,7 +627,7 @@ TEST_P(RtpSenderVideoTest, .HasExtension()); } -TEST_P(RtpSenderVideoTest, PropagatesChainDiffsIntoDependencyDescriptor) { +TEST_F(RtpSenderVideoTest, PropagatesChainDiffsIntoDependencyDescriptor) { const int64_t kFrameId = 100000; uint8_t kFrame[100]; rtp_module_->RegisterRtpHeaderExtension( @@ -666,7 +660,7 @@ TEST_P(RtpSenderVideoTest, PropagatesChainDiffsIntoDependencyDescriptor) { ContainerEq(generic.chain_diffs)); } -TEST_P(RtpSenderVideoTest, +TEST_F(RtpSenderVideoTest, PropagatesActiveDecodeTargetsIntoDependencyDescriptor) { const int64_t kFrameId = 100000; uint8_t kFrame[100]; @@ -700,7 +694,7 @@ TEST_P(RtpSenderVideoTest, EXPECT_EQ(descriptor_key.active_decode_targets_bitmask, 0b01u); } -TEST_P(RtpSenderVideoTest, +TEST_F(RtpSenderVideoTest, SetDiffentVideoStructureAvoidsCollisionWithThePreviousStructure) { const int64_t kFrameId = 100000; uint8_t kFrame[100]; @@ -774,7 +768,7 @@ TEST_P(RtpSenderVideoTest, descriptor_key2.attached_structure.get(), &descriptor_delta)); } -TEST_P(RtpSenderVideoTest, +TEST_F(RtpSenderVideoTest, AuthenticateVideoHeaderWhenDependencyDescriptorExtensionIsUsed) { static constexpr size_t kFrameSize = 100; uint8_t kFrame[kFrameSize] = {1, 2, 3, 4}; @@ -817,7 +811,7 @@ TEST_P(RtpSenderVideoTest, .HasExtension()); } -TEST_P(RtpSenderVideoTest, PopulateGenericFrameDescriptor) { +TEST_F(RtpSenderVideoTest, PopulateGenericFrameDescriptor) { const int64_t kFrameId = 100000; uint8_t kFrame[100]; rtp_module_->RegisterRtpHeaderExtension( @@ -874,17 +868,17 @@ void RtpSenderVideoTest:: EXPECT_EQ(transport_.last_sent_packet().payload_size(), 1 + kFrameSize); } -TEST_P(RtpSenderVideoTest, +TEST_F(RtpSenderVideoTest, UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed00) { UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed(0); } -TEST_P(RtpSenderVideoTest, +TEST_F(RtpSenderVideoTest, UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed01) { UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed(1); } -TEST_P(RtpSenderVideoTest, VideoLayersAllocationWithResolutionSentOnKeyFrames) { +TEST_F(RtpSenderVideoTest, VideoLayersAllocationWithResolutionSentOnKeyFrames) { const size_t kFrameSize = 100; uint8_t kFrame[kFrameSize]; rtp_module_->RegisterRtpHeaderExtension( @@ -920,7 +914,7 @@ TEST_P(RtpSenderVideoTest, VideoLayersAllocationWithResolutionSentOnKeyFrames) { .GetExtension(&sent_allocation)); } -TEST_P(RtpSenderVideoTest, +TEST_F(RtpSenderVideoTest, VideoLayersAllocationWithoutResolutionSentOnDeltaWhenUpdated) { const size_t kFrameSize = 100; uint8_t kFrame[kFrameSize]; @@ -968,7 +962,7 @@ TEST_P(RtpSenderVideoTest, SizeIs(1)); } -TEST_P(RtpSenderVideoTest, +TEST_F(RtpSenderVideoTest, VideoLayersAllocationWithResolutionSentOnDeltaWhenSpatialLayerAdded) { const size_t kFrameSize = 100; uint8_t kFrame[kFrameSize]; @@ -1014,7 +1008,7 @@ TEST_P(RtpSenderVideoTest, EXPECT_TRUE(sent_allocation.resolution_and_frame_rate_is_valid); } -TEST_P(RtpSenderVideoTest, +TEST_F(RtpSenderVideoTest, VideoLayersAllocationWithResolutionSentOnLargeFrameRateChange) { const size_t kFrameSize = 100; uint8_t kFrame[kFrameSize]; @@ -1056,7 +1050,7 @@ TEST_P(RtpSenderVideoTest, EXPECT_EQ(sent_allocation.active_spatial_layers[0].frame_rate_fps, 20); } -TEST_P(RtpSenderVideoTest, +TEST_F(RtpSenderVideoTest, VideoLayersAllocationWithoutResolutionSentOnSmallFrameRateChange) { const size_t kFrameSize = 100; uint8_t kFrame[kFrameSize]; @@ -1097,7 +1091,7 @@ TEST_P(RtpSenderVideoTest, EXPECT_FALSE(sent_allocation.resolution_and_frame_rate_is_valid); } -TEST_P(RtpSenderVideoTest, VideoLayersAllocationSentOnDeltaFramesOnlyOnUpdate) { +TEST_F(RtpSenderVideoTest, VideoLayersAllocationSentOnDeltaFramesOnlyOnUpdate) { const size_t kFrameSize = 100; uint8_t kFrame[kFrameSize]; rtp_module_->RegisterRtpHeaderExtension( @@ -1139,7 +1133,7 @@ TEST_P(RtpSenderVideoTest, VideoLayersAllocationSentOnDeltaFramesOnlyOnUpdate) { .GetExtension(&sent_allocation)); } -TEST_P(RtpSenderVideoTest, VideoLayersAllocationNotSentOnHigherTemporalLayers) { +TEST_F(RtpSenderVideoTest, VideoLayersAllocationNotSentOnHigherTemporalLayers) { const size_t kFrameSize = 100; uint8_t kFrame[kFrameSize]; rtp_module_->RegisterRtpHeaderExtension( @@ -1175,7 +1169,7 @@ TEST_P(RtpSenderVideoTest, VideoLayersAllocationNotSentOnHigherTemporalLayers) { .HasExtension()); } -TEST_P(RtpSenderVideoTest, AbsoluteCaptureTime) { +TEST_F(RtpSenderVideoTest, AbsoluteCaptureTime) { constexpr int64_t kAbsoluteCaptureTimestampMs = 12345678; uint8_t kFrame[kMaxPacketLength]; rtp_module_->RegisterRtpHeaderExtension(AbsoluteCaptureTimeExtension::Uri(), @@ -1212,7 +1206,7 @@ TEST_P(RtpSenderVideoTest, AbsoluteCaptureTime) { // Essentially the same test as AbsoluteCaptureTime but with a field trial. // After the field trial is experimented, we will remove AbsoluteCaptureTime. -TEST_P(RtpSenderVideoTest, AbsoluteCaptureTimeWithCaptureClockOffset) { +TEST_F(RtpSenderVideoTest, AbsoluteCaptureTimeWithCaptureClockOffset) { field_trials_.set_include_capture_clock_offset(true); rtp_sender_video_ = std::make_unique( &fake_clock_, rtp_module_->RtpSender(), field_trials_); @@ -1250,7 +1244,7 @@ TEST_P(RtpSenderVideoTest, AbsoluteCaptureTimeWithCaptureClockOffset) { EXPECT_EQ(absolute_capture_time->estimated_capture_clock_offset, 0); } -TEST_P(RtpSenderVideoTest, AbsoluteCaptureTimeWithExtensionProvided) { +TEST_F(RtpSenderVideoTest, AbsoluteCaptureTimeWithExtensionProvided) { constexpr AbsoluteCaptureTime kAbsoluteCaptureTime = { 123, absl::optional(456), @@ -1283,7 +1277,7 @@ TEST_P(RtpSenderVideoTest, AbsoluteCaptureTimeWithExtensionProvided) { EXPECT_EQ(absolute_capture_time, kAbsoluteCaptureTime); } -TEST_P(RtpSenderVideoTest, PopulatesPlayoutDelay) { +TEST_F(RtpSenderVideoTest, PopulatesPlayoutDelay) { // Single packet frames. constexpr size_t kPacketSize = 123; uint8_t kFrame[kPacketSize]; @@ -1340,7 +1334,7 @@ TEST_P(RtpSenderVideoTest, PopulatesPlayoutDelay) { EXPECT_EQ(received_delay, kExpectedDelay); } -TEST_P(RtpSenderVideoTest, SendGenericVideo) { +TEST_F(RtpSenderVideoTest, SendGenericVideo) { const uint8_t kPayloadType = 127; const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric; const uint8_t kPayload[] = {47, 11, 32, 93, 89}; @@ -1373,7 +1367,7 @@ TEST_P(RtpSenderVideoTest, SendGenericVideo) { EXPECT_THAT(sent_payload.subview(1), ElementsAreArray(kDeltaPayload)); } -TEST_P(RtpSenderVideoTest, SendRawVideo) { +TEST_F(RtpSenderVideoTest, SendRawVideo) { const uint8_t kPayloadType = 111; const uint8_t kPayload[] = {11, 22, 33, 44, 55}; @@ -1389,10 +1383,6 @@ TEST_P(RtpSenderVideoTest, SendRawVideo) { EXPECT_THAT(sent_payload, ElementsAreArray(kPayload)); } -INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, - RtpSenderVideoTest, - ::testing::Bool()); - class RtpSenderVideoWithFrameTransformerTest : public ::testing::Test { public: RtpSenderVideoWithFrameTransformerTest() diff --git a/sdk/objc/api/peerconnection/RTCFieldTrials.h b/sdk/objc/api/peerconnection/RTCFieldTrials.h index a42a961c01..3e8fcc8075 100644 --- a/sdk/objc/api/peerconnection/RTCFieldTrials.h +++ b/sdk/objc/api/peerconnection/RTCFieldTrials.h @@ -13,8 +13,7 @@ #import "RTCMacros.h" /** The only valid value for the following if set is kRTCFieldTrialEnabledValue. */ -RTC_EXTERN NSString * const kRTCFieldTrialAudioForceABWENoTWCCKey; -RTC_EXTERN NSString * const kRTCFieldTrialSendSideBweWithOverheadKey; +RTC_EXTERN NSString *const kRTCFieldTrialAudioForceABWENoTWCCKey; RTC_EXTERN NSString * const kRTCFieldTrialFlexFec03AdvertisedKey; RTC_EXTERN NSString * const kRTCFieldTrialFlexFec03Key; RTC_EXTERN NSString * const kRTCFieldTrialH264HighProfileKey; diff --git a/sdk/objc/api/peerconnection/RTCFieldTrials.mm b/sdk/objc/api/peerconnection/RTCFieldTrials.mm index e16d39e511..193da9e4f7 100644 --- a/sdk/objc/api/peerconnection/RTCFieldTrials.mm +++ b/sdk/objc/api/peerconnection/RTCFieldTrials.mm @@ -16,8 +16,7 @@ #include "system_wrappers/include/field_trial.h" -NSString * const kRTCFieldTrialAudioForceABWENoTWCCKey = @"WebRTC-Audio-ABWENoTWCC"; -NSString * const kRTCFieldTrialSendSideBweWithOverheadKey = @"WebRTC-SendSideBwe-WithOverhead"; +NSString *const kRTCFieldTrialAudioForceABWENoTWCCKey = @"WebRTC-Audio-ABWENoTWCC"; NSString * const kRTCFieldTrialFlexFec03AdvertisedKey = @"WebRTC-FlexFEC-03-Advertised"; NSString * const kRTCFieldTrialFlexFec03Key = @"WebRTC-FlexFEC-03"; NSString * const kRTCFieldTrialH264HighProfileKey = @"WebRTC-H264HighProfile"; diff --git a/test/scenario/audio_stream.cc b/test/scenario/audio_stream.cc index ea170bc17c..891ef4b4a6 100644 --- a/test/scenario/audio_stream.cc +++ b/test/scenario/audio_stream.cc @@ -129,10 +129,8 @@ SendAudioStream::SendAudioStream( sender_->SendTask([&] { send_stream_ = sender_->call_->CreateAudioSendStream(send_config); - if (field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")) { - sender->call_->OnAudioTransportOverheadChanged( - sender_->transport_->packet_overhead().bytes()); - } + sender->call_->OnAudioTransportOverheadChanged( + sender_->transport_->packet_overhead().bytes()); }); } diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 958d04e247..923c318c6d 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -2694,9 +2694,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { // bitrates than expected by this test, due to encoder pushback and subtracted // overhead. webrtc::test::ScopedKeyValueConfig field_trials( - field_trials_, - "WebRTC-VideoRateControl/bitrate_adjuster:false/" - "WebRTC-SendSideBwe-WithOverhead/Disabled/"); + field_trials_, "WebRTC-VideoRateControl/bitrate_adjuster:false/"); class EncoderBitrateThresholdObserver : public test::SendTest, public VideoBitrateAllocatorFactory, @@ -2722,8 +2720,8 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { EXPECT_LE(codec.startBitrate, codec.maxBitrate); if (num_rate_allocator_creations_ == 0) { EXPECT_EQ(static_cast(kMinBitrateKbps), codec.minBitrate); - EXPECT_EQ(static_cast(kStartBitrateKbps), - codec.startBitrate); + EXPECT_NEAR(static_cast(kStartBitrateKbps), + codec.startBitrate, 10); EXPECT_EQ(static_cast(kMaxBitrateKbps), codec.maxBitrate); } else if (num_rate_allocator_creations_ == 1) { EXPECT_EQ(static_cast(kLowerMaxBitrateKbps), @@ -2749,8 +2747,8 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { EXPECT_EQ(0, num_encoder_initializations_); EXPECT_EQ(static_cast(kMinBitrateKbps), codecSettings->minBitrate); - EXPECT_EQ(static_cast(kStartBitrateKbps), - codecSettings->startBitrate); + EXPECT_NEAR(static_cast(kStartBitrateKbps), + codecSettings->startBitrate, 10); EXPECT_EQ(static_cast(kMaxBitrateKbps), codecSettings->maxBitrate); @@ -2775,14 +2773,18 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { FakeEncoder::SetRates(parameters); } - void WaitForSetRates(uint32_t expected_bitrate) { + void WaitForSetRates(uint32_t expected_bitrate, int abs_error) { // Wait for the expected rate to be set. In some cases there can be // more than one update pending, in which case we keep waiting // until the correct value has been observed. + // The target_bitrate_ is reduced by the calculated packet overhead. const int64_t start_time = rtc::TimeMillis(); do { MutexLock lock(&mutex_); - if (target_bitrate_ == expected_bitrate) { + + int error = target_bitrate_ - expected_bitrate; + if ((error < 0 && error >= -abs_error) || + (error >= 0 && error <= abs_error)) { return; } } while (bitrate_changed_event_.Wait( @@ -2790,7 +2792,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { VideoSendStreamTest::kDefaultTimeout - TimeDelta::Millis(rtc::TimeMillis() - start_time)))); MutexLock lock(&mutex_); - EXPECT_EQ(target_bitrate_, expected_bitrate) + EXPECT_NEAR(target_bitrate_, expected_bitrate, abs_error) << "Timed out while waiting encoder rate to be set."; } @@ -2832,7 +2834,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { << "Timed out while waiting for rate allocator to be created."; ASSERT_TRUE(init_encode_event_.Wait(VideoSendStreamTest::kDefaultTimeout)) << "Timed out while waiting for encoder to be configured."; - WaitForSetRates(kStartBitrateKbps); + WaitForSetRates(kStartBitrateKbps, 80); BitrateConstraints bitrate_config; bitrate_config.start_bitrate_bps = kIncreasedStartBitrateKbps * 1000; bitrate_config.max_bitrate_bps = kIncreasedMaxBitrateKbps * 1000; @@ -2841,7 +2843,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { bitrate_config); }); // Encoder rate is capped by EncoderConfig max_bitrate_bps. - WaitForSetRates(kMaxBitrateKbps); + WaitForSetRates(kMaxBitrateKbps, 10); encoder_config_.max_bitrate_bps = kLowerMaxBitrateKbps * 1000; SendTask(task_queue_, [&]() { send_stream_->ReconfigureVideoEncoder(encoder_config_.Copy()); @@ -2851,7 +2853,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { EXPECT_EQ(2, num_rate_allocator_creations_) << "Rate allocator should have been recreated."; - WaitForSetRates(kLowerMaxBitrateKbps); + WaitForSetRates(kLowerMaxBitrateKbps, 10); EXPECT_EQ(1, num_encoder_initializations_); encoder_config_.max_bitrate_bps = kIncreasedMaxBitrateKbps * 1000; @@ -2865,7 +2867,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { // Expected target bitrate is the start bitrate set in the call to // call_->GetTransportControllerSend()->SetSdpBitrateParameters. - WaitForSetRates(kIncreasedStartBitrateKbps); + WaitForSetRates(kIncreasedStartBitrateKbps, 10); EXPECT_EQ(1, num_encoder_initializations_); } @@ -3701,8 +3703,6 @@ TEST_F(VideoSendStreamTest, EncoderConfigMaxFramerateReportedToSource) { // testing that the maximum possible target payload rate is smaller than the // maximum bandwidth estimate by the overhead rate. TEST_F(VideoSendStreamTest, RemoveOverheadFromBandwidth) { - test::ScopedFieldTrials override_field_trials( - "WebRTC-SendSideBwe-WithOverhead/Enabled/"); class RemoveOverheadFromBandwidthTest : public test::EndToEndTest, public test::FakeEncoder { public: