Remove TraceCallback use from Call.
Non-global logging isn't supported, and having a per-call logging dispatch seems over-eager and adds more complexity than it's worth. The current implementation is racy on initialization due to missing atomics support. Besides, logging support should be separate from use of Call. R=mflodman@webrtc.org BUG=3250,3157 Review URL: https://webrtc-codereview.appspot.com/12329006 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5971 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
parent
a5c8d2c9b3
commit
5ca6a5387e
@ -54,8 +54,6 @@ class Call {
|
||||
: webrtc_config(NULL),
|
||||
send_transport(send_transport),
|
||||
voice_engine(NULL),
|
||||
trace_callback(NULL),
|
||||
trace_filter(kTraceDefault),
|
||||
overuse_callback(NULL) {}
|
||||
|
||||
webrtc::Config* webrtc_config;
|
||||
@ -65,9 +63,6 @@ class Call {
|
||||
// VoiceEngine used for audio/video synchronization for this Call.
|
||||
VoiceEngine* voice_engine;
|
||||
|
||||
TraceCallback* trace_callback;
|
||||
uint32_t trace_filter;
|
||||
|
||||
// Callback for overuse and normal usage based on the jitter of incoming
|
||||
// captured frames. 'NULL' disables the callback.
|
||||
OveruseCallback* overuse_callback;
|
||||
|
@ -54,7 +54,6 @@ class BitrateEstimatorTest : public ::testing::Test {
|
||||
// Create receiver call first so that we are guaranteed to have a trace
|
||||
// callback when sender call is created.
|
||||
Call::Config receiver_call_config(&receive_transport_);
|
||||
receiver_call_config.trace_callback = &receiver_trace_;
|
||||
receiver_call_.reset(Call::Create(receiver_call_config));
|
||||
|
||||
Call::Config sender_call_config(&send_transport_);
|
||||
|
@ -115,83 +115,7 @@ class Call : public webrtc::Call, public PacketReceiver {
|
||||
};
|
||||
} // namespace internal
|
||||
|
||||
class TraceDispatcher : public TraceCallback {
|
||||
public:
|
||||
TraceDispatcher()
|
||||
: lock_(CriticalSectionWrapper::CreateCriticalSection()),
|
||||
filter_(kTraceNone) {
|
||||
Trace::CreateTrace();
|
||||
VideoEngine::SetTraceCallback(this);
|
||||
VideoEngine::SetTraceFilter(kTraceNone);
|
||||
}
|
||||
|
||||
~TraceDispatcher() {
|
||||
Trace::ReturnTrace();
|
||||
VideoEngine::SetTraceCallback(NULL);
|
||||
}
|
||||
|
||||
virtual void Print(TraceLevel level,
|
||||
const char* message,
|
||||
int length) OVERRIDE {
|
||||
CriticalSectionScoped crit(lock_.get());
|
||||
for (std::map<Call*, Call::Config*>::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 crit(lock_.get());
|
||||
callbacks_[call] = config;
|
||||
|
||||
filter_ |= config->trace_filter;
|
||||
VideoEngine::SetTraceFilter(filter_);
|
||||
}
|
||||
|
||||
void DeregisterCallback(Call* call) {
|
||||
CriticalSectionScoped crit(lock_.get());
|
||||
callbacks_.erase(call);
|
||||
|
||||
filter_ = kTraceNone;
|
||||
for (std::map<Call*, Call::Config*>::iterator it = callbacks_.begin();
|
||||
it != callbacks_.end();
|
||||
++it) {
|
||||
filter_ |= it->second->trace_filter;
|
||||
}
|
||||
|
||||
VideoEngine::SetTraceFilter(filter_);
|
||||
}
|
||||
|
||||
private:
|
||||
scoped_ptr<CriticalSectionWrapper> lock_;
|
||||
unsigned int filter_;
|
||||
std::map<Call*, Call::Config*> callbacks_;
|
||||
};
|
||||
|
||||
namespace internal {
|
||||
TraceDispatcher* global_trace_dispatcher = NULL;
|
||||
} // internal
|
||||
|
||||
void CreateTraceDispatcher() {
|
||||
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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Call* Call::Create(const Call::Config& config) {
|
||||
CreateTraceDispatcher();
|
||||
|
||||
VideoEngine* video_engine = config.webrtc_config != NULL
|
||||
? VideoEngine::Create(*config.webrtc_config)
|
||||
: VideoEngine::Create();
|
||||
@ -217,8 +141,6 @@ Call::Call(webrtc::VideoEngine* video_engine, const Call::Config& config)
|
||||
new CpuOveruseObserverProxy(config.overuse_callback));
|
||||
}
|
||||
|
||||
global_trace_dispatcher->RegisterCallback(this, &config_);
|
||||
|
||||
rtp_rtcp_ = ViERTP_RTCP::GetInterface(video_engine_);
|
||||
assert(rtp_rtcp_ != NULL);
|
||||
|
||||
@ -235,7 +157,6 @@ Call::Call(webrtc::VideoEngine* video_engine, const Call::Config& config)
|
||||
}
|
||||
|
||||
Call::~Call() {
|
||||
global_trace_dispatcher->DeregisterCallback(this);
|
||||
base_->DeleteChannel(base_channel_id_);
|
||||
base_->Release();
|
||||
codec_->Release();
|
||||
|
@ -222,63 +222,6 @@ class NackObserver : public test::RtpRtcpObserver {
|
||||
int nacks_left_;
|
||||
};
|
||||
|
||||
// Test disabled, ongoing work disabled traces causing UsesTraceCallback to
|
||||
// fail. Tracked by webrtc:3157.
|
||||
TEST_F(CallTest, DISABLED_UsesTraceCallback) {
|
||||
const unsigned int kSenderTraceFilter = kTraceDebug;
|
||||
const unsigned int kReceiverTraceFilter = kTraceDefault & (~kTraceDebug);
|
||||
class TraceObserver : public TraceCallback {
|
||||
public:
|
||||
explicit 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(kDefaultTimeoutMs); }
|
||||
|
||||
private:
|
||||
unsigned int filter_;
|
||||
unsigned int messages_left_;
|
||||
scoped_ptr<EventWrapper> 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, ReceiverCanBeStartedTwice) {
|
||||
test::NullTransport transport;
|
||||
CreateCalls(Call::Config(&transport), Call::Config(&transport));
|
||||
|
Loading…
Reference in New Issue
Block a user