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)

defect

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: jesup, Assigned: jesup)

Details

(Whiteboard: [WebRTC] [blocking-webrtc-] [qa-])

Attachments

(2 files)

We want to be able to turn on WEBRTC_TRACE() logging without recompiling the code

Also moving 'ikran' to 'signaling'
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
Attachment #693479 - Flags: review?(tterribe)
Not blocking but very handy
Priority: -- → P2
Whiteboard: [WebRTC] [blocking-webrtc-]
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+
(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.
Attached patch interdiffsSplinter Review
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 on attachment 693608 [details] [diff] [review]
interdiffs

Review of attachment 693608 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.
Attachment #693608 - Flags: review?(tterribe) → review+
https://hg.mozilla.org/mozilla-central/rev/ccf586b1e679
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [WebRTC] [blocking-webrtc-] → [WebRTC] [blocking-webrtc-] [qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: