From aea1d1ad3fa1d4225f581e39ceb040fa641b5c7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sami=20Kalliom=C3=A4ki?= Date: Thu, 23 Nov 2017 14:42:21 +0100 Subject: [PATCH] Android: Fix leaking software video codec instances. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, wrapped native codec instances would leak the native object if it was never used. This change fixes it by changing getNative method to createNative. Also fixes "Video codec hardware acceleration" setting in AppRTCMobile. Bug: webrtc:7925 Change-Id: I53f6dc1dd5e37dea8d14278423122dede17719c5 Reviewed-on: https://webrtc-review.googlesource.com/24881 Reviewed-by: Magnus Jedvert Commit-Queue: Sami Kalliomäki Cr-Commit-Position: refs/heads/master@{#20859} --- .../appspot/apprtc/PeerConnectionClient.java | 21 +++++++++++++++---- .../api/org/webrtc/VideoDecoderFallback.java | 11 +++++++++- .../api/org/webrtc/VideoEncoderFallback.java | 11 +++++++++- .../src/java/org/webrtc/VP8Decoder.java | 6 +----- .../src/java/org/webrtc/VP8Encoder.java | 6 +----- .../src/java/org/webrtc/VP9Decoder.java | 6 +----- .../src/java/org/webrtc/VP9Encoder.java | 6 +----- .../org/webrtc/WrappedNativeVideoDecoder.java | 13 ++---------- .../org/webrtc/WrappedNativeVideoEncoder.java | 18 +++++++--------- sdk/android/src/jni/vp8codec.cc | 4 ++-- sdk/android/src/jni/vp9codec.cc | 4 ++-- sdk/android/src/jni/wrappednativecodec.cc | 9 +++----- 12 files changed, 57 insertions(+), 58 deletions(-) diff --git a/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java b/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java index b641555143..fb11e4ae0b 100644 --- a/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java +++ b/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java @@ -49,9 +49,13 @@ import org.webrtc.RtpReceiver; import org.webrtc.RtpSender; import org.webrtc.SdpObserver; import org.webrtc.SessionDescription; +import org.webrtc.SoftwareVideoDecoderFactory; +import org.webrtc.SoftwareVideoEncoderFactory; import org.webrtc.StatsObserver; import org.webrtc.StatsReport; import org.webrtc.VideoCapturer; +import org.webrtc.VideoDecoderFactory; +import org.webrtc.VideoEncoderFactory; import org.webrtc.VideoRenderer; import org.webrtc.VideoSink; import org.webrtc.VideoSource; @@ -520,10 +524,19 @@ public class PeerConnectionClient { } final boolean enableH264HighProfile = VIDEO_CODEC_H264_HIGH.equals(peerConnectionParameters.videoCodec); - factory = new PeerConnectionFactory(options, - new DefaultVideoEncoderFactory(rootEglBase.getEglBaseContext(), - true /* enableIntelVp8Encoder */, enableH264HighProfile), - new DefaultVideoDecoderFactory(rootEglBase.getEglBaseContext())); + final VideoEncoderFactory encoderFactory; + final VideoDecoderFactory decoderFactory; + + if (peerConnectionParameters.videoCodecHwAcceleration) { + encoderFactory = new DefaultVideoEncoderFactory( + rootEglBase.getEglBaseContext(), true /* enableIntelVp8Encoder */, enableH264HighProfile); + decoderFactory = new DefaultVideoDecoderFactory(rootEglBase.getEglBaseContext()); + } else { + encoderFactory = new SoftwareVideoEncoderFactory(); + decoderFactory = new SoftwareVideoDecoderFactory(); + } + + factory = new PeerConnectionFactory(options, encoderFactory, decoderFactory); Log.d(TAG, "Peer connection factory created."); } diff --git a/sdk/android/api/org/webrtc/VideoDecoderFallback.java b/sdk/android/api/org/webrtc/VideoDecoderFallback.java index 7c4d8caa0e..67e746f83c 100644 --- a/sdk/android/api/org/webrtc/VideoDecoderFallback.java +++ b/sdk/android/api/org/webrtc/VideoDecoderFallback.java @@ -14,8 +14,17 @@ package org.webrtc; * A combined video decoder that falls back on a secondary decoder if the primary decoder fails. */ public class VideoDecoderFallback extends WrappedNativeVideoDecoder { + private final VideoDecoder fallback; + private final VideoDecoder primary; + public VideoDecoderFallback(VideoDecoder fallback, VideoDecoder primary) { - super(createNativeDecoder(fallback, primary)); + this.fallback = fallback; + this.primary = primary; + } + + @Override + long createNativeDecoder() { + return createNativeDecoder(fallback, primary); } private static native long createNativeDecoder(VideoDecoder fallback, VideoDecoder primary); diff --git a/sdk/android/api/org/webrtc/VideoEncoderFallback.java b/sdk/android/api/org/webrtc/VideoEncoderFallback.java index 0072c136ad..6ee635104b 100644 --- a/sdk/android/api/org/webrtc/VideoEncoderFallback.java +++ b/sdk/android/api/org/webrtc/VideoEncoderFallback.java @@ -14,8 +14,17 @@ package org.webrtc; * A combined video encoder that falls back on a secondary encoder if the primary encoder fails. */ public class VideoEncoderFallback extends WrappedNativeVideoEncoder { + private final VideoEncoder fallback; + private final VideoEncoder primary; + public VideoEncoderFallback(VideoEncoder fallback, VideoEncoder primary) { - super(createNativeEncoder(fallback, primary)); + this.fallback = fallback; + this.primary = primary; + } + + @Override + long createNativeEncoder() { + return createNativeEncoder(fallback, primary); } private static native long createNativeEncoder(VideoEncoder fallback, VideoEncoder primary); diff --git a/sdk/android/src/java/org/webrtc/VP8Decoder.java b/sdk/android/src/java/org/webrtc/VP8Decoder.java index 474107b32b..ea7ff9730d 100644 --- a/sdk/android/src/java/org/webrtc/VP8Decoder.java +++ b/sdk/android/src/java/org/webrtc/VP8Decoder.java @@ -11,9 +11,5 @@ package org.webrtc; class VP8Decoder extends WrappedNativeVideoDecoder { - VP8Decoder() { - super(createNativeDecoder()); - } - - private static native long createNativeDecoder(); + @Override native long createNativeDecoder(); } diff --git a/sdk/android/src/java/org/webrtc/VP8Encoder.java b/sdk/android/src/java/org/webrtc/VP8Encoder.java index 505a7a9aee..232e1a8429 100644 --- a/sdk/android/src/java/org/webrtc/VP8Encoder.java +++ b/sdk/android/src/java/org/webrtc/VP8Encoder.java @@ -11,9 +11,5 @@ package org.webrtc; class VP8Encoder extends WrappedNativeVideoEncoder { - VP8Encoder() { - super(createNativeEncoder()); - } - - private static native long createNativeEncoder(); + @Override native long createNativeEncoder(); } diff --git a/sdk/android/src/java/org/webrtc/VP9Decoder.java b/sdk/android/src/java/org/webrtc/VP9Decoder.java index a01557eec0..619ba91768 100644 --- a/sdk/android/src/java/org/webrtc/VP9Decoder.java +++ b/sdk/android/src/java/org/webrtc/VP9Decoder.java @@ -11,11 +11,7 @@ package org.webrtc; class VP9Decoder extends WrappedNativeVideoDecoder { - VP9Decoder() { - super(createNativeDecoder()); - } + @Override native long createNativeDecoder(); static native boolean isSupported(); - - private static native long createNativeDecoder(); } diff --git a/sdk/android/src/java/org/webrtc/VP9Encoder.java b/sdk/android/src/java/org/webrtc/VP9Encoder.java index 16cc34820e..56315c4b0c 100644 --- a/sdk/android/src/java/org/webrtc/VP9Encoder.java +++ b/sdk/android/src/java/org/webrtc/VP9Encoder.java @@ -11,11 +11,7 @@ package org.webrtc; class VP9Encoder extends WrappedNativeVideoEncoder { - VP9Encoder() { - super(createNativeEncoder()); - } + @Override native long createNativeEncoder(); static native boolean isSupported(); - - private static native long createNativeEncoder(); } diff --git a/sdk/android/src/java/org/webrtc/WrappedNativeVideoDecoder.java b/sdk/android/src/java/org/webrtc/WrappedNativeVideoDecoder.java index d94c4ada97..916d07bd8e 100644 --- a/sdk/android/src/java/org/webrtc/WrappedNativeVideoDecoder.java +++ b/sdk/android/src/java/org/webrtc/WrappedNativeVideoDecoder.java @@ -13,17 +13,8 @@ package org.webrtc; /** * Wraps a native webrtc::VideoDecoder. */ -class WrappedNativeVideoDecoder implements VideoDecoder { - private final long nativeDecoder; - - WrappedNativeVideoDecoder(long nativeDecoder) { - this.nativeDecoder = nativeDecoder; - } - - @CalledByNative - public long getNativeDecoder() { - return this.nativeDecoder; - } +abstract class WrappedNativeVideoDecoder implements VideoDecoder { + @CalledByNative abstract long createNativeDecoder(); @Override public VideoCodecStatus initDecode(Settings settings, Callback decodeCallback) { diff --git a/sdk/android/src/java/org/webrtc/WrappedNativeVideoEncoder.java b/sdk/android/src/java/org/webrtc/WrappedNativeVideoEncoder.java index c0b145ea58..287566ed9e 100644 --- a/sdk/android/src/java/org/webrtc/WrappedNativeVideoEncoder.java +++ b/sdk/android/src/java/org/webrtc/WrappedNativeVideoEncoder.java @@ -13,17 +13,8 @@ package org.webrtc; /** * Wraps a native webrtc::VideoEncoder. */ -class WrappedNativeVideoEncoder implements VideoEncoder { - private final long nativeEncoder; - - WrappedNativeVideoEncoder(long nativeEncoder) { - this.nativeEncoder = nativeEncoder; - } - - @CalledByNative - public long getNativeEncoder() { - return this.nativeEncoder; - } +abstract class WrappedNativeVideoEncoder implements VideoEncoder { + @CalledByNative abstract long createNativeEncoder(); @Override public VideoCodecStatus initEncode(Settings settings, Callback encodeCallback) { @@ -59,4 +50,9 @@ class WrappedNativeVideoEncoder implements VideoEncoder { public String getImplementationName() { throw new UnsupportedOperationException("Not implemented."); } + + @CalledByNative + static boolean isInstanceOf(VideoEncoder encoder) { + return encoder instanceof WrappedNativeVideoEncoder; + } } diff --git a/sdk/android/src/jni/vp8codec.cc b/sdk/android/src/jni/vp8codec.cc index 1108ed0540..000652ce80 100644 --- a/sdk/android/src/jni/vp8codec.cc +++ b/sdk/android/src/jni/vp8codec.cc @@ -19,14 +19,14 @@ namespace jni { JNI_FUNCTION_DECLARATION(jlong, VP8Encoder_createNativeEncoder, JNIEnv* jni, - jclass) { + jobject) { return jlongFromPointer(VP8Encoder::Create().release()); } JNI_FUNCTION_DECLARATION(jlong, VP8Decoder_createNativeDecoder, JNIEnv* jni, - jclass) { + jobject) { return jlongFromPointer(VP8Decoder::Create().release()); } diff --git a/sdk/android/src/jni/vp9codec.cc b/sdk/android/src/jni/vp9codec.cc index 599fee5087..8ec51ae336 100644 --- a/sdk/android/src/jni/vp9codec.cc +++ b/sdk/android/src/jni/vp9codec.cc @@ -19,7 +19,7 @@ namespace jni { JNI_FUNCTION_DECLARATION(jlong, VP9Encoder_createNativeEncoder, JNIEnv* jni, - jclass) { + jobject) { return jlongFromPointer(VP9Encoder::Create().release()); } @@ -33,7 +33,7 @@ JNI_FUNCTION_DECLARATION(jboolean, JNI_FUNCTION_DECLARATION(jlong, VP9Decoder_createNativeDecoder, JNIEnv* jni, - jclass) { + jobject) { return jlongFromPointer(VP9Decoder::Create().release()); } diff --git a/sdk/android/src/jni/wrappednativecodec.cc b/sdk/android/src/jni/wrappednativecodec.cc index a29be73def..e5eb63656b 100644 --- a/sdk/android/src/jni/wrappednativecodec.cc +++ b/sdk/android/src/jni/wrappednativecodec.cc @@ -28,7 +28,7 @@ std::unique_ptr JavaToNativeVideoDecoder(JNIEnv* jni, VideoDecoder* decoder; if (jni->IsInstanceOf(j_decoder, wrapped_native_decoder_class)) { jlong native_decoder = - Java_WrappedNativeVideoDecoder_getNativeDecoder(jni, j_decoder); + Java_WrappedNativeVideoDecoder_createNativeDecoder(jni, j_decoder); decoder = reinterpret_cast(native_decoder); } else { decoder = new VideoDecoderWrapper(jni, j_decoder); @@ -39,13 +39,10 @@ std::unique_ptr JavaToNativeVideoDecoder(JNIEnv* jni, std::unique_ptr JavaToNativeVideoEncoder(JNIEnv* jni, jobject j_encoder) { - jclass wrapped_native_encoder_class = - GetClass(jni, "org/webrtc/WrappedNativeVideoEncoder"); - VideoEncoder* encoder; - if (jni->IsInstanceOf(j_encoder, wrapped_native_encoder_class)) { + if (Java_WrappedNativeVideoEncoder_isInstanceOf(jni, j_encoder)) { jlong native_encoder = - Java_WrappedNativeVideoEncoder_getNativeEncoder(jni, j_encoder); + Java_WrappedNativeVideoEncoder_createNativeEncoder(jni, j_encoder); encoder = reinterpret_cast(native_encoder); } else { encoder = new VideoEncoderWrapper(jni, j_encoder);