From a39254da593bbdb0b1e072a44827229680afe3ee Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Tue, 26 Mar 2019 14:10:16 +0100 Subject: [PATCH] in WebrtcVoiceEngine allow to set TaskQueueFactory in production code keep using GlobalTaskQueueFactory() in tests switch to use DefaultTaskQueueFactory directly. Bug: webrtc:10284 Change-Id: I170274a98324796623089a965a39f0cbb7e281d9 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/128878 Reviewed-by: Steve Anton Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#27296} --- media/BUILD.gn | 4 ++ .../null_webrtc_video_engine_unittest.cc | 37 ++++++++-------- media/engine/webrtc_media_engine.cc | 7 +-- media/engine/webrtc_voice_engine.cc | 18 ++++---- media/engine/webrtc_voice_engine.h | 4 +- media/engine/webrtc_voice_engine_unittest.cc | 43 +++++++++++++++---- 6 files changed, 71 insertions(+), 42 deletions(-) diff --git a/media/BUILD.gn b/media/BUILD.gn index 26a17a1bdf..d02ef89d18 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -260,6 +260,8 @@ rtc_static_library("rtc_audio_video") { libs = [] deps = [ "../api:scoped_refptr", + "../api/task_queue", + "../api/task_queue:global_task_queue_factory", "../api/video:video_bitrate_allocation", "../api/video:video_bitrate_allocator_factory", "../modules/audio_processing:api", @@ -493,6 +495,8 @@ if (rtc_include_tests) { "../:webrtc_common", "../api:fake_media_transport", "../api:scoped_refptr", + "../api/task_queue", + "../api/task_queue:default_task_queue_factory", "../api/test/video:function_video_factory", "../api/units:time_delta", "../api/video:video_frame_i420", diff --git a/media/engine/null_webrtc_video_engine_unittest.cc b/media/engine/null_webrtc_video_engine_unittest.cc index 85eaea3a92..343e167fcb 100644 --- a/media/engine/null_webrtc_video_engine_unittest.cc +++ b/media/engine/null_webrtc_video_engine_unittest.cc @@ -9,7 +9,13 @@ */ #include "media/engine/null_webrtc_video_engine.h" + +#include +#include + #include "absl/memory/memory.h" +#include "api/task_queue/default_task_queue_factory.h" +#include "api/task_queue/task_queue_factory.h" #include "media/engine/webrtc_voice_engine.h" #include "modules/audio_device/include/mock_audio_device.h" #include "modules/audio_processing/include/audio_processing.h" @@ -19,30 +25,21 @@ namespace cricket { -class WebRtcMediaEngineNullVideo : public CompositeMediaEngine { - public: - WebRtcMediaEngineNullVideo( - webrtc::AudioDeviceModule* adm, - const rtc::scoped_refptr& - audio_encoder_factory, - const rtc::scoped_refptr& - audio_decoder_factory) - : CompositeMediaEngine(absl::make_unique( - adm, - audio_encoder_factory, - audio_decoder_factory, - nullptr, - webrtc::AudioProcessingBuilder().Create()), - absl::make_unique()) {} -}; - // Simple test to check if NullWebRtcVideoEngine implements the methods // required by CompositeMediaEngine. TEST(NullWebRtcVideoEngineTest, CheckInterface) { + std::unique_ptr task_queue_factory = + webrtc::CreateDefaultTaskQueueFactory(); testing::NiceMock adm; - WebRtcMediaEngineNullVideo engine( - &adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), - webrtc::MockAudioDecoderFactory::CreateUnusedFactory()); + auto audio_engine = absl::make_unique( + task_queue_factory.get(), &adm, + webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), + webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, + webrtc::AudioProcessingBuilder().Create()); + + CompositeMediaEngine engine(std::move(audio_engine), + absl::make_unique()); + EXPECT_TRUE(engine.Init()); } diff --git a/media/engine/webrtc_media_engine.cc b/media/engine/webrtc_media_engine.cc index 5de0927a52..48269ee51b 100644 --- a/media/engine/webrtc_media_engine.cc +++ b/media/engine/webrtc_media_engine.cc @@ -14,6 +14,7 @@ #include "absl/algorithm/container.h" #include "absl/memory/memory.h" +#include "api/task_queue/global_task_queue_factory.h" #include "api/video/builtin_video_bitrate_allocator_factory.h" #include "api/video_codecs/video_decoder_factory.h" #include "api/video_codecs/video_encoder_factory.h" @@ -61,9 +62,9 @@ std::unique_ptr WebRtcMediaEngineFactory::Create( auto video_engine = absl::make_unique(); #endif return std::unique_ptr(new CompositeMediaEngine( - absl::make_unique(adm, audio_encoder_factory, - audio_decoder_factory, audio_mixer, - audio_processing), + absl::make_unique( + &webrtc::GlobalTaskQueueFactory(), adm, audio_encoder_factory, + audio_decoder_factory, audio_mixer, audio_processing), std::move(video_engine))); } diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 9fc9d08362..a94fa7ab03 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -178,12 +178,16 @@ absl::optional ComputeSendBitrate(int max_send_bitrate_bps, } // namespace WebRtcVoiceEngine::WebRtcVoiceEngine( + webrtc::TaskQueueFactory* task_queue_factory, webrtc::AudioDeviceModule* adm, const rtc::scoped_refptr& encoder_factory, const rtc::scoped_refptr& decoder_factory, rtc::scoped_refptr audio_mixer, rtc::scoped_refptr audio_processing) - : adm_(adm), + : low_priority_worker_queue_(task_queue_factory->CreateTaskQueue( + "rtc-low-prio", + webrtc::TaskQueueFactory::Priority::LOW)), + adm_(adm), encoder_factory_(encoder_factory), decoder_factory_(decoder_factory), audio_mixer_(audio_mixer), @@ -216,10 +220,6 @@ void WebRtcVoiceEngine::Init() { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); RTC_LOG(LS_INFO) << "WebRtcVoiceEngine::Init"; - // TaskQueue expects to be created/destroyed on the same thread. - low_priority_worker_queue_.reset( - new rtc::TaskQueue("rtc-low-prio", rtc::TaskQueue::Priority::LOW)); - // Load our audio codec lists. RTC_LOG(LS_INFO) << "Supported send codecs in order of preference:"; send_codecs_ = CollectCodecs(encoder_factory_->GetSupportedEncoders()); @@ -580,8 +580,8 @@ void WebRtcVoiceEngine::UnregisterChannel(WebRtcVoiceMediaChannel* channel) { bool WebRtcVoiceEngine::StartAecDump(rtc::PlatformFile file, int64_t max_size_bytes) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); - auto aec_dump = webrtc::AecDumpFactory::Create( - file, max_size_bytes, low_priority_worker_queue_.get()); + auto aec_dump = webrtc::AecDumpFactory::Create(file, max_size_bytes, + &low_priority_worker_queue_); if (!aec_dump) { return false; } @@ -592,8 +592,8 @@ bool WebRtcVoiceEngine::StartAecDump(rtc::PlatformFile file, void WebRtcVoiceEngine::StartAecDump(const std::string& filename) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); - auto aec_dump = webrtc::AecDumpFactory::Create( - filename, -1, low_priority_worker_queue_.get()); + auto aec_dump = + webrtc::AecDumpFactory::Create(filename, -1, &low_priority_worker_queue_); if (aec_dump) { apm()->AttachAecDump(std::move(aec_dump)); } diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h index 3bce78de0a..1dbba82d29 100644 --- a/media/engine/webrtc_voice_engine.h +++ b/media/engine/webrtc_voice_engine.h @@ -19,6 +19,7 @@ #include "api/audio_codecs/audio_encoder_factory.h" #include "api/rtp_receiver_interface.h" #include "api/scoped_refptr.h" +#include "api/task_queue/task_queue_factory.h" #include "call/audio_state.h" #include "call/call.h" #include "media/base/rtp_utils.h" @@ -46,6 +47,7 @@ class WebRtcVoiceEngine final : public VoiceEngineInterface { public: WebRtcVoiceEngine( + webrtc::TaskQueueFactory* task_queue_factory, webrtc::AudioDeviceModule* adm, const rtc::scoped_refptr& encoder_factory, const rtc::scoped_refptr& decoder_factory, @@ -95,7 +97,7 @@ class WebRtcVoiceEngine final : public VoiceEngineInterface { void StartAecDump(const std::string& filename); int CreateVoEChannel(); - std::unique_ptr low_priority_worker_queue_; + rtc::TaskQueue low_priority_worker_queue_; webrtc::AudioDeviceModule* adm(); webrtc::AudioProcessing* apm() const; diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc index a8935e1f8f..2f6d74a3dc 100644 --- a/media/engine/webrtc_voice_engine_unittest.cc +++ b/media/engine/webrtc_voice_engine_unittest.cc @@ -16,6 +16,7 @@ #include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "api/rtp_parameters.h" #include "api/scoped_refptr.h" +#include "api/task_queue/default_task_queue_factory.h" #include "call/call.h" #include "logging/rtc_event_log/rtc_event_log.h" #include "media/base/fake_media_engine.h" @@ -135,6 +136,8 @@ void AdmSetupExpectations(webrtc::test::MockAudioDeviceModule* adm) { // Tests that our stub library "works". TEST(WebRtcVoiceEngineTestStubLibrary, StartupShutdown) { + std::unique_ptr task_queue_factory = + webrtc::CreateDefaultTaskQueueFactory(); StrictMock adm; AdmSetupExpectations(&adm); rtc::scoped_refptr> apm = @@ -147,7 +150,8 @@ TEST(WebRtcVoiceEngineTestStubLibrary, StartupShutdown) { EXPECT_CALL(*apm, DetachAecDump()); { cricket::WebRtcVoiceEngine engine( - &adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), + task_queue_factory.get(), &adm, + webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, apm); engine.Init(); } @@ -167,7 +171,8 @@ class WebRtcVoiceEngineTestFake : public testing::Test { WebRtcVoiceEngineTestFake() : WebRtcVoiceEngineTestFake("") {} explicit WebRtcVoiceEngineTestFake(const char* field_trials) - : apm_(new rtc::RefCountedObject< + : task_queue_factory_(webrtc::CreateDefaultTaskQueueFactory()), + apm_(new rtc::RefCountedObject< StrictMock>()), apm_gc_(*apm_->gain_control()), apm_ns_(*apm_->noise_suppression()), @@ -198,7 +203,8 @@ class WebRtcVoiceEngineTestFake : public testing::Test { auto encoder_factory = webrtc::CreateBuiltinAudioEncoderFactory(); auto decoder_factory = webrtc::CreateBuiltinAudioDecoderFactory(); engine_.reset(new cricket::WebRtcVoiceEngine( - &adm_, encoder_factory, decoder_factory, nullptr, apm_)); + task_queue_factory_.get(), &adm_, encoder_factory, decoder_factory, + nullptr, apm_)); engine_->Init(); send_parameters_.codecs.push_back(kPcmuCodec); recv_parameters_.codecs.push_back(kPcmuCodec); @@ -750,6 +756,7 @@ class WebRtcVoiceEngineTestFake : public testing::Test { } protected: + std::unique_ptr task_queue_factory_; StrictMock adm_; rtc::scoped_refptr> apm_; webrtc::test::MockGainControl& apm_gc_; @@ -3492,11 +3499,14 @@ TEST_F(WebRtcVoiceEngineTestFake, GetSourcesWithNonExistingSsrc) { TEST(WebRtcVoiceEngineTest, StartupShutdown) { // If the VoiceEngine wants to gather available codecs early, that's fine but // we never want it to create a decoder at this stage. + std::unique_ptr task_queue_factory = + webrtc::CreateDefaultTaskQueueFactory(); testing::NiceMock adm; rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); cricket::WebRtcVoiceEngine engine( - &adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), + task_queue_factory.get(), &adm, + webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, apm); engine.Init(); webrtc::RtcEventLogNullImpl event_log; @@ -3511,6 +3521,8 @@ TEST(WebRtcVoiceEngineTest, StartupShutdown) { // Tests that reference counting on the external ADM is correct. TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) { + std::unique_ptr task_queue_factory = + webrtc::CreateDefaultTaskQueueFactory(); testing::NiceMock adm; EXPECT_CALL(adm, AddRef()).Times(3); EXPECT_CALL(adm, Release()) @@ -3520,7 +3532,8 @@ TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) { rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); cricket::WebRtcVoiceEngine engine( - &adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), + task_queue_factory.get(), &adm, + webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, apm); engine.Init(); webrtc::RtcEventLogNullImpl event_log; @@ -3536,13 +3549,16 @@ TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) { // Verify the payload id of common audio codecs, including CN, ISAC, and G722. TEST(WebRtcVoiceEngineTest, HasCorrectPayloadTypeMapping) { + std::unique_ptr task_queue_factory = + webrtc::CreateDefaultTaskQueueFactory(); // TODO(ossu): Why are the payload types of codecs with non-static payload // type assignments checked here? It shouldn't really matter. testing::NiceMock adm; rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); cricket::WebRtcVoiceEngine engine( - &adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), + task_queue_factory.get(), &adm, + webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, apm); engine.Init(); for (const cricket::AudioCodec& codec : engine.send_codecs()) { @@ -3583,11 +3599,14 @@ TEST(WebRtcVoiceEngineTest, HasCorrectPayloadTypeMapping) { // Tests that VoE supports at least 32 channels TEST(WebRtcVoiceEngineTest, Has32Channels) { + std::unique_ptr task_queue_factory = + webrtc::CreateDefaultTaskQueueFactory(); testing::NiceMock adm; rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); cricket::WebRtcVoiceEngine engine( - &adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), + task_queue_factory.get(), &adm, + webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, apm); engine.Init(); webrtc::RtcEventLogNullImpl event_log; @@ -3615,6 +3634,8 @@ TEST(WebRtcVoiceEngineTest, Has32Channels) { // Test that we set our preferred codecs properly. TEST(WebRtcVoiceEngineTest, SetRecvCodecs) { + std::unique_ptr task_queue_factory = + webrtc::CreateDefaultTaskQueueFactory(); // TODO(ossu): I'm not sure of the intent of this test. It's either: // - Check that our builtin codecs are usable by Channel. // - The codecs provided by the engine is usable by Channel. @@ -3626,7 +3647,8 @@ TEST(WebRtcVoiceEngineTest, SetRecvCodecs) { rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); cricket::WebRtcVoiceEngine engine( - &adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), + task_queue_factory.get(), &adm, + webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::CreateBuiltinAudioDecoderFactory(), nullptr, apm); engine.Init(); webrtc::RtcEventLogNullImpl event_log; @@ -3658,6 +3680,8 @@ TEST(WebRtcVoiceEngineTest, CollectRecvCodecs) { specs.push_back( webrtc::AudioCodecSpec{{"codec5", 8000, 2}, {8000, 1, 64000}}); + std::unique_ptr task_queue_factory = + webrtc::CreateDefaultTaskQueueFactory(); rtc::scoped_refptr unused_encoder_factory = webrtc::MockAudioEncoderFactory::CreateUnusedFactory(); rtc::scoped_refptr mock_decoder_factory = @@ -3668,7 +3692,8 @@ TEST(WebRtcVoiceEngineTest, CollectRecvCodecs) { rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); - cricket::WebRtcVoiceEngine engine(&adm, unused_encoder_factory, + cricket::WebRtcVoiceEngine engine(task_queue_factory.get(), &adm, + unused_encoder_factory, mock_decoder_factory, nullptr, apm); engine.Init(); auto codecs = engine.recv_codecs();