Closed
Bug 822704
Opened 11 years ago
Closed 11 years ago
Enable webrtc/trunk WEBRTC_TRACE() logging via NSPR_LOG_MODULES
Categories
(Core :: WebRTC, defect, P2)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: jesup, Assigned: jesup)
Details
(Whiteboard: [WebRTC] [blocking-webrtc-] [qa-])
Attachments
(2 files)
4.69 KB,
patch
|
derf
:
review+
|
Details | Diff | Splinter Review |
3.96 KB,
patch
|
derf
:
review+
|
Details | Diff | Splinter Review |
We want to be able to turn on WEBRTC_TRACE() logging without recompiling the code Also moving 'ikran' to 'signaling'
Assignee | ||
Comment 1•11 years ago
|
||
Note: the level is the webrtc TraceLevel value (bitmask); kTraceAll logs means setting to 65535 (not -1), see media/webrtc/trunk/src/common_types.h. Note that kTraceAll logs are BIG, and they also are a circular log to avoid filling your disk. The filename is 'WebRTC.log' in the executable dir unless you set the env var WEBRTC_TRACE_FILE
Assignee | ||
Updated•11 years ago
|
Attachment #693479 -
Flags: review?(tterribe)
Assignee | ||
Comment 2•11 years ago
|
||
Not blocking but very handy
Priority: -- → P2
Whiteboard: [WebRTC] [blocking-webrtc-]
Comment 3•11 years ago
|
||
Comment on attachment 693479 [details] [diff] [review] Enable WEBRTC_TRACE() logging via NSPR_LOG_MODULES and rename signaling log module Review of attachment 693479 [details] [diff] [review]: ----------------------------------------------------------------- r=me, but see the comment about VoE. ::: media/webrtc/signaling/src/common/browser_logging/CSFLog.cpp @@ +12,4 @@ > > static PRLogModuleInfo *gLogModuleInfo = NULL; > > +PRLogModuleInfo *GetSignalingLogInfo() These parts should really be in a separate patch, but I'm not going to argue. ::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp @@ +108,5 @@ > + } > + CSFLogDebug(logTag, "%s Logging webrtc to %s level %d", __FUNCTION__, > + file, logs->level); > + mVideoEngine->SetTraceFilter(logs->level); > + mVideoEngine->SetTraceFile(file); Don't we need one of these for the voice engine as well? Not sure if you'd want to share the same module for both, in which case you might want to rethink the module name.
Attachment #693479 -
Flags: review?(tterribe) → review+
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Timothy B. Terriberry (:derf) from comment #3) > ::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp > @@ +108,5 @@ > > + } > > + CSFLogDebug(logTag, "%s Logging webrtc to %s level %d", __FUNCTION__, > > + file, logs->level); > > + mVideoEngine->SetTraceFilter(logs->level); > > + mVideoEngine->SetTraceFile(file); > > Don't we need one of these for the voice engine as well? Not sure if you'd > want to share the same module for both, in which case you might want to > rethink the module name. Voice and Video engines both call Trace::SetTraceLevel/File. I.e. they're shared; there's only one control. I may want to modify it to have a shared "turn on logging" from both conduits, but right now we're always creating a video conduit I think.
Assignee | ||
Comment 5•11 years ago
|
||
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 693608 [details] [diff] [review] interdiffs Interdiffs (actually I had to use int instead of bool to keep the compiler happy)
Attachment #693608 -
Flags: review?(tterribe)
Comment 7•11 years ago
|
||
Comment on attachment 693608 [details] [diff] [review] interdiffs Review of attachment 693608 [details] [diff] [review]: ----------------------------------------------------------------- LGTM.
Attachment #693608 -
Flags: review?(tterribe) → review+
Assignee | ||
Comment 8•11 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/ccf586b1e679
Target Milestone: --- → mozilla20
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ccf586b1e679
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Whiteboard: [WebRTC] [blocking-webrtc-] → [WebRTC] [blocking-webrtc-] [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•