From f570a2804ed8af6f6586f4aea51e089bd55d7e42 Mon Sep 17 00:00:00 2001 From: philipel Date: Mon, 21 Nov 2016 08:29:59 -0800 Subject: [PATCH] Revert of Allow custom metrics implementations on Android. (patchset #11 id:260001 of https://codereview.webrtc.org/2403463002/ ) Reason for revert: Break downstream tests. Original issue's description: > Allow custom metrics implementations on Android. > > BUG=webrtc:6499 > > Committed: https://crrev.com/de609b26c5fc77fd3388eae5594ee8a634edf8da > Cr-Commit-Position: refs/heads/master@{#15169} TBR=kjellander@webrtc.org,magjed@webrtc.org,sakal@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:6499 Review-Url: https://codereview.webrtc.org/2516403002 Cr-Commit-Position: refs/heads/master@{#15171} --- webrtc/BUILD.gn | 1 + webrtc/api/BUILD.gn | 30 ++----------- .../java/src/org/webrtc/Camera1Session.java | 1 + .../java/src/org/webrtc/Camera2Session.java | 1 + .../org/webrtc/CameraEnumerationAndroid.java | 1 + .../java/src/org/webrtc/Histogram.java | 44 ------------------- .../android/java/src/org/webrtc/Metrics.java | 35 +++++++++++++++ webrtc/api/android/java/src/org/webrtc/OWNERS | 1 - .../src/org/webrtc/VideoCapturerAndroid.java | 1 + webrtc/api/android/jni/OWNERS | 1 - .../api/android/jni/androidhistogram_jni.cc | 44 ------------------- webrtc/api/android/jni/androidmetrics_jni.cc | 27 +++++++++++- .../api/android/jni/classreferenceholder.cc | 2 + webrtc/examples/BUILD.gn | 1 - 14 files changed, 71 insertions(+), 119 deletions(-) delete mode 100644 webrtc/api/android/java/src/org/webrtc/Histogram.java delete mode 100644 webrtc/api/android/jni/androidhistogram_jni.cc diff --git a/webrtc/BUILD.gn b/webrtc/BUILD.gn index cefe10638a..17e3c2ac88 100644 --- a/webrtc/BUILD.gn +++ b/webrtc/BUILD.gn @@ -708,6 +708,7 @@ if (rtc_include_tests) { deps = [ "//base:base_java_test_support", "//webrtc/api:libjingle_peerconnection_java", + "//webrtc/api:libjingle_peerconnection_jni", "//webrtc/examples:AppRTCMobile_javalib", ] } diff --git a/webrtc/api/BUILD.gn b/webrtc/api/BUILD.gn index ee64e609df..01aacb7459 100644 --- a/webrtc/api/BUILD.gn +++ b/webrtc/api/BUILD.gn @@ -171,12 +171,12 @@ if (is_android && !build_with_chromium) { rtc_static_library("libjingle_peerconnection_jni") { sources = [ - "android/jni/androidhistogram_jni.cc", "android/jni/androidmediacodeccommon.h", "android/jni/androidmediadecoder_jni.cc", "android/jni/androidmediadecoder_jni.h", "android/jni/androidmediaencoder_jni.cc", "android/jni/androidmediaencoder_jni.h", + "android/jni/androidmetrics_jni.cc", "android/jni/androidnetworkmonitor_jni.cc", "android/jni/androidnetworkmonitor_jni.h", "android/jni/androidvideotracksource_jni.cc", @@ -215,6 +215,8 @@ if (is_android && !build_with_chromium) { deps = [ ":libjingle_peerconnection", + "../system_wrappers:field_trial_default", + "../system_wrappers:metrics_default", ] if (rtc_build_libyuv) { @@ -228,20 +230,6 @@ if (is_android && !build_with_chromium) { } } - rtc_static_library("libjingle_peerconnection_metrics_default_jni") { - sources = [ - "android/jni/androidmetrics_jni.cc", - ] - - configs += [ ":libjingle_peerconnection_jni_warnings_config" ] - - deps = [ - ":libjingle_peerconnection", - "../system_wrappers:field_trial_default", - "../system_wrappers:metrics_default", - ] - } - rtc_shared_library("libjingle_peerconnection_so") { sources = [ "android/jni/jni_onload.cc", @@ -252,7 +240,6 @@ if (is_android && !build_with_chromium) { deps = [ ":libjingle_peerconnection", ":libjingle_peerconnection_jni", - ":libjingle_peerconnection_metrics_default_jni", ] output_extension = "so" } @@ -292,7 +279,6 @@ if (is_android) { "android/java/src/org/webrtc/GlShader.java", "android/java/src/org/webrtc/GlTextureFrameBuffer.java", "android/java/src/org/webrtc/GlUtil.java", - "android/java/src/org/webrtc/Histogram.java", "android/java/src/org/webrtc/IceCandidate.java", "android/java/src/org/webrtc/MediaCodecVideoDecoder.java", "android/java/src/org/webrtc/MediaCodecVideoEncoder.java", @@ -300,6 +286,7 @@ if (is_android) { "android/java/src/org/webrtc/MediaSource.java", "android/java/src/org/webrtc/MediaStream.java", "android/java/src/org/webrtc/MediaStreamTrack.java", + "android/java/src/org/webrtc/Metrics.java", "android/java/src/org/webrtc/NetworkMonitor.java", "android/java/src/org/webrtc/NetworkMonitorAutoDetect.java", "android/java/src/org/webrtc/PeerConnection.java", @@ -329,14 +316,6 @@ if (is_android) { "../base:base_java", ] } - - android_library("libjingle_peerconnection_metrics_default_java") { - java_files = [ "android/java/src/org/webrtc/Metrics.java" ] - - deps = [ - "//webrtc/base:base_java", - ] - } } rtc_source_set("rtc_stats_api") { @@ -506,7 +485,6 @@ if (rtc_include_tests) { deps = [ ":libjingle_peerconnection_java", - ":libjingle_peerconnection_metrics_default_java", "../base:base_java", "//base:base_java", ] diff --git a/webrtc/api/android/java/src/org/webrtc/Camera1Session.java b/webrtc/api/android/java/src/org/webrtc/Camera1Session.java index 8f2988642f..6ff9c8aa66 100644 --- a/webrtc/api/android/java/src/org/webrtc/Camera1Session.java +++ b/webrtc/api/android/java/src/org/webrtc/Camera1Session.java @@ -20,6 +20,7 @@ import java.nio.ByteBuffer; import java.util.List; import java.util.concurrent.TimeUnit; import org.webrtc.CameraEnumerationAndroid.CaptureFormat; +import org.webrtc.Metrics.Histogram; @SuppressWarnings("deprecation") public class Camera1Session implements CameraSession { diff --git a/webrtc/api/android/java/src/org/webrtc/Camera2Session.java b/webrtc/api/android/java/src/org/webrtc/Camera2Session.java index 2552b2413e..37348609a5 100644 --- a/webrtc/api/android/java/src/org/webrtc/Camera2Session.java +++ b/webrtc/api/android/java/src/org/webrtc/Camera2Session.java @@ -29,6 +29,7 @@ import java.util.Arrays; import java.util.List; import java.util.concurrent.TimeUnit; import org.webrtc.CameraEnumerationAndroid.CaptureFormat; +import org.webrtc.Metrics.Histogram; @TargetApi(21) public class Camera2Session implements CameraSession { diff --git a/webrtc/api/android/java/src/org/webrtc/CameraEnumerationAndroid.java b/webrtc/api/android/java/src/org/webrtc/CameraEnumerationAndroid.java index de2b9199b4..c5cdcf7d56 100644 --- a/webrtc/api/android/java/src/org/webrtc/CameraEnumerationAndroid.java +++ b/webrtc/api/android/java/src/org/webrtc/CameraEnumerationAndroid.java @@ -18,6 +18,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.List; +import org.webrtc.Metrics.Histogram; @SuppressWarnings("deprecation") public class CameraEnumerationAndroid { diff --git a/webrtc/api/android/java/src/org/webrtc/Histogram.java b/webrtc/api/android/java/src/org/webrtc/Histogram.java deleted file mode 100644 index 877986134a..0000000000 --- a/webrtc/api/android/java/src/org/webrtc/Histogram.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2016 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -package org.webrtc; - -/** - * Class for holding the native pointer of a histogram. Since there is no way to destroy a - * histogram, please don't create unnecessary instances of this object. This class is thread safe. - * - * Usage example: - * private static final Histogram someMetricHistogram = - * Histogram.createCounts("WebRTC.Video.SomeMetric", 1, 10000, 50); - * someMetricHistogram.addSample(someVariable); - */ -class Histogram { - private final long handle; - - private Histogram(long handle) { - this.handle = handle; - } - - static public Histogram createCounts(String name, int min, int max, int bucketCount) { - return new Histogram(nativeCreateCounts(name, min, max, bucketCount)); - } - - static public Histogram createEnumeration(String name, int max) { - return new Histogram(nativeCreateEnumeration(name, max)); - } - - public void addSample(int sample) { - nativeAddSample(handle, sample); - } - - private static native long nativeCreateCounts(String name, int min, int max, int bucketCount); - private static native long nativeCreateEnumeration(String name, int max); - private static native void nativeAddSample(long handle, int sample); -} diff --git a/webrtc/api/android/java/src/org/webrtc/Metrics.java b/webrtc/api/android/java/src/org/webrtc/Metrics.java index ca19489e89..7d715fb7ae 100644 --- a/webrtc/api/android/java/src/org/webrtc/Metrics.java +++ b/webrtc/api/android/java/src/org/webrtc/Metrics.java @@ -59,6 +59,41 @@ public class Metrics { } } + /** + * Class for holding the native pointer of a histogram. Since there is no way to destroy a + * histogram, please don't create unnecessary instances of this object. This class is thread safe. + * + * Usage example: + * private static final Histogram someMetricHistogram = + * Histogram.createCounts("WebRTC.Video.SomeMetric", 1, 10000, 50); + * someMetricHistogram.addSample(someVariable); + */ + static class Histogram { + private final long handle; + private final String name; // Only used for logging. + + private Histogram(long handle, String name) { + this.handle = handle; + this.name = name; + } + + static public Histogram createCounts(String name, int min, int max, int bucketCount) { + return new Histogram(nativeCreateCounts(name, min, max, bucketCount), name); + } + + static public Histogram createEnumeration(String name, int max) { + return new Histogram(nativeCreateEnumeration(name, max), name); + } + + public void addSample(int sample) { + nativeAddSample(handle, sample); + } + + private static native long nativeCreateCounts(String name, int min, int max, int bucketCount); + private static native long nativeCreateEnumeration(String name, int max); + private static native void nativeAddSample(long handle, int sample); + } + private void add(String name, HistogramInfo info) { map.put(name, info); } diff --git a/webrtc/api/android/java/src/org/webrtc/OWNERS b/webrtc/api/android/java/src/org/webrtc/OWNERS index e6ccc2dda2..38f3f6f50f 100644 --- a/webrtc/api/android/java/src/org/webrtc/OWNERS +++ b/webrtc/api/android/java/src/org/webrtc/OWNERS @@ -1,3 +1,2 @@ per-file Camera*=sakal@webrtc.org -per-file Histogram.java=sakal@webrtc.org per-file Metrics.java=sakal@webrtc.org diff --git a/webrtc/api/android/java/src/org/webrtc/VideoCapturerAndroid.java b/webrtc/api/android/java/src/org/webrtc/VideoCapturerAndroid.java index da32eb5433..08c5c134ac 100644 --- a/webrtc/api/android/java/src/org/webrtc/VideoCapturerAndroid.java +++ b/webrtc/api/android/java/src/org/webrtc/VideoCapturerAndroid.java @@ -24,6 +24,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import org.webrtc.CameraEnumerationAndroid.CaptureFormat; +import org.webrtc.Metrics.Histogram; // Android specific implementation of VideoCapturer. // An instance of this class can be created by an application using diff --git a/webrtc/api/android/jni/OWNERS b/webrtc/api/android/jni/OWNERS index 4da3451506..96da1b9594 100644 --- a/webrtc/api/android/jni/OWNERS +++ b/webrtc/api/android/jni/OWNERS @@ -1,3 +1,2 @@ -per-file androidhistogram_jni.cc=sakal@webrtc.org per-file androidmetrics_jni.cc=sakal@webrtc.org per-file androidvideotracksource_jni.cc=sakal@webrtc.org diff --git a/webrtc/api/android/jni/androidhistogram_jni.cc b/webrtc/api/android/jni/androidhistogram_jni.cc deleted file mode 100644 index 9ccd2b78ec..0000000000 --- a/webrtc/api/android/jni/androidhistogram_jni.cc +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2016 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include -#include - -#include "webrtc/api/android/jni/classreferenceholder.h" -#include "webrtc/api/android/jni/jni_helpers.h" -#include "webrtc/api/android/jni/native_handle_impl.h" -#include "webrtc/system_wrappers/include/metrics.h" - -// Enables collection of native histograms and creating them. -namespace webrtc_jni { - -JOW(jlong, Histogram_nativeCreateCounts) -(JNIEnv* jni, jclass, jstring j_name, jint min, jint max, jint buckets) { - std::string name = JavaToStdString(jni, j_name); - return jlongFromPointer( - webrtc::metrics::HistogramFactoryGetCounts(name, min, max, buckets)); -} - -JOW(jlong, Histogram_nativeCreateEnumeration) -(JNIEnv* jni, jclass, jstring j_name, jint max) { - std::string name = JavaToStdString(jni, j_name); - return jlongFromPointer( - webrtc::metrics::HistogramFactoryGetEnumeration(name, max)); -} - -JOW(void, Histogram_nativeAddSample) -(JNIEnv* jni, jclass, jlong histogram, jint sample) { - if (histogram) { - HistogramAdd(reinterpret_cast(histogram), - sample); - } -} - -} // namespace webrtc_jni diff --git a/webrtc/api/android/jni/androidmetrics_jni.cc b/webrtc/api/android/jni/androidmetrics_jni.cc index 62efc1b553..f63e60452e 100644 --- a/webrtc/api/android/jni/androidmetrics_jni.cc +++ b/webrtc/api/android/jni/androidmetrics_jni.cc @@ -13,6 +13,7 @@ #include "webrtc/api/android/jni/classreferenceholder.h" #include "webrtc/api/android/jni/jni_helpers.h" +#include "webrtc/api/android/jni/native_handle_impl.h" #include "webrtc/system_wrappers/include/metrics.h" #include "webrtc/system_wrappers/include/metrics_default.h" @@ -25,11 +26,11 @@ JOW(void, Metrics_nativeEnable)(JNIEnv* jni, jclass) { // Gets and clears native histograms. JOW(jobject, Metrics_nativeGetAndReset)(JNIEnv* jni, jclass) { - jclass j_metrics_class = jni->FindClass("org/webrtc/Metrics"); + jclass j_metrics_class = FindClass(jni, "org/webrtc/Metrics"); jmethodID j_add = GetMethodID(jni, j_metrics_class, "add", "(Ljava/lang/String;Lorg/webrtc/Metrics$HistogramInfo;)V"); - jclass j_info_class = jni->FindClass("org/webrtc/Metrics$HistogramInfo"); + jclass j_info_class = FindClass(jni, "org/webrtc/Metrics$HistogramInfo"); jmethodID j_add_sample = GetMethodID(jni, j_info_class, "addSample", "(II)V"); // Create |Metrics|. @@ -58,4 +59,26 @@ JOW(jobject, Metrics_nativeGetAndReset)(JNIEnv* jni, jclass) { return j_metrics; } +JOW(jlong, Metrics_00024Histogram_nativeCreateCounts) +(JNIEnv* jni, jclass, jstring j_name, jint min, jint max, jint buckets) { + std::string name = JavaToStdString(jni, j_name); + return jlongFromPointer( + webrtc::metrics::HistogramFactoryGetCounts(name, min, max, buckets)); +} + +JOW(jlong, Metrics_00024Histogram_nativeCreateEnumeration) +(JNIEnv* jni, jclass, jstring j_name, jint max) { + std::string name = JavaToStdString(jni, j_name); + return jlongFromPointer( + webrtc::metrics::HistogramFactoryGetEnumeration(name, max)); +} + +JOW(void, Metrics_00024Histogram_nativeAddSample) +(JNIEnv* jni, jclass, jlong histogram, jint sample) { + if (histogram) { + HistogramAdd(reinterpret_cast(histogram), + sample); + } +} + } // namespace webrtc_jni diff --git a/webrtc/api/android/jni/classreferenceholder.cc b/webrtc/api/android/jni/classreferenceholder.cc index ee68112435..bfc03a3ab5 100644 --- a/webrtc/api/android/jni/classreferenceholder.cc +++ b/webrtc/api/android/jni/classreferenceholder.cc @@ -69,6 +69,8 @@ ClassReferenceHolder::ClassReferenceHolder(JNIEnv* jni) { LoadClass(jni, "org/webrtc/MediaSource$State"); LoadClass(jni, "org/webrtc/MediaStream"); LoadClass(jni, "org/webrtc/MediaStreamTrack$State"); + LoadClass(jni, "org/webrtc/Metrics"); + LoadClass(jni, "org/webrtc/Metrics$HistogramInfo"); LoadClass(jni, "org/webrtc/NetworkMonitor"); LoadClass(jni, "org/webrtc/NetworkMonitorAutoDetect$ConnectionType"); LoadClass(jni, "org/webrtc/NetworkMonitorAutoDetect$IPAddress"); diff --git a/webrtc/examples/BUILD.gn b/webrtc/examples/BUILD.gn index d419d948ac..58ae8a4c1a 100644 --- a/webrtc/examples/BUILD.gn +++ b/webrtc/examples/BUILD.gn @@ -93,7 +93,6 @@ if (is_android) { deps = [ ":AppRTCMobile_resources", "//webrtc/api:libjingle_peerconnection_java", - "//webrtc/api:libjingle_peerconnection_metrics_default_java", "//webrtc/base:base_java", "//webrtc/examples/androidapp/third_party/autobanh:autobanh_java", ]