From cb4ab65dfc503ea29c319f54b850508eb27fefa2 Mon Sep 17 00:00:00 2001 From: "perkj@webrtc.org" Date: Tue, 4 Oct 2011 17:54:34 +0000 Subject: [PATCH] Moved creation of objects to the signaling thread. Fixed defect of not initializing remote_media_streams in peerconnection_impl.cc Fixed defect in glare case of peerconnectionsignaling.cc BUG= TEST= Review URL: http://webrtc-codereview.appspot.com/196001 git-svn-id: http://webrtc.googlecode.com/svn/trunk@690 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../talk/app/webrtc_dev/peerconnectionimpl.cc | 1 + .../webrtc_dev/peerconnectionmanagerimpl.cc | 90 +++++++++++++++---- .../webrtc_dev/peerconnectionmanagerimpl.h | 12 ++- .../app/webrtc_dev/peerconnectionsignaling.cc | 14 +-- 4 files changed, 91 insertions(+), 26 deletions(-) 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 bed82541e0..64b04055ed 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 @@ -128,6 +128,7 @@ PeerConnectionImpl::PeerConnectionImpl( PcPacketSocketFactory* socket_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), diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.cc b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.cc index 0e2f159a63..f8044bdbff 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.cc +++ b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.cc @@ -30,8 +30,6 @@ #include "talk/app/webrtc_dev/peerconnectionimpl.h" #include "talk/app/webrtc_dev/webrtc_devicemanager.h" #include "talk/base/basicpacketsocketfactory.h" -#include "talk/base/thread.h" -#include "talk/session/phone/channelmanager.h" #include "talk/session/phone/webrtcmediaengine.h" #ifdef WEBRTC_RELATIVE_PATH @@ -40,6 +38,27 @@ #include "third_party/webrtc/files/include/audio_device.h" #endif +namespace { + +typedef talk_base::TypedMessageData InitMessageData; + +struct CreatePeerConnectionParams : public talk_base::MessageData { + CreatePeerConnectionParams(const std::string& configuration, + webrtc::PeerConnectionObserver* observer) + : configuration(configuration), observer(observer) { + } + scoped_refptr peerconnection; + const std::string& configuration; + webrtc::PeerConnectionObserver* observer; +}; + +enum { + MSG_INIT_MANAGER = 1, + MSG_CREATE_PEERCONNECTION = 2, +}; + +} // namespace anonymous + namespace webrtc { @@ -85,6 +104,7 @@ talk_base::PacketSocketFactory* PcPacketSocketFactory::socket_factory() const { scoped_refptr PeerConnectionManager::Create() { RefCountImpl* pc_manager = new RefCountImpl(); + if (!pc_manager->Initialize()) { delete pc_manager; pc_manager = NULL; @@ -113,13 +133,13 @@ scoped_refptr PeerConnectionManager::Create( PeerConnectionManagerImpl::PeerConnectionManagerImpl() : worker_thread_(new talk_base::Thread), - signaling_thread_(new talk_base::Thread), - network_manager_(PcNetworkManager::Create( - new talk_base::BasicNetworkManager())), - socket_factory_(PcPacketSocketFactory::Create( - new talk_base::BasicPacketSocketFactory)) { + signaling_thread_(new talk_base::Thread) { worker_thread_ptr_ = worker_thread_.get(); signaling_thread_ptr_ = signaling_thread_.get(); + bool result = worker_thread_->Start(); + ASSERT(result); + result = signaling_thread_->Start(); + ASSERT(result); } PeerConnectionManagerImpl::PeerConnectionManagerImpl( @@ -144,19 +164,43 @@ PeerConnectionManagerImpl::~PeerConnectionManagerImpl() { } bool PeerConnectionManagerImpl::Initialize() { - if (worker_thread_.get() && !worker_thread_->Start()) - return false; - if (signaling_thread_.get() && !signaling_thread_->Start()) - return false; - cricket::DeviceManager* device_manager(new WebRtcDeviceManager()); - cricket::WebRtcMediaEngine* webrtc_media_engine = NULL; + InitMessageData result(false); + signaling_thread_->Send(this, MSG_INIT_MANAGER, &result); + return result.data(); +} +void PeerConnectionManagerImpl::OnMessage(talk_base::Message* msg) { + switch (msg->message_id) { + case MSG_INIT_MANAGER: { + InitMessageData* pdata = static_cast (msg->pdata); + pdata->data() = Initialize_s(); + break; + } + case MSG_CREATE_PEERCONNECTION: { + CreatePeerConnectionParams* pdata = + static_cast (msg->pdata); + pdata->peerconnection = CreatePeerConnection_s(pdata->configuration, + pdata->observer); + break; + } + } +} + +bool PeerConnectionManagerImpl::Initialize_s() { + if (!network_manager_.get()) + network_manager_ = PcNetworkManager::Create( + new talk_base::BasicNetworkManager()); + if (!socket_factory_.get()) + socket_factory_ = PcPacketSocketFactory::Create( + new talk_base::BasicPacketSocketFactory(worker_thread_ptr_)); + + cricket::DeviceManager* device_manager(new WebRtcDeviceManager()); // TODO(perkj): Need to make sure only one VoE is created inside // WebRtcMediaEngine. - webrtc_media_engine = new cricket::WebRtcMediaEngine( - default_adm_.get(), - NULL, // No secondary adm. - NULL); // No vcm available. + cricket::WebRtcMediaEngine* webrtc_media_engine( + new cricket::WebRtcMediaEngine(default_adm_.get(), + NULL, // No secondary adm. + NULL)); // No vcm available. channel_manager_.reset(new cricket::ChannelManager( webrtc_media_engine, device_manager, worker_thread_ptr_)); @@ -169,12 +213,20 @@ bool PeerConnectionManagerImpl::Initialize() { scoped_refptr PeerConnectionManagerImpl::CreatePeerConnection( const std::string& configuration, PeerConnectionObserver* observer) { - RefCountImpl* pc = + CreatePeerConnectionParams params(configuration, observer); + signaling_thread_->Send(this, MSG_CREATE_PEERCONNECTION, ¶ms); + return params.peerconnection; +} + +scoped_refptr PeerConnectionManagerImpl::CreatePeerConnection_s( + const std::string& configuration, + PeerConnectionObserver* observer) { + RefCountImpl* pc( new RefCountImpl(channel_manager_.get(), signaling_thread_ptr_, worker_thread_ptr_, network_manager_, - socket_factory_); + socket_factory_)); if (!pc->Initialize(configuration, observer)) { delete pc; pc = NULL; diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.h b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.h index b2c2bf054f..26933176e5 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.h +++ b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionmanagerimpl.h @@ -32,11 +32,13 @@ #include "talk/base/scoped_ptr.h" #include "talk/app/webrtc_dev/peerconnection.h" #include "talk/app/webrtc_dev/mediastream.h" +#include "talk/base/thread.h" #include "talk/session/phone/channelmanager.h" namespace webrtc { -class PeerConnectionManagerImpl : public PeerConnectionManager { +class PeerConnectionManagerImpl : public PeerConnectionManager, + public talk_base::MessageHandler { public: scoped_refptr CreatePeerConnection( const std::string& config, @@ -52,7 +54,15 @@ class PeerConnectionManagerImpl : public PeerConnectionManager { AudioDeviceModule* default_adm); virtual ~PeerConnectionManagerImpl(); + private: + bool Initialize_s(); + scoped_refptr CreatePeerConnection_s( + const std::string& configuration, + PeerConnectionObserver* observer); + // Implements talk_base::MessageHandler. + void OnMessage(talk_base::Message* msg); + talk_base::scoped_ptr worker_thread_; talk_base::Thread* worker_thread_ptr_; talk_base::scoped_ptr signaling_thread_; diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionsignaling.cc b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionsignaling.cc index 3edcd8502f..6c842d03b6 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionsignaling.cc +++ b/third_party_mods/libjingle/source/talk/app/webrtc_dev/peerconnectionsignaling.cc @@ -144,6 +144,8 @@ void PeerConnectionSignaling::ProcessSignalingMessage( if (state_ == kGlare) { state_ = kIdle; } + // Clear the MSG_SEND_QUEUED_OFFER we posted delayed. + signaling_thread_->Clear(this, MSG_SEND_QUEUED_OFFER); signaling_thread_->Post(this, MSG_GENERATE_ANSWER); break; } @@ -169,13 +171,13 @@ void PeerConnectionSignaling::ProcessSignalingMessage( break; } case PeerConnectionMessage::kError: { - if (signaling_message->error() != PeerConnectionMessage::kWrongState) + if (signaling_message->error() != PeerConnectionMessage::kWrongState) { SignalErrorMessageReceived(signaling_message->error()); - - // An error have occurred that we can't do anything about. - // Reset the state and wait for user action. - queued_offers_.clear(); - state_ = kIdle; + // An error have occurred that we can't do anything about. + // Reset the state and wait for user action. + queued_offers_.clear(); + state_ = kIdle; + } break; } }