From a8a4e09900c864ccfba71903dbeca49ef349ac24 Mon Sep 17 00:00:00 2001 From: Mattia Iavarone Date: Fri, 11 Jan 2019 13:03:58 +0100 Subject: [PATCH] V2 bug fixes (#356) * Fix permissions error * Fix #355 * Fix #357 * Improve CameraOptions tests --- .../cameraview/CameraOptions1Test.java | 21 ++++++++++ .../cameraview/CameraViewCallbacksTest.java | 2 +- .../cameraview/CameraViewTest.java | 2 +- .../com/otaliastudios/cameraview/Camera1.java | 11 ++++- .../cameraview/CameraController.java | 9 ++++- .../cameraview/CameraOptions.java | 13 +++++- .../otaliastudios/cameraview/CameraView.java | 40 ++++++------------- docs/_posts/2018-12-20-runtime-permissions.md | 9 +++-- 8 files changed, 69 insertions(+), 38 deletions(-) diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraOptions1Test.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraOptions1Test.java index 00261196..7ab9e4da 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraOptions1Test.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraOptions1Test.java @@ -130,6 +130,27 @@ public class CameraOptions1Test extends BaseTest { } } + @Test + public void testVideoSizesNull() { + // When videoSizes is null, we take the preview sizes. + List sizes = Arrays.asList( + mockCameraSize(100, 200), + mockCameraSize(50, 50), + mockCameraSize(1600, 900), + mockCameraSize(1000, 2000) + ); + Camera.Parameters params = mock(Camera.Parameters.class); + when(params.getSupportedVideoSizes()).thenReturn(null); + when(params.getSupportedPreviewSizes()).thenReturn(sizes); + CameraOptions o = new CameraOptions(params, false); + Collection supportedSizes = o.getSupportedVideoSizes(); + assertEquals(supportedSizes.size(), sizes.size()); + for (Camera.Size size : sizes) { + Size internalSize = new Size(size.width, size.height); + assertTrue(supportedSizes.contains(internalSize)); + } + } + @Test public void testVideoSizesFlip() { List sizes = Arrays.asList( diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewCallbacksTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewCallbacksTest.java index 44319ea1..f66b74fe 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewCallbacksTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewCallbacksTest.java @@ -62,7 +62,7 @@ public class CameraViewCallbacksTest extends BaseTest { } @Override - protected boolean checkPermissions(@NonNull Mode mode, @NonNull Audio audio) { + protected boolean checkPermissions(@NonNull Audio audio) { return true; } }; diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewTest.java index fcb8ae2e..aed9b4af 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewTest.java @@ -50,7 +50,7 @@ public class CameraViewTest extends BaseTest { } @Override - protected boolean checkPermissions(@NonNull Mode mode, @NonNull Audio audio) { + protected boolean checkPermissions(@NonNull Audio audio) { return hasPermissions; } }; diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java b/cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java index 3e34236b..80dd92ba 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java @@ -173,7 +173,16 @@ class Camera1 extends CameraController implements Camera.PreviewCallback, Camera Camera.Parameters params = mCamera.getParameters(); mPreviewFormat = params.getPreviewFormat(); params.setPreviewSize(mPreviewSize.getWidth(), mPreviewSize.getHeight()); // <- not allowed during preview - params.setPictureSize(mCaptureSize.getWidth(), mCaptureSize.getHeight()); // <- allowed + if (mMode == Mode.PICTURE) { + params.setPictureSize(mCaptureSize.getWidth(), mCaptureSize.getHeight()); // <- allowed + } else { + // mCaptureSize in this case is a video size. The available video sizes are not necessarily + // a subset of the picture sizes, so we can't use the mCaptureSize value: it might crash. + // However, the setPictureSize() passed here is useless : we don't allow HQ pictures in video mode. + // While this might be lifted in the future, for now, just use a picture capture size. + Size pictureSize = computeCaptureSize(Mode.PICTURE); + params.setPictureSize(pictureSize.getWidth(), pictureSize.getHeight()); + } mCamera.setParameters(params); mCamera.setPreviewCallbackWithBuffer(null); // Release anything left diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java index f283e5af..d6063787 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java @@ -523,12 +523,17 @@ abstract class CameraController implements @NonNull @SuppressWarnings("WeakerAccess") protected final Size computeCaptureSize() { + return computeCaptureSize(mMode); + } + + @SuppressWarnings("WeakerAccess") + protected final Size computeCaptureSize(Mode mode) { // We want to pass stuff into the REF_VIEW reference, not the sensor one. // This is already managed by CameraOptions, so we just flip again at the end. boolean flip = flip(REF_SENSOR, REF_VIEW); SizeSelector selector; Collection sizes; - if (mMode == Mode.PICTURE) { + if (mode == Mode.PICTURE) { selector = mPictureSizeSelector; sizes = mCameraOptions.getSupportedPictureSizes(); } else { @@ -538,7 +543,7 @@ abstract class CameraController implements selector = SizeSelectors.or(selector, SizeSelectors.biggest()); List list = new ArrayList<>(sizes); Size result = selector.select(list).get(0); - LOG.i("computeCaptureSize:", "result:", result, "flip:", flip); + LOG.i("computeCaptureSize:", "result:", result, "flip:", flip, "mode:", mode); if (flip) result = result.flip(); // Go back to REF_SENSOR return result; } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraOptions.java b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraOptions.java index 8042c505..dc566960 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraOptions.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraOptions.java @@ -87,7 +87,7 @@ public class CameraOptions { exposureCorrectionSupported = params.getMinExposureCompensation() != 0 || params.getMaxExposureCompensation() != 0; - // Sizes + // Picture Sizes List sizes = params.getSupportedPictureSizes(); for (Camera.Size size : sizes) { int width = flipSizes ? size.height : size.width; @@ -95,6 +95,8 @@ public class CameraOptions { supportedPictureSizes.add(new Size(width, height)); supportedPictureAspectRatio.add(AspectRatio.of(width, height)); } + + // Video Sizes List vsizes = params.getSupportedVideoSizes(); if (vsizes != null) { for (Camera.Size size : vsizes) { @@ -103,6 +105,15 @@ public class CameraOptions { supportedVideoSizes.add(new Size(width, height)); supportedVideoAspectRatio.add(AspectRatio.of(width, height)); } + } else { + // StackOverflow threads seems to agree that if getSupportedVideoSizes is null, previews can be used. + List fallback = params.getSupportedPreviewSizes(); + for (Camera.Size size : fallback) { + int width = flipSizes ? size.height : size.width; + int height = flipSizes ? size.width : size.height; + supportedVideoSizes.add(new Size(width, height)); + supportedVideoAspectRatio.add(AspectRatio.of(width, height)); + } } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java index ea7a8429..f7c979ff 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java @@ -603,7 +603,7 @@ public class CameraView extends FrameLayout implements LifecycleObserver { if (!isEnabled()) return; if (mCameraPreview != null) mCameraPreview.onResume(); - if (checkPermissions(getMode(), getAudio())) { + if (checkPermissions(getAudio())) { // Update display orientation for current CameraController mOrientationHelper.enable(getContext()); mCameraController.setDisplayOffset(mOrientationHelper.getDisplayOffset()); @@ -613,15 +613,15 @@ public class CameraView extends FrameLayout implements LifecycleObserver { /** - * Checks that we have appropriate permissions for this session type. - * Throws if session = audio and manifest did not add the microphone permissions. - * @param mode the sessionType to be checked + * Checks that we have appropriate permissions. + * This means checking that we have audio permissions if audio = Audio.ON. * @param audio the audio setting to be checked * @return true if we can go on, false otherwise. */ + @SuppressWarnings("ConstantConditions") @SuppressLint("NewApi") - protected boolean checkPermissions(@NonNull Mode mode, @NonNull Audio audio) { - checkPermissionsManifestOrThrow(mode, audio); + protected boolean checkPermissions(@NonNull Audio audio) { + checkPermissionsManifestOrThrow(audio); // Manifest is OK at this point. Let's check runtime permissions. if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) return true; @@ -641,12 +641,11 @@ public class CameraView extends FrameLayout implements LifecycleObserver { /** - * If mSessionType == SESSION_TYPE_VIDEO we will ask for RECORD_AUDIO permission. + * If audio is on we will ask for RECORD_AUDIO permission. * If the developer did not add this to its manifest, throw and fire warnings. - * (Hoping this is not caught elsewhere... we should test). */ - private void checkPermissionsManifestOrThrow(@NonNull Mode mode, @NonNull Audio audio) { - if (mode == Mode.VIDEO && audio == Audio.ON) { + private void checkPermissionsManifestOrThrow(@NonNull Audio audio) { + if (audio == Audio.ON) { try { PackageManager manager = getContext().getPackageManager(); PackageInfo info = manager.getPackageInfo(getContext().getPackageName(), PackageManager.GET_PERMISSIONS); @@ -655,7 +654,7 @@ public class CameraView extends FrameLayout implements LifecycleObserver { return; } } - LOG.e("Permission error:", "When the session type is set to video,", + LOG.e("Permission error:", "When audio is enabled (Audio.ON),", "the RECORD_AUDIO permission should be added to the app manifest file."); throw new IllegalStateException(CameraLogger.lastMessage); } catch (PackageManager.NameNotFoundException e) { @@ -1027,7 +1026,7 @@ public class CameraView extends FrameLayout implements LifecycleObserver { // Check did took place, or will happen on start(). mCameraController.setAudio(audio); - } else if (checkPermissions(getMode(), audio)) { + } else if (checkPermissions(audio)) { // Camera is running. Pass. mCameraController.setAudio(audio); @@ -1096,22 +1095,7 @@ public class CameraView extends FrameLayout implements LifecycleObserver { * @param mode desired session type. */ public void setMode(@NonNull Mode mode) { - - if (mode == getMode() || isClosed()) { - // Check did took place, or will happen on start(). - mCameraController.setMode(mode); - - } else if (checkPermissions(mode, getAudio())) { - // Camera is running. CameraImpl setMode will do the trick. - mCameraController.setMode(mode); - - } else { - // This means that the audio permission is being asked. - // Stop the camera so it can be restarted by the developer onPermissionResult. - // Developer must also set the session type again... - // Not ideal but good for now. - close(); - } + mCameraController.setMode(mode); } diff --git a/docs/_posts/2018-12-20-runtime-permissions.md b/docs/_posts/2018-12-20-runtime-permissions.md index 0c978a8b..0cbcd03c 100644 --- a/docs/_posts/2018-12-20-runtime-permissions.md +++ b/docs/_posts/2018-12-20-runtime-permissions.md @@ -41,8 +41,9 @@ device has cameras, and then start the camera view. On Marshmallow+, the user must explicitly approve our permissions. You can -- handle permissions yourself and then call `cameraView.start()` once they are acquired -- or call `cameraView.start()` anyway: `CameraView` will present a permission request to the user based on +- handle permissions yourself and then call `open()` or `setLifecycleOwner()` once they are acquired +- ignore this: `CameraView` will present a permission request to the user based on whether they are needed or not with the current configuration. - -In the second case, you should restart the camera if you have a successful response from `onRequestPermissionResults()`. + +Note however, that this is done at the activity level, so the permission request callback +`onRequestPermissionResults()` will be invoked on the parent activity, not the fragment. \ No newline at end of file