From a6cba3ab5c899339d577adf1824e0e007c12863e Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Sat, 29 Aug 2015 15:57:43 +0200 Subject: [PATCH] Java VideoRenderer.Callbacks: Make renderFrame() interface asynchronous This CL makes the Java render interface asynchronous by requiring every call to renderFrame() to be followed by an explicit renderFrameDone() call. In JNI, this is implemented with cricket::VideoFrame::Copy() before calling renderFrame(), and a corresponding call to delete in renderFrameDone(). This CL is primarily done to prepare for a new renderer implementation. BUG=webrtc:4742, webrtc:4909 R=glaznev@webrtc.org Review URL: https://codereview.webrtc.org/1313563002 . Cr-Commit-Position: refs/heads/master@{#9814} --- .../org/webrtc/VideoCapturerAndroidTest.java | 79 +++++++++++++++++++ talk/app/webrtc/androidvideocapturer.cc | 2 + .../org/webrtc/VideoCapturerAndroid.java | 6 ++ .../android/org/webrtc/VideoRendererGui.java | 9 ++- .../app/webrtc/java/jni/peerconnection_jni.cc | 16 +++- .../java/src/org/webrtc/VideoRenderer.java | 36 ++++++++- .../src/org/webrtc/PeerConnectionTest.java | 1 + .../apprtc/test/PeerConnectionClientTest.java | 1 + 8 files changed, 140 insertions(+), 10 deletions(-) diff --git a/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java b/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java index 54f77c5166..162fad1c3b 100644 --- a/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java +++ b/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java @@ -48,6 +48,7 @@ public class VideoCapturerAndroidTest extends ActivityTestCase { ++framesRendered; frameLock.notify(); } + VideoRenderer.renderFrameDone(frame); } public int WaitForNextFrameToRender() throws InterruptedException { @@ -58,6 +59,34 @@ public class VideoCapturerAndroidTest extends ActivityTestCase { } } + static class AsyncRenderer implements VideoRenderer.Callbacks { + private final List pendingFrames = new ArrayList(); + + @Override + public void renderFrame(I420Frame frame) { + synchronized (pendingFrames) { + pendingFrames.add(frame); + pendingFrames.notifyAll(); + } + } + + // Wait until at least one frame have been received, before returning them. + public List WaitForFrames() { + synchronized (pendingFrames) { + while (pendingFrames.isEmpty()) { + try { + pendingFrames.wait(); + } catch (InterruptedException e) { + // Ignore. + } + } + final List frames = new ArrayList(pendingFrames); + pendingFrames.clear(); + return frames; + } + } + } + static class FakeCapturerObserver implements VideoCapturerAndroid.CapturerObserver { private int framesCaptured = 0; @@ -306,4 +335,54 @@ public class VideoCapturerAndroidTest extends ActivityTestCase { capturer.returnBuffer(timeStamp); } } + + @SmallTest + // This test that we can capture frames, stop capturing, keep the frames for rendering, and then + // return the frames. It tests both the Java and the C++ layer. + public void testCaptureAndAsyncRender() { + final VideoCapturerAndroid capturer = VideoCapturerAndroid.create("", null); + // Helper class that sets everything up, captures at least one frame, and then shuts + // everything down. + class CaptureFramesRunnable implements Runnable { + public List frames; + + @Override + public void run() { + PeerConnectionFactory factory = new PeerConnectionFactory(); + VideoSource source = factory.createVideoSource(capturer, new MediaConstraints()); + VideoTrack track = factory.createVideoTrack("dummy", source); + AsyncRenderer renderer = new AsyncRenderer(); + track.addRenderer(new VideoRenderer(renderer)); + + // Wait until we get at least one frame. + frames = renderer.WaitForFrames(); + + // Stop everything. + track.dispose(); + source.dispose(); + factory.dispose(); + } + } + + // Capture frames on a separate thread. + CaptureFramesRunnable captureFramesRunnable = new CaptureFramesRunnable(); + Thread captureThread = new Thread(captureFramesRunnable); + captureThread.start(); + + // Wait until frames are captured, and then kill the thread. + try { + captureThread.join(); + } catch (InterruptedException e) { + fail("Capture thread was interrupted"); + } + captureThread = null; + + // Assert that we have frames that have not been returned. + assertTrue(!captureFramesRunnable.frames.isEmpty()); + // Return the frame(s). + for (I420Frame frame : captureFramesRunnable.frames) { + VideoRenderer.renderFrameDone(frame); + } + assertEquals(capturer.pendingFramesTimeStamps(), "[]"); + } } diff --git a/talk/app/webrtc/androidvideocapturer.cc b/talk/app/webrtc/androidvideocapturer.cc index cc7821760c..747dd43c5e 100644 --- a/talk/app/webrtc/androidvideocapturer.cc +++ b/talk/app/webrtc/androidvideocapturer.cc @@ -129,6 +129,8 @@ AndroidVideoCapturer::AndroidVideoCapturer( formats.push_back(format); } SetSupportedFormats(formats); + // Do not apply frame rotation by default. + SetApplyRotation(false); } AndroidVideoCapturer::~AndroidVideoCapturer() { diff --git a/talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java b/talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java index 00034fc143..d4251f6021 100644 --- a/talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java +++ b/talk/app/webrtc/java/android/org/webrtc/VideoCapturerAndroid.java @@ -223,6 +223,12 @@ public class VideoCapturerAndroid extends VideoCapturer implements PreviewCallba return CameraEnumerationAndroid.supportedFormats.get(id); } + // Return a list of timestamps for the frames that have been sent out, but not returned yet. + // Useful for logging and testing. + public String pendingFramesTimeStamps() { + return videoBuffers.pendingFramesTimeStamps(); + } + private VideoCapturerAndroid() { Log.d(TAG, "VideoCapturerAndroid"); } diff --git a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java index d4f754b1e1..1720cd86b9 100644 --- a/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java +++ b/talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java @@ -368,9 +368,9 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { frameToRenderQueue.poll(); // Re-allocate / allocate the frame. yuvFrameToRender = new I420Frame(videoWidth, videoHeight, rotationDegree, - strides, null); + strides, null, 0); textureFrameToRender = new I420Frame(videoWidth, videoHeight, rotationDegree, - null, -1); + null, -1, 0); updateTextureProperties = true; Log.d(TAG, " YuvImageRenderer.setSize done."); } @@ -380,6 +380,7 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { public synchronized void renderFrame(I420Frame frame) { if (surface == null) { // This object has been released. + VideoRenderer.renderFrameDone(frame); return; } if (!seenFrame && rendererEvents != null) { @@ -393,6 +394,7 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { // Skip rendering of this frame if setSize() was not called. if (yuvFrameToRender == null || textureFrameToRender == null) { framesDropped++; + VideoRenderer.renderFrameDone(frame); return; } // Check input frame parameters. @@ -402,6 +404,7 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { frame.yuvStrides[2] < frame.width / 2) { Log.e(TAG, "Incorrect strides " + frame.yuvStrides[0] + ", " + frame.yuvStrides[1] + ", " + frame.yuvStrides[2]); + VideoRenderer.renderFrameDone(frame); return; } // Check incoming frame dimensions. @@ -415,6 +418,7 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { if (frameToRenderQueue.size() > 0) { // Skip rendering of this frame if previous frame was not rendered yet. framesDropped++; + VideoRenderer.renderFrameDone(frame); return; } @@ -431,6 +435,7 @@ public class VideoRendererGui implements GLSurfaceView.Renderer { } copyTimeNs += (System.nanoTime() - now); seenFrame = true; + VideoRenderer.renderFrameDone(frame); // Request rendering. surface.requestRender(); diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc index 503a75ee88..517d543da5 100644 --- a/talk/app/webrtc/java/jni/peerconnection_jni.cc +++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc @@ -763,10 +763,10 @@ class JavaVideoRendererWrapper : public VideoRendererInterface { j_frame_class_(jni, FindClass(jni, "org/webrtc/VideoRenderer$I420Frame")), j_i420_frame_ctor_id_(GetMethodID( - jni, *j_frame_class_, "", "(III[I[Ljava/nio/ByteBuffer;)V")), + jni, *j_frame_class_, "", "(III[I[Ljava/nio/ByteBuffer;J)V")), j_texture_frame_ctor_id_(GetMethodID( jni, *j_frame_class_, "", - "(IIILjava/lang/Object;I)V")), + "(IIILjava/lang/Object;IJ)V")), j_byte_buffer_class_(jni, FindClass(jni, "java/nio/ByteBuffer")) { CHECK_EXCEPTION(jni); } @@ -775,6 +775,9 @@ class JavaVideoRendererWrapper : public VideoRendererInterface { void RenderFrame(const cricket::VideoFrame* video_frame) override { ScopedLocalRefFrame local_ref_frame(jni()); + // Make a shallow copy. |j_callbacks_| is responsible for releasing the + // copy by calling VideoRenderer.renderFrameDone(). + video_frame = video_frame->Copy(); jobject j_frame = (video_frame->GetNativeHandle() != nullptr) ? CricketToJavaTextureFrame(video_frame) : CricketToJavaI420Frame(video_frame); @@ -810,7 +813,7 @@ class JavaVideoRendererWrapper : public VideoRendererInterface { *j_frame_class_, j_i420_frame_ctor_id_, frame->GetWidth(), frame->GetHeight(), static_cast(frame->GetVideoRotation()), - strides, planes); + strides, planes, frame); } // Return a VideoRenderer.I420Frame referring texture object in |frame|. @@ -823,7 +826,7 @@ class JavaVideoRendererWrapper : public VideoRendererInterface { *j_frame_class_, j_texture_frame_ctor_id_, frame->GetWidth(), frame->GetHeight(), static_cast(frame->GetVideoRotation()), - texture_object, texture_id); + texture_object, texture_id, frame); } JNIEnv* jni() { @@ -944,6 +947,11 @@ JOW(void, VideoRenderer_freeWrappedVideoRenderer)(JNIEnv*, jclass, jlong j_p) { delete reinterpret_cast(j_p); } +JOW(void, VideoRenderer_releaseNativeFrame)( + JNIEnv* jni, jclass, jlong j_frame_ptr) { + delete reinterpret_cast(j_frame_ptr); +} + JOW(void, MediaStreamTrack_free)(JNIEnv*, jclass, jlong j_p) { CHECK_RELEASE(reinterpret_cast(j_p)); } diff --git a/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java b/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java index b496f3ebcc..a7b88ba939 100644 --- a/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java +++ b/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java @@ -42,10 +42,12 @@ public class VideoRenderer { public final int width; public final int height; public final int[] yuvStrides; - public final ByteBuffer[] yuvPlanes; + public ByteBuffer[] yuvPlanes; public final boolean yuvFrame; public Object textureObject; public int textureId; + // If |nativeFramePointer| is non-zero, the memory is allocated on the C++ side. + private long nativeFramePointer; // rotationDegree is the degree that the frame must be rotated clockwisely // to be rendered correctly. @@ -58,7 +60,7 @@ public class VideoRenderer { */ public I420Frame( int width, int height, int rotationDegree, - int[] yuvStrides, ByteBuffer[] yuvPlanes) { + int[] yuvStrides, ByteBuffer[] yuvPlanes, long nativeFramePointer) { this.width = width; this.height = height; this.yuvStrides = yuvStrides; @@ -71,6 +73,7 @@ public class VideoRenderer { this.yuvPlanes = yuvPlanes; this.yuvFrame = true; this.rotationDegree = rotationDegree; + this.nativeFramePointer = nativeFramePointer; if (rotationDegree % 90 != 0) { throw new IllegalArgumentException("Rotation degree not multiple of 90: " + rotationDegree); } @@ -81,7 +84,7 @@ public class VideoRenderer { */ public I420Frame( int width, int height, int rotationDegree, - Object textureObject, int textureId) { + Object textureObject, int textureId, long nativeFramePointer) { this.width = width; this.height = height; this.yuvStrides = null; @@ -90,6 +93,7 @@ public class VideoRenderer { this.textureId = textureId; this.yuvFrame = false; this.rotationDegree = rotationDegree; + this.nativeFramePointer = nativeFramePointer; if (rotationDegree % 90 != 0) { throw new IllegalArgumentException("Rotation degree not multiple of 90: " + rotationDegree); } @@ -109,6 +113,13 @@ public class VideoRenderer { * error and will likely crash. */ public I420Frame copyFrom(I420Frame source) { + // |nativeFramePointer| is not copied from |source|, because resources in this object are + // still allocated in Java. After copyFrom() is done, this object should not hold any + // references to |source|. This is violated for texture frames however, because |textureId| + // is copied without making a deep copy. + if (this.nativeFramePointer != 0) { + throw new RuntimeException("Trying to overwrite a frame allocated in C++"); + } if (source.yuvFrame && yuvFrame) { if (width != source.width || height != source.height) { throw new RuntimeException("Mismatched dimensions! Source: " + @@ -170,10 +181,25 @@ public class VideoRenderer { /** The real meat of VideoRendererInterface. */ public static interface Callbacks { // |frame| might have pending rotation and implementation of Callbacks - // should handle that by applying rotation during rendering. + // should handle that by applying rotation during rendering. The callee + // is responsible for signaling when it is done with |frame| by calling + // renderFrameDone(frame). public void renderFrame(I420Frame frame); } + /** + * This must be called after every renderFrame() to release the frame. + */ + public static void renderFrameDone(I420Frame frame) { + frame.yuvPlanes = null; + frame.textureObject = null; + frame.textureId = 0; + if (frame.nativeFramePointer != 0) { + releaseNativeFrame(frame.nativeFramePointer); + frame.nativeFramePointer = 0; + } + } + // |this| either wraps a native (GUI) renderer or a client-supplied Callbacks // (Java) implementation; this is indicated by |isWrappedVideoRenderer|. long nativeVideoRenderer; @@ -215,4 +241,6 @@ public class VideoRenderer { private static native void freeGuiVideoRenderer(long nativeVideoRenderer); private static native void freeWrappedVideoRenderer(long nativeVideoRenderer); + + private static native void releaseNativeFrame(long nativeFramePointer); } diff --git a/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java b/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java index 5133723ea6..870b2e5827 100644 --- a/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java +++ b/talk/app/webrtc/java/testcommon/src/org/webrtc/PeerConnectionTest.java @@ -136,6 +136,7 @@ public class PeerConnectionTest { public synchronized void renderFrame(VideoRenderer.I420Frame frame) { setSize(frame.rotatedWidth(), frame.rotatedHeight()); --expectedFramesDelivered; + VideoRenderer.renderFrameDone(frame); } public synchronized void expectSignalingChange(SignalingState newState) { diff --git a/webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java b/webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java index d7df8d7f1f..4e433a19a7 100644 --- a/webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java +++ b/webrtc/examples/androidtests/src/org/appspot/apprtc/test/PeerConnectionClientTest.java @@ -94,6 +94,7 @@ public class PeerConnectionClientTest extends InstrumentationTestCase } } renderFrameCalled = true; + VideoRenderer.renderFrameDone(frame); doneRendering.countDown(); }