Fix preview on Pixel devices, fix deadlock with surface preview (#1020)

cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java

@@ -214,6 +214,7 @@ public class Camera1Engine extends CameraBaseEngine implements
 
         mCaptureSize = computeCaptureSize();
         mPreviewStreamSize = computePreviewStreamSize();
+        LOG.i("onStartBind:", "Returning");
         return Tasks.forResult(null);
     }
 

cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java

@@ -19,6 +19,8 @@ import android.location.Location;
 import android.media.Image;
 import android.media.ImageReader;
 import android.os.Build;
+import android.os.Handler;
+import android.os.Looper;
 import android.util.Pair;
 import android.util.Range;
 import android.util.Rational;
@@ -61,6 +63,7 @@ import com.otaliastudios.cameraview.frame.FrameManager;
 import com.otaliastudios.cameraview.frame.ImageFrameManager;
 import com.otaliastudios.cameraview.gesture.Gesture;
 import com.otaliastudios.cameraview.internal.CropHelper;
+import com.otaliastudios.cameraview.internal.FpsRangeValidator;
 import com.otaliastudios.cameraview.metering.MeteringRegions;
 import com.otaliastudios.cameraview.picture.Full2PictureRecorder;
 import com.otaliastudios.cameraview.picture.Snapshot2PictureRecorder;
@@ -76,6 +79,7 @@ import java.util.Comparator;
 import java.util.List;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 
 @RequiresApi(Build.VERSION_CODES.LOLLIPOP)
@@ -498,6 +502,7 @@ public class Camera2Engine extends CameraBaseEngine implements
         if (outputClass == SurfaceHolder.class) {
             try {
                 // This must be called from the UI thread...
+                LOG.i("onStartBind:", "Waiting on UI thread...");
                 Tasks.await(Tasks.call(new Callable<Void>() {
                     @Override
                     public Void call() {
@@ -1374,14 +1379,13 @@ public class Camera2Engine extends CameraBaseEngine implements
     protected boolean applyPreviewFrameRate(@NonNull CaptureRequest.Builder builder,
                                             float oldPreviewFrameRate) {
         //noinspection unchecked
-        Range<Integer>[] fallback = new Range[]{};
         Range<Integer>[] fpsRanges = readCharacteristic(
                 CameraCharacteristics.CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,
-                fallback);
+                new Range[]{});
         sortRanges(fpsRanges);
         if (mPreviewFrameRate == 0F) {
             // 0F is a special value. Fallback to a reasonable default.
-            for (Range<Integer> fpsRange : fpsRanges) {
+            for (Range<Integer> fpsRange : filterRanges(fpsRanges)) {
                 if (fpsRange.contains(30) || fpsRange.contains(24)) {
                     builder.set(CaptureRequest.CONTROL_AE_TARGET_FPS_RANGE, fpsRange);
                     return true;
@@ -1393,7 +1397,7 @@ public class Camera2Engine extends CameraBaseEngine implements
                     mCameraOptions.getPreviewFrameRateMaxValue());
             mPreviewFrameRate = Math.max(mPreviewFrameRate,
                     mCameraOptions.getPreviewFrameRateMinValue());
-            for (Range<Integer> fpsRange : fpsRanges) {
+            for (Range<Integer> fpsRange : filterRanges(fpsRanges)) {
                 if (fpsRange.contains(Math.round(mPreviewFrameRate))) {
                     builder.set(CaptureRequest.CONTROL_AE_TARGET_FPS_RANGE, fpsRange);
                     return true;
@@ -1405,23 +1409,32 @@ public class Camera2Engine extends CameraBaseEngine implements
     }
 
     private void sortRanges(Range<Integer>[] fpsRanges) {
-        if (getPreviewFrameRateExact() && mPreviewFrameRate != 0) { // sort by range width in ascending order
-            Arrays.sort(fpsRanges, new Comparator<Range<Integer>>() {
-                @Override
-                public int compare(Range<Integer> range1, Range<Integer> range2) {
+        final boolean ascending = getPreviewFrameRateExact() && mPreviewFrameRate != 0;
+        Arrays.sort(fpsRanges, new Comparator<Range<Integer>>() {
+            @Override
+            public int compare(Range<Integer> range1, Range<Integer> range2) {
+                if (ascending) {
                     return (range1.getUpper() - range1.getLower())
                             - (range2.getUpper() - range2.getLower());
-                }
-            });
-        } else { // sort by range width in descending order
-            Arrays.sort(fpsRanges, new Comparator<Range<Integer>>() {
-                @Override
-                public int compare(Range<Integer> range1, Range<Integer> range2) {
+                } else {
                     return (range2.getUpper() - range2.getLower())
                             - (range1.getUpper() - range1.getLower());
                 }
-            });
+            }
+        });
+    }
+
+    private List<Range<Integer>> filterRanges(Range<Integer>[] fpsRanges) {
+        List<Range<Integer>> results = new ArrayList<>();
+        int min = Math.round(mCameraOptions.getPreviewFrameRateMinValue());
+        int max = Math.round(mCameraOptions.getPreviewFrameRateMaxValue());
+        for (Range<Integer> fpsRange : fpsRanges) {
+            if (!fpsRange.contains(min)) continue;
+            if (!fpsRange.contains(max)) continue;
+            if (!FpsRangeValidator.validate(fpsRange)) continue;
+            results.add(fpsRange);
         }
+        return results;
     }
 
     @Override

cameraview/src/main/java/com/otaliastudios/cameraview/engine/orchestrator/CameraOrchestrator.java

@@ -2,6 +2,7 @@ package com.otaliastudios.cameraview.engine.orchestrator;
 
 import androidx.annotation.GuardedBy;
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 
 import com.google.android.gms.tasks.OnCompleteListener;
 import com.google.android.gms.tasks.Task;
@@ -108,37 +109,40 @@ public class CameraOrchestrator {
         return job.source.getTask();
     }
 
+    @GuardedBy("mJobsLock")
     private void sync(long after) {
         // Jumping on the message handler even if after = 0L should avoid StackOverflow errors.
         mCallback.getJobWorker("_sync").post(after, new Runnable() {
             @SuppressWarnings("StatementWithEmptyBody")
             @Override
             public void run() {
+                Job<?> job = null;
                 synchronized (mJobsLock) {
-                    if (!mJobRunning) {
+                    if (mJobRunning) {
+                        // Do nothing, job will be picked in executed().
+                    } else {
                         long now = System.currentTimeMillis();
-                        Job<?> job = null;
                         for (Job<?> candidate : mJobs) {
                             if (candidate.startTime <= now) {
                                 job = candidate;
                                 break;
                             }
                         }
-                        if (job != null) execute(job);
-                    } else {
-                        // Do nothing, job will be picked in executed().
+                        if (job != null) {
+                            mJobRunning = true;
+                        }
                     }
                 }
+                // This must be out of mJobsLock! See comments in execute().
+                if (job != null) execute(job);
             }
         });
     }
 
-    @GuardedBy("mJobsLock")
+    // Since we use WorkerHandler.run(), the job can end up being executed on the current thread.
+    // For this reason, it's important that this method is never guarded by mJobsLock! Because
+    // all threads can be waiting on that, even the UI thread e.g. through scheduleInternal.
     private <T> void execute(@NonNull final Job<T> job) {
-        if (mJobRunning) {
-            throw new IllegalStateException("mJobRunning is already true! job=" + job.name);
-        }
-        mJobRunning = true;
         final WorkerHandler worker = mCallback.getJobWorker(job.name);
         worker.run(new Runnable() {
             @Override

cameraview/src/main/java/com/otaliastudios/cameraview/internal/FpsRangeValidator.java

@@ -0,0 +1,38 @@
+package com.otaliastudios.cameraview.internal;
+
+import android.os.Build;
+import android.util.Log;
+import android.util.Range;
+
+import androidx.annotation.RequiresApi;
+
+import com.otaliastudios.cameraview.CameraLogger;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+@RequiresApi(21)
+public class FpsRangeValidator {
+
+    private final static CameraLogger LOG = CameraLogger.create("FpsRangeValidator");
+    private final static Map<String, List<Range<Integer>>> sIssues = new HashMap<>();
+
+    static {
+        sIssues.put("Google Pixel 4", Arrays.asList(new Range<>(15, 60)));
+    }
+
+    public static boolean validate(Range<Integer> range) {
+        LOG.i("Build.MODEL:", Build.MODEL, "Build.BRAND:", Build.BRAND, "Build.MANUFACTURER:", Build.MANUFACTURER);
+        String descriptor = Build.MANUFACTURER + " " + Build.MODEL;
+        List<Range<Integer>> ranges = sIssues.get(descriptor);
+        if (ranges != null && ranges.contains(range)) {
+            LOG.i("Dropping range:", range);
+            return false;
+        }
+        return true;
+    }
+}

demo/src/main/kotlin/com/otaliastudios/cameraview/demo/CameraActivity.kt

@@ -28,7 +28,7 @@ class CameraActivity : AppCompatActivity(), View.OnClickListener, OptionView.Cal
 
     companion object {
         private val LOG = CameraLogger.create("DemoApp")
-        private const val USE_FRAME_PROCESSOR = true
+        private const val USE_FRAME_PROCESSOR = false
         private const val DECODE_BITMAP = false
     }