AppRTCDemo: Render each video in a separate SurfaceView

talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java

Index: talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java
diff --git a/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java b/talk/app/webrtc/java/src/org/webrtc/VideoRenderer.java
index 401640b463bfdfff7f832ebf75ce73100a0b40fb..f4e2eabc9b4d28d57a8134fcd46b4eb57d40a57e 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,11 +93,46 @@ 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);
       }
     }
 
+    /**
+     * Shallow copy constructor.
+     */
+    private I420Frame(I420Frame source) {

[AlexG, 2015/08/04 00:27:23]

This implementation will do frame memory allocation / freeing for every rendered
frame - right? We may have 30 huge memory allocs/deallocs every second. I am not
sure, but I am afraid it may cause heap fragmentation. Why not to use one time
allocation and then VideoRenderer.copyFrom() instead of multiple allocations?
Plus it will keep I420Frame interface easier with less amount of constructors /
extra param.

[magjed_webrtc, 2015/08/04 17:05:05]

It will not allocate or copy the pixel data, it will just allocate and assign
some pointers and header data, i.e. ~100 byte per frame including
cricket::WebRtcVideoFrame in C++. Anyway, I removed all this, see next comment.

+      width = source.width;
+      height = source.height;
+      yuvStrides = source.yuvStrides;
+      yuvPlanes = source.yuvPlanes;
+      yuvFrame = source.yuvFrame;
+      textureObject = source.textureObject;
+      textureId = source.textureId;
+      rotationDegree = source.rotationDegree;
+      // If source.nativeFramePointer == 0, then the memory is managed by Java's garbage collector.
+      // Otherwise, we need to ask C++ to make a shallow copy.
+      nativeFramePointer = (source.nativeFramePointer == 0)
+          ? 0
+          : shallowCopyNativeFrame(source.nativeFramePointer);
+    }
+
+    // Every shallow copy should be released to make sure we don't leak memory in C++.
+    public I420Frame shallowCopy() {
+      return new I420Frame(this);
+    }
+
+    public void release() {

[AlexG, 2015/08/04 00:27:23]

I would add a flag here to avoid accidental case if I420Frame comes from C++ (in
RenderFrame()), shallow copy is not created, but I420Frame.release() is called
to avoid accidental freeing of C++ memory from Java. Overall interface looks not
very clear for me though - for some I420 frames we should call release(), for
another - not.

[magjed_webrtc, 2015/08/04 17:05:05]

Ok, how about this interface instead; for every call to renderFrame(), the
callee is responsible for calling static VideoRenderer.renderFrameDone(frame) to
signal that rendering is done. This return call is allowed to take place after
renderFrame() has returned, making async rendering possible. I have updated the
CL to this interface instead. What do you think?

[AlexG, 2015/08/05 00:47:11]

If you are making a copy of only pointers in VideoFrame, but not makig a copy of
actual yuv data then after VideoRendererInterface::RenderFrame() is called by
say camera  you return right away and just copy pointers and sending the request
to render thread to do the rendering. How do you ensure than that camera will
not modify yuv data of this frame if the frame is returned back to camera pool?

The reason why VideoRenderrGui.java made a copy of whole frame including yuv
data was because C++ VideoRendererInterface::RenderFrame() API assumes sync
rendering - i.e. after the call is done it is assumed that frame is rendered and
its yuv data may be updated with the next image. 

[magjed_webrtc, 2015/08/07 17:14:50]

It is possible to make async rendering with
VideoRendererInterface::RenderFrame(const cricket::VideoFrame* frame). You call
cricket::VideoFrame::Copy(). The documentation say:
// Make a shallow copy of the frame. The frame buffer itself is not copied.
// Both the current and new VideoFrame will share a single reference-counted
// frame buffer.
virtual VideoFrame *Copy() const = 0;
The frame won't be reused until the last reference is gone. In this particular
case, the cricket::VideoFrame implementation is cricket::WebRtcVideoFrame and
the Copy() function will create an extra reference to the
rtc::scoped_refptr<WrappedI420Buffer> that is created here:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjin...
When the last reference is gone, the frame is returned back to the camera pool.

+      yuvPlanes = null;
+      textureObject = null;
+      textureId = 0;
+      if (nativeFramePointer != 0) {
+        releaseNativeFrame(nativeFramePointer);
+        nativeFramePointer = 0;
+      }
+    }
+
     public int rotatedWidth() {
       return (rotationDegree % 180 == 0) ? width : height;
     }
@@ -212,4 +250,7 @@ public class VideoRenderer {
 
   private static native void freeGuiVideoRenderer(long nativeVideoRenderer);
   private static native void freeWrappedVideoRenderer(long nativeVideoRenderer);
+
+  private static native long shallowCopyNativeFrame(long nativeFramePointer);
+  private static native void releaseNativeFrame(long nativeFramePointer);
 }