From 3e9e5b39d14861920036f66b0493ba7239ed36d6 Mon Sep 17 00:00:00 2001 From: Karl Wiberg Date: Mon, 6 Nov 2017 05:01:56 +0100 Subject: [PATCH] OrtcFactoryInterface::Create(): Require caller to supply audio codec factories So that we don't have to be capable of creating one ourselves, which requires a dependency on the audio codecs. BUG=webrtc:8396 Change-Id: I5600da5e17f613b0e61a9fb0fbdb373fe42f855c Reviewed-on: https://webrtc-review.googlesource.com/20220 Reviewed-by: Peter Thatcher Commit-Queue: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#20641} --- api/ortc/ortcfactoryinterface.h | 24 +++++++++----- ortc/BUILD.gn | 4 +-- ortc/ortcfactory.cc | 43 ++++++++++++++++--------- ortc/ortcfactory.h | 12 +++++-- ortc/ortcfactory_integrationtest.cc | 22 ++++++++----- ortc/ortcfactory_unittest.cc | 6 +++- ortc/ortcrtpreceiver_unittest.cc | 5 ++- ortc/ortcrtpsender_unittest.cc | 5 ++- ortc/rtptransport_unittest.cc | 5 ++- ortc/rtptransportcontroller_unittest.cc | 11 ++++--- ortc/srtptransport_unittest.cc | 5 ++- 11 files changed, 96 insertions(+), 46 deletions(-) diff --git a/api/ortc/ortcfactoryinterface.h b/api/ortc/ortcfactoryinterface.h index 15be4f0250..a279a7b33f 100644 --- a/api/ortc/ortcfactoryinterface.h +++ b/api/ortc/ortcfactoryinterface.h @@ -76,21 +76,29 @@ class OrtcFactoryInterface { // be injected; otherwise a platform-specific module will be used that will // use the default audio input. // + // |audio_encoder_factory| and |audio_decoder_factory| are used to + // instantiate audio codecs; they determine what codecs are supported. + // // Note that the OrtcFactoryInterface does not take ownership of any of the - // objects passed in, and as previously stated, these objects can't be - // destroyed before the factory is. + // objects passed in by raw pointer, and as previously stated, these objects + // can't be destroyed before the factory is. static RTCErrorOr> Create( rtc::Thread* network_thread, rtc::Thread* signaling_thread, rtc::NetworkManager* network_manager, rtc::PacketSocketFactory* socket_factory, - AudioDeviceModule* adm); + AudioDeviceModule* adm, + rtc::scoped_refptr audio_encoder_factory, + rtc::scoped_refptr audio_decoder_factory); - // Constructor for convenience which uses default implementations of - // everything (though does still require that the current thread runs a - // message loop; see above). - static RTCErrorOr> Create() { - return Create(nullptr, nullptr, nullptr, nullptr, nullptr); + // Constructor for convenience which uses default implementations where + // possible (though does still require that the current thread runs a message + // loop; see above). + static RTCErrorOr> Create( + rtc::scoped_refptr audio_encoder_factory, + rtc::scoped_refptr audio_decoder_factory) { + return Create(nullptr, nullptr, nullptr, nullptr, nullptr, + audio_encoder_factory, audio_decoder_factory); } virtual ~OrtcFactoryInterface() {} diff --git a/ortc/BUILD.gn b/ortc/BUILD.gn index 485771c9a4..ec1a5c4a7f 100644 --- a/ortc/BUILD.gn +++ b/ortc/BUILD.gn @@ -34,8 +34,6 @@ rtc_static_library("ortc") { # libjingle_peerconnection. deps = [ "../api:optional", - "../api/audio_codecs:builtin_audio_decoder_factory", - "../api/audio_codecs:builtin_audio_encoder_factory", "../call:call_interfaces", "../call:rtp_sender", "../logging:rtc_event_log_api", @@ -78,6 +76,8 @@ if (rtc_include_tests) { deps = [ ":ortc", + "../api/audio_codecs:builtin_audio_decoder_factory", + "../api/audio_codecs:builtin_audio_encoder_factory", "../media:rtc_media_tests_utils", "../p2p:p2p_test_utils", "../p2p:rtc_p2p", diff --git a/ortc/ortcfactory.cc b/ortc/ortcfactory.cc index 62939b9b55..7492dd47aa 100644 --- a/ortc/ortcfactory.cc +++ b/ortc/ortcfactory.cc @@ -14,8 +14,6 @@ #include // For std::move. #include -#include "api/audio_codecs/builtin_audio_decoder_factory.h" -#include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "api/mediastreamtrackproxy.h" #include "api/proxy.h" #include "api/rtcerror.h" @@ -137,7 +135,9 @@ RTCErrorOr> OrtcFactory::Create( rtc::NetworkManager* network_manager, rtc::PacketSocketFactory* socket_factory, AudioDeviceModule* adm, - std::unique_ptr media_engine) { + std::unique_ptr media_engine, + rtc::scoped_refptr audio_encoder_factory, + rtc::scoped_refptr audio_decoder_factory) { // Hop to signaling thread if needed. if (signaling_thread && !signaling_thread->IsCurrent()) { return signaling_thread @@ -145,10 +145,12 @@ RTCErrorOr> OrtcFactory::Create( RTC_FROM_HERE, rtc::Bind(&OrtcFactory::Create_s, network_thread, signaling_thread, network_manager, socket_factory, adm, - media_engine.release())); + media_engine.release(), audio_encoder_factory, + audio_decoder_factory)); } return Create_s(network_thread, signaling_thread, network_manager, - socket_factory, adm, media_engine.release()); + socket_factory, adm, media_engine.release(), + audio_encoder_factory, audio_decoder_factory); } RTCErrorOr> OrtcFactoryInterface::Create( @@ -156,24 +158,30 @@ RTCErrorOr> OrtcFactoryInterface::Create( rtc::Thread* signaling_thread, rtc::NetworkManager* network_manager, rtc::PacketSocketFactory* socket_factory, - AudioDeviceModule* adm) { + AudioDeviceModule* adm, + rtc::scoped_refptr audio_encoder_factory, + rtc::scoped_refptr audio_decoder_factory) { return OrtcFactory::Create(network_thread, signaling_thread, network_manager, - socket_factory, adm, nullptr); + socket_factory, adm, nullptr, + audio_encoder_factory, audio_decoder_factory); } -OrtcFactory::OrtcFactory(rtc::Thread* network_thread, - rtc::Thread* signaling_thread, - rtc::NetworkManager* network_manager, - rtc::PacketSocketFactory* socket_factory, - AudioDeviceModule* adm) +OrtcFactory::OrtcFactory( + rtc::Thread* network_thread, + rtc::Thread* signaling_thread, + rtc::NetworkManager* network_manager, + rtc::PacketSocketFactory* socket_factory, + AudioDeviceModule* adm, + rtc::scoped_refptr audio_encoder_factory, + rtc::scoped_refptr audio_decoder_factory) : network_thread_(network_thread), signaling_thread_(signaling_thread), network_manager_(network_manager), socket_factory_(socket_factory), adm_(adm), null_event_log_(RtcEventLog::CreateNull()), - audio_encoder_factory_(CreateBuiltinAudioEncoderFactory()), - audio_decoder_factory_(CreateBuiltinAudioDecoderFactory()) { + audio_encoder_factory_(audio_encoder_factory), + audio_decoder_factory_(audio_decoder_factory) { if (!rtc::CreateRandomString(kDefaultRtcpCnameLength, &default_cname_)) { RTC_LOG(LS_ERROR) << "Failed to generate CNAME?"; RTC_NOTREACHED(); @@ -502,12 +510,15 @@ RTCErrorOr> OrtcFactory::Create_s( rtc::NetworkManager* network_manager, rtc::PacketSocketFactory* socket_factory, AudioDeviceModule* adm, - cricket::MediaEngineInterface* media_engine) { + cricket::MediaEngineInterface* media_engine, + rtc::scoped_refptr audio_encoder_factory, + rtc::scoped_refptr audio_decoder_factory) { // Add the unique_ptr wrapper back. std::unique_ptr owned_media_engine( media_engine); std::unique_ptr new_factory(new OrtcFactory( - network_thread, signaling_thread, network_manager, socket_factory, adm)); + network_thread, signaling_thread, network_manager, socket_factory, adm, + audio_encoder_factory, audio_decoder_factory)); RTCError err = new_factory->Initialize(std::move(owned_media_engine)); if (!err.ok()) { return std::move(err); diff --git a/ortc/ortcfactory.h b/ortc/ortcfactory.h index 89f1ee009b..750fcf7586 100644 --- a/ortc/ortcfactory.h +++ b/ortc/ortcfactory.h @@ -38,7 +38,9 @@ class OrtcFactory : public OrtcFactoryInterface { rtc::NetworkManager* network_manager, rtc::PacketSocketFactory* socket_factory, AudioDeviceModule* adm, - std::unique_ptr media_engine); + std::unique_ptr media_engine, + rtc::scoped_refptr audio_encoder_factory, + rtc::scoped_refptr audio_decoder_factory); RTCErrorOr> CreateRtpTransportController() override; @@ -101,7 +103,9 @@ class OrtcFactory : public OrtcFactoryInterface { rtc::Thread* signaling_thread, rtc::NetworkManager* network_manager, rtc::PacketSocketFactory* socket_factory, - AudioDeviceModule* adm); + AudioDeviceModule* adm, + rtc::scoped_refptr audio_encoder_factory, + rtc::scoped_refptr audio_decoder_factory); RTCErrorOr> CreateRtpTransportController(const RtpTransportParameters& parameters); @@ -114,7 +118,9 @@ class OrtcFactory : public OrtcFactoryInterface { rtc::NetworkManager* network_manager, rtc::PacketSocketFactory* socket_factory, AudioDeviceModule* adm, - cricket::MediaEngineInterface* media_engine); + cricket::MediaEngineInterface* media_engine, + rtc::scoped_refptr audio_encoder_factory, + rtc::scoped_refptr audio_decoder_factory); // Performs initialization that can fail. Called by factory method after // construction, and if it fails, no object is returned. diff --git a/ortc/ortcfactory_integrationtest.cc b/ortc/ortcfactory_integrationtest.cc index 70b315ca10..6385dee916 100644 --- a/ortc/ortcfactory_integrationtest.cc +++ b/ortc/ortcfactory_integrationtest.cc @@ -11,6 +11,8 @@ #include #include // For std::pair, std::move. +#include "api/audio_codecs/builtin_audio_decoder_factory.h" +#include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "api/ortc/ortcfactoryinterface.h" #include "ortc/testrtpparameters.h" #include "p2p/base/udptransport.h" @@ -77,14 +79,18 @@ class OrtcFactoryIntegrationTest : public testing::Test { virtual_socket_server_.SetDefaultRoute(kIPv4LocalHostAddress); network_thread_.Start(); // Need to create after network thread is started. - ortc_factory1_ = OrtcFactoryInterface::Create( - &network_thread_, nullptr, &fake_network_manager_, - nullptr, fake_audio_capture_module1_) - .MoveValue(); - ortc_factory2_ = OrtcFactoryInterface::Create( - &network_thread_, nullptr, &fake_network_manager_, - nullptr, fake_audio_capture_module2_) - .MoveValue(); + ortc_factory1_ = + OrtcFactoryInterface::Create( + &network_thread_, nullptr, &fake_network_manager_, nullptr, + fake_audio_capture_module1_, CreateBuiltinAudioEncoderFactory(), + CreateBuiltinAudioDecoderFactory()) + .MoveValue(); + ortc_factory2_ = + OrtcFactoryInterface::Create( + &network_thread_, nullptr, &fake_network_manager_, nullptr, + fake_audio_capture_module2_, CreateBuiltinAudioEncoderFactory(), + CreateBuiltinAudioDecoderFactory()) + .MoveValue(); } protected: diff --git a/ortc/ortcfactory_unittest.cc b/ortc/ortcfactory_unittest.cc index 649dafbdf1..40afbd328b 100644 --- a/ortc/ortcfactory_unittest.cc +++ b/ortc/ortcfactory_unittest.cc @@ -10,6 +10,8 @@ #include +#include "api/audio_codecs/builtin_audio_decoder_factory.h" +#include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "media/base/fakemediaengine.h" #include "ortc/ortcfactory.h" #include "ortc/testrtpparameters.h" @@ -32,7 +34,9 @@ class OrtcFactoryTest : public testing::Test { OrtcFactory::Create(&thread_, nullptr, &fake_network_manager_, nullptr, nullptr, std::unique_ptr( - new cricket::FakeMediaEngine())) + new cricket::FakeMediaEngine()), + CreateBuiltinAudioEncoderFactory(), + CreateBuiltinAudioDecoderFactory()) .MoveValue(); } diff --git a/ortc/ortcrtpreceiver_unittest.cc b/ortc/ortcrtpreceiver_unittest.cc index 47a3d76edb..f0e9238714 100644 --- a/ortc/ortcrtpreceiver_unittest.cc +++ b/ortc/ortcrtpreceiver_unittest.cc @@ -10,6 +10,8 @@ #include +#include "api/audio_codecs/builtin_audio_decoder_factory.h" +#include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "media/base/fakemediaengine.h" #include "ortc/ortcfactory.h" #include "ortc/testrtpparameters.h" @@ -34,7 +36,8 @@ class OrtcRtpReceiverTest : public testing::Test { // use FakePacketTransport. auto ortc_factory_result = OrtcFactory::Create( nullptr, nullptr, nullptr, nullptr, nullptr, - std::unique_ptr(fake_media_engine_)); + std::unique_ptr(fake_media_engine_), + CreateBuiltinAudioEncoderFactory(), CreateBuiltinAudioDecoderFactory()); ortc_factory_ = ortc_factory_result.MoveValue(); RtpTransportParameters parameters; parameters.rtcp.mux = true; diff --git a/ortc/ortcrtpsender_unittest.cc b/ortc/ortcrtpsender_unittest.cc index 233c8fadc7..c8bd8f9393 100644 --- a/ortc/ortcrtpsender_unittest.cc +++ b/ortc/ortcrtpsender_unittest.cc @@ -10,6 +10,8 @@ #include +#include "api/audio_codecs/builtin_audio_decoder_factory.h" +#include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "media/base/fakemediaengine.h" #include "ortc/ortcfactory.h" #include "ortc/testrtpparameters.h" @@ -38,7 +40,8 @@ class OrtcRtpSenderTest : public testing::Test { // use FakePacketTransport. auto ortc_factory_result = OrtcFactory::Create( nullptr, nullptr, nullptr, nullptr, nullptr, - std::unique_ptr(fake_media_engine_)); + std::unique_ptr(fake_media_engine_), + CreateBuiltinAudioEncoderFactory(), CreateBuiltinAudioDecoderFactory()); ortc_factory_ = ortc_factory_result.MoveValue(); RtpTransportParameters parameters; parameters.rtcp.mux = true; diff --git a/ortc/rtptransport_unittest.cc b/ortc/rtptransport_unittest.cc index 46c3965d92..92ad43fab3 100644 --- a/ortc/rtptransport_unittest.cc +++ b/ortc/rtptransport_unittest.cc @@ -10,6 +10,8 @@ #include +#include "api/audio_codecs/builtin_audio_decoder_factory.h" +#include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "media/base/fakemediaengine.h" #include "ortc/ortcfactory.h" #include "ortc/testrtpparameters.h" @@ -29,7 +31,8 @@ class RtpTransportTest : public testing::Test { // FakePacketTransports. auto result = OrtcFactory::Create( nullptr, nullptr, nullptr, nullptr, nullptr, - std::unique_ptr(fake_media_engine_)); + std::unique_ptr(fake_media_engine_), + CreateBuiltinAudioEncoderFactory(), CreateBuiltinAudioDecoderFactory()); ortc_factory_ = result.MoveValue(); } diff --git a/ortc/rtptransportcontroller_unittest.cc b/ortc/rtptransportcontroller_unittest.cc index ff07627fdf..a663b82e7f 100644 --- a/ortc/rtptransportcontroller_unittest.cc +++ b/ortc/rtptransportcontroller_unittest.cc @@ -10,6 +10,8 @@ #include +#include "api/audio_codecs/builtin_audio_decoder_factory.h" +#include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "media/base/fakemediaengine.h" #include "ortc/ortcfactory.h" #include "ortc/testrtpparameters.h" @@ -31,10 +33,11 @@ class RtpTransportControllerTest : public testing::Test { RtpTransportControllerTest() { // Note: This doesn't need to use fake network classes, since it uses // FakePacketTransports. - auto result = - OrtcFactory::Create(nullptr, nullptr, nullptr, nullptr, nullptr, - std::unique_ptr( - new cricket::FakeMediaEngine())); + auto result = OrtcFactory::Create( + nullptr, nullptr, nullptr, nullptr, nullptr, + std::unique_ptr( + new cricket::FakeMediaEngine()), + CreateBuiltinAudioEncoderFactory(), CreateBuiltinAudioDecoderFactory()); ortc_factory_ = result.MoveValue(); rtp_transport_controller_ = ortc_factory_->CreateRtpTransportController().MoveValue(); diff --git a/ortc/srtptransport_unittest.cc b/ortc/srtptransport_unittest.cc index 0a2532c94a..8323603af0 100644 --- a/ortc/srtptransport_unittest.cc +++ b/ortc/srtptransport_unittest.cc @@ -10,6 +10,8 @@ #include +#include "api/audio_codecs/builtin_audio_decoder_factory.h" +#include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "media/base/fakemediaengine.h" #include "ortc/ortcfactory.h" #include "ortc/testrtpparameters.h" @@ -51,7 +53,8 @@ class SrtpTransportTest : public testing::Test { // FakePacketTransports. auto result = OrtcFactory::Create( nullptr, nullptr, nullptr, nullptr, nullptr, - std::unique_ptr(fake_media_engine_)); + std::unique_ptr(fake_media_engine_), + CreateBuiltinAudioEncoderFactory(), CreateBuiltinAudioDecoderFactory()); ortc_factory_ = result.MoveValue(); rtp_transport_controller_ = ortc_factory_->CreateRtpTransportController().MoveValue();