From 0482d0190259322aab4b8ee45a74cb621b383de2 Mon Sep 17 00:00:00 2001 From: "tommi@webrtc.org" Date: Mon, 2 Mar 2015 17:51:27 +0000 Subject: [PATCH] Implement TraceCallback in a nested class of WebRtcVideoEngine. This is to fix a race that occurs in unit tests when the tests inherit from the engine class that also implements the callback interface for tracing. If tracing happens while the most derived class is still being constructed, we're in trouble. So, instead, factoring out the TraceCallback implementation. R=pbos@webrtc.org BUG= Review URL: https://webrtc-codereview.appspot.com/43489004 Cr-Commit-Position: refs/heads/master@{#8562} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8562 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/media/webrtc/webrtcvideoengine.cc | 35 ++++++++------------------ talk/media/webrtc/webrtcvideoengine.h | 26 +++++++++++-------- 2 files changed, 27 insertions(+), 34 deletions(-) diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index a86e6107bf..0e06cfa9fc 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -1066,21 +1066,24 @@ static bool GetCpuOveruseOptions(const VideoOptions& options, return true; } -WebRtcVideoEngine::WebRtcVideoEngine(WebRtcVoiceEngine* voice_engine) { +WebRtcVideoEngine::WebRtcVideoEngine(WebRtcVoiceEngine* voice_engine) + : voice_engine_(voice_engine), trace_callback_(voice_engine) { Construct(new ViEWrapper(), new ViETraceWrapper(), voice_engine, new rtc::CpuMonitor(NULL)); } WebRtcVideoEngine::WebRtcVideoEngine(WebRtcVoiceEngine* voice_engine, ViEWrapper* vie_wrapper, - rtc::CpuMonitor* cpu_monitor) { + rtc::CpuMonitor* cpu_monitor) + : voice_engine_(voice_engine), trace_callback_(voice_engine) { Construct(vie_wrapper, new ViETraceWrapper(), voice_engine, cpu_monitor); } WebRtcVideoEngine::WebRtcVideoEngine(WebRtcVoiceEngine* voice_engine, ViEWrapper* vie_wrapper, ViETraceWrapper* tracing, - rtc::CpuMonitor* cpu_monitor) { + rtc::CpuMonitor* cpu_monitor) + : voice_engine_(voice_engine), trace_callback_(voice_engine) { Construct(vie_wrapper, tracing, voice_engine, cpu_monitor); } @@ -1093,7 +1096,6 @@ void WebRtcVideoEngine::Construct(ViEWrapper* vie_wrapper, vie_wrapper_.reset(vie_wrapper); vie_wrapper_base_initialized_ = false; tracing_.reset(tracing); - voice_engine_ = voice_engine; initialized_ = false; SetTraceFilter(SeverityToFilter(kDefaultLogSeverity)); render_module_.reset(new WebRtcPassthroughRender()); @@ -1103,8 +1105,8 @@ void WebRtcVideoEngine::Construct(ViEWrapper* vie_wrapper, cpu_monitor_.reset(cpu_monitor); SetTraceOptions(""); - if (tracing_->SetTraceCallback(this) != 0) { - LOG_RTCERR1(SetTraceCallback, this); + if (tracing_->SetTraceCallback(&trace_callback_) != 0) { + LOG_RTCERR1(SetTraceCallback, &trace_callback_); } default_video_codec_list_ = DefaultVideoCodecList(); @@ -1548,27 +1550,13 @@ bool WebRtcVideoEngine::RebuildCodecList(const VideoCodec& in_codec) { return true; } -// Ignore spammy trace messages, mostly from the stats API when we haven't -// gotten RTCP info yet from the remote side. -bool WebRtcVideoEngine::ShouldIgnoreTrace(const std::string& trace) { - static const char* const kTracesToIgnore[] = { - NULL - }; - for (const char* const* p = kTracesToIgnore; *p; ++p) { - if (trace.find(*p) == 0) { - return true; - } - } - return false; -} - int WebRtcVideoEngine::GetNumOfChannels() { rtc::CritScope cs(&channels_crit_); return static_cast(channels_.size()); } -void WebRtcVideoEngine::Print(webrtc::TraceLevel level, const char* trace, - int length) { +void WebRtcVideoEngine::TraceCallbackImpl::Print( + webrtc::TraceLevel level, const char* trace, int length) { rtc::LoggingSeverity sev = rtc::LS_VERBOSE; if (level == webrtc::kTraceError || level == webrtc::kTraceCritical) sev = rtc::LS_ERROR; @@ -1586,8 +1574,7 @@ void WebRtcVideoEngine::Print(webrtc::TraceLevel level, const char* trace, LOG_V(sev) << msg; } else { std::string msg(trace + 71, length - 72); - if (!ShouldIgnoreTrace(msg) && - (!voice_engine_ || !voice_engine_->ShouldIgnoreTrace(msg))) { + if (!voice_engine_ || !voice_engine_->ShouldIgnoreTrace(msg)) { LOG_V(sev) << "webrtc: " << msg; } } diff --git a/talk/media/webrtc/webrtcvideoengine.h b/talk/media/webrtc/webrtcvideoengine.h index 25db0a966a..98c444a0c3 100644 --- a/talk/media/webrtc/webrtcvideoengine.h +++ b/talk/media/webrtc/webrtcvideoengine.h @@ -94,8 +94,7 @@ bool IsNackEnabled(const VideoCodec& codec); bool IsRembEnabled(const VideoCodec& codec); void AddDefaultFeedbackParams(VideoCodec* codec); -class WebRtcVideoEngine : public sigslot::has_slots<>, - public webrtc::TraceCallback { +class WebRtcVideoEngine : public sigslot::has_slots<> { public: // Creates the WebRtcVideoEngine with internal VideoCaptureModule. explicit WebRtcVideoEngine(WebRtcVoiceEngine* voice_engine); @@ -174,8 +173,6 @@ class WebRtcVideoEngine : public sigslot::has_slots<>, void UnregisterChannel(WebRtcVideoMediaChannel* channel); bool ConvertFromCricketVideoCodec(const VideoCodec& in_codec, webrtc::VideoCodec* out_codec); - // Check whether the supplied trace should be ignored. - bool ShouldIgnoreTrace(const std::string& trace); int GetNumOfChannels(); VideoFormat GetStartCaptureFormat() const { return default_codec_format_; } @@ -183,6 +180,19 @@ class WebRtcVideoEngine : public sigslot::has_slots<>, rtc::CpuMonitor* cpu_monitor() { return cpu_monitor_.get(); } protected: + class TraceCallbackImpl : public webrtc::TraceCallback { + public: + TraceCallbackImpl(WebRtcVoiceEngine* voice_engine) + : voice_engine_(voice_engine) {} + ~TraceCallbackImpl() override {} + + private: + void Print(webrtc::TraceLevel level, const char* trace, + int length) override; + + WebRtcVoiceEngine* const voice_engine_; + }; + bool initialized() const { return initialized_; } @@ -206,16 +216,11 @@ class WebRtcVideoEngine : public sigslot::has_slots<>, bool InitVideoEngine(); bool VerifyApt(const VideoCodec& in, int expected_apt) const; - // webrtc::TraceCallback implementation. - virtual void Print(webrtc::TraceLevel level, - const char* trace, - int length) OVERRIDE; - rtc::Thread* worker_thread_; rtc::scoped_ptr vie_wrapper_; bool vie_wrapper_base_initialized_; rtc::scoped_ptr tracing_; - WebRtcVoiceEngine* voice_engine_; + WebRtcVoiceEngine* const voice_engine_; rtc::scoped_ptr render_module_; rtc::scoped_ptr simulcast_encoder_factory_; WebRtcVideoEncoderFactory* encoder_factory_; @@ -224,6 +229,7 @@ class WebRtcVideoEngine : public sigslot::has_slots<>, std::vector default_video_codec_list_; std::vector rtp_header_extensions_; VideoFormat default_codec_format_; + TraceCallbackImpl trace_callback_; bool initialized_; rtc::CriticalSection channels_crit_;