From 4c4cb5b98456cb16d9174485bd59e58a861fd0f5 Mon Sep 17 00:00:00 2001 From: skvlad Date: Wed, 29 Jun 2016 15:30:41 -0700 Subject: [PATCH] Separate the JNI function that controls logging levels into two. The parameters for Logging.enableTracing() were creating the impression that they control level and severity of one tracing system and they are meant to be used together. In fact the "levels" parameter controlled one tracing system (WEBRTC_TRACE), and the "severity" parameter was responsible for a completely different one: setting the severity level above which log messages from LOG() will be directed to the platform-specific debug output (logcat on Android). The method signature suggested that the "path" parameter applied to both systems - while it was only meaningful for the WEBRTC_TRACE; LOG messages were directed to ADB logcat no matter what the Path value was. It is possible to redirect LOG messages to a file, but that is done using a completely different set of APIs - PeerConnectionFactory.startInternalTracingCapture(). I've separated these two methods to make it more clear which of the parameters controls which system. NOTRY=true Review-Url: https://codereview.webrtc.org/2110853003 Cr-Commit-Position: refs/heads/master@{#13334} --- webrtc/api/java/jni/peerconnection_jni.cc | 9 ++++--- webrtc/base/java/src/org/webrtc/Logging.java | 27 ++++++++++++++----- .../appspot/apprtc/PeerConnectionClient.java | 4 +-- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/webrtc/api/java/jni/peerconnection_jni.cc b/webrtc/api/java/jni/peerconnection_jni.cc index a5ba254c32..db89001a1f 100644 --- a/webrtc/api/java/jni/peerconnection_jni.cc +++ b/webrtc/api/java/jni/peerconnection_jni.cc @@ -884,8 +884,7 @@ JOW(void, DataChannel_dispose)(JNIEnv* jni, jobject j_dc) { } JOW(void, Logging_nativeEnableTracing)( - JNIEnv* jni, jclass, jstring j_path, jint nativeLevels, - jint nativeSeverity) { + JNIEnv* jni, jclass, jstring j_path, jint nativeLevels) { std::string path = JavaToStdString(jni, j_path); if (nativeLevels != webrtc::kTraceNone) { webrtc::Trace::set_level_filter(nativeLevels); @@ -898,7 +897,11 @@ JOW(void, Logging_nativeEnableTracing)( static LogcatTraceContext* g_trace_callback = new LogcatTraceContext(); } } - if (nativeSeverity >= rtc::LS_SENSITIVE && nativeSeverity <= rtc::LS_ERROR) { +} + +JOW(void, Logging_nativeEnableLogToDebugOutput) + (JNIEnv *jni, jclass, jint nativeSeverity) { + if (nativeSeverity >= rtc::LS_SENSITIVE && nativeSeverity <= rtc::LS_NONE) { rtc::LogMessage::LogToDebug( static_cast(nativeSeverity)); } diff --git a/webrtc/base/java/src/org/webrtc/Logging.java b/webrtc/base/java/src/org/webrtc/Logging.java index ea1bca3bdb..684e5bc5b2 100644 --- a/webrtc/base/java/src/org/webrtc/Logging.java +++ b/webrtc/base/java/src/org/webrtc/Logging.java @@ -20,6 +20,7 @@ import java.util.logging.Level; public class Logging { private static final Logger fallbackLogger = Logger.getLogger("org.webrtc.Logging"); private static volatile boolean tracingEnabled; + private static volatile boolean loggingEnabled; private static volatile boolean nativeLibLoaded; static { @@ -60,7 +61,7 @@ public class Logging { // Keep in sync with webrtc/base/logging.h:LoggingSeverity. public enum Severity { - LS_SENSITIVE, LS_VERBOSE, LS_INFO, LS_WARNING, LS_ERROR, + LS_SENSITIVE, LS_VERBOSE, LS_INFO, LS_WARNING, LS_ERROR, LS_NONE }; public static void enableLogThreads() { @@ -80,10 +81,11 @@ public class Logging { nativeEnableLogTimeStamps(); } - // Enable tracing to |path| of messages of |levels| and |severity|. + // Enable tracing to |path| of messages of |levels|. // On Android, use "logcat:" for |path| to send output there. + // Note: this function controls the output of the WEBRTC_TRACE() macros. public static synchronized void enableTracing( - String path, EnumSet levels, Severity severity) { + String path, EnumSet levels) { if (!nativeLibLoaded) { fallbackLogger.log(Level.WARNING, "Cannot enable tracing because native lib not loaded."); return; @@ -96,12 +98,24 @@ public class Logging { for (TraceLevel level : levels) { nativeLevel |= level.level; } - nativeEnableTracing(path, nativeLevel, severity.ordinal()); + nativeEnableTracing(path, nativeLevel); tracingEnabled = true; } + // Enable diagnostic logging for messages of |severity| to the platform debug + // output. On Android, the output will be directed to Logcat. + // Note: this function starts collecting the output of the LOG() macros. + public static synchronized void enableLogToDebugOutput(Severity severity) { + if (!nativeLibLoaded) { + fallbackLogger.log(Level.WARNING, "Cannot enable logging because native lib not loaded."); + return; + } + nativeEnableLogToDebugOutput(severity.ordinal()); + loggingEnabled = true; + } + public static void log(Severity severity, String tag, String message) { - if (tracingEnabled) { + if (loggingEnabled) { nativeLog(severity.ordinal(), tag, message); return; } @@ -165,7 +179,8 @@ public class Logging { } private static native void nativeEnableTracing( - String path, int nativeLevels, int nativeSeverity); + String path, int nativeLevels); + private static native void nativeEnableLogToDebugOutput(int nativeSeverity); private static native void nativeEnableLogThreads(); private static native void nativeEnableLogTimeStamps(); private static native void nativeLog(int severity, String tag, String message); diff --git a/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java b/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java index 532c016612..e8e1f85039 100644 --- a/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java +++ b/webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java @@ -490,8 +490,8 @@ public class PeerConnectionClient { // NOTE: this _must_ happen while |factory| is alive! Logging.enableTracing( "logcat:", - EnumSet.of(Logging.TraceLevel.TRACE_DEFAULT), - Logging.Severity.LS_INFO); + EnumSet.of(Logging.TraceLevel.TRACE_DEFAULT)); + Logging.enableLogToDebugOutput(Logging.Severity.LS_INFO); mediaStream = factory.createLocalMediaStream("ARDAMS"); if (videoCallEnabled) {