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}
This commit is contained in:
Sami Kalliomäki
2018-02-15 13:58:15 +01:00
committed by Commit Bot
parent 2c599d663d
commit 2bde85046a
8 changed files with 32 additions and 14 deletions

View File

@ -177,6 +177,7 @@ public class VideoFrame {
buffer.retain();
}
@CalledByNative
public void release() {
buffer.release();
}

View File

@ -26,7 +26,7 @@ std::unique_ptr<rtc::VideoSinkInterface<VideoFrame>> JavaToNativeVideoSink(
ScopedJavaLocalRef<jobject> NativeToJavaVideoFrame(JNIEnv* jni,
const VideoFrame& frame) {
return jni::NativeToJavaFrame(jni, frame);
return jni::NativeToJavaVideoFrame(jni, frame);
}
} // namespace webrtc

View File

@ -26,7 +26,8 @@ std::unique_ptr<rtc::VideoSinkInterface<VideoFrame>> JavaToNativeVideoSink(
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);

View File

@ -728,11 +728,14 @@ int32_t MediaCodecVideoEncoder::Encode(
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;

View File

@ -128,8 +128,10 @@ int32_t VideoEncoderWrapper::Encode(
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);
}

View File

@ -383,8 +383,8 @@ static bool IsJavaVideoBuffer(rtc::scoped_refptr<VideoFrameBuffer> buffer) {
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 @@ ScopedJavaLocalRef<jobject> NativeToJavaFrame(JNIEnv* jni,
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 @@ ScopedJavaLocalRef<jobject> NativeToJavaFrame(JNIEnv* jni,
}
}
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>&,

View File

@ -155,8 +155,11 @@ VideoFrame JavaToNativeFrame(JNIEnv* jni,
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);

View File

@ -23,7 +23,9 @@ VideoSinkWrapper::~VideoSinkWrapper() {}
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