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}
diff --git a/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java b/talk/app/webrtc/androidtests/src/org/webrtc/VideoCapturerAndroidTest.java
index 54f77c5..162fad1 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 @@
         ++framesRendered;
         frameLock.notify();
       }
+      VideoRenderer.renderFrameDone(frame);
     }
 
     public int WaitForNextFrameToRender() throws InterruptedException {
@@ -58,6 +59,34 @@
     }
   }
 
+  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 @@
       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(), "[]");
+  }
 }
diff --git a/talk/app/webrtc/androidvideocapturer.cc b/talk/app/webrtc/androidvideocapturer.cc
index cc78217..747dd43 100644
--- a/talk/app/webrtc/androidvideocapturer.cc
+++ b/talk/app/webrtc/androidvideocapturer.cc
@@ -129,6 +129,8 @@
       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 00034fc..d4251f6 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 @@
     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 d4f754b..1720cd8 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 @@
         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 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 @@
         // 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 @@
               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 @@
         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 @@
       }
       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 503a75e..517d543 100644
--- a/talk/app/webrtc/java/jni/peerconnection_jni.cc
+++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc
@@ -763,10 +763,10 @@
         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 @@
 
   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 @@
         *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 @@
         *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 @@
   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));
 }
diff --git a/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java b/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java
index b496f3e..a7b88ba 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 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 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 @@
       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 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 @@
       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 @@
      * 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 @@
   /** 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 @@
 
   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 5133723..870b2e5 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 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 d7df8d7..4e433a1 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 @@
         }
       }
       renderFrameCalled = true;
+      VideoRenderer.renderFrameDone(frame);
       doneRendering.countDown();
     }