diff --git a/audio/BUILD.gn b/audio/BUILD.gn index 80ce0998f9..e4ab39a209 100644 --- a/audio/BUILD.gn +++ b/audio/BUILD.gn @@ -149,7 +149,6 @@ if (rtc_include_tests) { "../logging:rtc_event_log_api", "../modules/audio_device:mock_audio_device", "../rtc_base:rtc_base_tests_utils", - "../test:field_trial", # For TestAudioDeviceModule "../modules/audio_device:audio_device_impl", diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 2e4830c1f2..0c597ed321 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -264,8 +264,8 @@ void AudioSendStream::ConfigureStream( RtcpBandwidthObserver* bandwidth_observer = nullptr; - if (stream->allocation_settings_.ShouldSendTransportSequenceNumber( - new_ids.transport_sequence_number)) { + if (stream->allocation_settings_.IncludeAudioInFeedback( + new_ids.transport_sequence_number != 0)) { channel_send->EnableSendTransportSequenceNumber( new_ids.transport_sequence_number); // Probing in application limited region is only used in combination with diff --git a/audio/audio_send_stream_tests.cc b/audio/audio_send_stream_tests.cc index 55de03dd51..7deeff3b4d 100644 --- a/audio/audio_send_stream_tests.cc +++ b/audio/audio_send_stream_tests.cc @@ -9,8 +9,6 @@ */ #include "test/call_test.h" -#include "test/constants.h" -#include "test/field_trial.h" #include "test/gtest.h" #include "test/rtcp_packet_parser.h" @@ -139,54 +137,42 @@ TEST_F(AudioSendStreamCallTest, SupportsAudioLevel) { RunBaseTest(&test); } -class TransportWideSequenceNumberObserver : public AudioSendTest { - public: - explicit TransportWideSequenceNumberObserver(bool expect_sequence_number) - : AudioSendTest(), expect_sequence_number_(expect_sequence_number) { - EXPECT_TRUE(parser_->RegisterRtpHeaderExtension( - kRtpExtensionTransportSequenceNumber, - kTransportSequenceNumberExtensionId)); - } +TEST_F(AudioSendStreamCallTest, SupportsTransportWideSequenceNumbers) { + static const uint8_t kExtensionId = test::kTransportSequenceNumberExtensionId; + class TransportWideSequenceNumberObserver : public AudioSendTest { + public: + TransportWideSequenceNumberObserver() : AudioSendTest() { + EXPECT_TRUE(parser_->RegisterRtpHeaderExtension( + kRtpExtensionTransportSequenceNumber, kExtensionId)); + } - private: - Action OnSendRtp(const uint8_t* packet, size_t length) override { - RTPHeader header; - EXPECT_TRUE(parser_->Parse(packet, length, &header)); + private: + Action OnSendRtp(const uint8_t* packet, size_t length) override { + RTPHeader header; + EXPECT_TRUE(parser_->Parse(packet, length, &header)); - EXPECT_EQ(header.extension.hasTransportSequenceNumber, - expect_sequence_number_); - EXPECT_FALSE(header.extension.hasTransmissionTimeOffset); - EXPECT_FALSE(header.extension.hasAbsoluteSendTime); + EXPECT_TRUE(header.extension.hasTransportSequenceNumber); + EXPECT_FALSE(header.extension.hasTransmissionTimeOffset); + EXPECT_FALSE(header.extension.hasAbsoluteSendTime); - observation_complete_.Set(); + observation_complete_.Set(); - return SEND_PACKET; - } + return SEND_PACKET; + } - void ModifyAudioConfigs( - AudioSendStream::Config* send_config, - std::vector* receive_configs) override { - send_config->rtp.extensions.clear(); - send_config->rtp.extensions.push_back( - RtpExtension(RtpExtension::kTransportSequenceNumberUri, - kTransportSequenceNumberExtensionId)); - } + void ModifyAudioConfigs( + AudioSendStream::Config* send_config, + std::vector* receive_configs) override { + send_config->rtp.extensions.clear(); + send_config->rtp.extensions.push_back(RtpExtension( + RtpExtension::kTransportSequenceNumberUri, kExtensionId)); + } - void PerformTest() override { - EXPECT_TRUE(Wait()) << "Timed out while waiting for a single RTP packet."; - } - const bool expect_sequence_number_; -}; + void PerformTest() override { + EXPECT_TRUE(Wait()) << "Timed out while waiting for a single RTP packet."; + } + } test; -TEST_F(AudioSendStreamCallTest, SendsTransportWideSequenceNumbersInFieldTrial) { - ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/"); - TransportWideSequenceNumberObserver test(/*expect_sequence_number=*/true); - RunBaseTest(&test); -} - -TEST_F(AudioSendStreamCallTest, - DoesNotSendTransportWideSequenceNumbersPerDefault) { - TransportWideSequenceNumberObserver test(/*expect_sequence_number=*/false); RunBaseTest(&test); } diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index e20b031079..70f6c7238e 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -28,7 +28,6 @@ #include "modules/rtp_rtcp/mocks/mock_rtcp_rtt_stats.h" #include "modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" #include "rtc_base/task_queue.h" -#include "test/field_trial.h" #include "test/gtest.h" #include "test/mock_audio_encoder.h" #include "test/mock_audio_encoder_factory.h" @@ -354,7 +353,6 @@ TEST(AudioSendStreamTest, SetMuted) { } TEST(AudioSendStreamTest, AudioBweCorrectObjectsOnChannelProxy) { - ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/"); ConfigHelper helper(true, true); auto send_stream = helper.CreateAudioSendStream(); } @@ -506,7 +504,6 @@ TEST(AudioSendStreamTest, DontRecreateEncoder) { } TEST(AudioSendStreamTest, ReconfigureTransportCcResetsFirst) { - ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/"); ConfigHelper helper(false, true); auto send_stream = helper.CreateAudioSendStream(); auto new_config = helper.config(); diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index a055ae9ed3..07a47d789c 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -559,8 +559,11 @@ RtpCapabilities WebRtcVoiceEngine::GetCapabilities() const { int id = 1; capabilities.header_extensions.push_back( webrtc::RtpExtension(webrtc::RtpExtension::kAudioLevelUri, id++)); - capabilities.header_extensions.push_back(webrtc::RtpExtension( - webrtc::RtpExtension::kTransportSequenceNumberUri, id++)); + if (allocation_settings_.EnableTransportSequenceNumberExtension()) { + capabilities.header_extensions.push_back(webrtc::RtpExtension( + webrtc::RtpExtension::kTransportSequenceNumberUri, id++)); + } + return capabilities; } diff --git a/rtc_base/experiments/audio_allocation_settings.cc b/rtc_base/experiments/audio_allocation_settings.cc index a505357869..79c5a0dfe7 100644 --- a/rtc_base/experiments/audio_allocation_settings.cc +++ b/rtc_base/experiments/audio_allocation_settings.cc @@ -65,12 +65,16 @@ bool AudioAllocationSettings::ConfigureRateAllocationRange() const { return audio_send_side_bwe_; } -bool AudioAllocationSettings::ShouldSendTransportSequenceNumber( +bool AudioAllocationSettings::EnableTransportSequenceNumberExtension() const { + // TODO(srte): Update this to be more accurate. + return audio_send_side_bwe_ && !allocate_audio_without_feedback_; +} + +bool AudioAllocationSettings::IncludeAudioInFeedback( int transport_seq_num_extension_header_id) const { if (force_no_audio_feedback_) return false; - return audio_send_side_bwe_ && !allocate_audio_without_feedback_ && - transport_seq_num_extension_header_id != 0; + return transport_seq_num_extension_header_id != 0; } bool AudioAllocationSettings::UpdateAudioTargetBitrate( diff --git a/rtc_base/experiments/audio_allocation_settings.h b/rtc_base/experiments/audio_allocation_settings.h index f05b4a33ed..bcc0f3e90b 100644 --- a/rtc_base/experiments/audio_allocation_settings.h +++ b/rtc_base/experiments/audio_allocation_settings.h @@ -27,13 +27,14 @@ class AudioAllocationSettings { bool IgnoreSeqNumIdChange() const; // Returns true if the bitrate allocation range should be configured. bool ConfigureRateAllocationRange() const; - // Returns true if sent audio packets should have transport wide sequence - // numbers. + // Returns true if the transport sequence number extension should be enabled. + bool EnableTransportSequenceNumberExtension() const; + // Returns true if audio traffic should be included in transport wide feedback + // packets. // |transport_seq_num_extension_header_id| the extension header id for // transport sequence numbers. Set to 0 if not the extension is not // configured. - bool ShouldSendTransportSequenceNumber( - int transport_seq_num_extension_header_id) const; + bool IncludeAudioInFeedback(int transport_seq_num_extension_header_id) const; // Returns true if target bitrate for audio streams should be updated. // |transport_seq_num_extension_header_id| the extension header id for // transport sequence numbers. Set to 0 if not the extension is not diff --git a/video/end_to_end_tests/transport_feedback_tests.cc b/video/end_to_end_tests/transport_feedback_tests.cc index 9aacd9a26f..cc0cb7beb1 100644 --- a/video/end_to_end_tests/transport_feedback_tests.cc +++ b/video/end_to_end_tests/transport_feedback_tests.cc @@ -316,7 +316,6 @@ TEST_F(TransportFeedbackEndToEndTest, VideoTransportFeedbackNotConfigured) { } TEST_F(TransportFeedbackEndToEndTest, AudioReceivesTransportFeedback) { - test::ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/"); TransportFeedbackTester test(true, 0, 1); RunBaseTest(&test); } @@ -425,7 +424,6 @@ TEST_F(TransportFeedbackEndToEndTest, } TEST_F(TransportFeedbackEndToEndTest, TransportSeqNumOnAudioAndVideo) { - test::ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/"); static constexpr int kExtensionId = 8; static constexpr size_t kMinPacketsToWaitFor = 50; class TransportSequenceNumberTest : public test::EndToEndTest {