admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reland "in WebrtcVoiceEngine allow to set TaskQueueFactory"

Browse Source 
in production code keep using GlobalTaskQueueFactory()
in tests switch to use DefaultTaskQueueFactory directly.

This reverts commit e27ccf9a1681e0e4ff9281f9a18fea357d2bc890.

Reason for revert: addressed the failure with patchset#2

Original change's description:
> Revert "in WebrtcVoiceEngine allow to set TaskQueueFactory"
>
> This reverts commit a39254da593bbdb0b1e072a44827229680afe3ee.
>
> Reason for revert: Tests are failing due to ThreadChecker's called on valid thread.
>
> Original change's description:
> > 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 <steveanton@webrtc.org>
> > Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#27296}
>
> TBR=danilchap@webrtc.org,steveanton@webrtc.org
>
> Change-Id: I9742e5d0171a94f3840e197c40fdb44523e4963b
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:10284
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/129780
> Reviewed-by: Amit Hilbuch <amithi@webrtc.org>
> Commit-Queue: Amit Hilbuch <amithi@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#27297}

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:10284
Change-Id: I55fd5811c68d04c3e8cf537974496460b38c1d4f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/129933
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27348}
This commit is contained in:
Danil Chapovalov 2019-03-27 18:51:45 +01:00 committed by Commit Bot
parent 90d782619e
commit 4c7112a27a
6 changed files with 67 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
media/BUILD.gn
View File

@ -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",

37
media/engine/null_webrtc_video_engine_unittest.cc
View File

@ -9,7 +9,13 @@
*/
#include "media/engine/null_webrtc_video_engine.h"
#include <memory>
#include <utility>
#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<webrtc::AudioEncoderFactory>&
audio_encoder_factory,
const rtc::scoped_refptr<webrtc::AudioDecoderFactory>&
audio_decoder_factory)
: CompositeMediaEngine(absl::make_unique<WebRtcVoiceEngine>(
adm,
audio_encoder_factory,
audio_decoder_factory,
nullptr,
webrtc::AudioProcessingBuilder().Create()),
absl::make_unique<NullWebRtcVideoEngine>()) {}
};
// Simple test to check if NullWebRtcVideoEngine implements the methods
// required by CompositeMediaEngine.
TEST(NullWebRtcVideoEngineTest, CheckInterface) {
std::unique_ptr<webrtc::TaskQueueFactory> task_queue_factory =
webrtc::CreateDefaultTaskQueueFactory();
testing::NiceMock<webrtc::test::MockAudioDeviceModule> adm;
WebRtcMediaEngineNullVideo engine(
&adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(),
webrtc::MockAudioDecoderFactory::CreateUnusedFactory());
auto audio_engine = absl::make_unique<WebRtcVoiceEngine>(
task_queue_factory.get(), &adm,
webrtc::MockAudioEncoderFactory::CreateUnusedFactory(),
webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr,
webrtc::AudioProcessingBuilder().Create());
CompositeMediaEngine engine(std::move(audio_engine),
absl::make_unique<NullWebRtcVideoEngine>());
EXPECT_TRUE(engine.Init());
}

7
media/engine/webrtc_media_engine.cc
View File

@ -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<MediaEngineInterface> WebRtcMediaEngineFactory::Create(
auto video_engine = absl::make_unique<NullWebRtcVideoEngine>();
#endif
return std::unique_ptr<MediaEngineInterface>(new CompositeMediaEngine(
absl::make_unique<WebRtcVoiceEngine>(adm, audio_encoder_factory,
audio_decoder_factory, audio_mixer,
audio_processing),
absl::make_unique<WebRtcVoiceEngine>(
&webrtc::GlobalTaskQueueFactory(), adm, audio_encoder_factory,
audio_decoder_factory, audio_mixer, audio_processing),
std::move(video_engine)));
}

7
media/engine/webrtc_voice_engine.cc
View File

@ -178,12 +178,14 @@ absl::optional<int> ComputeSendBitrate(int max_send_bitrate_bps,
} // namespace
WebRtcVoiceEngine::WebRtcVoiceEngine(
webrtc::TaskQueueFactory* task_queue_factory,
webrtc::AudioDeviceModule* adm,
const rtc::scoped_refptr<webrtc::AudioEncoderFactory>& encoder_factory,
const rtc::scoped_refptr<webrtc::AudioDecoderFactory>& decoder_factory,
rtc::scoped_refptr<webrtc::AudioMixer> audio_mixer,
rtc::scoped_refptr<webrtc::AudioProcessing> audio_processing)
: adm_(adm),
: task_queue_factory_(task_queue_factory),
adm_(adm),
encoder_factory_(encoder_factory),
decoder_factory_(decoder_factory),
audio_mixer_(audio_mixer),
@ -218,7 +220,8 @@ void 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));
new rtc::TaskQueue(task_queue_factory_->CreateTaskQueue(
"rtc-low-prio", webrtc::TaskQueueFactory::Priority::LOW)));
// Load our audio codec lists.
RTC_LOG(LS_INFO) << "Supported send codecs in order of preference:";

3
media/engine/webrtc_voice_engine.h
View File

@ -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<webrtc::AudioEncoderFactory>& encoder_factory,
const rtc::scoped_refptr<webrtc::AudioDecoderFactory>& decoder_factory,
@ -91,6 +93,7 @@ class WebRtcVoiceEngine final : public VoiceEngineInterface {
void StartAecDump(const std::string& filename);
int CreateVoEChannel();
webrtc::TaskQueueFactory* const task_queue_factory_;
std::unique_ptr<rtc::TaskQueue> low_priority_worker_queue_;
webrtc::AudioDeviceModule* adm();

43
media/engine/webrtc_voice_engine_unittest.cc
View File

@ -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"
@ -136,6 +137,8 @@ void AdmSetupExpectations(webrtc::test::MockAudioDeviceModule* adm) {
// Tests that our stub library "works".
TEST(WebRtcVoiceEngineTestStubLibrary, StartupShutdown) {
std::unique_ptr<webrtc::TaskQueueFactory> task_queue_factory =
webrtc::CreateDefaultTaskQueueFactory();
StrictMock<webrtc::test::MockAudioDeviceModule> adm;
AdmSetupExpectations(&adm);
rtc::scoped_refptr<StrictMock<webrtc::test::MockAudioProcessing>> apm =
@ -148,7 +151,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();
}
@ -168,7 +172,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<webrtc::test::MockAudioProcessing>>()),
apm_ns_(*apm_->noise_suppression()),
call_(),
@ -189,7 +194,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);
@ -756,6 +762,7 @@ class WebRtcVoiceEngineTestFake : public testing::Test {
}
protected:
std::unique_ptr<webrtc::TaskQueueFactory> task_queue_factory_;
StrictMock<webrtc::test::MockAudioDeviceModule> adm_;
rtc::scoped_refptr<StrictMock<webrtc::test::MockAudioProcessing>> apm_;
webrtc::test::MockNoiseSuppression& apm_ns_;
@ -3499,11 +3506,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<webrtc::TaskQueueFactory> task_queue_factory =
webrtc::CreateDefaultTaskQueueFactory();
testing::NiceMock<webrtc::test::MockAudioDeviceModule> adm;
rtc::scoped_refptr<webrtc::AudioProcessing> 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;
@ -3518,6 +3528,8 @@ TEST(WebRtcVoiceEngineTest, StartupShutdown) {
// Tests that reference counting on the external ADM is correct.
TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) {
std::unique_ptr<webrtc::TaskQueueFactory> task_queue_factory =
webrtc::CreateDefaultTaskQueueFactory();
testing::NiceMock<webrtc::test::MockAudioDeviceModule> adm;
EXPECT_CALL(adm, AddRef()).Times(3);
EXPECT_CALL(adm, Release())
@ -3527,7 +3539,8 @@ TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) {
rtc::scoped_refptr<webrtc::AudioProcessing> 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;
@ -3543,13 +3556,16 @@ TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) {
// Verify the payload id of common audio codecs, including CN, ISAC, and G722.
TEST(WebRtcVoiceEngineTest, HasCorrectPayloadTypeMapping) {
std::unique_ptr<webrtc::TaskQueueFactory> 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<webrtc::test::MockAudioDeviceModule> adm;
rtc::scoped_refptr<webrtc::AudioProcessing> 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()) {
@ -3590,11 +3606,14 @@ TEST(WebRtcVoiceEngineTest, HasCorrectPayloadTypeMapping) {
// Tests that VoE supports at least 32 channels
TEST(WebRtcVoiceEngineTest, Has32Channels) {
std::unique_ptr<webrtc::TaskQueueFactory> task_queue_factory =
webrtc::CreateDefaultTaskQueueFactory();
testing::NiceMock<webrtc::test::MockAudioDeviceModule> adm;
rtc::scoped_refptr<webrtc::AudioProcessing> 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;
@ -3622,6 +3641,8 @@ TEST(WebRtcVoiceEngineTest, Has32Channels) {
// Test that we set our preferred codecs properly.
TEST(WebRtcVoiceEngineTest, SetRecvCodecs) {
std::unique_ptr<webrtc::TaskQueueFactory> 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.
@ -3633,7 +3654,8 @@ TEST(WebRtcVoiceEngineTest, SetRecvCodecs) {
rtc::scoped_refptr<webrtc::AudioProcessing> 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;
@ -3665,6 +3687,8 @@ TEST(WebRtcVoiceEngineTest, CollectRecvCodecs) {
specs.push_back(
webrtc::AudioCodecSpec{{"codec5", 8000, 2}, {8000, 1, 64000}});
std::unique_ptr<webrtc::TaskQueueFactory> task_queue_factory =
webrtc::CreateDefaultTaskQueueFactory();
rtc::scoped_refptr<webrtc::MockAudioEncoderFactory> unused_encoder_factory =
webrtc::MockAudioEncoderFactory::CreateUnusedFactory();
rtc::scoped_refptr<webrtc::MockAudioDecoderFactory> mock_decoder_factory =
@ -3675,7 +3699,8 @@ TEST(WebRtcVoiceEngineTest, CollectRecvCodecs) {
rtc::scoped_refptr<webrtc::AudioProcessing> 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();