From defe1358a5da8e27356bac6f7ae04ac9d7ee714c Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 12 Aug 2024 12:50:44 +0200 Subject: [PATCH] Pass Environment into NetEqImpl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To propagate field trials in addition to clock Bug: webrtc:356878416 Change-Id: Idefc4848ec4af30c8aed0f93b7fadfc3181bddb1 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/358980 Commit-Queue: Danil Chapovalov Reviewed-by: Jakob Ivarsson‎ Cr-Commit-Position: refs/heads/main@{#42761} --- api/neteq/BUILD.gn | 4 +--- api/neteq/custom_neteq_factory.cc | 9 +++++---- api/neteq/custom_neteq_factory.h | 8 ++++---- api/neteq/neteq_factory.h | 15 +-------------- modules/audio_coding/BUILD.gn | 3 ++- .../audio_coding/neteq/default_neteq_factory.cc | 9 +++++---- .../audio_coding/neteq/default_neteq_factory.h | 8 ++++---- modules/audio_coding/neteq/neteq_impl.cc | 12 ++++++------ modules/audio_coding/neteq/neteq_impl.h | 12 ++++++------ modules/audio_coding/neteq/neteq_impl_unittest.cc | 10 ++++++---- 10 files changed, 40 insertions(+), 50 deletions(-) diff --git a/api/neteq/BUILD.gn b/api/neteq/BUILD.gn index 63881ae6b8..5678d2b731 100644 --- a/api/neteq/BUILD.gn +++ b/api/neteq/BUILD.gn @@ -21,9 +21,7 @@ rtc_source_set("neteq_api") { "..:rtp_headers", "..:rtp_packet_info", "..:scoped_refptr", - "../../rtc_base:checks", "../../rtc_base:stringutils", - "../../system_wrappers:system_wrappers", "../audio_codecs:audio_codecs_api", "../environment", "../units:timestamp", @@ -43,8 +41,8 @@ rtc_source_set("custom_neteq_factory") { ":neteq_controller_api", "..:scoped_refptr", "../../modules/audio_coding:neteq", - "../../system_wrappers:system_wrappers", "../audio_codecs:audio_codecs_api", + "../environment", ] } diff --git a/api/neteq/custom_neteq_factory.cc b/api/neteq/custom_neteq_factory.cc index b2df5df9ff..82ee7b3e48 100644 --- a/api/neteq/custom_neteq_factory.cc +++ b/api/neteq/custom_neteq_factory.cc @@ -12,6 +12,7 @@ #include +#include "api/environment/environment.h" #include "modules/audio_coding/neteq/neteq_impl.h" namespace webrtc { @@ -22,12 +23,12 @@ CustomNetEqFactory::CustomNetEqFactory( CustomNetEqFactory::~CustomNetEqFactory() = default; -std::unique_ptr CustomNetEqFactory::CreateNetEq( +std::unique_ptr CustomNetEqFactory::Create( + const Environment& env, const NetEq::Config& config, - const rtc::scoped_refptr& decoder_factory, - Clock* clock) const { + scoped_refptr decoder_factory) const { return std::make_unique( - config, NetEqImpl::Dependencies(config, clock, decoder_factory, + config, NetEqImpl::Dependencies(env, config, std::move(decoder_factory), *controller_factory_)); } diff --git a/api/neteq/custom_neteq_factory.h b/api/neteq/custom_neteq_factory.h index 1e9a1fca9d..b77eb06894 100644 --- a/api/neteq/custom_neteq_factory.h +++ b/api/neteq/custom_neteq_factory.h @@ -14,11 +14,11 @@ #include #include "api/audio_codecs/audio_decoder_factory.h" +#include "api/environment/environment.h" #include "api/neteq/neteq.h" #include "api/neteq/neteq_controller_factory.h" #include "api/neteq/neteq_factory.h" #include "api/scoped_refptr.h" -#include "system_wrappers/include/clock.h" namespace webrtc { @@ -32,10 +32,10 @@ class CustomNetEqFactory : public NetEqFactory { CustomNetEqFactory(const CustomNetEqFactory&) = delete; CustomNetEqFactory& operator=(const CustomNetEqFactory&) = delete; - std::unique_ptr CreateNetEq( + std::unique_ptr Create( + const Environment& env, const NetEq::Config& config, - const rtc::scoped_refptr& decoder_factory, - Clock* clock) const override; + scoped_refptr decoder_factory) const override; private: std::unique_ptr controller_factory_; diff --git a/api/neteq/neteq_factory.h b/api/neteq/neteq_factory.h index 8ee3680f22..98165c9841 100644 --- a/api/neteq/neteq_factory.h +++ b/api/neteq/neteq_factory.h @@ -17,8 +17,6 @@ #include "api/environment/environment.h" #include "api/neteq/neteq.h" #include "api/scoped_refptr.h" -#include "rtc_base/checks.h" -#include "system_wrappers/include/clock.h" namespace webrtc { @@ -33,18 +31,7 @@ class NetEqFactory { virtual std::unique_ptr Create( const Environment& env, const NetEq::Config& config, - scoped_refptr decoder_factory) const { - return CreateNetEq(config, decoder_factory, &env.clock()); - } - - virtual std::unique_ptr CreateNetEq( - const NetEq::Config& config, - const rtc::scoped_refptr& decoder_factory, - Clock* clock) const { - // TODO: b/356878416 - Delete this function when all callers are migrated - // to `Create` function above. - RTC_CHECK_NOTREACHED(); - } + scoped_refptr decoder_factory) const = 0; }; } // namespace webrtc diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn index a9559205b4..15eb29c787 100644 --- a/modules/audio_coding/BUILD.gn +++ b/modules/audio_coding/BUILD.gn @@ -716,6 +716,7 @@ rtc_library("neteq") { "../../api:scoped_refptr", "../../api/audio:audio_frame_api", "../../api/audio_codecs:audio_codecs_api", + "../../api/environment", "../../api/neteq:neteq_api", "../../api/neteq:neteq_controller_api", "../../api/neteq:tick_timer", @@ -754,9 +755,9 @@ rtc_source_set("default_neteq_factory") { ":neteq", "../../api:scoped_refptr", "../../api/audio_codecs:audio_codecs_api", + "../../api/environment", "../../api/neteq:default_neteq_controller_factory", "../../api/neteq:neteq_api", - "../../system_wrappers:system_wrappers", ] } diff --git a/modules/audio_coding/neteq/default_neteq_factory.cc b/modules/audio_coding/neteq/default_neteq_factory.cc index 487450fe0f..6c04a8a127 100644 --- a/modules/audio_coding/neteq/default_neteq_factory.cc +++ b/modules/audio_coding/neteq/default_neteq_factory.cc @@ -12,6 +12,7 @@ #include +#include "api/environment/environment.h" #include "modules/audio_coding/neteq/neteq_impl.h" namespace webrtc { @@ -19,12 +20,12 @@ namespace webrtc { DefaultNetEqFactory::DefaultNetEqFactory() = default; DefaultNetEqFactory::~DefaultNetEqFactory() = default; -std::unique_ptr DefaultNetEqFactory::CreateNetEq( +std::unique_ptr DefaultNetEqFactory::Create( + const Environment& env, const NetEq::Config& config, - const rtc::scoped_refptr& decoder_factory, - Clock* clock) const { + scoped_refptr decoder_factory) const { return std::make_unique( - config, NetEqImpl::Dependencies(config, clock, decoder_factory, + config, NetEqImpl::Dependencies(env, config, std::move(decoder_factory), controller_factory_)); } diff --git a/modules/audio_coding/neteq/default_neteq_factory.h b/modules/audio_coding/neteq/default_neteq_factory.h index 24d2bae419..a0c44d1d9d 100644 --- a/modules/audio_coding/neteq/default_neteq_factory.h +++ b/modules/audio_coding/neteq/default_neteq_factory.h @@ -14,10 +14,10 @@ #include #include "api/audio_codecs/audio_decoder_factory.h" +#include "api/environment/environment.h" #include "api/neteq/default_neteq_controller_factory.h" #include "api/neteq/neteq_factory.h" #include "api/scoped_refptr.h" -#include "system_wrappers/include/clock.h" namespace webrtc { @@ -28,10 +28,10 @@ class DefaultNetEqFactory : public NetEqFactory { DefaultNetEqFactory(const DefaultNetEqFactory&) = delete; DefaultNetEqFactory& operator=(const DefaultNetEqFactory&) = delete; - std::unique_ptr CreateNetEq( + std::unique_ptr Create( + const Environment& env, const NetEq::Config& config, - const rtc::scoped_refptr& decoder_factory, - Clock* clock) const override; + scoped_refptr decoder_factory) const override; private: const DefaultNetEqControllerFactory controller_factory_; diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc index fa3d47aace..5ea78e3424 100644 --- a/modules/audio_coding/neteq/neteq_impl.cc +++ b/modules/audio_coding/neteq/neteq_impl.cc @@ -107,11 +107,11 @@ bool EqualSampleRates(uint8_t pt1, } // namespace NetEqImpl::Dependencies::Dependencies( + const Environment& env, const NetEq::Config& config, - Clock* clock, - const rtc::scoped_refptr& decoder_factory, + scoped_refptr decoder_factory, const NetEqControllerFactory& controller_factory) - : clock(clock), + : env(env), tick_timer(new TickTimer), stats(new StatisticsCalculator), decoder_database( @@ -127,7 +127,7 @@ NetEqImpl::Dependencies::Dependencies( config.max_packets_in_buffer, !config.for_test_no_time_stretching, tick_timer.get(), - clock)), + &env.clock())), red_payload_splitter(new RedPayloadSplitter), timestamp_scaler(new TimestampScaler(*decoder_database)), accelerate_factory(new AccelerateFactory), @@ -139,7 +139,7 @@ NetEqImpl::Dependencies::~Dependencies() = default; NetEqImpl::NetEqImpl(const NetEq::Config& config, Dependencies&& deps, bool create_components) - : clock_(deps.clock), + : env_(deps.env), tick_timer_(std::move(deps.tick_timer)), decoder_database_(std::move(deps.decoder_database)), dtmf_buffer_(std::move(deps.dtmf_buffer)), @@ -1981,7 +1981,7 @@ int NetEqImpl::ExtractPackets(size_t required_samples, if (packet->packet_info.has_value() && !packet->packet_info->receive_time().IsMinusInfinity()) { processing_time = - clock_->CurrentTime() - packet->packet_info->receive_time(); + env_.clock().CurrentTime() - packet->packet_info->receive_time(); } stats_->JitterBufferDelay( diff --git a/modules/audio_coding/neteq/neteq_impl.h b/modules/audio_coding/neteq/neteq_impl.h index 8730d9a85f..73d2920407 100644 --- a/modules/audio_coding/neteq/neteq_impl.h +++ b/modules/audio_coding/neteq/neteq_impl.h @@ -20,6 +20,7 @@ #include "absl/types/optional.h" #include "api/audio/audio_frame.h" +#include "api/environment/environment.h" #include "api/neteq/neteq.h" #include "api/neteq/neteq_controller.h" #include "api/neteq/neteq_controller_factory.h" @@ -40,7 +41,6 @@ namespace webrtc { // Forward declarations. class Accelerate; class BackgroundNoise; -class Clock; class ComfortNoise; class DecoderDatabase; class DtmfBuffer; @@ -96,13 +96,13 @@ class NetEqImpl : public webrtc::NetEq { // before sending the struct to the NetEqImpl constructor. However, there // are dependencies between some of the classes inside the struct, so // swapping out one may make it necessary to re-create another one. - Dependencies(const NetEq::Config& config, - Clock* clock, - const rtc::scoped_refptr& decoder_factory, + Dependencies(const Environment& env, + const NetEq::Config& config, + scoped_refptr decoder_factory, const NetEqControllerFactory& controller_factory); ~Dependencies(); - Clock* const clock; + const Environment env; std::unique_ptr tick_timer; std::unique_ptr stats; std::unique_ptr decoder_database; @@ -340,7 +340,7 @@ class NetEqImpl : public webrtc::NetEq { NetEqController::PacketArrivedInfo ToPacketArrivedInfo( const Packet& packet) const RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_); - Clock* const clock_; + const Environment env_; mutable Mutex mutex_; const std::unique_ptr tick_timer_ RTC_GUARDED_BY(mutex_); diff --git a/modules/audio_coding/neteq/neteq_impl_unittest.cc b/modules/audio_coding/neteq/neteq_impl_unittest.cc index 9e43c4a1f6..c28bd8c453 100644 --- a/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -72,13 +72,14 @@ int DeletePacketsAndReturnOk(PacketList* packet_list) { class NetEqImplTest : public ::testing::Test { protected: - NetEqImplTest() : clock_(0) { config_.sample_rate_hz = 8000; } + NetEqImplTest() : clock_(0), env_(CreateEnvironment(&clock_)) { + config_.sample_rate_hz = 8000; + } - void CreateInstance( - const rtc::scoped_refptr& decoder_factory) { + void CreateInstance(scoped_refptr decoder_factory) { ASSERT_TRUE(decoder_factory); config_.enable_muted_state = enable_muted_state_; - NetEqImpl::Dependencies deps(config_, &clock_, decoder_factory, + NetEqImpl::Dependencies deps(env_, config_, std::move(decoder_factory), DefaultNetEqControllerFactory()); // Get a local pointer to NetEq's TickTimer object. @@ -230,6 +231,7 @@ class NetEqImplTest : public ::testing::Test { std::unique_ptr neteq_; NetEq::Config config_; SimulatedClock clock_; + const Environment env_; TickTimer* tick_timer_ = nullptr; MockDecoderDatabase* mock_decoder_database_ = nullptr; DecoderDatabase* decoder_database_ = nullptr;