Android video rendering: Apply SurfaceTexture.getTransformationMatrix()

talk/app/webrtc/java/android/org/webrtc/VideoRendererGui.java

 /*
  * libjingle
  * Copyright 2014 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 23 matching lines @@
 import javax.microedition.khronos.opengles.GL10;
 
 import android.annotation.SuppressLint;
 import android.graphics.Point;
 import android.graphics.Rect;
 import android.graphics.SurfaceTexture;
 import android.opengl.EGL14;
 import android.opengl.EGLContext;
 import android.opengl.GLES20;
 import android.opengl.GLSurfaceView;
+import android.opengl.Matrix;
 import android.util.Log;
 
 import org.webrtc.VideoRenderer.I420Frame;
 
 /**
  * Efficiently renders YUV frames using the GPU for CSC.
  * Clients will want first to call setView() to pass GLSurfaceView
  * and then for each video stream either create instance of VideoRenderer using
  * createGui() call or VideoRenderer.Callbacks interface using create() call.
  * Only one instance of the class can be created.
@@ skipping 72 matching lines @@
     // Time in ns spent in draw() function.
     private long drawTimeNs;
     // Time in ns spent in draw() copying resources from |pendingFrame| - including uploading frame
     // data to rendering planes.
     private long copyTimeNs;
     // The allowed view area in percentage of screen size.
     private final Rect layoutInPercentage;
     // The actual view area in pixels. It is a centered subrectangle of the rectangle defined by
     // |layoutInPercentage|.
     private final Rect displayLayout = new Rect();
-    // Cached texture transformation matrix, calculated from current layout parameters.
-    private final float[] texMatrix = new float[16];
-    // Flag if texture vertices or coordinates update is needed.
-    private boolean updateTextureProperties;
+    // Cached layout transformation matrix, calculated from current layout parameters.
+    private float[] layoutMatrix = new float[16];

[hbos, 2015/09/08 14:08:47]

This probably shouldn't be used at all until it has been properly set (and could
be null). But if it does have a default value, wouldn't the identity matrix make
more sense (setIdentity in constr)?

[magjed_webrtc, 2015/09/08 15:24:28]

True, I will default to null instead.

[hbos, 2015/09/09 08:27:54]

Acknowledged.

+    // Flag if layout transformation matrix update is needed.
+    private boolean updateLayoutProperties;
     // Texture properties update lock.
-    private final Object updateTextureLock = new Object();
+    private final Object updateLayoutLock = new Object();

[hbos, 2015/09/08 14:08:47]

What variables are guarded by this lock? Can you list them in the comment?

[magjed_webrtc, 2015/09/08 15:24:28]

Done.

+    // Texture sampling matrix.
+    private float[] samplingMatrix;
     // Viewport dimensions.
     private int screenWidth;
     private int screenHeight;
     // Video dimension.
     private int videoWidth;
     private int videoHeight;
 
     // This is the degree that the frame should be rotated clockwisely to have
     // it rendered up right.
     private int rotationDegree;
 
     private YuvImageRenderer(
         GLSurfaceView surface, int id,
         int x, int y, int width, int height,
         RendererCommon.ScalingType scalingType, boolean mirror) {
       Log.d(TAG, "YuvImageRenderer.Create id: " + id);
       this.surface = surface;
       this.id = id;
       this.scalingType = scalingType;
       this.mirror = mirror;
       layoutInPercentage = new Rect(x, y, Math.min(100, x + width), Math.min(100, y + height));
-      updateTextureProperties = false;
+      updateLayoutProperties = false;
       rotationDegree = 0;
     }
 
     private synchronized void release() {
       surface = null;
       synchronized (pendingFrameLock) {
         if (pendingFrame != null) {
           VideoRenderer.renderFrameDone(pendingFrame);
           pendingFrame = null;
         }
       }
     }
 
     private void createTextures() {
       Log.d(TAG, "  YuvImageRenderer.createTextures " + id + " on GL thread:" +
           Thread.currentThread().getId());
 
       // Generate 3 texture ids for Y/U/V and place them into |yuvTextures|.
       for (int i = 0; i < 3; i++)  {
         yuvTextures[i] = GlUtil.generateTexture(GLES20.GL_TEXTURE_2D);
       }
     }
 
     private void checkAdjustTextureCoords() {

[hbos, 2015/09/08 14:08:47]

Rename to checkAdjustLayoutMatrix?

[magjed_webrtc, 2015/09/08 15:24:28]

Renamed to getLayoutMatrix() and changed it a bit.

-      synchronized(updateTextureLock) {
-        if (!updateTextureProperties) {
+      synchronized(updateLayoutLock) {
+        if (!updateLayoutProperties) {
           return;
         }
         // Initialize to maximum allowed area. Round to integer coordinates inwards the layout
         // bounding box (ceil left/top and floor right/bottom) to not break constraints.
         displayLayout.set(
             (screenWidth * layoutInPercentage.left + 99) / 100,
             (screenHeight * layoutInPercentage.top + 99) / 100,
             (screenWidth * layoutInPercentage.right) / 100,
             (screenHeight * layoutInPercentage.bottom) / 100);
         Log.d(TAG, "ID: "  + id + ". AdjustTextureCoords. Allowed display size: "
             + displayLayout.width() + " x " + displayLayout.height() + ". Video: " + videoWidth
             + " x " + videoHeight + ". Rotation: " + rotationDegree + ". Mirror: " + mirror);
         final float videoAspectRatio = (rotationDegree % 180 == 0)
             ? (float) videoWidth / videoHeight
             : (float) videoHeight / videoWidth;
         // Adjust display size based on |scalingType|.
         final Point displaySize = RendererCommon.getDisplaySize(scalingType,
             videoAspectRatio, displayLayout.width(), displayLayout.height());
         displayLayout.inset((displayLayout.width() - displaySize.x) / 2,
                             (displayLayout.height() - displaySize.y) / 2);
         Log.d(TAG, "  Adjusted display size: " + displayLayout.width() + " x "
             + displayLayout.height());
-        RendererCommon.getTextureMatrix(texMatrix, rotationDegree, mirror, videoAspectRatio,
-            (float) displayLayout.width() / displayLayout.height());
-        updateTextureProperties = false;
+        layoutMatrix = RendererCommon.getLayoutMatrix(
+            mirror, videoAspectRatio, (float) displayLayout.width() / displayLayout.height());

[hbos, 2015/09/08 14:08:47]

What are the implications of mirroring? I'm thinking about trying to rotate or
some other operation before or after mirroring and how that might reverse the
direction.
But the layout matrix is both mirrored and rotated in the correct order here and
then not modified?

What if we want to mirror afterwards? Or do we simply not do that?
Thinking about how we might mirror in order to avoid the back-facing camera's
default mirroring. Is this different if textures are used or not? If we want to
do the final decision about whether or not we want to mirror later does the
matrix operation order bite us in the ass?

[magjed_webrtc, 2015/09/08 15:24:28]

We need to take care when we multiply matrices together, i.e. A * B is not
always the same as B * A, but we can still do whatever we want. The final
decision about mirroring is done in the Matrix.multiplyMM() call a few lines
below.

[hbos, 2015/09/09 08:27:54]

Acknowledged.

+        updateLayoutProperties = false;
         Log.d(TAG, "  AdjustTextureCoords done");
       }
     }
 
     private void draw(GlRectDrawer drawer) {
       if (!seenFrame) {
         // No frame received yet - nothing to render.
         return;
       }
       long now = System.nanoTime();
@@ skipping 29 matching lines @@
                   (SurfaceTexture) pendingFrame.textureObject;
               surfaceTexture.updateTexImage();
             }
           }
           copyTimeNs += (System.nanoTime() - now);
           VideoRenderer.renderFrameDone(pendingFrame);
           pendingFrame = null;
         }
       }
 
+      final float[] texMatrix = new float[16];
+      Matrix.multiplyMM(texMatrix, 0, samplingMatrix, 0, layoutMatrix, 0);

[hbos, 2015/09/08 14:08:47]

Is reading from samplingMatrix thread-safe without synchronized on this?

Is reading from layoutMatrix thread-safe without synchronized on
updateLayoutLock?

[magjed_webrtc, 2015/09/08 15:24:28]

Thanks, this is a bug. I moved all usage of |samplingMatrix| into this function.
I also should not call getTransformationMatrix() before updateTexImage().

[hbos, 2015/09/09 08:27:54]

You're still updating and returning a reference to layoutMatrix. We're safe
since layoutMatrix is only updated in draw() but I think the name
"getLayoutMatrix" is misleading since it is not a normal getter and the return
value is not thread-safe. What influences if the matrix is updated or not is
guarded by a lock, but the actual matrix being updated is otherwise not guarded.

In my opinion it is more clear that we are updating and dealing with a member
variable if we use the member variable directly and make the updater a "void
updateLayoutMatrix()" or similar.

       if (rendererType == RendererType.RENDERER_YUV) {
         drawer.drawYuv(yuvTextures, texMatrix);
       } else {
         drawer.drawOes(oesTexture, texMatrix);
       }
 
       if (isNewFrame) {
         framesRendered++;
         drawTimeNs += (System.nanoTime() - now);
         if ((framesRendered % 300) == 0) {
@@ skipping 10 matching lines @@
       if (framesReceived > 0 && framesRendered > 0) {
         Log.d(TAG, "Duration: " + (int)(timeSinceFirstFrameNs / 1e6) +
             " ms. FPS: " + (float)framesRendered * 1e9 / timeSinceFirstFrameNs);
         Log.d(TAG, "Draw time: " +
             (int) (drawTimeNs / (1000 * framesRendered)) + " us. Copy time: " +
             (int) (copyTimeNs / (1000 * framesReceived)) + " us");
       }
     }
 
     public void setScreenSize(final int screenWidth, final int screenHeight) {
-      synchronized(updateTextureLock) {
+      synchronized(updateLayoutLock) {
         if (screenWidth == this.screenWidth && screenHeight == this.screenHeight) {
           return;
         }
         Log.d(TAG, "ID: " + id + ". YuvImageRenderer.setScreenSize: " +
             screenWidth + " x " + screenHeight);
         this.screenWidth = screenWidth;
         this.screenHeight = screenHeight;
-        updateTextureProperties = true;
+        updateLayoutProperties = true;
       }
     }
 
     public void setPosition(int x, int y, int width, int height,
         RendererCommon.ScalingType scalingType, boolean mirror) {
       final Rect layoutInPercentage =
           new Rect(x, y, Math.min(100, x + width), Math.min(100, y + height));
-      synchronized(updateTextureLock) {
+      synchronized(updateLayoutLock) {
         if (layoutInPercentage.equals(this.layoutInPercentage) && scalingType == this.scalingType
             && mirror == this.mirror) {
           return;
         }
         Log.d(TAG, "ID: " + id + ". YuvImageRenderer.setPosition: (" + x + ", " + y +
             ") " +  width + " x " + height + ". Scaling: " + scalingType +
             ". Mirror: " + mirror);
         this.layoutInPercentage.set(layoutInPercentage);
         this.scalingType = scalingType;
         this.mirror = mirror;
-        updateTextureProperties = true;
+        updateLayoutProperties = true;
       }
     }
 
     private void setSize(final int videoWidth, final int videoHeight, final int rotation) {
       if (videoWidth == this.videoWidth && videoHeight == this.videoHeight
           && rotation == rotationDegree) {
         return;
       }
       if (rendererEvents != null) {
         Log.d(TAG, "ID: " + id +
             ". Reporting frame resolution changed to " + videoWidth + " x " + videoHeight);
         rendererEvents.onFrameResolutionChanged(videoWidth, videoHeight, rotation);
       }
 
-      synchronized (updateTextureLock) {
+      synchronized (updateLayoutLock) {
         Log.d(TAG, "ID: " + id + ". YuvImageRenderer.setSize: " +
             videoWidth + " x " + videoHeight + " rotation " + rotation);
 
         this.videoWidth = videoWidth;
         this.videoHeight = videoHeight;
         rotationDegree = rotation;
-        updateTextureProperties = true;
+        updateLayoutProperties = true;
         Log.d(TAG, "  YuvImageRenderer.setSize done.");
       }
     }
 
     @Override
     public synchronized void renderFrame(I420Frame frame) {
       if (surface == null) {
         // This object has been released.
         VideoRenderer.renderFrameDone(frame);
         return;
@@ skipping 17 matching lines @@
         }
 
         if (pendingFrame != null) {
           // Skip rendering of this frame if previous frame was not rendered yet.
           framesDropped++;
           VideoRenderer.renderFrameDone(frame);
           return;
         }
         pendingFrame = frame;
       }
+      samplingMatrix = RendererCommon.getSamplingMatrix(
+          (SurfaceTexture) frame.textureObject, frame.rotationDegree);
       setSize(frame.width, frame.height, frame.rotationDegree);
       seenFrame = true;
 
       // Request rendering.
       surface.requestRender();
     }
   }
 
   /** Passes GLSurfaceView to video renderer. */
   public static synchronized void setView(GLSurfaceView surface,
@@ skipping 190 matching lines @@
     GLES20.glViewport(0, 0, screenWidth, screenHeight);
     GLES20.glClear(GLES20.GL_COLOR_BUFFER_BIT);
     synchronized (yuvImageRenderers) {
       for (YuvImageRenderer yuvImageRenderer : yuvImageRenderers) {
         yuvImageRenderer.draw(drawer);
       }
     }
   }
 
 }