From 1819fd711aa081b7303f3fbe1e041cf5bf26706b Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Mon, 10 Jun 2013 13:48:26 +0000 Subject: [PATCH] RW lock access to ssrc maps in VideoCall. BUG= R=mflodman@webrtc.org Review URL: https://webrtc-codereview.appspot.com/1640004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@4202 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/video_engine/internal/video_call.cc | 46 +++++++++++----------- webrtc/video_engine/internal/video_call.h | 8 +++- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/webrtc/video_engine/internal/video_call.cc b/webrtc/video_engine/internal/video_call.cc index a7773e6c5..adaa14bf4 100644 --- a/webrtc/video_engine/internal/video_call.cc +++ b/webrtc/video_engine/internal/video_call.cc @@ -29,7 +29,10 @@ namespace internal { VideoCall::VideoCall(webrtc::VideoEngine* video_engine, newapi::Transport* send_transport) - : send_transport(send_transport), video_engine_(video_engine) { + : send_transport(send_transport), + receive_lock_(RWLockWrapper::CreateRWLock()), + send_lock_(RWLockWrapper::CreateRWLock()), + video_engine_(video_engine) { assert(video_engine != NULL); assert(send_transport != NULL); @@ -66,19 +69,18 @@ VideoSendStream::Config VideoCall::GetDefaultSendConfig() { } newapi::VideoSendStream* VideoCall::CreateSendStream( - const newapi::VideoSendStream::Config& send_stream_config) { - assert(send_stream_config.rtp.ssrcs.size() > 0); - assert(send_stream_config.codec.numberOfSimulcastStreams == 0 || - send_stream_config.codec.numberOfSimulcastStreams == - send_stream_config.rtp.ssrcs.size()); + const newapi::VideoSendStream::Config& config) { + assert(config.rtp.ssrcs.size() > 0); + assert(config.codec.numberOfSimulcastStreams == 0 || + config.codec.numberOfSimulcastStreams == config.rtp.ssrcs.size()); + VideoSendStream* send_stream = - new VideoSendStream(send_transport, video_engine_, send_stream_config); - for (size_t i = 0; i < send_stream_config.rtp.ssrcs.size(); ++i) { - uint32_t ssrc = send_stream_config.rtp.ssrcs[i]; - // SSRC must be previously unused! - assert(send_ssrcs_[ssrc] == NULL && - receive_ssrcs_.find(ssrc) == receive_ssrcs_.end()); - send_ssrcs_[ssrc] = send_stream; + new VideoSendStream(send_transport, video_engine_, config); + + WriteLockScoped write_lock(*send_lock_); + for (size_t i = 0; i < config.rtp.ssrcs.size(); ++i) { + assert(send_ssrcs_.find(config.rtp.ssrcs[i]) == send_ssrcs_.end()); + send_ssrcs_[config.rtp.ssrcs[i]] = send_stream; } return send_stream; } @@ -100,14 +102,13 @@ VideoReceiveStream::Config VideoCall::GetDefaultReceiveConfig() { } newapi::VideoReceiveStream* VideoCall::CreateReceiveStream( - const newapi::VideoReceiveStream::Config& receive_stream_config) { - assert(receive_ssrcs_[receive_stream_config.rtp.ssrc] == NULL); - + const newapi::VideoReceiveStream::Config& config) { VideoReceiveStream* receive_stream = new VideoReceiveStream( - video_engine_, receive_stream_config, send_transport); - - receive_ssrcs_[receive_stream_config.rtp.ssrc] = receive_stream; + video_engine_, config, send_transport); + WriteLockScoped write_lock(*receive_lock_); + assert(receive_ssrcs_.find(config.rtp.ssrc) == receive_ssrcs_.end()); + receive_ssrcs_[config.rtp.ssrc] = receive_stream; return receive_stream; } @@ -135,6 +136,7 @@ bool VideoCall::DeliverRtcp(ModuleRTPUtility::RTPHeaderParser* rtp_parser, // TODO(pbos): Figure out what channel needs it actually. // Do NOT broadcast! Also make sure it's a valid packet. bool rtcp_delivered = false; + ReadLockScoped read_lock(*receive_lock_); for (std::map::iterator it = receive_ssrcs_.begin(); it != receive_ssrcs_.end(); ++it) { @@ -156,14 +158,14 @@ bool VideoCall::DeliverRtp(ModuleRTPUtility::RTPHeaderParser* rtp_parser, return false; } - uint32_t ssrc = rtp_header.ssrc; - if (receive_ssrcs_.find(ssrc) == receive_ssrcs_.end()) { + ReadLockScoped read_lock(*receive_lock_); + if (receive_ssrcs_.find(rtp_header.ssrc) == receive_ssrcs_.end()) { // TODO(pbos): Log some warning, SSRC without receiver. return false; } VideoReceiveStream* receiver = - static_cast(receive_ssrcs_[ssrc]); + static_cast(receive_ssrcs_[rtp_header.ssrc]); return receiver->DeliverRtp(packet, length); } diff --git a/webrtc/video_engine/internal/video_call.h b/webrtc/video_engine/internal/video_call.h index fcdc60731..817d70fc3 100644 --- a/webrtc/video_engine/internal/video_call.h +++ b/webrtc/video_engine/internal/video_call.h @@ -15,6 +15,8 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" +#include "webrtc/system_wrappers/interface/rw_lock_wrapper.h" +#include "webrtc/system_wrappers/interface/scoped_ptr.h" #include "webrtc/video_engine/internal/video_receive_stream.h" #include "webrtc/video_engine/internal/video_send_stream.h" #include "webrtc/video_engine/new_include/video_engine.h" @@ -41,7 +43,7 @@ class VideoCall : public newapi::VideoCall, public newapi::PacketReceiver { virtual newapi::VideoSendStream::Config GetDefaultSendConfig() OVERRIDE; virtual newapi::VideoSendStream* CreateSendStream( - const newapi::VideoSendStream::Config& send_stream_config) OVERRIDE; + const newapi::VideoSendStream::Config& config) OVERRIDE; virtual newapi::SendStreamState* DestroySendStream( newapi::VideoSendStream* send_stream) OVERRIDE; @@ -49,7 +51,7 @@ class VideoCall : public newapi::VideoCall, public newapi::PacketReceiver { virtual newapi::VideoReceiveStream::Config GetDefaultReceiveConfig() OVERRIDE; virtual newapi::VideoReceiveStream* CreateReceiveStream( - const newapi::VideoReceiveStream::Config& receive_stream_config) OVERRIDE; + const newapi::VideoReceiveStream::Config& config) OVERRIDE; virtual void DestroyReceiveStream(newapi::VideoReceiveStream* receive_stream) OVERRIDE; @@ -68,7 +70,9 @@ class VideoCall : public newapi::VideoCall, public newapi::PacketReceiver { newapi::Transport* send_transport; std::map receive_ssrcs_; + scoped_ptr receive_lock_; std::map send_ssrcs_; + scoped_ptr send_lock_; webrtc::VideoEngine* video_engine_; ViERTP_RTCP* rtp_rtcp_;