diff --git a/api/audio_codecs/opus/BUILD.gn b/api/audio_codecs/opus/BUILD.gn index b495661cfd..cbf03c229a 100644 --- a/api/audio_codecs/opus/BUILD.gn +++ b/api/audio_codecs/opus/BUILD.gn @@ -50,6 +50,8 @@ rtc_library("audio_encoder_opus") { "../../../modules/audio_coding:webrtc_opus", "../../../rtc_base:checks", "../../../rtc_base/system:rtc_export", + "../../environment", + "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/types:optional", ] } diff --git a/api/audio_codecs/opus/audio_encoder_opus.cc b/api/audio_codecs/opus/audio_encoder_opus.cc index 4073d0854c..6a6d5664b6 100644 --- a/api/audio_codecs/opus/audio_encoder_opus.cc +++ b/api/audio_codecs/opus/audio_encoder_opus.cc @@ -13,9 +13,11 @@ #include #include +#include "absl/memory/memory.h" #include "absl/types/optional.h" #include "api/audio_codecs/audio_codec_pair_id.h" #include "api/audio_codecs/audio_encoder.h" +#include "api/audio_codecs/audio_encoder_factory.h" #include "api/audio_codecs/audio_format.h" #include "api/audio_codecs/opus/audio_encoder_opus_config.h" #include "api/field_trials_view.h" @@ -48,7 +50,23 @@ std::unique_ptr AudioEncoderOpus::MakeAudioEncoder( RTC_DCHECK_NOTREACHED(); return nullptr; } - return AudioEncoderOpusImpl::MakeAudioEncoder(config, payload_type); +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + // Use WrapUnique to call deprecated constructor. + return absl::WrapUnique(new AudioEncoderOpusImpl(config, payload_type)); +#pragma clang diagnostic pop +} + +std::unique_ptr AudioEncoderOpus::MakeAudioEncoder( + const Environment& env, + const AudioEncoderOpusConfig& config, + const AudioEncoderFactory::Options& options) { + if (!config.IsOk()) { + RTC_DCHECK_NOTREACHED(); + return nullptr; + } + return std::make_unique(env, config, + options.payload_type); } } // namespace webrtc diff --git a/api/audio_codecs/opus/audio_encoder_opus.h b/api/audio_codecs/opus/audio_encoder_opus.h index df93ae5303..414fe45012 100644 --- a/api/audio_codecs/opus/audio_encoder_opus.h +++ b/api/audio_codecs/opus/audio_encoder_opus.h @@ -17,8 +17,10 @@ #include "absl/types/optional.h" #include "api/audio_codecs/audio_codec_pair_id.h" #include "api/audio_codecs/audio_encoder.h" +#include "api/audio_codecs/audio_encoder_factory.h" #include "api/audio_codecs/audio_format.h" #include "api/audio_codecs/opus/audio_encoder_opus_config.h" +#include "api/environment/environment.h" #include "api/field_trials_view.h" #include "rtc_base/system/rtc_export.h" @@ -32,6 +34,10 @@ struct RTC_EXPORT AudioEncoderOpus { const SdpAudioFormat& audio_format); static void AppendSupportedEncoders(std::vector* specs); static AudioCodecInfo QueryAudioEncoder(const AudioEncoderOpusConfig& config); + static std::unique_ptr MakeAudioEncoder( + const Environment& env, + const AudioEncoderOpusConfig& config, + const AudioEncoderFactory::Options& options); static std::unique_ptr MakeAudioEncoder( const AudioEncoderOpusConfig& config, int payload_type, diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn index bb707774a3..20c55108d7 100644 --- a/modules/audio_coding/BUILD.gn +++ b/modules/audio_coding/BUILD.gn @@ -469,8 +469,11 @@ rtc_library("webrtc_opus") { ":audio_coding_opus_common", ":audio_network_adaptor", "../../api:array_view", + "../../api:field_trials_view", "../../api/audio_codecs:audio_codecs_api", "../../api/audio_codecs/opus:audio_encoder_opus_config", + "../../api/environment", + "../../api/transport:field_trial_based_config", "../../common_audio", "../../rtc_base:buffer", "../../rtc_base:checks", @@ -483,6 +486,7 @@ rtc_library("webrtc_opus") { "../../rtc_base:stringutils", "../../rtc_base:timeutils", "../../system_wrappers:field_trial", + "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/strings:string_view", "//third_party/abseil-cpp/absl/types:optional", @@ -1723,6 +1727,7 @@ if (rtc_include_tests) { "../../system_wrappers", "../../test:audio_codec_mocks", "../../test:audio_test_common", + "../../test:explicit_key_value_config", "../../test:field_trial", "../../test:fileutils", "../../test:rtc_expect_death", diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc index 51b0fcd492..8437706122 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc @@ -16,8 +16,11 @@ #include #include +#include "absl/memory/memory.h" #include "absl/strings/match.h" #include "absl/strings/string_view.h" +#include "api/field_trials_view.h" +#include "api/transport/field_trial_based_config.h" #include "modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h" #include "modules/audio_coding/audio_network_adaptor/controller_manager.h" #include "modules/audio_coding/codecs/opus/audio_coder_opus_common.h" @@ -31,7 +34,6 @@ #include "rtc_base/string_encode.h" #include "rtc_base/string_to_number.h" #include "rtc_base/time_utils.h" -#include "system_wrappers/include/field_trial.h" namespace webrtc { @@ -163,14 +165,14 @@ int GetBitrateBps(const AudioEncoderOpusConfig& config) { return *config.bitrate_bps; } -std::vector GetBitrateMultipliers() { +std::vector GetBitrateMultipliers(const FieldTrialsView& field_trials) { constexpr char kBitrateMultipliersName[] = "WebRTC-Audio-OpusBitrateMultipliers"; const bool use_bitrate_multipliers = - webrtc::field_trial::IsEnabled(kBitrateMultipliersName); + field_trials.IsEnabled(kBitrateMultipliersName); if (use_bitrate_multipliers) { const std::string field_trial_string = - webrtc::field_trial::FindFullName(kBitrateMultipliersName); + field_trials.Lookup(kBitrateMultipliersName); std::vector pieces; rtc::tokenize(field_trial_string, '-', &pieces); if (pieces.size() < 2 || pieces[0] != "Enabled") { @@ -227,16 +229,6 @@ AudioCodecInfo AudioEncoderOpusImpl::QueryAudioEncoder( return info; } -std::unique_ptr AudioEncoderOpusImpl::MakeAudioEncoder( - const AudioEncoderOpusConfig& config, - int payload_type) { - if (!config.IsOk()) { - RTC_DCHECK_NOTREACHED(); - return nullptr; - } - return std::make_unique(config, payload_type); -} - absl::optional AudioEncoderOpusImpl::SdpToConfig( const SdpAudioFormat& format) { if (!absl::EqualsIgnoreCase(format.name, "opus") || @@ -345,9 +337,35 @@ class AudioEncoderOpusImpl::PacketLossFractionSmoother { rtc::ExpFilter smoother_; }; +std::unique_ptr AudioEncoderOpusImpl::CreateForTesting( + const Environment& env, + const AudioEncoderOpusConfig& config, + int payload_type, + const AudioNetworkAdaptorCreator& audio_network_adaptor_creator, + std::unique_ptr bitrate_smoother) { + // Using `new` to access a non-public constructor. + return absl::WrapUnique(new AudioEncoderOpusImpl( + env.field_trials(), config, payload_type, audio_network_adaptor_creator, + std::move(bitrate_smoother))); +} + +AudioEncoderOpusImpl::AudioEncoderOpusImpl(const Environment& env, + const AudioEncoderOpusConfig& config, + int payload_type) + : AudioEncoderOpusImpl( + env.field_trials(), + config, + payload_type, + [this](absl::string_view config_string, RtcEventLog* event_log) { + return DefaultAudioNetworkAdaptorCreator(config_string, event_log); + }, + // We choose 5sec as initial time constant due to empirical data. + std::make_unique(5'000)) {} + AudioEncoderOpusImpl::AudioEncoderOpusImpl(const AudioEncoderOpusConfig& config, int payload_type) : AudioEncoderOpusImpl( + FieldTrialBasedConfig(), config, payload_type, [this](absl::string_view config_string, RtcEventLog* event_log) { @@ -357,17 +375,17 @@ AudioEncoderOpusImpl::AudioEncoderOpusImpl(const AudioEncoderOpusConfig& config, std::make_unique(5000)) {} AudioEncoderOpusImpl::AudioEncoderOpusImpl( + const FieldTrialsView& field_trials, const AudioEncoderOpusConfig& config, int payload_type, const AudioNetworkAdaptorCreator& audio_network_adaptor_creator, std::unique_ptr bitrate_smoother) : payload_type_(payload_type), - use_stable_target_for_adaptation_(!webrtc::field_trial::IsDisabled( - "WebRTC-Audio-StableTargetAdaptation")), - adjust_bandwidth_( - webrtc::field_trial::IsEnabled("WebRTC-AdjustOpusBandwidth")), + use_stable_target_for_adaptation_( + !field_trials.IsDisabled("WebRTC-Audio-StableTargetAdaptation")), + adjust_bandwidth_(field_trials.IsEnabled("WebRTC-AdjustOpusBandwidth")), bitrate_changed_(true), - bitrate_multipliers_(GetBitrateMultipliers()), + bitrate_multipliers_(GetBitrateMultipliers(field_trials)), packet_loss_rate_(0.0), inst_(nullptr), packet_loss_fraction_smoother_(new PacketLossFractionSmoother()), @@ -384,10 +402,6 @@ AudioEncoderOpusImpl::AudioEncoderOpusImpl( SetProjectedPacketLossRate(packet_loss_rate_); } -AudioEncoderOpusImpl::AudioEncoderOpusImpl(int payload_type, - const SdpAudioFormat& format) - : AudioEncoderOpusImpl(*SdpToConfig(format), payload_type) {} - AudioEncoderOpusImpl::~AudioEncoderOpusImpl() { RTC_CHECK_EQ(0, WebRtcOpus_EncoderFree(inst_)); } diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.h b/modules/audio_coding/codecs/opus/audio_encoder_opus.h index 8c5c235016..7568631b4e 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus.h +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.h @@ -21,6 +21,7 @@ #include "api/audio_codecs/audio_encoder.h" #include "api/audio_codecs/audio_format.h" #include "api/audio_codecs/opus/audio_encoder_opus_config.h" +#include "api/environment/environment.h" #include "common_audio/smoothing_filter.h" #include "modules/audio_coding/audio_network_adaptor/include/audio_network_adaptor.h" #include "modules/audio_coding/codecs/opus/opus_interface.h" @@ -49,16 +50,21 @@ class AudioEncoderOpusImpl final : public AudioEncoder { std::function(absl::string_view, RtcEventLog*)>; - AudioEncoderOpusImpl(const AudioEncoderOpusConfig& config, int payload_type); - - // Dependency injection for testing. - AudioEncoderOpusImpl( + static std::unique_ptr CreateForTesting( + const Environment& env, const AudioEncoderOpusConfig& config, int payload_type, const AudioNetworkAdaptorCreator& audio_network_adaptor_creator, std::unique_ptr bitrate_smoother); - AudioEncoderOpusImpl(int payload_type, const SdpAudioFormat& format); + AudioEncoderOpusImpl(const Environment& env, + const AudioEncoderOpusConfig& config, + int payload_type); + + [[deprecated("bugs.webrtc.org/343086059")]] AudioEncoderOpusImpl( + const AudioEncoderOpusConfig& config, + int payload_type); + ~AudioEncoderOpusImpl() override; AudioEncoderOpusImpl(const AudioEncoderOpusImpl&) = delete; @@ -120,13 +126,19 @@ class AudioEncoderOpusImpl final : public AudioEncoder { private: class PacketLossFractionSmoother; + // TODO: bugs.webrtc.org/343086059 - Replace field_trials with Environment + // when public constructors that do not provide the Environment are removed. + AudioEncoderOpusImpl( + const FieldTrialsView& field_trials, + const AudioEncoderOpusConfig& config, + int payload_type, + const AudioNetworkAdaptorCreator& audio_network_adaptor_creator, + std::unique_ptr bitrate_smoother); + static absl::optional SdpToConfig( const SdpAudioFormat& format); static void AppendSupportedEncoders(std::vector* specs); static AudioCodecInfo QueryAudioEncoder(const AudioEncoderOpusConfig& config); - static std::unique_ptr MakeAudioEncoder( - const AudioEncoderOpusConfig&, - int payload_type); size_t Num10msFramesPerPacket() const; size_t SamplesPer10msFrame() const; 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 f82ef965db..996bf53f03 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc @@ -15,6 +15,7 @@ #include #include "absl/strings/string_view.h" +#include "api/environment/environment_factory.h" #include "common_audio/mocks/mock_smoothing_filter.h" #include "modules/audio_coding/audio_network_adaptor/mock/mock_audio_network_adaptor.h" #include "modules/audio_coding/codecs/opus/audio_encoder_opus.h" @@ -22,17 +23,18 @@ #include "modules/audio_coding/neteq/tools/audio_loop.h" #include "rtc_base/checks.h" #include "rtc_base/fake_clock.h" -#include "test/field_trial.h" +#include "test/explicit_key_value_config.h" #include "test/gmock.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" #include "test/testsupport/file_utils.h" namespace webrtc { +namespace { +using test::ExplicitKeyValueConfig; using ::testing::NiceMock; using ::testing::Return; -namespace { - constexpr int kDefaultOpusPayloadType = 105; constexpr int kDefaultOpusRate = 32000; constexpr int kDefaultOpusPacSize = 960; @@ -52,8 +54,10 @@ struct AudioEncoderOpusStates { AudioEncoderOpusConfig config; }; -std::unique_ptr CreateCodec(int sample_rate_hz, - size_t num_channels) { +std::unique_ptr CreateCodec( + int sample_rate_hz, + size_t num_channels, + const FieldTrialsView* field_trials = nullptr) { std::unique_ptr states = std::make_unique(); states->mock_audio_network_adaptor = nullptr; @@ -85,9 +89,9 @@ std::unique_ptr CreateCodec(int sample_rate_hz, new MockSmoothingFilter()); states->mock_bitrate_smoother = bitrate_smoother.get(); - states->encoder.reset( - new AudioEncoderOpusImpl(states->config, kDefaultOpusPayloadType, creator, - std::move(bitrate_smoother))); + states->encoder = AudioEncoderOpusImpl::CreateForTesting( + CreateEnvironment(field_trials), states->config, kDefaultOpusPayloadType, + creator, std::move(bitrate_smoother)); return states; } @@ -264,9 +268,9 @@ TEST_P(AudioEncoderOpusTest, TEST_P(AudioEncoderOpusTest, InvokeAudioNetworkAdaptorOnReceivedUplinkBandwidth) { - test::ScopedFieldTrials override_field_trials( + ExplicitKeyValueConfig field_trials( "WebRTC-Audio-StableTargetAdaptation/Disabled/"); - auto states = CreateCodec(sample_rate_hz_, 2); + auto states = CreateCodec(sample_rate_hz_, 2, &field_trials); states->encoder->EnableAudioNetworkAdaptor("", nullptr); auto config = CreateEncoderRuntimeConfig(); @@ -485,9 +489,9 @@ TEST_P(AudioEncoderOpusTest, EmptyConfigDoesNotAffectEncoderSettings) { } TEST_P(AudioEncoderOpusTest, UpdateUplinkBandwidthInAudioNetworkAdaptor) { - test::ScopedFieldTrials override_field_trials( + ExplicitKeyValueConfig field_trials( "WebRTC-Audio-StableTargetAdaptation/Disabled/"); - auto states = CreateCodec(sample_rate_hz_, 2); + auto states = CreateCodec(sample_rate_hz_, 2, &field_trials); states->encoder->EnableAudioNetworkAdaptor("", nullptr); const size_t opus_rate_khz = rtc::CheckedDivExact(sample_rate_hz_, 1000); const std::vector audio(opus_rate_khz * 10 * 2, 0); @@ -819,7 +823,16 @@ TEST_P(AudioEncoderOpusTest, OpusFlagDtxAsNonSpeech) { } TEST(AudioEncoderOpusTest, OpusDtxFilteringHighEnergyRefreshPackets) { - test::ScopedFieldTrials override_field_trials( + // TODO: bugs.webrtc.org/343086059 - Use `ExplicitKeyValueConfig` type to + // ensure global field trial string is not used when this field trial is + // queried from the passed Environment. + // There are currently two complications for that: + // - field trial is queried by WebRtcOpus_EncoderCreate that follows c-style + // interface, and thus is not ready to accept c++ interface FieldTrialsView + // - field trial is queried during `RecreateEncoderInstance`, i.e., opus + // encoder needs to save field trials passed at construction. That will be + // simpler once all public constructors accept webrtc::Environment. + test::ScopedKeyValueConfig field_trials( "WebRTC-Audio-OpusAvoidNoisePumpingDuringDtx/Enabled/"); const std::string kInputFileName = webrtc::test::ResourcePath("audio_coding/testfile16kHz", "pcm"); @@ -828,7 +841,8 @@ TEST(AudioEncoderOpusTest, OpusDtxFilteringHighEnergyRefreshPackets) { config.dtx_enabled = true; config.sample_rate_hz = kSampleRateHz; constexpr int payload_type = 17; - const auto encoder = AudioEncoderOpus::MakeAudioEncoder(config, payload_type); + AudioEncoderOpusImpl encoder(CreateEnvironment(&field_trials), config, + payload_type); test::AudioLoop audio_loop; constexpr size_t kMaxLoopLengthSaples = kSampleRateHz * 11.6f; constexpr size_t kInputBlockSizeSamples = kSampleRateHz / 100; @@ -849,7 +863,7 @@ TEST(AudioEncoderOpusTest, OpusDtxFilteringHighEnergyRefreshPackets) { // Every second call to the encoder will generate an Opus packet. for (int j = 0; j < 2; j++) { auto next_frame = audio_loop.GetNextBlock(); - info = encoder->Encode(rtp_timestamp, next_frame, &encoded); + info = encoder.Encode(rtp_timestamp, next_frame, &encoded); if (opus_entered_dtx) { size_t silence_frame_start = rtp_timestamp - timestamp_start_silence; silence_filled = silence_frame_start >= kSilenceDurationSamples; @@ -890,7 +904,7 @@ TEST(AudioEncoderOpusTest, OpusDtxFilteringHighEnergyRefreshPackets) { silence.begin() + silence_frame_start, silence.begin() + silence_frame_start + kInputBlockSizeSamples, silence_frame.begin(), [gain](float s) { return gain * s; }); - info = encoder->Encode(rtp_timestamp, silence_frame, &encoded); + info = encoder.Encode(rtp_timestamp, silence_frame, &encoded); rtp_timestamp += kInputBlockSizeSamples; } EXPECT_TRUE(info.encoded_bytes > 0 || last_packet_dtx_frame);