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}

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<I420Frame> pendingFrames = new ArrayList<I420Frame>();
+
+    @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<I420Frame> WaitForFrames() {
+      synchronized (pendingFrames) {
+        while (pendingFrames.isEmpty()) {
+          try {
+            pendingFrames.wait();
+          } catch (InterruptedException e) {
+            // Ignore.
+          }
+        }
+        final List<I420Frame> frames = new ArrayList<I420Frame>(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<I420Frame> 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(), "[]");
+  }
 }

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() {

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");
   }

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();

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_, "<init>", "(III[I[Ljava/nio/ByteBuffer;)V")),
+            jni, *j_frame_class_, "<init>", "(III[I[Ljava/nio/ByteBuffer;J)V")),
         j_texture_frame_ctor_id_(GetMethodID(
             jni, *j_frame_class_, "<init>",
-            "(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<int>(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<int>(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<JavaVideoRendererWrapper*>(j_p);
 }
 
+JOW(void, VideoRenderer_releaseNativeFrame)(
+    JNIEnv* jni, jclass, jlong j_frame_ptr) {
+  delete reinterpret_cast<const cricket::VideoFrame*>(j_frame_ptr);
+}
+
 JOW(void, MediaStreamTrack_free)(JNIEnv*, jclass, jlong j_p) {
   CHECK_RELEASE(reinterpret_cast<MediaStreamTrackInterface*>(j_p));
 }

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);
 }

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) {

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();
     }