Fix video snapshots rotation

MIGRATION.md

@@ -135,11 +135,8 @@ Might be used in the future to speed up development.
   If device has no `BACK` cameras, defaults to `FRONT`.  
   
 
-TODO: think bout getPreviewSize() being removed, and think about adding a setPreviewSize().
+TODO: do we want getPreviewSize() / setPreviewSize() ? probably not the getter.
-      If adding getPreviewSize() back, update this doc.
-TODO: fix video recording rotation: with front camera, it does not work.
 TODO: opencollective
-
 TODO: document cameraGridColor
 TODO: document setVideoBitRate
 TODO: document setAudioBitRate

cameraview/src/androidTest/java/com/otaliastudios/cameraview/MockCameraController.java

@@ -100,7 +100,7 @@ public class MockCameraController extends CameraController {
     }
 
     @Override
-    void takeVideoSnapshot(@NonNull File file) {
+    void takeVideoSnapshot(@NonNull File file, @NonNull AspectRatio viewAspectRatio) {
 
     }
 

cameraview/src/main/gles/com/otaliastudios/cameraview/EglBaseSurface.java

@@ -40,6 +40,7 @@ import java.nio.ByteOrder;
 @RequiresApi(api = Build.VERSION_CODES.JELLY_BEAN_MR2)
 class EglBaseSurface extends EglElement {
     protected static final String TAG = EglBaseSurface.class.getSimpleName();
+    private final static CameraLogger LOG = CameraLogger.create(TAG);
 
     // EglCore object we're associated with.  It may be associated with multiple surfaces.
     protected EglCore mEglCore;
@@ -65,6 +66,10 @@ class EglBaseSurface extends EglElement {
 
         // Don't cache width/height here, because the size of the underlying surface can change
         // out from under us (see e.g. HardwareScalerActivity).
+        int tempWidth = mEglCore.querySurface(mEGLSurface, EGL14.EGL_WIDTH);
+        int tempHeight = mEglCore.querySurface(mEGLSurface, EGL14.EGL_HEIGHT);
+        // TODO remove these logs
+        LOG.e("ROTBUG", "eglSurfaceWidth:", tempWidth, "eglSurfaceHeight:", tempHeight);
         //mWidth = mEglCore.querySurface(mEGLSurface, EGL14.EGL_WIDTH);
         //mHeight = mEglCore.querySurface(mEGLSurface, EGL14.EGL_HEIGHT);
     }

cameraview/src/main/gles/com/otaliastudios/cameraview/MediaEncoderEngine.java

@@ -19,6 +19,9 @@ import java.util.List;
 @RequiresApi(api = Build.VERSION_CODES.JELLY_BEAN_MR2)
 class MediaEncoderEngine {
 
+    private final static String TAG = MediaEncoder.class.getSimpleName();
+    private final static CameraLogger LOG = CameraLogger.create(TAG);
+
     @SuppressWarnings("WeakerAccess")
     final static int STOP_BY_USER = 0;
     final static int STOP_BY_MAX_DURATION = 1;
@@ -76,7 +79,7 @@ class MediaEncoderEngine {
                     mPossibleStopReason = STOP_BY_MAX_DURATION;
                     finalMaxDuration = maxDuration;
                 }
-                Log.e("MediaEncoderEngine", "Computed a max duration of " + (finalMaxDuration / 1000F));
+                LOG.i("Computed a max duration of", (finalMaxDuration / 1000F));
                 for (MediaEncoder encoder : mEncoders) {
                     encoder.prepare(mController, finalMaxDuration);
                 }
@@ -113,7 +116,6 @@ class MediaEncoderEngine {
             if (!mMediaMuxerStarted) {
                 throw new IllegalStateException("Trying to write before muxer started");
             }
-            Log.e("MediaEncoderEngine", "Writing data." + track);
             mMediaMuxer.writeSampleData(track, encodedData, info);
         }
 

cameraview/src/main/gles/com/otaliastudios/cameraview/TextureMediaEncoder.java

@@ -19,6 +19,9 @@ import java.io.IOException;
 @RequiresApi(api = Build.VERSION_CODES.JELLY_BEAN_MR2)
 class TextureMediaEncoder extends VideoMediaEncoder<TextureMediaEncoder.Config> {
 
+    private static final String TAG = TextureMediaEncoder.class.getSimpleName();
+    private static final CameraLogger LOG = CameraLogger.create(TAG);
+
     final static String FRAME_EVENT = "frame";
 
     static class Frame {
@@ -31,10 +34,14 @@ class TextureMediaEncoder extends VideoMediaEncoder<TextureMediaEncoder.Config>
         float scaleY;
         boolean scaleFlipped;
         EGLContext eglContext;
+        int transformRotation;
 
         Config(int width, int height, int bitRate, int frameRate, int rotation, String mimeType,
                int textureId, float scaleX, float scaleY, boolean scaleFlipped, EGLContext eglContext) {
-            super(width, height, bitRate, frameRate, rotation, mimeType);
+            // We rotate the texture using transformRotation. Pass rotation=0 to super so that
+            // no rotation metadata is written into the output file.
+            super(width, height, bitRate, frameRate, 0, mimeType);
+            this.transformRotation = rotation;
             this.textureId = textureId;
             this.scaleX = scaleX;
             this.scaleY = scaleY;
@@ -124,7 +131,7 @@ class TextureMediaEncoder extends VideoMediaEncoder<TextureMediaEncoder.Config>
             // translate to origin, rotate, then back to where we were.
 
             Matrix.translateM(transform, 0, 0.5F, 0.5F, 0);
-            Matrix.rotateM(transform, 0, mConfig.rotation /* VIEW-OUTPUT */, 0, 0, 1);
+            Matrix.rotateM(transform, 0, mConfig.transformRotation, 0, 0, 1);
             Matrix.translateM(transform, 0, -0.5F, -0.5F, 0);
 
             drain(false);

cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java

@@ -567,6 +567,15 @@ class Camera1 extends CameraController implements Camera.PreviewCallback, Camera
                 result.size = getPreviewSize(REF_OUTPUT); // Not the real size: it will be cropped to match the view ratio
                 result.rotation = offset(REF_SENSOR, REF_OUTPUT); // Actually it will be rotated and set to 0.
                 AspectRatio outputRatio = flip(REF_OUTPUT, REF_VIEW) ? viewAspectRatio.inverse() : viewAspectRatio;
+                // LOG.e("ROTBUG_pic", "aspectRatio (REF_VIEW):", viewAspectRatio);
+                // LOG.e("ROTBUG_pic", "aspectRatio (REF_OUTPUT):", outputRatio);
+                // LOG.e("ROTBUG_pic", "sizeUncropped (REF_OUTPUT):", result.size);
+                // LOG.e("ROTBUG_pic", "rotation:", result.rotation);
+
+                LOG.v("Rotations", "SV", offset(REF_SENSOR, REF_VIEW), "VS", offset(REF_VIEW, REF_SENSOR));
+                LOG.v("Rotations", "SO", offset(REF_SENSOR, REF_OUTPUT), "OS", offset(REF_OUTPUT, REF_SENSOR));
+                LOG.v("Rotations", "VO", offset(REF_VIEW, REF_OUTPUT), "OV", offset(REF_OUTPUT, REF_VIEW));
+
                 mPictureRecorder = new SnapshotPictureRecorder(result, Camera1.this, mCamera, outputRatio);
                 mPictureRecorder.take();
             }
@@ -660,7 +669,7 @@ class Camera1 extends CameraController implements Camera.PreviewCallback, Camera
 
     @SuppressLint("NewApi")
     @Override
-    void takeVideoSnapshot(@NonNull final File file) {
+    void takeVideoSnapshot(@NonNull final File file, @NonNull final AspectRatio viewAspectRatio) {
         if (!(mPreview instanceof GlCameraPreview)) {
             throw new IllegalStateException("Video snapshots are only supported with GlCameraPreview.");
         }
@@ -680,44 +689,59 @@ class Camera1 extends CameraController implements Camera.PreviewCallback, Camera
                 videoResult.location = mLocation;
                 videoResult.videoBitRate = mVideoBitRate;
                 videoResult.audioBitRate = mAudioBitRate;
-
-                // What matters as size here is the preview size, which is passed to the GlCameraPreview
-                // surface texture, which is then passed to the encoder. The view size has no influence.
-
-                // CROPPING: We must make sure that the encoder applies our special transformation
-                // to the data so that it is cropped. In this case, we must CROP the preview size based
-                // on the view bounds, like below:
-                Size preview = getPreviewSize(REF_VIEW); // The preview stream size in REF_VIEW
-                Size view = mPreview.getOutputSurfaceSize(); // The view size in REF_VIEW
-                Rect crop = CropHelper.computeCrop(preview, AspectRatio.of(view.getWidth(), view.getHeight()));
-                Size finalSize = new Size(crop.width(), crop.height()); // The visible size in REF_VIEW
-                // ^ We could flip to REF_OUTPUT here and this is what we are doing, but since the video codec recorder
-                // does not account rotation, we must stay in REF_VIEW for this to work well.
-
-                // Without cropping (missing ATM - Edit: DONE), the actual size is the preview size with no crops.
-                // Passing a cropped size while the cropping is not implemented at the encoder surface level,
-                // would cause distortions and crashes in the video encoder.
-                // Size finalSize2 = getPreviewSize(REF_VIEW);
-
-                // With phone landscape on the left:
-                // offset(REF_SENSOR, REF_VIEW) -> 270 (correct)
-                // offset(REF_VIEW, REF_OUTPUT) -> 270 (correct)
-                // offset(REF_SENSOR, REF_OUTPUT) -> 180 (not correct)
-
-                // With straight phone:
-                // offset(REF_SENSOR, REF_VIEW) -> 270 (not correct)
-                // offset(REF_VIEW, REF_OUTPUT) -> 0 (correct)
-                // offset(REF_SENSOR, REF_OUTPUT) -> 270 (not correct)
-
-                // So it looks like REF_VIEW REF_OUTPUT is the correct one, meaning that
-                // the input data in this case is not in the REF_SENSOR coordinates but rather
-                // in the REF_VIEW ones.
-                videoResult.size = flip(REF_VIEW, REF_OUTPUT) ? finalSize.flip() : finalSize;
-                videoResult.rotation = offset(REF_VIEW, REF_OUTPUT);
                 videoResult.audio = mAudio;
                 videoResult.maxSize = mVideoMaxSize;
                 videoResult.maxDuration = mVideoMaxDuration;
 
+                // Size and rotation turned out to be extremely tricky. In case of SnapshotPictureRecorder
+                // we use the preview size in REF_OUTPUT (cropped) and offset(REF_SENSOR, REF_OUTPUT) as rotation.
+                // These values mean that we expect input to be in the REF_SENSOR system.
+
+                // Here everything seems different. We would expect a difference because the two snapshot
+                // recorders have different mechanics (the picture one uses a SurfaceTexture with setBufferSize,
+                // the video one here uses the MediaCodec input surface which we can't control).
+
+                // The strangest thing is the fact that the correct angle seems to be the same for FRONT and
+                // BACK sensor, which means that our sensor correction actually screws things up. For this reason
+                // facing value is temporarily set to BACK.
+                Facing realFacing = mFacing;
+                mFacing = Facing.BACK;
+
+                // These are the angles that make it work on a Nexus5X, compared to the offset() results.
+                // For instance, SV means offset(REF_SENSOR, REF_VIEW). The rest should be clear.
+                //    CONFIG   | WANTED |   SV   |   VS   |   VO   |   OV   |   SO   |   OS   |
+                // ------------|--------|--------|--------|--------|--------|--------|--------|
+                //   Vertical  |   0    |   270  |   90   |   0    |   0    |   270  |   90   |
+                //     Left    |   270  |   270  |   90   |  270   |   90   |   180  |   180  |
+                //    Right    |   90   |   270  |   90   |   90   |   270  |    0   |    0   |
+                // Upside down |   180  |   270  |   90   |  180   |   180  |   90   |   270  |
+
+                // The VO is the only correct value. Things change when using FRONT camera, in which case,
+                // no value is actually correct, and the needed values are the same of BACK!
+                //    CONFIG   | WANTED |   SV   |   VS   |   VO   |   OV   |   SO   |   OS   |
+                // ------------|--------|--------|--------|--------|--------|--------|--------|
+                //   Vertical  |   0    |   90   |   270  |  180   |   180  |   270  |   90   |
+                //     Left    |   270  |   90   |   270  |  270   |   90   |    0   |    0   |
+                //    Right    |   90   |   90   |   270  |   90   |   270  |   180  |   180  |
+                // Upside down |   180  |   90   |   270  |   0    |    0   |   90   |   270  |
+
+                // Based on this we will use VO for everything. See if we get issues about distortion
+                // and maybe we can improve. The reason why this happen is beyond my understanding.
+
+                Size outputSize = getPreviewSize(REF_OUTPUT);
+                AspectRatio outputRatio = flip(REF_OUTPUT, REF_VIEW) ? viewAspectRatio.inverse() : viewAspectRatio;
+                Rect outputCrop = CropHelper.computeCrop(outputSize, outputRatio);
+                outputSize = new Size(outputCrop.width(), outputCrop.height());
+                videoResult.size = outputSize;
+                videoResult.rotation = offset(REF_VIEW, REF_OUTPUT);
+                // LOG.e("ROTBUG_video", "aspectRatio (REF_VIEW):", viewAspectRatio);
+                // LOG.e("ROTBUG_video", "aspectRatio (REF_OUTPUT):", outputRatio);
+                // LOG.e("ROTBUG_video", "sizeUncropped (REF_OUTPUT):", outputSize);
+                // LOG.e("ROTBUG_video", "sizeCropped (REF_OUTPUT):", videoResult.size);
+                // LOG.e("ROTBUG_video", "rotation:", videoResult.rotation);
+
+                // Reset facing and start.
+                mFacing = realFacing;
                 GlCameraPreview cameraPreview = (GlCameraPreview) mPreview;
                 mVideoRecorder = new SnapshotVideoRecorder(videoResult, Camera1.this, cameraPreview);
                 mVideoRecorder.start();

cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java

@@ -339,7 +339,7 @@ abstract class CameraController implements
 
     abstract void takeVideo(@NonNull File file);
 
-    abstract void takeVideoSnapshot(@NonNull File file);
+    abstract void takeVideoSnapshot(@NonNull File file, @NonNull AspectRatio viewAspectRatio);
 
     abstract void stopVideo();
 
@@ -469,7 +469,7 @@ abstract class CameraController implements
         }
         // Maybe the sensor is the other.
         if (toReference == REF_SENSOR) {
-            return -offset(toReference, fromReference) + 360;
+            return (-offset(toReference, fromReference) + 360) % 360;
         }
         // None of them is the sensor. Use a difference.
         return (offset(REF_SENSOR, toReference) - offset(REF_SENSOR, fromReference) + 360) % 360;

cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java

@@ -1281,10 +1281,11 @@ public class CameraView extends FrameLayout implements LifecycleObserver {
     }
 
     public void takeVideoSnapshot(@Nullable File file) {
+        if (getWidth() == 0 || getHeight() == 0) return;
         if (file == null) {
             file = new File(getContext().getFilesDir(), "video.mp4");
         }
-        mCameraController.takeVideoSnapshot(file);
+        mCameraController.takeVideoSnapshot(file, AspectRatio.of(getWidth(), getHeight()));
         mUiHandler.post(new Runnable() {
             @Override
             public void run() {

cameraview/src/main/java/com/otaliastudios/cameraview/SnapshotPictureRecorder.java

@@ -114,7 +114,8 @@ class SnapshotPictureRecorder extends PictureRecorder {
                         Matrix.scaleM(mTransform, 0, realScaleX, realScaleY, 1);
 
                         // Apply rotation:
-                        // Not sure why we need the minus here... It makes no sense.
+                        // TODO Not sure why we need the minus here... It makes no sense to me.
+                        LOG.w("Recording frame. Rotation:", mResult.rotation, "Actual:", -mResult.rotation);
                         int rotation = -mResult.rotation;
                         mResult.rotation = 0;
                         Matrix.translateM(mTransform, 0, 0.5F, 0.5F, 0);
@@ -124,9 +125,6 @@ class SnapshotPictureRecorder extends PictureRecorder {
                         float[] scaleXY = new float[] { realScaleX, realScaleY };
                         viewport.drawFrame(mTextureId, mTransform, scaleXY);
                         // don't - surface.swapBuffers();
-                        // TODO: need PNG when the preview is rounded, which is an experimental thing
-                        // In all the other cases this must be JPEG!
-                        // Although PNG is extremely slow.
                         mResult.data = surface.saveFrameTo(Bitmap.CompressFormat.JPEG);
                         mResult.format = PictureResult.FORMAT_JPEG;
                         mSurfaceTexture.releaseTexImage();

cameraview/src/main/java/com/otaliastudios/cameraview/SnapshotVideoRecorder.java

@@ -73,6 +73,7 @@ class SnapshotVideoRecorder extends VideoRecorder implements GlCameraPreview.Ren
             if (mResult.videoBitRate <= 0) mResult.videoBitRate = DEFAULT_VIDEO_BITRATE;
             if (mResult.audioBitRate <= 0) mResult.audioBitRate = DEFAULT_AUDIO_BITRATE;
             if (mResult.videoFrameRate <= 0) mResult.videoFrameRate = DEFAULT_VIDEO_FRAMERATE;
+            LOG.w("Creating frame encoder. Rotation:", mResult.rotation);
             TextureMediaEncoder.Config config = new TextureMediaEncoder.Config(width, height,
                     mResult.videoBitRate,
                     mResult.videoFrameRate,
@@ -103,9 +104,9 @@ class SnapshotVideoRecorder extends VideoRecorder implements GlCameraPreview.Ren
         }
 
         if (mCurrentState == STATE_RECORDING && mDesiredState == STATE_NOT_RECORDING) {
+            mCurrentState = STATE_NOT_RECORDING; // before nulling encoderEngine!
             mEncoderEngine.stop();
             mEncoderEngine = null;
-            mCurrentState = STATE_NOT_RECORDING;
             mPreview.removeRendererFrameCallback(SnapshotVideoRecorder.this);
             mPreview = null;
         }