From 3eb0c2fb2efca42313964c01f29d9819600374ff Mon Sep 17 00:00:00 2001 From: Mattia Iavarone Date: Thu, 20 Dec 2018 11:47:23 +0100 Subject: [PATCH] Fix video snapshots rotation --- MIGRATION.md | 5 +- .../cameraview/MockCameraController.java | 2 +- .../cameraview/EglBaseSurface.java | 5 + .../cameraview/MediaEncoderEngine.java | 6 +- .../cameraview/TextureMediaEncoder.java | 11 ++- .../com/otaliastudios/cameraview/Camera1.java | 94 ++++++++++++------- .../cameraview/CameraController.java | 4 +- .../otaliastudios/cameraview/CameraView.java | 3 +- .../cameraview/SnapshotPictureRecorder.java | 6 +- .../cameraview/SnapshotVideoRecorder.java | 3 +- 10 files changed, 87 insertions(+), 52 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 4fbcc68f..39c953ee 100644 --- a/MIGRATION.md +++ b/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(). - If adding getPreviewSize() back, update this doc. -TODO: fix video recording rotation: with front camera, it does not work. +TODO: do we want getPreviewSize() / setPreviewSize() ? probably not the getter. TODO: opencollective - TODO: document cameraGridColor TODO: document setVideoBitRate TODO: document setAudioBitRate diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/MockCameraController.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/MockCameraController.java index 157d0985..2eac6e8a 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/MockCameraController.java +++ b/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) { } diff --git a/cameraview/src/main/gles/com/otaliastudios/cameraview/EglBaseSurface.java b/cameraview/src/main/gles/com/otaliastudios/cameraview/EglBaseSurface.java index cd5858ca..b359bc22 100644 --- a/cameraview/src/main/gles/com/otaliastudios/cameraview/EglBaseSurface.java +++ b/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); } diff --git a/cameraview/src/main/gles/com/otaliastudios/cameraview/MediaEncoderEngine.java b/cameraview/src/main/gles/com/otaliastudios/cameraview/MediaEncoderEngine.java index 9ed08a72..8535ee16 100644 --- a/cameraview/src/main/gles/com/otaliastudios/cameraview/MediaEncoderEngine.java +++ b/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); } diff --git a/cameraview/src/main/gles/com/otaliastudios/cameraview/TextureMediaEncoder.java b/cameraview/src/main/gles/com/otaliastudios/cameraview/TextureMediaEncoder.java index 7582b192..9a5f83dc 100644 --- a/cameraview/src/main/gles/com/otaliastudios/cameraview/TextureMediaEncoder.java +++ b/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 { + 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 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 // 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); diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java b/cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java index a6279743..4d54bd0f 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java +++ b/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(); diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java index 3565c75d..d6bb6870 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java +++ b/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; diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java index a5058d54..bc781fc5 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java +++ b/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() { diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/SnapshotPictureRecorder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/SnapshotPictureRecorder.java index 8260d04e..ea4ee3df 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/SnapshotPictureRecorder.java +++ b/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(); diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/SnapshotVideoRecorder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/SnapshotVideoRecorder.java index 81de7f24..af7466e6 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/SnapshotVideoRecorder.java +++ b/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; }