diff --git a/webrtc/DEPS b/webrtc/DEPS index 40ec7a3d62..5fab15ccfd 100644 --- a/webrtc/DEPS +++ b/webrtc/DEPS @@ -40,6 +40,9 @@ specific_include_rules = { "audio_send_stream\.h": [ "+webrtc/modules/audio_coding", ], + "audio_receive_stream\.h": [ + "+webrtc/modules/audio_coding/codecs/audio_decoder_factory.h", + ], "video_frame\.h": [ "+webrtc/common_video", ], diff --git a/webrtc/api/DEPS b/webrtc/api/DEPS index 956d4d95c8..fcb506dc00 100644 --- a/webrtc/api/DEPS +++ b/webrtc/api/DEPS @@ -19,5 +19,8 @@ specific_include_rules = { ], "peerconnection_jni\.cc": [ "+webrtc/voice_engine", + ], + "peerconnectionfactory.\cc": [ + "+webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.h", ] } diff --git a/webrtc/api/peerconnectionfactory.cc b/webrtc/api/peerconnectionfactory.cc index 8500422a1a..08fe952abd 100644 --- a/webrtc/api/peerconnectionfactory.cc +++ b/webrtc/api/peerconnectionfactory.cc @@ -28,6 +28,7 @@ #include "webrtc/media/engine/webrtcmediaengine.h" #include "webrtc/media/engine/webrtcvideodecoderfactory.h" #include "webrtc/media/engine/webrtcvideoencoderfactory.h" +#include "webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.h" #include "webrtc/modules/audio_device/include/audio_device.h" #include "webrtc/p2p/base/basicpacketsocketfactory.h" #include "webrtc/p2p/client/basicportallocator.h" @@ -58,8 +59,13 @@ rtc::scoped_refptr CreatePeerConnectionFactory( cricket::WebRtcVideoDecoderFactory* decoder_factory) { rtc::scoped_refptr pc_factory( new rtc::RefCountedObject( - network_thread, worker_thread, signaling_thread, default_adm, - encoder_factory, decoder_factory)); + network_thread, + worker_thread, + signaling_thread, + default_adm, + CreateBuiltinAudioDecoderFactory(), + encoder_factory, + decoder_factory)); // Call Initialize synchronously but make sure its executed on // |signaling_thread|. @@ -93,6 +99,8 @@ PeerConnectionFactory::PeerConnectionFactory( rtc::Thread* worker_thread, rtc::Thread* signaling_thread, AudioDeviceModule* default_adm, + const rtc::scoped_refptr& + audio_decoder_factory, cricket::WebRtcVideoEncoderFactory* video_encoder_factory, cricket::WebRtcVideoDecoderFactory* video_decoder_factory) : owns_ptrs_(false), @@ -101,6 +109,7 @@ PeerConnectionFactory::PeerConnectionFactory( worker_thread_(worker_thread), signaling_thread_(signaling_thread), default_adm_(default_adm), + audio_decoder_factory_(audio_decoder_factory), video_encoder_factory_(video_encoder_factory), video_decoder_factory_(video_decoder_factory) { RTC_DCHECK(network_thread); @@ -319,7 +328,9 @@ rtc::Thread* PeerConnectionFactory::network_thread() { cricket::MediaEngineInterface* PeerConnectionFactory::CreateMediaEngine_w() { ASSERT(worker_thread_ == rtc::Thread::Current()); return cricket::WebRtcMediaEngineFactory::Create( - default_adm_.get(), video_encoder_factory_.get(), + default_adm_.get(), + audio_decoder_factory_, + video_encoder_factory_.get(), video_decoder_factory_.get()); } diff --git a/webrtc/api/peerconnectionfactory.h b/webrtc/api/peerconnectionfactory.h index 0e52efcc96..d4bc0da62b 100644 --- a/webrtc/api/peerconnectionfactory.h +++ b/webrtc/api/peerconnectionfactory.h @@ -101,6 +101,8 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { rtc::Thread* worker_thread, rtc::Thread* signaling_thread, AudioDeviceModule* default_adm, + const rtc::scoped_refptr& + audio_decoder_factory, cricket::WebRtcVideoEncoderFactory* video_encoder_factory, cricket::WebRtcVideoDecoderFactory* video_decoder_factory); virtual ~PeerConnectionFactory(); @@ -116,6 +118,7 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { Options options_; // External Audio device used for audio playback. rtc::scoped_refptr default_adm_; + rtc::scoped_refptr audio_decoder_factory_; std::unique_ptr channel_manager_; // External Video encoder factory. This can be NULL if the client has not // injected any. In that case, video engine will use the internal SW encoder. diff --git a/webrtc/audio/DEPS b/webrtc/audio/DEPS index 63711ab428..b9605bb611 100644 --- a/webrtc/audio/DEPS +++ b/webrtc/audio/DEPS @@ -1,6 +1,7 @@ include_rules = [ "+webrtc/base", "+webrtc/voice_engine", + "+webrtc/modules/audio_coding/codecs/mock", "+webrtc/modules/bitrate_controller", "+webrtc/modules/congestion_controller", "+webrtc/modules/pacing", diff --git a/webrtc/audio/audio_receive_stream.cc b/webrtc/audio/audio_receive_stream.cc index e391df471e..c6c9b30b2a 100644 --- a/webrtc/audio/audio_receive_stream.cc +++ b/webrtc/audio/audio_receive_stream.cc @@ -94,6 +94,15 @@ AudioReceiveStream::AudioReceiveStream( channel_proxy_ = voe_impl->GetChannelProxy(config_.voe_channel_id); channel_proxy_->SetLocalSSRC(config.rtp.local_ssrc); + // TODO(ossu): This is where we'd like to set the decoder factory to + // use. However, since it needs to be included when constructing Channel, we + // cannot do that until we're able to move Channel ownership into the + // Audio{Send,Receive}Streams. The best we can do is check that we're not + // trying to use two different factories using the different interfaces. + RTC_CHECK(config.decoder_factory); + RTC_CHECK_EQ(config.decoder_factory, + channel_proxy_->GetAudioDecoderFactory()); + channel_proxy_->RegisterExternalTransport(config.rtcp_send_transport); for (const auto& extension : config.rtp.extensions) { diff --git a/webrtc/audio/audio_receive_stream_unittest.cc b/webrtc/audio/audio_receive_stream_unittest.cc index fd913dbf87..7380649648 100644 --- a/webrtc/audio/audio_receive_stream_unittest.cc +++ b/webrtc/audio/audio_receive_stream_unittest.cc @@ -15,6 +15,7 @@ #include "webrtc/audio/audio_receive_stream.h" #include "webrtc/audio/conversion.h" +#include "webrtc/modules/audio_coding/codecs/mock/mock_audio_decoder_factory.h" #include "webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h" #include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h" #include "webrtc/modules/pacing/packet_router.h" @@ -30,6 +31,7 @@ namespace { using testing::_; using testing::Return; +using testing::ReturnRef; AudioDecodingCallStats MakeAudioDecodeStatsForTest() { AudioDecodingCallStats audio_decode_stats; @@ -64,6 +66,7 @@ const AudioDecodingCallStats kAudioDecodeStats = MakeAudioDecodeStatsForTest(); struct ConfigHelper { ConfigHelper() : simulated_clock_(123456), + decoder_factory_(new rtc::RefCountedObject), congestion_controller_(&simulated_clock_, &bitrate_observer_, &remote_bitrate_observer_) { @@ -102,6 +105,8 @@ struct ConfigHelper { .Times(1); EXPECT_CALL(*channel_proxy_, DeRegisterExternalTransport()) .Times(1); + EXPECT_CALL(*channel_proxy_, GetAudioDecoderFactory()) + .WillOnce(ReturnRef(decoder_factory_)); return channel_proxy_; })); stream_config_.voe_channel_id = kChannelId; @@ -113,6 +118,7 @@ struct ConfigHelper { RtpExtension(RtpExtension::kAudioLevelUri, kAudioLevelId)); stream_config_.rtp.extensions.push_back(RtpExtension( RtpExtension::kTransportSequenceNumberUri, kTransportSequenceNumberId)); + stream_config_.decoder_factory = decoder_factory_; } MockCongestionController* congestion_controller() { @@ -159,6 +165,7 @@ struct ConfigHelper { PacketRouter packet_router_; testing::NiceMock bitrate_observer_; testing::NiceMock remote_bitrate_observer_; + rtc::scoped_refptr decoder_factory_; MockCongestionController congestion_controller_; MockRemoteBitrateEstimator remote_bitrate_estimator_; testing::StrictMock voice_engine_; diff --git a/webrtc/audio_receive_stream.h b/webrtc/audio_receive_stream.h index 6d72b4d318..9af84a71fa 100644 --- a/webrtc/audio_receive_stream.h +++ b/webrtc/audio_receive_stream.h @@ -16,14 +16,14 @@ #include #include +#include "webrtc/base/scoped_ref_ptr.h" +#include "webrtc/modules/audio_coding/codecs/audio_decoder_factory.h" #include "webrtc/common_types.h" #include "webrtc/config.h" #include "webrtc/transport.h" #include "webrtc/typedefs.h" namespace webrtc { - -class AudioDecoder; class AudioSinkInterface; // WORK IN PROGRESS @@ -101,6 +101,8 @@ class AudioReceiveStream { // Call::CreateReceiveStream(). // TODO(solenberg): Use unique_ptr<> once our std lib fully supports C++11. std::map decoder_map; + + rtc::scoped_refptr decoder_factory; }; // Starts stream activity. diff --git a/webrtc/call/bitrate_estimator_tests.cc b/webrtc/call/bitrate_estimator_tests.cc index 17ddb747c7..88da13c864 100644 --- a/webrtc/call/bitrate_estimator_tests.cc +++ b/webrtc/call/bitrate_estimator_tests.cc @@ -102,7 +102,8 @@ static const int kASTExtensionId = 5; class BitrateEstimatorTest : public test::CallTest { public: - BitrateEstimatorTest() : receive_config_(nullptr) {} + BitrateEstimatorTest() : mock_voice_engine_(decoder_factory_), + receive_config_(nullptr) {} virtual ~BitrateEstimatorTest() { EXPECT_TRUE(streams_.empty()); } @@ -190,6 +191,7 @@ class BitrateEstimatorTest : public test::CallTest { receive_config.voe_channel_id = 0; receive_config.rtp.extensions.push_back( RtpExtension(RtpExtension::kAbsSendTimeUri, kASTExtensionId)); + receive_config.decoder_factory = test_->decoder_factory_; audio_receive_stream_ = test_->receiver_call_->CreateAudioReceiveStream(receive_config); } else { diff --git a/webrtc/call/call_perf_tests.cc b/webrtc/call/call_perf_tests.cc index 50e1a62cfb..137bb2bf6d 100644 --- a/webrtc/call/call_perf_tests.cc +++ b/webrtc/call/call_perf_tests.cc @@ -158,7 +158,7 @@ void CallPerfTest::TestAudioVideoSync(FecMode fec, ASSERT_STRNE("", audio_filename.c_str()); FakeAudioDevice fake_audio_device(Clock::GetRealTimeClock(), audio_filename, audio_rtp_speed); - EXPECT_EQ(0, voe_base->Init(&fake_audio_device, nullptr)); + EXPECT_EQ(0, voe_base->Init(&fake_audio_device, nullptr, decoder_factory_)); Config voe_config; voe_config.Set(new VoicePacing(true)); int send_channel_id = voe_base->CreateChannel(voe_config); @@ -248,6 +248,7 @@ void CallPerfTest::TestAudioVideoSync(FecMode fec, audio_recv_config.rtp.local_ssrc = kAudioRecvSsrc; audio_recv_config.voe_channel_id = recv_channel_id; audio_recv_config.sync_group = kSyncGroup; + audio_recv_config.decoder_factory = decoder_factory_; AudioReceiveStream* audio_receive_stream; diff --git a/webrtc/call/call_unittest.cc b/webrtc/call/call_unittest.cc index 0da91a9bf7..fb6cac11bc 100644 --- a/webrtc/call/call_unittest.cc +++ b/webrtc/call/call_unittest.cc @@ -15,12 +15,15 @@ #include "webrtc/audio_state.h" #include "webrtc/call.h" +#include "webrtc/modules/audio_coding/codecs/mock/mock_audio_decoder_factory.h" #include "webrtc/test/mock_voice_engine.h" namespace { struct CallHelper { - CallHelper() { + explicit CallHelper( + rtc::scoped_refptr decoder_factory = nullptr) + : voice_engine_(decoder_factory) { webrtc::AudioState::Config audio_state_config; audio_state_config.voice_engine = &voice_engine_; webrtc::Call::Config config; @@ -53,10 +56,13 @@ TEST(CallTest, CreateDestroy_AudioSendStream) { } TEST(CallTest, CreateDestroy_AudioReceiveStream) { - CallHelper call; + rtc::scoped_refptr decoder_factory( + new rtc::RefCountedObject); + CallHelper call(decoder_factory); AudioReceiveStream::Config config; config.rtp.remote_ssrc = 42; config.voe_channel_id = 123; + config.decoder_factory = decoder_factory; AudioReceiveStream* stream = call->CreateAudioReceiveStream(config); EXPECT_NE(stream, nullptr); call->DestroyAudioReceiveStream(stream); @@ -86,9 +92,12 @@ TEST(CallTest, CreateDestroy_AudioSendStreams) { } TEST(CallTest, CreateDestroy_AudioReceiveStreams) { - CallHelper call; + rtc::scoped_refptr decoder_factory( + new rtc::RefCountedObject); + CallHelper call(decoder_factory); AudioReceiveStream::Config config; config.voe_channel_id = 123; + config.decoder_factory = decoder_factory; std::list streams; for (int i = 0; i < 2; ++i) { for (uint32_t ssrc = 0; ssrc < 1234567; ssrc += 34567) { diff --git a/webrtc/media/base/fakemediaengine.h b/webrtc/media/base/fakemediaengine.h index 883b474a49..784967ac4f 100644 --- a/webrtc/media/base/fakemediaengine.h +++ b/webrtc/media/base/fakemediaengine.h @@ -736,7 +736,10 @@ class FakeBaseEngine { class FakeVoiceEngine : public FakeBaseEngine { public: - explicit FakeVoiceEngine(webrtc::AudioDeviceModule* adm) + FakeVoiceEngine( + webrtc::AudioDeviceModule* adm, + const rtc::scoped_refptr& + audio_decoder_factory) : output_volume_(-1) { // Add a fake audio codec. Note that the name must not be "" as there are // sanity checks against that. @@ -850,8 +853,9 @@ class FakeVideoEngine : public FakeBaseEngine { class FakeMediaEngine : public CompositeMediaEngine { public: - FakeMediaEngine() : - CompositeMediaEngine(nullptr) {} + FakeMediaEngine() + : CompositeMediaEngine(nullptr, + nullptr) {} virtual ~FakeMediaEngine() {} void SetAudioCodecs(const std::vector& codecs) { diff --git a/webrtc/media/base/mediaengine.h b/webrtc/media/base/mediaengine.h index 8e31c7cbb8..bd1ce431ec 100644 --- a/webrtc/media/base/mediaengine.h +++ b/webrtc/media/base/mediaengine.h @@ -27,6 +27,7 @@ #include "webrtc/media/base/mediacommon.h" #include "webrtc/media/base/videocapturer.h" #include "webrtc/media/base/videocommon.h" +#include "webrtc/modules/audio_coding/codecs/audio_decoder_factory.h" #if defined(GOOGLE_CHROME_BUILD) || defined(CHROMIUM_BUILD) #define DISABLE_MEDIA_ENGINE_FACTORY @@ -127,7 +128,11 @@ class MediaEngineFactory { template class CompositeMediaEngine : public MediaEngineInterface { public: - explicit CompositeMediaEngine(webrtc::AudioDeviceModule* adm) : voice_(adm) {} + CompositeMediaEngine( + webrtc::AudioDeviceModule* adm, + const rtc::scoped_refptr& + audio_decoder_factory) + : voice_(adm, audio_decoder_factory) {} virtual ~CompositeMediaEngine() {} virtual bool Init() { video_.Init(); diff --git a/webrtc/media/engine/nullwebrtcvideoengine_unittest.cc b/webrtc/media/engine/nullwebrtcvideoengine_unittest.cc index 31a2754e19..38d6e77488 100644 --- a/webrtc/media/engine/nullwebrtcvideoengine_unittest.cc +++ b/webrtc/media/engine/nullwebrtcvideoengine_unittest.cc @@ -18,19 +18,23 @@ namespace cricket { class WebRtcMediaEngineNullVideo : public CompositeMediaEngine { public: - WebRtcMediaEngineNullVideo(webrtc::AudioDeviceModule* adm, - WebRtcVideoEncoderFactory* encoder_factory, - WebRtcVideoDecoderFactory* decoder_factory) - : CompositeMediaEngine(adm) { - video_.SetExternalDecoderFactory(decoder_factory); - video_.SetExternalEncoderFactory(encoder_factory); + WebRtcMediaEngineNullVideo( + webrtc::AudioDeviceModule* adm, + const rtc::scoped_refptr& + audio_decoder_factory, + WebRtcVideoEncoderFactory* video_encoder_factory, + WebRtcVideoDecoderFactory* video_decoder_factory) + : CompositeMediaEngine( + adm, audio_decoder_factory) { + video_.SetExternalDecoderFactory(video_decoder_factory); + video_.SetExternalEncoderFactory(video_encoder_factory); } }; // Simple test to check if NullWebRtcVideoEngine implements the methods // required by CompositeMediaEngine. TEST(NullWebRtcVideoEngineTest, CheckInterface) { - WebRtcMediaEngineNullVideo engine(nullptr, nullptr, nullptr); + WebRtcMediaEngineNullVideo engine(nullptr, nullptr, nullptr, nullptr); EXPECT_TRUE(engine.Init()); } diff --git a/webrtc/media/engine/webrtcmediaengine.cc b/webrtc/media/engine/webrtcmediaengine.cc index f452b74233..53e15d6575 100644 --- a/webrtc/media/engine/webrtcmediaengine.cc +++ b/webrtc/media/engine/webrtcmediaengine.cc @@ -29,15 +29,19 @@ class WebRtcMediaEngine2 #endif public: WebRtcMediaEngine2(webrtc::AudioDeviceModule* adm, - WebRtcVideoEncoderFactory* encoder_factory, - WebRtcVideoDecoderFactory* decoder_factory) + const rtc::scoped_refptr& + audio_decoder_factory, + WebRtcVideoEncoderFactory* video_encoder_factory, + WebRtcVideoDecoderFactory* video_decoder_factory) #ifdef HAVE_WEBRTC_VIDEO - : CompositeMediaEngine(adm) { + : CompositeMediaEngine( + adm, audio_decoder_factory){ #else - : CompositeMediaEngine(adm) { + : CompositeMediaEngine( + adm, audio_decoder_factory) { #endif - video_.SetExternalDecoderFactory(decoder_factory); - video_.SetExternalEncoderFactory(encoder_factory); + video_.SetExternalDecoderFactory(video_decoder_factory); + video_.SetExternalEncoderFactory(video_encoder_factory); } }; @@ -45,10 +49,12 @@ class WebRtcMediaEngine2 cricket::MediaEngineInterface* CreateWebRtcMediaEngine( webrtc::AudioDeviceModule* adm, - cricket::WebRtcVideoEncoderFactory* encoder_factory, - cricket::WebRtcVideoDecoderFactory* decoder_factory) { - return new cricket::WebRtcMediaEngine2(adm, encoder_factory, - decoder_factory); + const rtc::scoped_refptr& + audio_decoder_factory, + cricket::WebRtcVideoEncoderFactory* video_encoder_factory, + cricket::WebRtcVideoDecoderFactory* video_decoder_factory) { + return new cricket::WebRtcMediaEngine2( + adm, audio_decoder_factory, video_encoder_factory, video_decoder_factory); } void DestroyWebRtcMediaEngine(cricket::MediaEngineInterface* media_engine) { @@ -61,9 +67,12 @@ namespace cricket { // ChannelManager. MediaEngineInterface* WebRtcMediaEngineFactory::Create( webrtc::AudioDeviceModule* adm, - WebRtcVideoEncoderFactory* encoder_factory, - WebRtcVideoDecoderFactory* decoder_factory) { - return CreateWebRtcMediaEngine(adm, encoder_factory, decoder_factory); + const rtc::scoped_refptr& + audio_decoder_factory, + WebRtcVideoEncoderFactory* video_encoder_factory, + WebRtcVideoDecoderFactory* video_decoder_factory) { + return CreateWebRtcMediaEngine(adm, audio_decoder_factory, + video_encoder_factory, video_decoder_factory); } namespace { diff --git a/webrtc/media/engine/webrtcmediaengine.h b/webrtc/media/engine/webrtcmediaengine.h index 8aad94ccdf..332f8751e4 100644 --- a/webrtc/media/engine/webrtcmediaengine.h +++ b/webrtc/media/engine/webrtcmediaengine.h @@ -31,8 +31,10 @@ class WebRtcMediaEngineFactory { public: static MediaEngineInterface* Create( webrtc::AudioDeviceModule* adm, - WebRtcVideoEncoderFactory* encoder_factory, - WebRtcVideoDecoderFactory* decoder_factory); + const rtc::scoped_refptr& + audio_decoder_factory, + WebRtcVideoEncoderFactory* video_encoder_factory, + WebRtcVideoDecoderFactory* video_decoder_factory); }; // Verify that extension IDs are within 1-byte extension range and are not diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index fd4bf8de6b..111344f8c1 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -99,12 +99,8 @@ class WebRtcVideoEngine2Test : public ::testing::Test { public: WebRtcVideoEngine2Test() : WebRtcVideoEngine2Test("") {} explicit WebRtcVideoEngine2Test(const char* field_trials) - : WebRtcVideoEngine2Test(nullptr, field_trials) {} - WebRtcVideoEngine2Test(WebRtcVoiceEngine* voice_engine, - const char* field_trials) : override_field_trials_(field_trials), call_(webrtc::Call::Create(webrtc::Call::Config())), - voice_engine_(nullptr), engine_() { std::vector engine_codecs = engine_.codecs(); RTC_DCHECK(!engine_codecs.empty()); @@ -144,7 +140,6 @@ class WebRtcVideoEngine2Test : public ::testing::Test { // Used in WebRtcVideoEngine2VoiceTest, but defined here so it's properly // initialized when the constructor is called. std::unique_ptr call_; - WebRtcVoiceEngine voice_engine_; WebRtcVideoEngine2 engine_; VideoCodec default_codec_; VideoCodec default_red_codec_; diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index 95b08e7158..b78d73ad0b 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -520,14 +520,18 @@ bool WebRtcVoiceEngine::ToCodecInst(const AudioCodec& in, return WebRtcVoiceCodecs::ToCodecInst(in, out); } -WebRtcVoiceEngine::WebRtcVoiceEngine(webrtc::AudioDeviceModule* adm) - : WebRtcVoiceEngine(adm, new VoEWrapper()) { +WebRtcVoiceEngine::WebRtcVoiceEngine( + webrtc::AudioDeviceModule* adm, + const rtc::scoped_refptr& decoder_factory) + : WebRtcVoiceEngine(adm, decoder_factory, new VoEWrapper()) { audio_state_ = webrtc::AudioState::Create(MakeAudioStateConfig(voe())); } -WebRtcVoiceEngine::WebRtcVoiceEngine(webrtc::AudioDeviceModule* adm, - VoEWrapper* voe_wrapper) - : adm_(adm), voe_wrapper_(voe_wrapper) { +WebRtcVoiceEngine::WebRtcVoiceEngine( + webrtc::AudioDeviceModule* adm, + const rtc::scoped_refptr& decoder_factory, + VoEWrapper* voe_wrapper) + : adm_(adm), decoder_factory_(decoder_factory), voe_wrapper_(voe_wrapper) { RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); LOG(LS_INFO) << "WebRtcVoiceEngine::WebRtcVoiceEngine"; RTC_DCHECK(voe_wrapper); @@ -547,7 +551,8 @@ WebRtcVoiceEngine::WebRtcVoiceEngine(webrtc::AudioDeviceModule* adm, webrtc::Trace::SetTraceCallback(this); webrtc::Trace::set_level_filter(kElevatedTraceFilter); LOG(LS_INFO) << webrtc::VoiceEngine::GetVersionString(); - RTC_CHECK_EQ(0, voe_wrapper_->base()->Init(adm_.get())); + RTC_CHECK_EQ(0, voe_wrapper_->base()->Init(adm_.get(), nullptr, + decoder_factory_)); webrtc::Trace::set_level_filter(kDefaultTraceFilter); // No ADM supplied? Get the default one from VoE. @@ -1275,14 +1280,16 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { public: - WebRtcAudioReceiveStream(int ch, - uint32_t remote_ssrc, - uint32_t local_ssrc, - bool use_transport_cc, - const std::string& sync_group, - const std::vector& extensions, - webrtc::Call* call, - webrtc::Transport* rtcp_send_transport) + WebRtcAudioReceiveStream( + int ch, + uint32_t remote_ssrc, + uint32_t local_ssrc, + bool use_transport_cc, + const std::string& sync_group, + const std::vector& extensions, + webrtc::Call* call, + webrtc::Transport* rtcp_send_transport, + const rtc::scoped_refptr& decoder_factory) : call_(call), config_() { RTC_DCHECK_GE(ch, 0); RTC_DCHECK(call); @@ -1291,6 +1298,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream { config_.rtcp_send_transport = rtcp_send_transport; config_.voe_channel_id = ch; config_.sync_group = sync_group; + config_.decoder_factory = decoder_factory; RecreateAudioReceiveStream(use_transport_cc, extensions); } @@ -2168,7 +2176,8 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) { ssrc, new WebRtcAudioReceiveStream(channel, ssrc, receiver_reports_ssrc_, recv_transport_cc_enabled_, sp.sync_label, recv_rtp_extensions_, - call_, this))); + call_, this, + engine()->decoder_factory_))); SetNack(channel, send_codec_spec_.nack_enabled); SetPlayout(channel, playout_); diff --git a/webrtc/media/engine/webrtcvoiceengine.h b/webrtc/media/engine/webrtcvoiceengine.h index 5b435348e6..63ef3f305d 100644 --- a/webrtc/media/engine/webrtcvoiceengine.h +++ b/webrtc/media/engine/webrtcvoiceengine.h @@ -46,9 +46,14 @@ class WebRtcVoiceEngine final : public webrtc::TraceCallback { // Exposed for the WVoE/MC unit test. static bool ToCodecInst(const AudioCodec& in, webrtc::CodecInst* out); - explicit WebRtcVoiceEngine(webrtc::AudioDeviceModule* adm); + WebRtcVoiceEngine( + webrtc::AudioDeviceModule* adm, + const rtc::scoped_refptr& decoder_factory); // Dependency injection for testing. - WebRtcVoiceEngine(webrtc::AudioDeviceModule* adm, VoEWrapper* voe_wrapper); + WebRtcVoiceEngine( + webrtc::AudioDeviceModule* adm, + const rtc::scoped_refptr& decoder_factory, + VoEWrapper* voe_wrapper); ~WebRtcVoiceEngine() override; rtc::scoped_refptr GetAudioState() const; @@ -112,6 +117,7 @@ class WebRtcVoiceEngine final : public webrtc::TraceCallback { // The audio device manager. rtc::scoped_refptr adm_; + rtc::scoped_refptr decoder_factory_; // The primary instance of WebRtc VoiceEngine. std::unique_ptr voe_wrapper_; rtc::scoped_refptr audio_state_; diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc index 337f1d581f..331e6250ce 100644 --- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc +++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc @@ -24,6 +24,8 @@ #include "webrtc/media/engine/fakewebrtccall.h" #include "webrtc/media/engine/fakewebrtcvoiceengine.h" #include "webrtc/media/engine/webrtcvoiceengine.h" +#include "webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.h" +#include "webrtc/modules/audio_coding/codecs/mock/mock_audio_decoder_factory.h" #include "webrtc/modules/audio_device/include/mock_audio_device.h" using testing::Return; @@ -73,7 +75,7 @@ TEST(WebRtcVoiceEngineTestStubLibrary, StartupShutdown) { cricket::FakeWebRtcVoiceEngine voe; EXPECT_FALSE(voe.IsInited()); { - cricket::WebRtcVoiceEngine engine(&adm, new FakeVoEWrapper(&voe)); + cricket::WebRtcVoiceEngine engine(&adm, nullptr, new FakeVoEWrapper(&voe)); EXPECT_TRUE(voe.IsInited()); } EXPECT_FALSE(voe.IsInited()); @@ -99,7 +101,7 @@ class WebRtcVoiceEngineTestFake : public testing::Test { EXPECT_CALL(adm_, BuiltInAECIsAvailable()).WillOnce(Return(false)); EXPECT_CALL(adm_, BuiltInAGCIsAvailable()).WillOnce(Return(false)); EXPECT_CALL(adm_, BuiltInNSIsAvailable()).WillOnce(Return(false)); - engine_.reset(new cricket::WebRtcVoiceEngine(&adm_, + engine_.reset(new cricket::WebRtcVoiceEngine(&adm_, nullptr, new FakeVoEWrapper(&voe_))); send_parameters_.codecs.push_back(kPcmuCodec); recv_parameters_.codecs.push_back(kPcmuCodec); @@ -3535,7 +3537,20 @@ TEST_F(WebRtcVoiceEngineTestFake, OnReadyToSendSignalsNetworkState) { // Tests that the library initializes and shuts down properly. TEST(WebRtcVoiceEngineTest, StartupShutdown) { - cricket::WebRtcVoiceEngine engine(nullptr); + using testing::_; + using testing::AnyNumber; + + // If the VoiceEngine wants to gather available codecs early, that's fine but + // we never want it to create a decoder at this stage. + rtc::scoped_refptr factory = + new rtc::RefCountedObject; + ON_CALL(*factory.get(), GetSupportedFormats()) + .WillByDefault(Return(std::vector())); + EXPECT_CALL(*factory.get(), GetSupportedFormats()) + .Times(AnyNumber()); + EXPECT_CALL(*factory.get(), MakeAudioDecoderMock(_, _)).Times(0); + + cricket::WebRtcVoiceEngine engine(nullptr, factory); std::unique_ptr call( webrtc::Call::Create(webrtc::Call::Config())); cricket::VoiceMediaChannel* channel = engine.CreateChannel( @@ -3550,7 +3565,7 @@ TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) { EXPECT_CALL(adm, AddRef()).Times(3).WillRepeatedly(Return(0)); EXPECT_CALL(adm, Release()).Times(3).WillRepeatedly(Return(0)); { - cricket::WebRtcVoiceEngine engine(&adm); + cricket::WebRtcVoiceEngine engine(&adm, nullptr); std::unique_ptr call( webrtc::Call::Create(webrtc::Call::Config())); cricket::VoiceMediaChannel* channel = engine.CreateChannel( @@ -3562,6 +3577,9 @@ TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) { // Tests that the library is configured with the codecs we want. TEST(WebRtcVoiceEngineTest, HasCorrectCodecs) { + // TODO(ossu): These tests should move into a future "builtin audio codecs" + // module. + // Check codecs by name. EXPECT_TRUE(cricket::WebRtcVoiceEngine::ToCodecInst( cricket::AudioCodec(96, "OPUS", 48000, 0, 2), nullptr)); @@ -3615,7 +3633,8 @@ TEST(WebRtcVoiceEngineTest, HasCorrectCodecs) { cricket::AudioCodec(0, "", 0, 5000, 1), nullptr)); // Verify the payload id of common audio codecs, including CN, ISAC, and G722. - cricket::WebRtcVoiceEngine engine(nullptr); + cricket::WebRtcVoiceEngine engine(nullptr, + webrtc::CreateBuiltinAudioDecoderFactory()); for (std::vector::const_iterator it = engine.codecs().begin(); it != engine.codecs().end(); ++it) { if (it->name == "CN" && it->clockrate == 16000) { @@ -3644,7 +3663,7 @@ TEST(WebRtcVoiceEngineTest, HasCorrectCodecs) { // Tests that VoE supports at least 32 channels TEST(WebRtcVoiceEngineTest, Has32Channels) { - cricket::WebRtcVoiceEngine engine(nullptr); + cricket::WebRtcVoiceEngine engine(nullptr, nullptr); std::unique_ptr call( webrtc::Call::Create(webrtc::Call::Config())); @@ -3668,7 +3687,15 @@ TEST(WebRtcVoiceEngineTest, Has32Channels) { // Test that we set our preferred codecs properly. TEST(WebRtcVoiceEngineTest, SetRecvCodecs) { - cricket::WebRtcVoiceEngine engine(nullptr); + // 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. + // It does not check that the codecs in the RecvParameters are actually + // what we sent in - though it's probably reasonable to expect so, if + // SetRecvParameters returns true. + // I think it will become clear once audio decoder injection is completed. + cricket::WebRtcVoiceEngine engine( + nullptr, webrtc::CreateBuiltinAudioDecoderFactory()); std::unique_ptr call( webrtc::Call::Create(webrtc::Call::Config())); cricket::WebRtcVoiceMediaChannel channel(&engine, cricket::MediaConfig(), diff --git a/webrtc/test/call_test.cc b/webrtc/test/call_test.cc index 3ec3da4fc3..919ebe812b 100644 --- a/webrtc/test/call_test.cc +++ b/webrtc/test/call_test.cc @@ -10,6 +10,7 @@ #include "webrtc/base/checks.h" #include "webrtc/common.h" #include "webrtc/config.h" +#include "webrtc/modules/audio_coding/codecs/builtin_audio_decoder_factory.h" #include "webrtc/test/call_test.h" #include "webrtc/test/encoder_settings.h" #include "webrtc/test/testsupport/fileutils.h" @@ -32,6 +33,7 @@ CallTest::CallTest() fake_encoder_(clock_), num_video_streams_(1), num_audio_streams_(0), + decoder_factory_(CreateBuiltinAudioDecoderFactory()), fake_send_audio_device_(nullptr), fake_recv_audio_device_(nullptr) {} @@ -229,6 +231,7 @@ void CallTest::CreateMatchingReceiveConfigs(Transport* rtcp_send_transport) { audio_config.rtcp_send_transport = rtcp_send_transport; audio_config.voe_channel_id = voe_recv_.channel_id; audio_config.rtp.remote_ssrc = audio_send_config_.rtp.ssrc; + audio_config.decoder_factory = decoder_factory_; audio_receive_configs_.push_back(audio_config); } } @@ -307,7 +310,8 @@ void CallTest::CreateVoiceEngines() { voe_send_.voice_engine = VoiceEngine::Create(); voe_send_.base = VoEBase::GetInterface(voe_send_.voice_engine); voe_send_.codec = VoECodec::GetInterface(voe_send_.voice_engine); - EXPECT_EQ(0, voe_send_.base->Init(fake_send_audio_device_.get(), nullptr)); + EXPECT_EQ(0, voe_send_.base->Init(fake_send_audio_device_.get(), nullptr, + decoder_factory_)); Config voe_config; voe_config.Set(new VoicePacing(true)); voe_send_.channel_id = voe_send_.base->CreateChannel(voe_config); @@ -316,7 +320,8 @@ void CallTest::CreateVoiceEngines() { voe_recv_.voice_engine = VoiceEngine::Create(); voe_recv_.base = VoEBase::GetInterface(voe_recv_.voice_engine); voe_recv_.codec = VoECodec::GetInterface(voe_recv_.voice_engine); - EXPECT_EQ(0, voe_recv_.base->Init(fake_recv_audio_device_.get(), nullptr)); + EXPECT_EQ(0, voe_recv_.base->Init(fake_recv_audio_device_.get(), nullptr, + decoder_factory_)); voe_recv_.channel_id = voe_recv_.base->CreateChannel(); EXPECT_GE(voe_recv_.channel_id, 0); } diff --git a/webrtc/test/call_test.h b/webrtc/test/call_test.h index ebc2bb2a54..d8019e55b8 100644 --- a/webrtc/test/call_test.h +++ b/webrtc/test/call_test.h @@ -102,6 +102,7 @@ class CallTest : public ::testing::Test { std::vector> allocated_decoders_; size_t num_video_streams_; size_t num_audio_streams_; + rtc::scoped_refptr decoder_factory_; private: // TODO(holmer): Remove once VoiceEngine is fully refactored to the new API. diff --git a/webrtc/test/mock_voe_channel_proxy.h b/webrtc/test/mock_voe_channel_proxy.h index a27a739006..cc2c32b373 100644 --- a/webrtc/test/mock_voe_channel_proxy.h +++ b/webrtc/test/mock_voe_channel_proxy.h @@ -50,6 +50,8 @@ class MockVoEChannelProxy : public voe::ChannelProxy { size_t length, const PacketTime& packet_time)); MOCK_METHOD2(ReceivedRTCPPacket, bool(const uint8_t* packet, size_t length)); + MOCK_CONST_METHOD0(GetAudioDecoderFactory, + const rtc::scoped_refptr&()); }; } // namespace test } // namespace webrtc diff --git a/webrtc/test/mock_voice_engine.h b/webrtc/test/mock_voice_engine.h index aa71e434b7..be72e0d631 100644 --- a/webrtc/test/mock_voice_engine.h +++ b/webrtc/test/mock_voice_engine.h @@ -28,17 +28,26 @@ class MockVoiceEngine : public VoiceEngineImpl { // methods don't use any override declarations, and we want to avoid // warnings from -Winconsistent-missing-override. See // http://crbug.com/428099. - MockVoiceEngine() : VoiceEngineImpl(new Config(), true) { + MockVoiceEngine( + rtc::scoped_refptr decoder_factory = nullptr) + : VoiceEngineImpl(new Config(), true), + decoder_factory_(decoder_factory) { // Increase ref count so this object isn't automatically deleted whenever // interfaces are Release():d. ++_ref_count; // We add this default behavior to make the mock easier to use in tests. It // will create a NiceMock of a voe::ChannelProxy. + // TODO(ossu): As long as AudioReceiveStream is implmented as a wrapper + // around Channel, we need to make sure ChannelProxy returns the same + // decoder factory as the one passed in when creating an AudioReceiveStream. ON_CALL(*this, ChannelProxyFactory(testing::_)) - .WillByDefault( - testing::Invoke([](int channel_id) { - return new testing::NiceMock(); - })); + .WillByDefault(testing::Invoke([this](int channel_id) { + auto* proxy = + new testing::NiceMock(); + EXPECT_CALL(*proxy, GetAudioDecoderFactory()) + .WillRepeatedly(testing::ReturnRef(decoder_factory_)); + return proxy; + })); } ~MockVoiceEngine() /* override */ { // Decrease ref count before base class d-tor is called; otherwise it will @@ -323,6 +332,15 @@ class MockVoiceEngine : public VoiceEngineImpl { MOCK_METHOD2(GetChannelOutputVolumeScaling, int(int channel, float& scaling)); MOCK_METHOD3(SetOutputVolumePan, int(int channel, float left, float right)); MOCK_METHOD3(GetOutputVolumePan, int(int channel, float& left, float& right)); + + private: + // TODO(ossu): I'm not particularly happy about keeping the decoder factory + // here, but due to how gmock is implemented, I cannot just keep it in the + // functor implementing the default version of ChannelProxyFactory, above. + // GMock creates an unfortunate copy of the functor, which would cause us to + // return a dangling reference. Fortunately, this should go away once + // voe::Channel does. + rtc::scoped_refptr decoder_factory_; }; } // namespace test } // namespace webrtc diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index a17f546f4e..6ec001218c 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -827,7 +827,8 @@ Channel::Channel(int32_t channelId, pacing_enabled_(config.Get().enabled), feedback_observer_proxy_(new TransportFeedbackProxy()), seq_num_allocator_proxy_(new TransportSequenceNumberProxy()), - rtp_packet_sender_proxy_(new RtpPacketSenderProxy()) { + rtp_packet_sender_proxy_(new RtpPacketSenderProxy()), + decoder_factory_(decoder_factory) { WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::Channel() - ctor"); AudioCodingModule::Config acm_config; @@ -1083,6 +1084,11 @@ void Channel::SetSink(std::unique_ptr sink) { audio_sink_ = std::move(sink); } +const rtc::scoped_refptr& +Channel::GetAudioDecoderFactory() const { + return decoder_factory_; +} + int32_t Channel::StartPlayout() { WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::StartPlayout()"); diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h index 9f789dfac3..5f1d7f29c9 100644 --- a/webrtc/voice_engine/channel.h +++ b/webrtc/voice_engine/channel.h @@ -199,6 +199,12 @@ class Channel void SetSink(std::unique_ptr sink); + // TODO(ossu): Don't use! It's only here to confirm that the decoder factory + // passed into AudioReceiveStream is the same as the one set when creating the + // ADM. Once Channel creation is moved into Audio{Send,Receive}Stream this can + // go. + const rtc::scoped_refptr& GetAudioDecoderFactory() const; + // API methods // VoEBase @@ -584,6 +590,9 @@ class Channel std::unique_ptr feedback_observer_proxy_; std::unique_ptr seq_num_allocator_proxy_; std::unique_ptr rtp_packet_sender_proxy_; + + // TODO(ossu): Remove once GetAudioDecoderFactory() is no longer needed. + rtc::scoped_refptr decoder_factory_; }; } // namespace voe diff --git a/webrtc/voice_engine/channel_proxy.cc b/webrtc/voice_engine/channel_proxy.cc index 4cc7f5cbbc..d16167f9bb 100644 --- a/webrtc/voice_engine/channel_proxy.cc +++ b/webrtc/voice_engine/channel_proxy.cc @@ -181,6 +181,11 @@ bool ChannelProxy::ReceivedRTCPPacket(const uint8_t* packet, size_t length) { return channel()->ReceivedRTCPPacket(packet, length) == 0; } +const rtc::scoped_refptr& +ChannelProxy::GetAudioDecoderFactory() const { + return channel()->GetAudioDecoderFactory(); +} + Channel* ChannelProxy::channel() const { RTC_DCHECK(channel_owner_.channel()); return channel_owner_.channel(); diff --git a/webrtc/voice_engine/channel_proxy.h b/webrtc/voice_engine/channel_proxy.h index df0c3f22ef..8159606ec5 100644 --- a/webrtc/voice_engine/channel_proxy.h +++ b/webrtc/voice_engine/channel_proxy.h @@ -80,6 +80,9 @@ class ChannelProxy { const PacketTime& packet_time); virtual bool ReceivedRTCPPacket(const uint8_t* packet, size_t length); + virtual const rtc::scoped_refptr& + GetAudioDecoderFactory() const; + private: Channel* channel() const;