From 40ace54c19325d7e9d26f44d88ef5efc4fe2abd5 Mon Sep 17 00:00:00 2001 From: Mattia Iavarone Date: Sun, 3 Jan 2021 23:25:10 +0100 Subject: [PATCH] Fix preview on Pixel devices, fix deadlock with surface preview (#1020) --- .../cameraview/engine/Camera1Engine.java | 1 + .../cameraview/engine/Camera2Engine.java | 43 ++++++++++++------- .../orchestrator/CameraOrchestrator.java | 24 ++++++----- .../internal/FpsRangeValidator.java | 38 ++++++++++++++++ .../cameraview/demo/CameraActivity.kt | 2 +- 5 files changed, 82 insertions(+), 26 deletions(-) create mode 100644 cameraview/src/main/java/com/otaliastudios/cameraview/internal/FpsRangeValidator.java diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java index d8b498cf..6f976f08 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java +++ b/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); } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java index b945d4a0..0ee02c13 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java +++ b/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() { @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[] fallback = new Range[]{}; Range[] 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 fpsRange : fpsRanges) { + for (Range 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 fpsRange : fpsRanges) { + for (Range 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[] fpsRanges) { - if (getPreviewFrameRateExact() && mPreviewFrameRate != 0) { // sort by range width in ascending order - Arrays.sort(fpsRanges, new Comparator>() { - @Override - public int compare(Range range1, Range range2) { + final boolean ascending = getPreviewFrameRateExact() && mPreviewFrameRate != 0; + Arrays.sort(fpsRanges, new Comparator>() { + @Override + public int compare(Range range1, Range 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>() { - @Override - public int compare(Range range1, Range range2) { + } else { return (range2.getUpper() - range2.getLower()) - (range1.getUpper() - range1.getLower()); } - }); + } + }); + } + + private List> filterRanges(Range[] fpsRanges) { + List> results = new ArrayList<>(); + int min = Math.round(mCameraOptions.getPreviewFrameRateMinValue()); + int max = Math.round(mCameraOptions.getPreviewFrameRateMaxValue()); + for (Range fpsRange : fpsRanges) { + if (!fpsRange.contains(min)) continue; + if (!fpsRange.contains(max)) continue; + if (!FpsRangeValidator.validate(fpsRange)) continue; + results.add(fpsRange); } + return results; } @Override diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/orchestrator/CameraOrchestrator.java b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/orchestrator/CameraOrchestrator.java index 3339dccc..d79f6162 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/orchestrator/CameraOrchestrator.java +++ b/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 void execute(@NonNull final Job 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 diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/internal/FpsRangeValidator.java b/cameraview/src/main/java/com/otaliastudios/cameraview/internal/FpsRangeValidator.java new file mode 100644 index 00000000..16e57096 --- /dev/null +++ b/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>> sIssues = new HashMap<>(); + + static { + sIssues.put("Google Pixel 4", Arrays.asList(new Range<>(15, 60))); + } + + public static boolean validate(Range range) { + LOG.i("Build.MODEL:", Build.MODEL, "Build.BRAND:", Build.BRAND, "Build.MANUFACTURER:", Build.MANUFACTURER); + String descriptor = Build.MANUFACTURER + " " + Build.MODEL; + List> ranges = sIssues.get(descriptor); + if (ranges != null && ranges.contains(range)) { + LOG.i("Dropping range:", range); + return false; + } + return true; + } +} diff --git a/demo/src/main/kotlin/com/otaliastudios/cameraview/demo/CameraActivity.kt b/demo/src/main/kotlin/com/otaliastudios/cameraview/demo/CameraActivity.kt index 2e245562..5c5311ce 100644 --- a/demo/src/main/kotlin/com/otaliastudios/cameraview/demo/CameraActivity.kt +++ b/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 }