AppRTCDemo: Render each video in a separate SurfaceView

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

 /*
  * libjingle
  * Copyright 2013 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 24 matching lines @@
  * class also provides a createGui() method for creating a GUI-rendering window
  * on various platforms.
  */
 public class VideoRenderer {
 
   /** Java version of cricket::VideoFrame. */
   public static class I420Frame {
     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.
     public int rotationDegree;
 
     /**
      * Construct a frame of the given dimensions with the specified planar
      * data.  If |yuvPlanes| is null, new planes of the appropriate sizes are
      * allocated.
      */
     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;
       if (yuvPlanes == null) {
         yuvPlanes = new ByteBuffer[3];
         yuvPlanes[0] = ByteBuffer.allocateDirect(yuvStrides[0] * height);
         yuvPlanes[1] = ByteBuffer.allocateDirect(yuvStrides[1] * height / 2);
         yuvPlanes[2] = ByteBuffer.allocateDirect(yuvStrides[2] * height / 2);
       }
       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);
       }
     }
 
     /**
      * Construct a texture frame of the given dimensions with data in SurfaceTexture
      */
     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;
       this.yuvPlanes = null;
       this.textureObject = textureObject;
       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;
     }
 
     public int rotatedHeight() {
       return (rotationDegree % 180 == 0) ? height : width;
     }
 
     /**
      * Copy the planes out of |source| into |this| and return |this|.  Calling
@@ skipping 97 matching lines @@
     } else {
       freeWrappedVideoRenderer(nativeVideoRenderer);
     }
   }
 
   private static native long nativeCreateGuiVideoRenderer(int x, int y);
   private static native long nativeWrapVideoRenderer(Callbacks callbacks);
 
   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);
 }