From b64eef12341ea75ef4b3f40a8bc3490ffecb5e76 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Thu, 4 Jan 2024 14:45:23 +0100 Subject: [PATCH] In AecDump take raw pointer to TaskQueueBase instead of legacy rtc::TaskQueue Bug: webrtc:14169 Change-Id: I1e50a945a7637da07bec00ccd7b6b1847a7481cd Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/333480 Reviewed-by: Sam Zackrisson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#41477} --- modules/audio_processing/BUILD.gn | 1 + modules/audio_processing/aec_dump/BUILD.gn | 19 ++++++-- .../aec_dump/aec_dump_factory.h | 46 +++++++++++-------- .../aec_dump/aec_dump_impl.cc | 26 ++++++----- .../audio_processing/aec_dump/aec_dump_impl.h | 6 +-- .../aec_dump/aec_dump_unittest.cc | 4 +- .../aec_dump/null_aec_dump_factory.cc | 23 ++++++---- .../audio_processing/audio_processing_impl.cc | 7 +-- .../audio_processing_unittest.cc | 10 ++-- .../test/audio_processing_simulator.cc | 2 +- .../audio_processing/test/debug_dump_test.cc | 2 +- test/fuzzers/BUILD.gn | 7 ++- .../audio_processing_configs_fuzzer.cc | 17 ++++--- 13 files changed, 104 insertions(+), 66 deletions(-) diff --git a/modules/audio_processing/BUILD.gn b/modules/audio_processing/BUILD.gn index 6aca7dee46..3611c1a342 100644 --- a/modules/audio_processing/BUILD.gn +++ b/modules/audio_processing/BUILD.gn @@ -193,6 +193,7 @@ rtc_library("audio_processing") { "../../rtc_base:gtest_prod", "../../rtc_base:logging", "../../rtc_base:macromagic", + "../../rtc_base:rtc_task_queue", "../../rtc_base:safe_minmax", "../../rtc_base:sanitizer", "../../rtc_base:swap_queue", diff --git a/modules/audio_processing/aec_dump/BUILD.gn b/modules/audio_processing/aec_dump/BUILD.gn index 78bae56835..ffbc098ed4 100644 --- a/modules/audio_processing/aec_dump/BUILD.gn +++ b/modules/audio_processing/aec_dump/BUILD.gn @@ -14,10 +14,15 @@ rtc_source_set("aec_dump") { deps = [ "..:aec_dump_interface", + "../../../api/task_queue", + "../../../rtc_base:rtc_task_queue", "../../../rtc_base/system:file_wrapper", "../../../rtc_base/system:rtc_export", ] - absl_deps = [ "//third_party/abseil-cpp/absl/strings" ] + absl_deps = [ + "//third_party/abseil-cpp/absl/base:nullability", + "//third_party/abseil-cpp/absl/strings", + ] } if (rtc_include_tests) { @@ -71,11 +76,13 @@ if (rtc_enable_protobuf) { "../../../rtc_base:protobuf_utils", "../../../rtc_base:race_checker", "../../../rtc_base:rtc_event", - "../../../rtc_base:rtc_task_queue", "../../../rtc_base/system:file_wrapper", "../../../system_wrappers", ] - absl_deps = [ "//third_party/abseil-cpp/absl/strings" ] + absl_deps = [ + "//third_party/abseil-cpp/absl/base:nullability", + "//third_party/abseil-cpp/absl/strings", + ] deps += [ "../:audioproc_debug_proto" ] } @@ -106,6 +113,10 @@ rtc_library("null_aec_dump_factory") { deps = [ ":aec_dump", "..:aec_dump_interface", + "../../../api/task_queue", + ] + absl_deps = [ + "//third_party/abseil-cpp/absl/base:nullability", + "//third_party/abseil-cpp/absl/strings", ] - absl_deps = [ "//third_party/abseil-cpp/absl/strings" ] } diff --git a/modules/audio_processing/aec_dump/aec_dump_factory.h b/modules/audio_processing/aec_dump/aec_dump_factory.h index 20718c3d7f..c76e9739ea 100644 --- a/modules/audio_processing/aec_dump/aec_dump_factory.h +++ b/modules/audio_processing/aec_dump/aec_dump_factory.h @@ -13,34 +13,44 @@ #include +#include "absl/base/nullability.h" #include "absl/strings/string_view.h" +#include "api/task_queue/task_queue_base.h" #include "modules/audio_processing/include/aec_dump.h" #include "rtc_base/system/file_wrapper.h" #include "rtc_base/system/rtc_export.h" - -namespace rtc { -class TaskQueue; -} // namespace rtc +#include "rtc_base/task_queue.h" namespace webrtc { class RTC_EXPORT AecDumpFactory { public: - // The `worker_queue` may not be null and must outlive the created - // AecDump instance. `max_log_size_bytes == -1` means the log size - // will be unlimited. `handle` may not be null. The AecDump takes - // responsibility for `handle` and closes it in the destructor. A - // non-null return value indicates that the file has been + // The `worker_queue` must outlive the created AecDump instance. + // `max_log_size_bytes == -1` means the log size will be unlimited. + // The AecDump takes responsibility for `handle` and closes it in the + // destructor. A non-null return value indicates that the file has been // sucessfully opened. - static std::unique_ptr Create(webrtc::FileWrapper file, - int64_t max_log_size_bytes, - rtc::TaskQueue* worker_queue); - static std::unique_ptr Create(absl::string_view file_name, - int64_t max_log_size_bytes, - rtc::TaskQueue* worker_queue); - static std::unique_ptr Create(FILE* handle, - int64_t max_log_size_bytes, - rtc::TaskQueue* worker_queue); + static absl::Nullable> Create( + FileWrapper file, + int64_t max_log_size_bytes, + absl::Nonnull worker_queue); + static absl::Nullable> Create( + absl::string_view file_name, + int64_t max_log_size_bytes, + absl::Nonnull worker_queue); + static absl::Nullable> Create( + absl::Nonnull handle, + int64_t max_log_size_bytes, + absl::Nonnull worker_queue); + + // TODO: bugs.webrtc.org/14169 - Delete this variant when no longer used by + // chromium. + static absl::Nullable> Create( + absl::Nonnull handle, + int64_t max_log_size_bytes, + absl::Nonnull worker_queue) { + return Create(handle, max_log_size_bytes, worker_queue->Get()); + } }; } // namespace webrtc diff --git a/modules/audio_processing/aec_dump/aec_dump_impl.cc b/modules/audio_processing/aec_dump/aec_dump_impl.cc index 94c24048e0..8484fcc6e1 100644 --- a/modules/audio_processing/aec_dump/aec_dump_impl.cc +++ b/modules/audio_processing/aec_dump/aec_dump_impl.cc @@ -13,11 +13,12 @@ #include #include +#include "absl/base/nullability.h" #include "absl/strings/string_view.h" +#include "api/task_queue/task_queue_base.h" #include "modules/audio_processing/aec_dump/aec_dump_factory.h" #include "rtc_base/checks.h" #include "rtc_base/event.h" -#include "rtc_base/task_queue.h" namespace webrtc { @@ -59,7 +60,7 @@ void CopyFromConfigToEvent(const webrtc::InternalAPMConfig& config, AecDumpImpl::AecDumpImpl(FileWrapper debug_file, int64_t max_log_size_bytes, - rtc::TaskQueue* worker_queue) + absl::Nonnull worker_queue) : debug_file_(std::move(debug_file)), num_bytes_left_for_log_(max_log_size_bytes), worker_queue_(worker_queue) {} @@ -254,9 +255,10 @@ void AecDumpImpl::PostWriteToFileTask(std::unique_ptr event) { }); } -std::unique_ptr AecDumpFactory::Create(webrtc::FileWrapper file, - int64_t max_log_size_bytes, - rtc::TaskQueue* worker_queue) { +absl::Nullable> AecDumpFactory::Create( + FileWrapper file, + int64_t max_log_size_bytes, + absl::Nonnull worker_queue) { RTC_DCHECK(worker_queue); if (!file.is_open()) return nullptr; @@ -265,16 +267,18 @@ std::unique_ptr AecDumpFactory::Create(webrtc::FileWrapper file, worker_queue); } -std::unique_ptr AecDumpFactory::Create(absl::string_view file_name, - int64_t max_log_size_bytes, - rtc::TaskQueue* worker_queue) { +absl::Nullable> AecDumpFactory::Create( + absl::string_view file_name, + int64_t max_log_size_bytes, + absl::Nonnull worker_queue) { return Create(FileWrapper::OpenWriteOnly(file_name), max_log_size_bytes, worker_queue); } -std::unique_ptr AecDumpFactory::Create(FILE* handle, - int64_t max_log_size_bytes, - rtc::TaskQueue* worker_queue) { +absl::Nullable> AecDumpFactory::Create( + absl::Nonnull handle, + int64_t max_log_size_bytes, + absl::Nonnull worker_queue) { return Create(FileWrapper(handle), max_log_size_bytes, worker_queue); } diff --git a/modules/audio_processing/aec_dump/aec_dump_impl.h b/modules/audio_processing/aec_dump/aec_dump_impl.h index 429808f9af..d5af31b01e 100644 --- a/modules/audio_processing/aec_dump/aec_dump_impl.h +++ b/modules/audio_processing/aec_dump/aec_dump_impl.h @@ -15,11 +15,11 @@ #include #include +#include "api/task_queue/task_queue_base.h" #include "modules/audio_processing/aec_dump/capture_stream_info.h" #include "modules/audio_processing/include/aec_dump.h" #include "rtc_base/race_checker.h" #include "rtc_base/system/file_wrapper.h" -#include "rtc_base/task_queue.h" #include "rtc_base/thread_annotations.h" // Files generated at build-time by the protobuf compiler. @@ -39,7 +39,7 @@ class AecDumpImpl : public AecDump { // `max_log_size_bytes == -1` means the log size will be unlimited. AecDumpImpl(FileWrapper debug_file, int64_t max_log_size_bytes, - rtc::TaskQueue* worker_queue); + absl::Nonnull worker_queue); AecDumpImpl(const AecDumpImpl&) = delete; AecDumpImpl& operator=(const AecDumpImpl&) = delete; ~AecDumpImpl() override; @@ -74,7 +74,7 @@ class AecDumpImpl : public AecDump { FileWrapper debug_file_; int64_t num_bytes_left_for_log_ = 0; rtc::RaceChecker race_checker_; - rtc::TaskQueue* worker_queue_; + absl::Nonnull worker_queue_; CaptureStreamInfo capture_stream_info_; }; } // namespace webrtc diff --git a/modules/audio_processing/aec_dump/aec_dump_unittest.cc b/modules/audio_processing/aec_dump/aec_dump_unittest.cc index 62f896fe14..2a8110c4fc 100644 --- a/modules/audio_processing/aec_dump/aec_dump_unittest.cc +++ b/modules/audio_processing/aec_dump/aec_dump_unittest.cc @@ -28,7 +28,7 @@ TEST(AecDumper, APICallsDoNotCrash) { { std::unique_ptr aec_dump = - webrtc::AecDumpFactory::Create(filename, -1, &file_writer_queue); + webrtc::AecDumpFactory::Create(filename, -1, file_writer_queue.Get()); constexpr int kNumChannels = 1; constexpr int kNumSamplesPerChannel = 160; @@ -63,7 +63,7 @@ TEST(AecDumper, WriteToFile) { { std::unique_ptr aec_dump = - webrtc::AecDumpFactory::Create(filename, -1, &file_writer_queue); + webrtc::AecDumpFactory::Create(filename, -1, file_writer_queue.Get()); constexpr int kNumChannels = 1; constexpr int kNumSamplesPerChannel = 160; diff --git a/modules/audio_processing/aec_dump/null_aec_dump_factory.cc b/modules/audio_processing/aec_dump/null_aec_dump_factory.cc index 9bd9745069..63929afac4 100644 --- a/modules/audio_processing/aec_dump/null_aec_dump_factory.cc +++ b/modules/audio_processing/aec_dump/null_aec_dump_factory.cc @@ -8,27 +8,32 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include "absl/base/nullability.h" #include "absl/strings/string_view.h" +#include "api/task_queue/task_queue_base.h" #include "modules/audio_processing/aec_dump/aec_dump_factory.h" #include "modules/audio_processing/include/aec_dump.h" namespace webrtc { -std::unique_ptr AecDumpFactory::Create(webrtc::FileWrapper file, - int64_t max_log_size_bytes, - rtc::TaskQueue* worker_queue) { +absl::Nullable> AecDumpFactory::Create( + FileWrapper file, + int64_t max_log_size_bytes, + absl::Nonnull worker_queue) { return nullptr; } -std::unique_ptr AecDumpFactory::Create(absl::string_view file_name, - int64_t max_log_size_bytes, - rtc::TaskQueue* worker_queue) { +absl::Nullable> AecDumpFactory::Create( + absl::string_view file_name, + int64_t max_log_size_bytes, + absl::Nonnull worker_queue) { return nullptr; } -std::unique_ptr AecDumpFactory::Create(FILE* handle, - int64_t max_log_size_bytes, - rtc::TaskQueue* worker_queue) { +absl::Nullable> AecDumpFactory::Create( + absl::Nonnull handle, + int64_t max_log_size_bytes, + absl::Nonnull worker_queue) { return nullptr; } } // namespace webrtc diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index c80cc76a3d..2e18ef4259 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -33,6 +33,7 @@ #include "rtc_base/checks.h" #include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/logging.h" +#include "rtc_base/task_queue.h" #include "rtc_base/time_utils.h" #include "rtc_base/trace_event.h" #include "system_wrappers/include/denormal_disabler.h" @@ -2085,8 +2086,8 @@ void AudioProcessingImpl::UpdateRecommendedInputVolumeLocked() { bool AudioProcessingImpl::CreateAndAttachAecDump(absl::string_view file_name, int64_t max_log_size_bytes, rtc::TaskQueue* worker_queue) { - std::unique_ptr aec_dump = - AecDumpFactory::Create(file_name, max_log_size_bytes, worker_queue); + std::unique_ptr aec_dump = AecDumpFactory::Create( + file_name, max_log_size_bytes, worker_queue->Get()); if (!aec_dump) { return false; } @@ -2099,7 +2100,7 @@ bool AudioProcessingImpl::CreateAndAttachAecDump(FILE* handle, int64_t max_log_size_bytes, rtc::TaskQueue* worker_queue) { std::unique_ptr aec_dump = - AecDumpFactory::Create(handle, max_log_size_bytes, worker_queue); + AecDumpFactory::Create(handle, max_log_size_bytes, worker_queue->Get()); if (!aec_dump) { return false; } diff --git a/modules/audio_processing/audio_processing_unittest.cc b/modules/audio_processing/audio_processing_unittest.cc index c2bedb2da4..2d3684e9b5 100644 --- a/modules/audio_processing/audio_processing_unittest.cc +++ b/modules/audio_processing/audio_processing_unittest.cc @@ -1485,8 +1485,8 @@ void ApmTest::ProcessDebugDump(absl::string_view in_filename, if (first_init) { // AttachAecDump() writes an additional init message. Don't start // recording until after the first init to avoid the extra message. - auto aec_dump = - AecDumpFactory::Create(out_filename, max_size_bytes, &worker_queue); + auto aec_dump = AecDumpFactory::Create(out_filename, max_size_bytes, + worker_queue.Get()); EXPECT_TRUE(aec_dump); apm_->AttachAecDump(std::move(aec_dump)); first_init = false; @@ -1632,7 +1632,7 @@ TEST_F(ApmTest, DebugDump) { const std::string filename = test::TempFilename(test::OutputPath(), "debug_aec"); { - auto aec_dump = AecDumpFactory::Create("", -1, &worker_queue); + auto aec_dump = AecDumpFactory::Create("", -1, worker_queue.Get()); EXPECT_FALSE(aec_dump); } @@ -1640,7 +1640,7 @@ TEST_F(ApmTest, DebugDump) { // Stopping without having started should be OK. apm_->DetachAecDump(); - auto aec_dump = AecDumpFactory::Create(filename, -1, &worker_queue); + auto aec_dump = AecDumpFactory::Create(filename, -1, worker_queue.Get()); EXPECT_TRUE(aec_dump); apm_->AttachAecDump(std::move(aec_dump)); EXPECT_EQ(apm_->kNoError, @@ -1683,7 +1683,7 @@ TEST_F(ApmTest, DebugDumpFromFileHandle) { // Stopping without having started should be OK. apm_->DetachAecDump(); - auto aec_dump = AecDumpFactory::Create(std::move(f), -1, &worker_queue); + auto aec_dump = AecDumpFactory::Create(std::move(f), -1, worker_queue.Get()); EXPECT_TRUE(aec_dump); apm_->AttachAecDump(std::move(aec_dump)); EXPECT_EQ(apm_->kNoError, diff --git a/modules/audio_processing/test/audio_processing_simulator.cc b/modules/audio_processing/test/audio_processing_simulator.cc index 7bd6da0133..500005f26a 100644 --- a/modules/audio_processing/test/audio_processing_simulator.cc +++ b/modules/audio_processing/test/audio_processing_simulator.cc @@ -622,7 +622,7 @@ void AudioProcessingSimulator::ConfigureAudioProcessor() { if (settings_.aec_dump_output_filename) { ap_->AttachAecDump(AecDumpFactory::Create( - *settings_.aec_dump_output_filename, -1, &worker_queue_)); + *settings_.aec_dump_output_filename, -1, worker_queue_.Get())); } } diff --git a/modules/audio_processing/test/debug_dump_test.cc b/modules/audio_processing/test/debug_dump_test.cc index cded5de217..0d3eefa94a 100644 --- a/modules/audio_processing/test/debug_dump_test.cc +++ b/modules/audio_processing/test/debug_dump_test.cc @@ -197,7 +197,7 @@ void DebugDumpGenerator::SetOutputChannels(int channels) { void DebugDumpGenerator::StartRecording() { apm_->AttachAecDump( - AecDumpFactory::Create(dump_file_name_.c_str(), -1, &worker_queue_)); + AecDumpFactory::Create(dump_file_name_.c_str(), -1, worker_queue_.Get())); } void DebugDumpGenerator::Process(size_t num_blocks) { diff --git a/test/fuzzers/BUILD.gn b/test/fuzzers/BUILD.gn index 4384c315b9..4f3168f342 100644 --- a/test/fuzzers/BUILD.gn +++ b/test/fuzzers/BUILD.gn @@ -470,6 +470,7 @@ webrtc_fuzzer_test("audio_processing_fuzzer") { "../../api:scoped_refptr", "../../api/audio:aec3_factory", "../../api/audio:echo_detector_creator", + "../../api/task_queue", "../../api/task_queue:default_task_queue_factory", "../../modules/audio_processing", "../../modules/audio_processing:api", @@ -479,11 +480,13 @@ webrtc_fuzzer_test("audio_processing_fuzzer") { "../../modules/audio_processing/aec_dump", "../../modules/audio_processing/aec_dump:aec_dump_impl", "../../rtc_base:macromagic", - "../../rtc_base:rtc_task_queue", "../../rtc_base:safe_minmax", "../../system_wrappers:field_trial", ] - absl_deps = [ "//third_party/abseil-cpp/absl/memory" ] + absl_deps = [ + "//third_party/abseil-cpp/absl/base:nullability", + "//third_party/abseil-cpp/absl/memory", + ] seed_corpus = "corpora/audio_processing-corpus" } diff --git a/test/fuzzers/audio_processing_configs_fuzzer.cc b/test/fuzzers/audio_processing_configs_fuzzer.cc index 331a373f4e..93bce2f2e7 100644 --- a/test/fuzzers/audio_processing_configs_fuzzer.cc +++ b/test/fuzzers/audio_processing_configs_fuzzer.cc @@ -11,16 +11,17 @@ #include #include +#include "absl/base/nullability.h" #include "absl/memory/memory.h" #include "api/audio/echo_canceller3_factory.h" #include "api/audio/echo_detector_creator.h" #include "api/task_queue/default_task_queue_factory.h" +#include "api/task_queue/task_queue_base.h" #include "modules/audio_processing/aec_dump/aec_dump_factory.h" #include "modules/audio_processing/include/audio_processing.h" #include "modules/audio_processing/test/audio_processing_builder_for_testing.h" #include "rtc_base/arraysize.h" #include "rtc_base/numerics/safe_minmax.h" -#include "rtc_base/task_queue.h" #include "system_wrappers/include/field_trial.h" #include "test/fuzzers/audio_processing_fuzzer_helper.h" #include "test/fuzzers/fuzz_data_helper.h" @@ -33,9 +34,10 @@ const std::string kFieldTrialNames[] = { "WebRTC-Aec3ShortHeadroomKillSwitch", }; -rtc::scoped_refptr CreateApm(test::FuzzDataHelper* fuzz_data, - std::string* field_trial_string, - rtc::TaskQueue* worker_queue) { +rtc::scoped_refptr CreateApm( + test::FuzzDataHelper* fuzz_data, + std::string* field_trial_string, + absl::Nonnull worker_queue) { // Parse boolean values for optionally enabling different // configurable public components of APM. bool use_ts = fuzz_data->ReadOrDefaultValue(true); @@ -134,9 +136,10 @@ void FuzzOneInput(const uint8_t* data, size_t size) { // for field_trial.h. Hence it's created here and not in CreateApm. std::string field_trial_string = ""; - rtc::TaskQueue worker_queue(GetTaskQueueFactory()->CreateTaskQueue( - "rtc-low-prio", rtc::TaskQueue::Priority::LOW)); - auto apm = CreateApm(&fuzz_data, &field_trial_string, &worker_queue); + std::unique_ptr worker_queue = + GetTaskQueueFactory()->CreateTaskQueue("rtc-low-prio", + TaskQueueFactory::Priority::LOW); + auto apm = CreateApm(&fuzz_data, &field_trial_string, worker_queue.get()); if (apm) { FuzzAudioProcessing(&fuzz_data, std::move(apm));