V2 bug fixes (#356)

* Fix permissions error

* Fix #355

* Fix #357

* Improve CameraOptions tests

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<Camera.Size> 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<Size> 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<Camera.Size> sizes = Arrays.asList(

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;
                     }
                 };

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;
                     }
                 };

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

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<Size> 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<Size> 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;
     }

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<Camera.Size> 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<Camera.Size> 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<Camera.Size> 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));
+            }
         }
     }
 

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.
+     * Checks that we have appropriate permissions.
-     * Throws if session = audio and manifest did not add the microphone permissions.     
+     * This means checking that we have audio permissions if audio = Audio.ON.
-     * @param mode the sessionType to be checked
      * @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) {
+    protected boolean checkPermissions(@NonNull Audio audio) {
-        checkPermissionsManifestOrThrow(mode, 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) {
+    private void checkPermissionsManifestOrThrow(@NonNull Audio audio) {
-        if (mode == Mode.VIDEO && audio == Audio.ON) {
+        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) {
-
+        mCameraController.setMode(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();
-        }
     }
 
 

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
+- handle permissions yourself and then call `open()` or `setLifecycleOwner()` once they are acquired
-- or call `cameraView.start()` anyway: `CameraView` will present a permission request to the user based on
+- 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.