From 4d8c81878e890215fc1d7447f98641e2c5e2530c Mon Sep 17 00:00:00 2001 From: "henrike@webrtc.org" Date: Mon, 31 Oct 2011 18:00:10 +0000 Subject: [PATCH] The implementation before this change list keeps the ownership of memory that is used by peer connection instances in the peer connection manager. This means that if the peer connection manager is deleted before all peer connections it has created, these peer connections will be pointing to invalid memory. The solution in this CL is to create a bundle of the memory that needs to be alive as long as there are any peer connections or peer connection manager instances. This bundle is scoped reference counted so that it is deleted only when there are no references to it. This enables the peer connection and manager to be deleted in any order. Review URL: http://webrtc-codereview.appspot.com/246003 git-svn-id: http://webrtc.googlecode.com/svn/trunk@843 4adac7df-926f-26a2-2b94-8c16560cd09d --- third_party_mods/libjingle/libjingle.gyp | 6 +- .../talk/app/webrtc_dev/peerconnection.h | 78 +++------ .../app/webrtc_dev/peerconnection_unittest.cc | 8 +- ...t.cc => peerconnectionfactory_unittest.cc} | 46 ++--- ...erimpl.cc => peerconnectionfactoryimpl.cc} | 164 ++++++++---------- ...agerimpl.h => peerconnectionfactoryimpl.h} | 30 ++-- .../talk/app/webrtc_dev/peerconnectionimpl.cc | 35 ++-- .../talk/app/webrtc_dev/peerconnectionimpl.h | 22 ++- .../webrtc_dev/peerconnectionimpl_unittest.cc | 4 +- .../peerconnection_client/conductor.cc | 3 +- .../peerconnection_client/conductor.h | 2 +- 11 files changed, 178 insertions(+), 220 deletions(-) rename third_party_mods/libjingle/source/talk/app/webrtc_dev/{peerconnectionmanager_unittest.cc => peerconnectionfactory_unittest.cc} (69%) rename third_party_mods/libjingle/source/talk/app/webrtc_dev/{peerconnectionmanagerimpl.cc => peerconnectionfactoryimpl.cc} (58%) rename third_party_mods/libjingle/source/talk/app/webrtc_dev/{peerconnectionmanagerimpl.h => peerconnectionfactoryimpl.h} (77%) diff --git a/third_party_mods/libjingle/libjingle.gyp b/third_party_mods/libjingle/libjingle.gyp index 79e1d2a926..113482c8f5 100644 --- a/third_party_mods/libjingle/libjingle.gyp +++ b/third_party_mods/libjingle/libjingle.gyp @@ -711,8 +711,8 @@ '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnection.h', '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionimpl.cc', '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionimpl.h', - '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.cc', - '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.h', + '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionfactoryimpl.cc', + '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionfactoryimpl.h', '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionmessage.cc', '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionmessage.h', '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionsignaling.cc', @@ -776,7 +776,7 @@ '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnection_unittest.cc', '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnection_unittests.cc', '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionimpl_unittest.cc', - '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionmanager_unittest.cc', + '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionfactory_unittest.cc', '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionmessage_unittest.cc', '<(libjingle_mods)/source/talk/app/webrtc_dev/peerconnectionsignaling_unittest.cc', '<(libjingle_mods)/source/talk/app/webrtc_dev/webrtcsession_unittest.cc', diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnection.h b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnection.h index 946a81afbb..495b714742 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnection.h +++ b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnection.h @@ -32,8 +32,8 @@ // peerconnection, mediastream and media tracks objects. // // The Following steps are needed to setup a typical call. -// 1. Create a PeerConnectionManager. Check constructors for more information -// about input parameters. +// 1. Create a PeerConnectionFactoryInterface. Check constructors for more +// information about input parameters. // 2. Create a PeerConnection object. Provide a configuration string which // points either to stun or turn server to generate ICE candidates and provide // an object that implements the PeerConnectionObserver interface. @@ -53,7 +53,7 @@ // The Receiver of a call can decide to accept or reject the call. // This decision will be taken by the application not peerconnection. // If application decides to accept the call -// 1. Create PeerConnectionManager if it doesn't exist. +// 1. Create PeerConnectionFactoryInterface if it doesn't exist. // 2. Create new PeerConnection // 3. Provide the remote offer to the new PeerConnection object by calling // ProcessSignalingMessage. @@ -156,57 +156,16 @@ class PeerConnectionInterface : public talk_base::RefCountInterface { ~PeerConnectionInterface() {} }; -// Reference counted wrapper for talk_base::NetworkManager. -class PcNetworkManager : public talk_base::RefCountInterface { - public: - static talk_base::scoped_refptr Create( - talk_base::NetworkManager* network_manager); - virtual talk_base::NetworkManager* network_manager() const; - - protected: - explicit PcNetworkManager(talk_base::NetworkManager* network_manager); - virtual ~PcNetworkManager(); - - talk_base::NetworkManager* network_manager_; -}; - -// Reference counted wrapper for talk_base::PacketSocketFactory. -class PcPacketSocketFactory : public talk_base::RefCountInterface { - public: - static talk_base::scoped_refptr Create( - talk_base::PacketSocketFactory* socket_factory); - virtual talk_base::PacketSocketFactory* socket_factory() const; - - protected: - explicit PcPacketSocketFactory( - talk_base::PacketSocketFactory* socket_factory); - virtual ~PcPacketSocketFactory(); - - talk_base::PacketSocketFactory* socket_factory_; -}; - -// PeerConnectionManager is the factory interface use for creating +// PeerConnectionFactoryInterface is the factory interface use for creating // PeerConnection, MediaStream and media tracks. -// PeerConnectionManager will create required libjingle threads, socket and -// network manager factory classes for networking. +// PeerConnectionFactoryInterface will create required libjingle threads, +// socket and network manager factory classes for networking. // If application decides to provide its own implementation of these classes // it should use alternate create method which accepts these parameters // as input. -class PeerConnectionManager : public talk_base::RefCountInterface { + +class PeerConnectionFactoryInterface : public talk_base::RefCountInterface { public: - // Create a new instance of PeerConnectionManager. - static talk_base::scoped_refptr Create(); - - // Create a new instance of PeerConnectionManager. - // Ownership of the arguments are not transfered to this object and must - // remain in scope for the lifetime of the PeerConnectionManager. - static talk_base::scoped_refptr Create( - talk_base::Thread* worker_thread, - talk_base::Thread* signaling_thread, - PcNetworkManager* network_manager, - PcPacketSocketFactory* packet_socket_factory, - AudioDeviceModule* default_adm); - virtual talk_base::scoped_refptr CreatePeerConnection(const std::string& config, PeerConnectionObserver* observer) = 0; @@ -223,10 +182,27 @@ class PeerConnectionManager : public talk_base::RefCountInterface { AudioDeviceModule* audio_device) = 0; protected: - // Dtor protected as objects shouldn't be deleted via this interface. - ~PeerConnectionManager() {} + // Dtor and ctor protected as objects shouldn't be created or deleted via + // this interface. + PeerConnectionFactoryInterface() {} + ~PeerConnectionFactoryInterface() {} // NOLINT }; +// Create a new instance of PeerConnectionFactoryInterface. +talk_base::scoped_refptr +CreatePeerConnectionFactory(); + +// Create a new instance of PeerConnectionFactoryInterface. +// Ownership of the arguments are not transfered to this object and must +// remain in scope for the lifetime of the PeerConnectionFactoryInterface. +talk_base::scoped_refptr +CreatePeerConnectionFactory( + talk_base::Thread* worker_thread, + talk_base::Thread* signaling_thread, + talk_base::NetworkManager* network_manager, + talk_base::PacketSocketFactory* packet_socket_factory, + AudioDeviceModule* default_adm); + } // namespace webrtc #endif // TALK_APP_WEBRTC_PEERCONNECTION_H_ diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnection_unittest.cc b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnection_unittest.cc index 92759b1d42..b6a0bfb896 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnection_unittest.cc +++ b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnection_unittest.cc @@ -173,10 +173,6 @@ class PeerConnectionP2PTestClient } ~PeerConnectionP2PTestClient() { - // Ensure that webrtc::PeerConnection is deleted before - // webrtc::PeerConnectionManager or crash will occur - webrtc::PeerConnectionInterface* temp = peer_connection_.release(); - temp->Release(); } void StartSession() { @@ -256,7 +252,7 @@ class PeerConnectionP2PTestClient bool Init() { EXPECT_TRUE(peer_connection_.get() == NULL); EXPECT_TRUE(peer_connection_factory_.get() == NULL); - peer_connection_factory_ = webrtc::PeerConnectionManager::Create(); + peer_connection_factory_ = webrtc::CreatePeerConnectionFactory(); if (peer_connection_factory_.get() == NULL) { ADD_FAILURE(); return false; @@ -278,7 +274,7 @@ class PeerConnectionP2PTestClient int id_; talk_base::scoped_refptr peer_connection_; - talk_base::scoped_refptr + talk_base::scoped_refptr peer_connection_factory_; // Remote peer communication. diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanager_unittest.cc b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionfactory_unittest.cc similarity index 69% rename from third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanager_unittest.cc rename to third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionfactory_unittest.cc index ced022ef52..37d81bd6dc 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanager_unittest.cc +++ b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionfactory_unittest.cc @@ -27,7 +27,7 @@ #include "gtest/gtest.h" #include "talk/app/webrtc_dev/mediastreamimpl.h" -#include "talk/app/webrtc_dev/peerconnectionmanagerimpl.h" +#include "talk/app/webrtc_dev/peerconnectionfactoryimpl.h" #include "talk/base/basicpacketsocketfactory.h" #include "talk/base/scoped_ptr.h" #include "talk/base/thread.h" @@ -56,22 +56,22 @@ class MockPeerConnectionObserver : public PeerConnectionObserver { }; // TODO(mallinath) - Fix drash when components are created in factory. -TEST(PeerConnectionManager, DISABLED_CreatePCUsingInternalModules) { +TEST(PeerConnectionFactory, DISABLED_CreatePCUsingInternalModules) { MockPeerConnectionObserver observer; - talk_base::scoped_refptr manager( - PeerConnectionManager::Create()); - ASSERT_TRUE(manager.get() != NULL); + talk_base::scoped_refptr factory( + CreatePeerConnectionFactory()); + ASSERT_TRUE(factory.get() != NULL); talk_base::scoped_refptr pc1( - manager->CreatePeerConnection("", &observer)); + factory->CreatePeerConnection("", &observer)); EXPECT_TRUE(pc1.get() == NULL); talk_base::scoped_refptr pc2( - manager->CreatePeerConnection(kStunConfiguration, &observer)); + factory->CreatePeerConnection(kStunConfiguration, &observer)); EXPECT_TRUE(pc2.get() != NULL); } -TEST(PeerConnectionManager, CreatePCUsingExternalModules) { +TEST(PeerConnectionFactory, CreatePCUsingExternalModules) { // Create an audio device. Use the default sound card. talk_base::scoped_refptr audio_device( AudioDeviceModuleImpl::Create(0)); @@ -80,27 +80,31 @@ TEST(PeerConnectionManager, CreatePCUsingExternalModules) { talk_base::scoped_ptr w_thread(new talk_base::Thread); EXPECT_TRUE(w_thread->Start()); - talk_base::scoped_refptr network_manager( - PcNetworkManager::Create(new talk_base::BasicNetworkManager)); - talk_base::scoped_refptr socket_factory( - PcPacketSocketFactory::Create(new talk_base::BasicPacketSocketFactory)); + // Ownership of these pointers is handed over to the PeerConnectionFactory. + // TODO(henrike): add a check that ensures that the destructor is called for + // these classes. E.g. by writing a wrapper and set a flag in the wrappers + // destructor, or e.g. add a callback. + talk_base::NetworkManager* network_manager = + new talk_base::BasicNetworkManager(); + talk_base::PacketSocketFactory* socket_factory = + new talk_base::BasicPacketSocketFactory(); - talk_base::scoped_refptr manager = - PeerConnectionManager::Create(talk_base::Thread::Current(), - talk_base::Thread::Current(), - network_manager, - socket_factory, - audio_device); - ASSERT_TRUE(manager.get() != NULL); + talk_base::scoped_refptr factory = + CreatePeerConnectionFactory(talk_base::Thread::Current(), + talk_base::Thread::Current(), + network_manager, + socket_factory, + audio_device); + ASSERT_TRUE(factory.get() != NULL); MockPeerConnectionObserver observer; talk_base::scoped_refptr pc1( - manager->CreatePeerConnection("", &observer)); + factory->CreatePeerConnection("", &observer)); EXPECT_TRUE(pc1.get() == NULL); talk_base::scoped_refptr pc2( - manager->CreatePeerConnection(kStunConfiguration, &observer)); + factory->CreatePeerConnection(kStunConfiguration, &observer)); EXPECT_TRUE(pc2.get() != NULL); } diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.cc b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionfactoryimpl.cc similarity index 58% rename from third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.cc rename to third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionfactoryimpl.cc index 77c464fc1c..2d2481fd2c 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.cc +++ b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionfactoryimpl.cc @@ -25,7 +25,7 @@ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include "talk/app/webrtc_dev/peerconnectionmanagerimpl.h" +#include "talk/app/webrtc_dev/peerconnectionfactoryimpl.h" #include "talk/app/webrtc_dev/mediastreamproxy.h" #include "talk/app/webrtc_dev/mediastreamtrackproxy.h" @@ -40,6 +40,8 @@ #include "third_party/webrtc/files/include/audio_device.h" #endif +using talk_base::scoped_refptr; + namespace { typedef talk_base::TypedMessageData InitMessageData; @@ -49,13 +51,13 @@ struct CreatePeerConnectionParams : public talk_base::MessageData { webrtc::PeerConnectionObserver* observer) : configuration(configuration), observer(observer) { } - talk_base::scoped_refptr peerconnection; + scoped_refptr peerconnection; const std::string& configuration; webrtc::PeerConnectionObserver* observer; }; enum { - MSG_INIT_MANAGER = 1, + MSG_INIT_FACTORY = 1, MSG_CREATE_PEERCONNECTION = 2, }; @@ -63,78 +65,37 @@ enum { namespace webrtc { +scoped_refptr +CreatePeerConnectionFactory() { + talk_base::RefCountedObject* pc_factory = + new talk_base::RefCountedObject(); -talk_base::scoped_refptr PcNetworkManager::Create( - talk_base::NetworkManager* network_manager) { - talk_base::RefCountedObject* implementation = - new talk_base::RefCountedObject(network_manager); - return implementation; -} - -PcNetworkManager::PcNetworkManager(talk_base::NetworkManager* network_manager) - : network_manager_(network_manager) { -} - -talk_base::NetworkManager* PcNetworkManager::network_manager() const { - return network_manager_; -} - -PcNetworkManager::~PcNetworkManager() { - delete network_manager_; -} - -talk_base::scoped_refptr PcPacketSocketFactory::Create( - talk_base::PacketSocketFactory* socket_factory) { - talk_base::RefCountedObject* implementation = - new talk_base::RefCountedObject(socket_factory); - return implementation; -} - -PcPacketSocketFactory::PcPacketSocketFactory( - talk_base::PacketSocketFactory* socket_factory) - : socket_factory_(socket_factory) { -} - -PcPacketSocketFactory::~PcPacketSocketFactory() { - delete socket_factory_; -} - -talk_base::PacketSocketFactory* PcPacketSocketFactory::socket_factory() const { - return socket_factory_; -} - -talk_base::scoped_refptr -PeerConnectionManager::Create() { - talk_base::RefCountedObject* pc_manager = - new talk_base::RefCountedObject(); - - if (!pc_manager->Initialize()) { - delete pc_manager; - pc_manager = NULL; + if (!pc_factory->Initialize()) { + delete pc_factory; + pc_factory = NULL; } - return pc_manager; + return pc_factory; } -talk_base::scoped_refptr PeerConnectionManager::Create( +scoped_refptr +CreatePeerConnectionFactory( talk_base::Thread* worker_thread, talk_base::Thread* signaling_thread, - PcNetworkManager* network_manager, - PcPacketSocketFactory* socket_factory, + talk_base::NetworkManager* network_manager, + talk_base::PacketSocketFactory* socket_factory, AudioDeviceModule* default_adm) { - talk_base::RefCountedObject* pc_manager = - new talk_base::RefCountedObject(worker_thread, - signaling_thread, - network_manager, - socket_factory, - default_adm); - if (!pc_manager->Initialize()) { - delete pc_manager; - pc_manager = NULL; + talk_base::RefCountedObject* pc_factory = + new talk_base::RefCountedObject( + worker_thread, signaling_thread, network_manager, socket_factory, + default_adm); + if (!pc_factory->Initialize()) { + delete pc_factory; + pc_factory = NULL; } - return pc_manager; + return pc_factory; } -PeerConnectionManagerImpl::PeerConnectionManagerImpl() +PeerConnectionFactoryImpl::PeerConnectionFactoryImpl() : worker_thread_(new talk_base::Thread), signaling_thread_(new talk_base::Thread) { worker_thread_ptr_ = worker_thread_.get(); @@ -145,11 +106,11 @@ PeerConnectionManagerImpl::PeerConnectionManagerImpl() ASSERT(result); } -PeerConnectionManagerImpl::PeerConnectionManagerImpl( +PeerConnectionFactoryImpl::PeerConnectionFactoryImpl( talk_base::Thread* worker_thread, talk_base::Thread* signaling_thread, - PcNetworkManager* network_manager, - PcPacketSocketFactory* socket_factory, + talk_base::NetworkManager* network_manager, + talk_base::PacketSocketFactory* socket_factory, AudioDeviceModule* default_adm) : worker_thread_ptr_(worker_thread), signaling_thread_ptr_(signaling_thread), @@ -158,23 +119,23 @@ PeerConnectionManagerImpl::PeerConnectionManagerImpl( default_adm_(default_adm) { ASSERT(worker_thread); ASSERT(signaling_thread); - ASSERT(network_manager->network_manager()); - ASSERT(socket_factory->socket_factory()); + ASSERT(network_manager); + ASSERT(socket_factory); ASSERT(default_adm); } -PeerConnectionManagerImpl::~PeerConnectionManagerImpl() { +PeerConnectionFactoryImpl::~PeerConnectionFactoryImpl() { } -bool PeerConnectionManagerImpl::Initialize() { +bool PeerConnectionFactoryImpl::Initialize() { InitMessageData result(false); - signaling_thread_ptr_->Send(this, MSG_INIT_MANAGER, &result); + signaling_thread_ptr_->Send(this, MSG_INIT_FACTORY, &result); return result.data(); } -void PeerConnectionManagerImpl::OnMessage(talk_base::Message* msg) { +void PeerConnectionFactoryImpl::OnMessage(talk_base::Message* msg) { switch (msg->message_id) { - case MSG_INIT_MANAGER: { + case MSG_INIT_FACTORY: { InitMessageData* pdata = static_cast (msg->pdata); pdata->data() = Initialize_s(); break; @@ -189,12 +150,11 @@ void PeerConnectionManagerImpl::OnMessage(talk_base::Message* msg) { } } -bool PeerConnectionManagerImpl::Initialize_s() { +bool PeerConnectionFactoryImpl::Initialize_s() { if (!network_manager_.get()) - network_manager_ = PcNetworkManager::Create( - new talk_base::BasicNetworkManager()); + network_manager_.reset(new talk_base::BasicNetworkManager()); if (!socket_factory_.get()) - socket_factory_ = PcPacketSocketFactory::Create( + socket_factory_.reset( new talk_base::BasicPacketSocketFactory(worker_thread_ptr_)); cricket::DeviceManager* device_manager(new WebRtcDeviceManager()); @@ -213,8 +173,8 @@ bool PeerConnectionManagerImpl::Initialize_s() { return true; } -talk_base::scoped_refptr -PeerConnectionManagerImpl::CreatePeerConnection( +scoped_refptr +PeerConnectionFactoryImpl::CreatePeerConnection( const std::string& configuration, PeerConnectionObserver* observer) { CreatePeerConnectionParams params(configuration, observer); @@ -222,16 +182,12 @@ PeerConnectionManagerImpl::CreatePeerConnection( return params.peerconnection; } -talk_base::scoped_refptr -PeerConnectionManagerImpl::CreatePeerConnection_s( +scoped_refptr +PeerConnectionFactoryImpl::CreatePeerConnection_s( const std::string& configuration, PeerConnectionObserver* observer) { talk_base::RefCountedObject* pc( - new talk_base::RefCountedObject(channel_manager_.get(), - signaling_thread_ptr_, - worker_thread_ptr_, - network_manager_, - socket_factory_)); + new talk_base::RefCountedObject(this)); if (!pc->Initialize(configuration, observer)) { delete pc; pc = NULL; @@ -239,26 +195,46 @@ PeerConnectionManagerImpl::CreatePeerConnection_s( return pc; } -talk_base::scoped_refptr -PeerConnectionManagerImpl::CreateLocalMediaStream( +scoped_refptr +PeerConnectionFactoryImpl::CreateLocalMediaStream( const std::string& label) { return MediaStreamProxy::Create(label, signaling_thread_ptr_); } -talk_base::scoped_refptr -PeerConnectionManagerImpl::CreateLocalVideoTrack( +scoped_refptr +PeerConnectionFactoryImpl::CreateLocalVideoTrack( const std::string& label, VideoCaptureModule* video_device) { return VideoTrackProxy::CreateLocal(label, video_device, signaling_thread_ptr_); } -talk_base::scoped_refptr -PeerConnectionManagerImpl::CreateLocalAudioTrack( +scoped_refptr +PeerConnectionFactoryImpl::CreateLocalAudioTrack( const std::string& label, AudioDeviceModule* audio_device) { return AudioTrackProxy::CreateLocal(label, audio_device, signaling_thread_ptr_); } +cricket::ChannelManager* PeerConnectionFactoryImpl::channel_manager() { + return channel_manager_.get(); +} + +talk_base::Thread* PeerConnectionFactoryImpl::signaling_thread() { + return signaling_thread_ptr_; +} + +talk_base::Thread* PeerConnectionFactoryImpl::worker_thread() { + return worker_thread_ptr_; +} + +talk_base::NetworkManager* PeerConnectionFactoryImpl::network_manager() { + return network_manager_.get(); +} + +talk_base::PacketSocketFactory* PeerConnectionFactoryImpl::socket_factory() { + return socket_factory_.get(); +} + } // namespace webrtc diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.h b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionfactoryimpl.h similarity index 77% rename from third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.h rename to third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionfactoryimpl.h index f5a75873aa..71048c8e13 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.h +++ b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionfactoryimpl.h @@ -24,20 +24,20 @@ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#ifndef TALK_APP_WEBRTC_PEERCONNECTIONMANAGERIMPL_H_ -#define TALK_APP_WEBRTC_PEERCONNECTIONMANAGERIMPL_H_ +#ifndef TALK_APP_WEBRTC_PEERCONNECTIONFACTORYIMPL_H_ +#define TALK_APP_WEBRTC_PEERCONNECTIONFACTORYIMPL_H_ #include +#include "talk/base/scoped_ptr.h" #include "talk/app/webrtc_dev/peerconnection.h" #include "talk/app/webrtc_dev/mediastream.h" -#include "talk/base/scoped_ptr.h" #include "talk/base/thread.h" #include "talk/session/phone/channelmanager.h" namespace webrtc { -class PeerConnectionManagerImpl : public PeerConnectionManager, +class PeerConnectionFactoryImpl : public PeerConnectionFactoryInterface, public talk_base::MessageHandler { public: talk_base::scoped_refptr CreatePeerConnection( @@ -56,14 +56,20 @@ class PeerConnectionManagerImpl : public PeerConnectionManager, CreateLocalAudioTrack(const std::string& label, AudioDeviceModule* audio_device); + virtual cricket::ChannelManager* channel_manager(); + virtual talk_base::Thread* signaling_thread(); + virtual talk_base::Thread* worker_thread(); + virtual talk_base::NetworkManager* network_manager(); + virtual talk_base::PacketSocketFactory* socket_factory(); + protected: - PeerConnectionManagerImpl(); - PeerConnectionManagerImpl(talk_base::Thread* worker_thread, + PeerConnectionFactoryImpl(); + PeerConnectionFactoryImpl(talk_base::Thread* worker_thread, talk_base::Thread* signaling_thread, - PcNetworkManager* network_manager, - PcPacketSocketFactory* socket_factory, + talk_base::NetworkManager* network_manager, + talk_base::PacketSocketFactory* socket_factory, AudioDeviceModule* default_adm); - virtual ~PeerConnectionManagerImpl(); + virtual ~PeerConnectionFactoryImpl(); private: @@ -78,8 +84,8 @@ class PeerConnectionManagerImpl : public PeerConnectionManager, talk_base::Thread* signaling_thread_ptr_; talk_base::scoped_ptr worker_thread_; talk_base::Thread* worker_thread_ptr_; - talk_base::scoped_refptr network_manager_; - talk_base::scoped_refptr socket_factory_; + talk_base::scoped_ptr network_manager_; + talk_base::scoped_ptr socket_factory_; // External Audio device used for audio playback. talk_base::scoped_refptr default_adm_; talk_base::scoped_ptr channel_manager_; @@ -87,4 +93,4 @@ class PeerConnectionManagerImpl : public PeerConnectionManager, } // namespace webrtc -#endif // TALK_APP_WEBRTC_PEERCONNECTIONMANAGER_IMPL_H_ +#endif // TALK_APP_WEBRTC_PEERCONNECTIONFACTORYIMPL_H_ diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl.cc b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl.cc index 1fbe6994c4..18f32f19fe 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl.cc +++ b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl.cc @@ -127,25 +127,20 @@ struct StreamCollectionParams : public talk_base::MessageData { namespace webrtc { PeerConnectionImpl::PeerConnectionImpl( - cricket::ChannelManager* channel_manager, - talk_base::Thread* signaling_thread, - talk_base::Thread* worker_thread, - PcNetworkManager* network_manager, - PcPacketSocketFactory* socket_factory) - : observer_(NULL), + PeerConnectionFactoryImpl* factory) + : factory_(factory), + observer_(NULL), local_media_streams_(StreamCollectionImpl::Create()), remote_media_streams_(StreamCollectionImpl::Create()), - signaling_thread_(signaling_thread), - channel_manager_(channel_manager), - network_manager_(network_manager), - socket_factory_(socket_factory), port_allocator_(new cricket::HttpPortAllocator( - network_manager->network_manager(), - socket_factory->socket_factory(), + factory->network_manager(), + factory->socket_factory(), std::string(kUserAgent))), - session_(new WebRtcSession(channel_manager, signaling_thread, - worker_thread, port_allocator_.get())), - signaling_(new PeerConnectionSignaling(signaling_thread, + session_(new WebRtcSession(factory->channel_manager(), + factory->signaling_thread(), + factory->worker_thread(), + port_allocator_.get())), + signaling_(new PeerConnectionSignaling(factory->signaling_thread(), session_.get())), stream_handler_(new MediaStreamHandlers(session_.get())) { signaling_->SignalNewPeerConnectionMessage.connect( @@ -159,8 +154,8 @@ PeerConnectionImpl::PeerConnectionImpl( } PeerConnectionImpl::~PeerConnectionImpl() { - signaling_thread_->Clear(this); - signaling_thread_->Send(this, MSG_TERMINATE); + signaling_thread()->Clear(this); + signaling_thread()->Send(this, MSG_TERMINATE); } // Clean up what needs to be cleaned up on the signaling thread. @@ -212,14 +207,14 @@ PeerConnectionImpl::local_streams() { talk_base::scoped_refptr PeerConnectionImpl::remote_streams() { StreamCollectionParams msg(NULL); - signaling_thread_->Send(this, MSG_RETURNREMOTEMEDIASTREAMS, &msg); + signaling_thread()->Send(this, MSG_RETURNREMOTEMEDIASTREAMS, &msg); return msg.streams; } bool PeerConnectionImpl::ProcessSignalingMessage(const std::string& msg) { SignalingParams* parameter(new SignalingParams( msg, StreamCollectionImpl::Create(local_media_streams_))); - signaling_thread_->Post(this, MSG_PROCESSSIGNALINGMESSAGE, parameter); + signaling_thread()->Post(this, MSG_PROCESSSIGNALINGMESSAGE, parameter); } void PeerConnectionImpl::AddStream(LocalMediaStreamInterface* local_stream) { @@ -234,7 +229,7 @@ void PeerConnectionImpl::RemoveStream( void PeerConnectionImpl::CommitStreamChanges() { StreamCollectionParams* msg(new StreamCollectionParams( StreamCollectionImpl::Create(local_media_streams_))); - signaling_thread_->Post(this, MSG_COMMITSTREAMCHANGES, msg); + signaling_thread()->Post(this, MSG_COMMITSTREAMCHANGES, msg); } void PeerConnectionImpl::OnMessage(talk_base::Message* msg) { diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl.h b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl.h index 589f98fbe9..48435edcd7 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl.h +++ b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl.h @@ -32,6 +32,7 @@ #include #include "talk/app/webrtc_dev/peerconnection.h" +#include "talk/app/webrtc_dev/peerconnectionfactoryimpl.h" #include "talk/app/webrtc_dev/peerconnectionsignaling.h" #include "talk/app/webrtc_dev/webrtcsession.h" #include "talk/base/scoped_ptr.h" @@ -53,11 +54,7 @@ class PeerConnectionImpl : public PeerConnectionInterface, public talk_base::MessageHandler, public sigslot::has_slots<> { public: - PeerConnectionImpl(cricket::ChannelManager* channel_manager, - talk_base::Thread* signaling_thread, - talk_base::Thread* worker_thread, - PcNetworkManager* network_manager, - PcPacketSocketFactory* socket_factory); + explicit PeerConnectionImpl(PeerConnectionFactoryImpl* factory); bool Initialize(const std::string& configuration, PeerConnectionObserver* observer); @@ -86,14 +83,21 @@ class PeerConnectionImpl : public PeerConnectionInterface, void Terminate_s(); + talk_base::Thread* signaling_thread() { + return factory_->signaling_thread(); + } + + // Storing the factory as a scoped reference pointer ensures that the memory + // in the PeerConnectionFactoryImpl remains available as long as the + // PeerConnection is running. It is passed to PeerConnection as a raw pointer. + // However, since the reference counting is done in the + // PeerConnectionFactoryInteface all instances created using the raw pointer + // will refer to the same reference count. + talk_base::scoped_refptr factory_; PeerConnectionObserver* observer_; talk_base::scoped_refptr local_media_streams_; talk_base::scoped_refptr remote_media_streams_; - talk_base::Thread* signaling_thread_; // Weak ref from PeerConnectionManager. - cricket::ChannelManager* channel_manager_; - talk_base::scoped_refptr network_manager_; - talk_base::scoped_refptr socket_factory_; talk_base::scoped_ptr port_allocator_; talk_base::scoped_ptr session_; talk_base::scoped_ptr signaling_; diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl_unittest.cc b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl_unittest.cc index a70e8c47b1..b5a57776b8 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl_unittest.cc +++ b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionimpl_unittest.cc @@ -53,13 +53,13 @@ class PeerConnectionImplTest : public testing::Test { public: protected: virtual void SetUp() { - pc_factory_ = webrtc::PeerConnectionManager::Create(); + pc_factory_ = webrtc::CreatePeerConnectionFactory(); ASSERT_TRUE(pc_factory_.get() != NULL); pc_ = pc_factory_->CreatePeerConnection(kStunConfiguration, &observer_); ASSERT_TRUE(pc_.get() != NULL); } - talk_base::scoped_refptr pc_factory_; + talk_base::scoped_refptr pc_factory_; talk_base::scoped_refptr pc_; MockPeerConnectionObserver observer_; }; diff --git a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.cc b/third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.cc index 45157b4b15..692b1fa85b 100644 --- a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.cc +++ b/third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.cc @@ -13,6 +13,7 @@ #include #include "modules/video_capture/main/interface/video_capture_factory.h" +#include "talk/app/webrtc_dev/peerconnection.h" #include "talk/examples/peerconnection_client/defaults.h" #include "talk/base/common.h" #include "talk/base/logging.h" @@ -44,7 +45,7 @@ bool Conductor::InitializePeerConnection() { ASSERT(peer_connection_factory_.get() == NULL); ASSERT(peer_connection_.get() == NULL); - peer_connection_factory_ = webrtc::PeerConnectionManager::Create(); + peer_connection_factory_ = webrtc::CreatePeerConnectionFactory(); if (!peer_connection_factory_.get()) { main_wnd_->MessageBox("Error", diff --git a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.h b/third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.h index 7cbe07c700..c54ebfa86e 100644 --- a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.h +++ b/third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.h @@ -104,7 +104,7 @@ class Conductor protected: int peer_id_; talk_base::scoped_refptr peer_connection_; - talk_base::scoped_refptr + talk_base::scoped_refptr peer_connection_factory_; PeerConnectionClient* client_; MainWindow* main_wnd_;