Fix memory leak in NativeToJavaVideoFrame.
This method used to just wrap frame when passed a native frame and
create a new one when passed non-native frame. This caused a memory
leak when a new frame was returned because the caller didn't release
the frame. Now the method always returns a new frame and the caller is
responsible for releasing it.
Bug: webrtc:8892, b/72675429
Change-Id: I06d67a6ed4c059cae1d709c51b0266f9c72fef1a
Reviewed-on: https://webrtc-review.googlesource.com/53840
Reviewed-by: Anders Carlsson <andersc@webrtc.org>
Commit-Queue: Sami Kalliomäki <sakal@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22033}diff --git a/sdk/android/api/org/webrtc/VideoFrame.java b/sdk/android/api/org/webrtc/VideoFrame.java
index 3e8a1e4..3b73543 100644
--- a/sdk/android/api/org/webrtc/VideoFrame.java
+++ b/sdk/android/api/org/webrtc/VideoFrame.java
@@ -177,6 +177,7 @@
buffer.retain();
}
+ @CalledByNative
public void release() {
buffer.release();
}
diff --git a/sdk/android/native_api/video/wrapper.cc b/sdk/android/native_api/video/wrapper.cc
index 0f1bcaf..cc626c8 100644
--- a/sdk/android/native_api/video/wrapper.cc
+++ b/sdk/android/native_api/video/wrapper.cc
@@ -26,7 +26,7 @@
ScopedJavaLocalRef<jobject> NativeToJavaVideoFrame(JNIEnv* jni,
const VideoFrame& frame) {
- return jni::NativeToJavaFrame(jni, frame);
+ return jni::NativeToJavaVideoFrame(jni, frame);
}
} // namespace webrtc
diff --git a/sdk/android/native_api/video/wrapper.h b/sdk/android/native_api/video/wrapper.h
index 35c305c..fc670ea 100644
--- a/sdk/android/native_api/video/wrapper.h
+++ b/sdk/android/native_api/video/wrapper.h
@@ -26,7 +26,8 @@
JNIEnv* jni,
jobject video_sink);
-// Creates a Java VideoFrame object from a native VideoFrame.
+// Creates a Java VideoFrame object from a native VideoFrame. The returned
+// object has to be released by calling release.
ScopedJavaLocalRef<jobject> NativeToJavaVideoFrame(JNIEnv* jni,
const VideoFrame& frame);
diff --git a/sdk/android/src/jni/androidmediaencoder.cc b/sdk/android/src/jni/androidmediaencoder.cc
index f71b508..7f835f7 100644
--- a/sdk/android/src/jni/androidmediaencoder.cc
+++ b/sdk/android/src/jni/androidmediaencoder.cc
@@ -728,11 +728,14 @@
case AndroidVideoFrameBuffer::AndroidType::kTextureBuffer:
encode_status = EncodeTexture(jni, key_frame, input_frame);
break;
- case AndroidVideoFrameBuffer::AndroidType::kJavaBuffer:
+ case AndroidVideoFrameBuffer::AndroidType::kJavaBuffer: {
+ ScopedJavaLocalRef<jobject> j_frame =
+ NativeToJavaVideoFrame(jni, frame);
encode_status =
- EncodeJavaFrame(jni, key_frame, NativeToJavaFrame(jni, input_frame),
- j_input_buffer_index);
+ EncodeJavaFrame(jni, key_frame, j_frame, j_input_buffer_index);
+ ReleaseJavaVideoFrame(jni, j_frame);
break;
+ }
default:
RTC_NOTREACHED();
return WEBRTC_VIDEO_CODEC_ERROR;
diff --git a/sdk/android/src/jni/videoencoderwrapper.cc b/sdk/android/src/jni/videoencoderwrapper.cc
index 5ef30c1..ef0cdb4 100644
--- a/sdk/android/src/jni/videoencoderwrapper.cc
+++ b/sdk/android/src/jni/videoencoderwrapper.cc
@@ -128,8 +128,10 @@
info.timestamp_rtp = frame.timestamp();
frame_extra_infos_.push_back(info);
- ScopedJavaLocalRef<jobject> ret = Java_VideoEncoder_encode(
- jni, encoder_, NativeToJavaFrame(jni, frame), encode_info);
+ ScopedJavaLocalRef<jobject> j_frame = NativeToJavaVideoFrame(jni, frame);
+ ScopedJavaLocalRef<jobject> ret =
+ Java_VideoEncoder_encode(jni, encoder_, j_frame, encode_info);
+ ReleaseJavaVideoFrame(jni, j_frame);
return HandleReturnCode(jni, ret);
}
diff --git a/sdk/android/src/jni/videoframe.cc b/sdk/android/src/jni/videoframe.cc
index f638e19..22867e4 100644
--- a/sdk/android/src/jni/videoframe.cc
+++ b/sdk/android/src/jni/videoframe.cc
@@ -383,8 +383,8 @@
AndroidVideoFrameBuffer::AndroidType::kJavaBuffer;
}
-ScopedJavaLocalRef<jobject> NativeToJavaFrame(JNIEnv* jni,
- const VideoFrame& frame) {
+ScopedJavaLocalRef<jobject> NativeToJavaVideoFrame(JNIEnv* jni,
+ const VideoFrame& frame) {
rtc::scoped_refptr<VideoFrameBuffer> buffer = frame.video_frame_buffer();
if (IsJavaVideoBuffer(buffer)) {
@@ -396,9 +396,11 @@
AndroidVideoBuffer* android_video_buffer =
static_cast<AndroidVideoBuffer*>(android_buffer);
+ ScopedJavaLocalRef<jobject> j_video_frame_buffer(
+ jni, android_video_buffer->video_frame_buffer());
+ Java_Buffer_retain(jni, j_video_frame_buffer);
return Java_VideoFrame_Constructor(
- jni, android_video_buffer->video_frame_buffer(),
- static_cast<jint>(frame.rotation()),
+ jni, j_video_frame_buffer, static_cast<jint>(frame.rotation()),
static_cast<jlong>(frame.timestamp_us() *
rtc::kNumNanosecsPerMicrosec));
} else {
@@ -410,6 +412,10 @@
}
}
+void ReleaseJavaVideoFrame(JNIEnv* jni, const JavaRef<jobject>& j_video_frame) {
+ Java_VideoFrame_release(jni, j_video_frame);
+}
+
static void JNI_VideoFrame_CropAndScaleI420(
JNIEnv* jni,
const JavaParamRef<jclass>&,
diff --git a/sdk/android/src/jni/videoframe.h b/sdk/android/src/jni/videoframe.h
index 5cb7a2d..6f5720d 100644
--- a/sdk/android/src/jni/videoframe.h
+++ b/sdk/android/src/jni/videoframe.h
@@ -155,8 +155,11 @@
const JavaRef<jobject>& j_video_frame,
uint32_t timestamp_rtp);
-ScopedJavaLocalRef<jobject> NativeToJavaFrame(JNIEnv* jni,
- const VideoFrame& frame);
+// NOTE: Returns a new video frame that has to be released by calling
+// ReleaseJavaVideoFrame.
+ScopedJavaLocalRef<jobject> NativeToJavaVideoFrame(JNIEnv* jni,
+ const VideoFrame& frame);
+void ReleaseJavaVideoFrame(JNIEnv* jni, const JavaRef<jobject>& j_video_frame);
int64_t GetJavaVideoFrameTimestampNs(JNIEnv* jni,
const JavaRef<jobject>& j_video_frame);
diff --git a/sdk/android/src/jni/videosink.cc b/sdk/android/src/jni/videosink.cc
index 89d6d2e..bbdda31 100644
--- a/sdk/android/src/jni/videosink.cc
+++ b/sdk/android/src/jni/videosink.cc
@@ -23,7 +23,9 @@
void VideoSinkWrapper::OnFrame(const VideoFrame& frame) {
JNIEnv* jni = AttachCurrentThreadIfNeeded();
- Java_VideoSink_onFrame(jni, j_sink_, NativeToJavaFrame(jni, frame));
+ ScopedJavaLocalRef<jobject> j_frame = NativeToJavaVideoFrame(jni, frame);
+ Java_VideoSink_onFrame(jni, j_sink_, j_frame);
+ ReleaseJavaVideoFrame(jni, j_frame);
}
} // namespace jni
