From de74b64184db0c1404067a6649a415ce3ba00197 Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Wed, 2 Oct 2013 13:36:09 +0000 Subject: [PATCH] Implement TraceCallbacks in Call. Uses a global TraceDispatcher in Call. Lazy initialization of it misses an atomic compare and exchange to be correct. This is expected to work fine so long as no Calls are created concurrently. BUG=2421 R=mflodman@webrtc.org Review URL: https://webrtc-codereview.appspot.com/2321005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@4900 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/video_engine/internal/call.cc | 83 ++++++++++++++++++++++++++ webrtc/video_engine/new_include/call.h | 2 +- webrtc/video_engine/test/call_tests.cc | 69 ++++++++++++++++++--- 3 files changed, 146 insertions(+), 8 deletions(-) diff --git a/webrtc/video_engine/internal/call.cc b/webrtc/video_engine/internal/call.cc index 94c5b7ed48..c3780fdc48 100644 --- a/webrtc/video_engine/internal/call.cc +++ b/webrtc/video_engine/internal/call.cc @@ -16,6 +16,9 @@ #include #include +#include "webrtc/system_wrappers/interface/critical_section_wrapper.h" +#include "webrtc/system_wrappers/interface/scoped_ptr.h" +#include "webrtc/system_wrappers/interface/trace.h" #include "webrtc/video_engine/include/vie_base.h" #include "webrtc/video_engine/include/vie_codec.h" #include "webrtc/video_engine/include/vie_rtp_rtcp.h" @@ -24,7 +27,84 @@ namespace webrtc { +class TraceDispatcher : public TraceCallback { + public: + TraceDispatcher() + : crit_(CriticalSectionWrapper::CreateCriticalSection()), + filter_(kTraceNone) {} + + virtual void Print(TraceLevel level, + const char* message, + int length) OVERRIDE { + CriticalSectionScoped lock(crit_.get()); + for (std::map::iterator it = callbacks_.begin(); + it != callbacks_.end(); + ++it) { + if ((level & it->second->trace_filter) != kTraceNone) + it->second->trace_callback->Print(level, message, length); + } + } + + void RegisterCallback(Call* call, Call::Config* config) { + if (config->trace_callback == NULL) + return; + + CriticalSectionScoped lock(crit_.get()); + callbacks_[call] = config; + + if ((filter_ | config->trace_filter) != filter_) { + if (filter_ == kTraceNone) { + Trace::CreateTrace(); + VideoEngine::SetTraceCallback(this); + } + filter_ |= config->trace_filter; + VideoEngine::SetTraceFilter(filter_); + } + } + + void DeregisterCallback(Call* call) { + CriticalSectionScoped lock(crit_.get()); + callbacks_.erase(call); + // Return early if there was no filter, this is required to prevent + // returning the Trace handle more than once. + if (filter_ == kTraceNone) + return; + + filter_ = kTraceNone; + for (std::map::iterator it = callbacks_.begin(); + it != callbacks_.end(); + ++it) { + filter_ |= it->second->trace_filter; + } + + VideoEngine::SetTraceFilter(filter_); + if (filter_ == kTraceNone) { + VideoEngine::SetTraceCallback(NULL); + Trace::ReturnTrace(); + } + } + + private: + scoped_ptr crit_; + unsigned int filter_; + std::map callbacks_; +}; + +namespace internal { + TraceDispatcher* global_trace_dispatcher = NULL; +} // internal + Call* Call::Create(const Call::Config& config) { + if (internal::global_trace_dispatcher == NULL) { + TraceDispatcher* dispatcher = new TraceDispatcher(); + // TODO(pbos): Atomic compare and exchange. + if (internal::global_trace_dispatcher == NULL) { + internal::global_trace_dispatcher = dispatcher; + } else { + delete dispatcher; + } + } + VideoEngine* video_engine = VideoEngine::Create(); assert(video_engine != NULL); @@ -42,6 +122,8 @@ Call::Call(webrtc::VideoEngine* video_engine, const Call::Config& config) assert(video_engine != NULL); assert(config.send_transport != NULL); + global_trace_dispatcher->RegisterCallback(this, &config_); + rtp_rtcp_ = ViERTP_RTCP::GetInterface(video_engine_); assert(rtp_rtcp_ != NULL); @@ -50,6 +132,7 @@ Call::Call(webrtc::VideoEngine* video_engine, const Call::Config& config) } Call::~Call() { + global_trace_dispatcher->DeregisterCallback(this); codec_->Release(); rtp_rtcp_->Release(); webrtc::VideoEngine::Delete(video_engine_); diff --git a/webrtc/video_engine/new_include/call.h b/webrtc/video_engine/new_include/call.h index 6650216318..f8fa9bed9f 100644 --- a/webrtc/video_engine/new_include/call.h +++ b/webrtc/video_engine/new_include/call.h @@ -42,7 +42,7 @@ class Call { overuse_detection(false), voice_engine(NULL), trace_callback(NULL), - trace_filter(kTraceNone) {} + trace_filter(kTraceDefault) {} newapi::Transport* send_transport; bool overuse_detection; diff --git a/webrtc/video_engine/test/call_tests.cc b/webrtc/video_engine/test/call_tests.cc index 9e1813e99e..bb10906ad3 100644 --- a/webrtc/video_engine/test/call_tests.cc +++ b/webrtc/video_engine/test/call_tests.cc @@ -41,10 +41,8 @@ class CallTest : public ::testing::Test { } protected: - void CreateCalls(newapi::Transport* sender_transport, - newapi::Transport* receiver_transport) { - Call::Config sender_config(sender_transport); - Call::Config receiver_config(receiver_transport); + void CreateCalls(const Call::Config& sender_config, + const Call::Config& receiver_config) { sender_call_.reset(Call::Create(sender_config)); receiver_call_.reset(Call::Create(receiver_config)); } @@ -228,10 +226,66 @@ class NackObserver : public test::RtpRtcpObserver { static const int kRequiredRtcpsWithoutNack = 2; }; +TEST_F(CallTest, UsesTraceCallback) { + const unsigned int kSenderTraceFilter = kTraceDebug; + const unsigned int kReceiverTraceFilter = kTraceDefault & (~kTraceDebug); + class TraceObserver: public TraceCallback { + public: + TraceObserver(unsigned int filter) + : filter_(filter), messages_left_(50), done_(EventWrapper::Create()) {} + + virtual void Print(TraceLevel level, + const char* message, + int length) OVERRIDE { + EXPECT_EQ(0u, level & (~filter_)); + if (--messages_left_ == 0) + done_->Set(); + } + + EventTypeWrapper Wait() { return done_->Wait(30 * 1000); } + + private: + unsigned int filter_; + unsigned int messages_left_; + scoped_ptr done_; + } sender_trace(kSenderTraceFilter), receiver_trace(kReceiverTraceFilter); + + test::DirectTransport send_transport, receive_transport; + Call::Config sender_call_config(&send_transport); + sender_call_config.trace_callback = &sender_trace; + sender_call_config.trace_filter = kSenderTraceFilter; + Call::Config receiver_call_config(&receive_transport); + receiver_call_config.trace_callback = &receiver_trace; + receiver_call_config.trace_filter = kReceiverTraceFilter; + CreateCalls(sender_call_config, receiver_call_config); + send_transport.SetReceiver(receiver_call_->Receiver()); + receive_transport.SetReceiver(sender_call_->Receiver()); + + CreateTestConfigs(); + + CreateStreams(); + CreateFrameGenerator(); + StartSending(); + + // Wait() waits for a couple of trace callbacks to occur. + EXPECT_EQ(kEventSignaled, sender_trace.Wait()); + EXPECT_EQ(kEventSignaled, receiver_trace.Wait()); + + StopSending(); + send_transport.StopSending(); + receive_transport.StopSending(); + DestroyStreams(); + + // The TraceCallback instance MUST outlive Calls, destroy Calls explicitly. + sender_call_.reset(); + receiver_call_.reset(); +} + TEST_F(CallTest, ReceivesAndRetransmitsNack) { NackObserver observer; - CreateCalls(observer.SendTransport(), observer.ReceiveTransport()); + CreateCalls(Call::Config(observer.SendTransport()), + Call::Config(observer.ReceiveTransport())); observer.SetReceivers(receiver_call_->Receiver(), sender_call_->Receiver()); @@ -333,7 +387,8 @@ class PliObserver : public test::RtpRtcpObserver, public VideoRenderer { void CallTest::ReceivesPliAndRecovers(int rtp_history_ms) { PliObserver observer(rtp_history_ms > 0); - CreateCalls(observer.SendTransport(), observer.ReceiveTransport()); + CreateCalls(Call::Config(observer.SendTransport()), + Call::Config(observer.ReceiveTransport())); observer.SetReceivers(receiver_call_->Receiver(), sender_call_->Receiver()); @@ -392,7 +447,7 @@ TEST_F(CallTest, SurvivesIncomingRtpPacketsToDestroyedReceiveStream) { test::DirectTransport send_transport, receive_transport; - CreateCalls(&send_transport, &receive_transport); + CreateCalls(Call::Config(&send_transport), Call::Config(&receive_transport)); PacketInputObserver input_observer(receiver_call_->Receiver()); send_transport.SetReceiver(&input_observer);