Merge "[Extensions] Fixed surface abandoned crash when pause/resume too quickly on Samsung Z Fold2" into androidx-main
diff --git a/camera/camera-extensions/src/androidTest/java/androidx/camera/extensions/internal/compat/workaround/OnEnableDisableSessionDurationCheckTest.kt b/camera/camera-extensions/src/androidTest/java/androidx/camera/extensions/internal/compat/workaround/OnEnableDisableSessionDurationCheckTest.kt
new file mode 100644
index 0000000..dd416ee
--- /dev/null
+++ b/camera/camera-extensions/src/androidTest/java/androidx/camera/extensions/internal/compat/workaround/OnEnableDisableSessionDurationCheckTest.kt
@@ -0,0 +1,97 @@
+/*
+ * Copyright 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package androidx.camera.extensions.internal.compat.workaround
+
+import android.os.SystemClock
+import androidx.camera.extensions.internal.compat.workaround.OnEnableDisableSessionDurationCheck.MIN_DURATION_FOR_ENABLE_DISABLE_SESSION
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.filters.SdkSuppress
+import androidx.test.filters.SmallTest
+import com.google.common.collect.Range
+import com.google.common.truth.Truth.assertThat
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@SmallTest
+@RunWith(AndroidJUnit4::class)
+@SdkSuppress(minSdkVersion = 21)
+class OnEnableDisableSessionDurationCheckTest {
+ companion object {
+ const val TOLERANCE = 15L
+ }
+
+ @Test
+ fun enabled_ensureMinimalDuration() {
+ // Arrange
+ val check = OnEnableDisableSessionDurationCheck(/* enabledMinimumDuration */true)
+
+ val duration = 40L
+ // Act
+ val startTime = SystemClock.elapsedRealtime()
+ check.onEnableSessionInvoked()
+ Thread.sleep(duration)
+ check.onDisableSessionInvoked()
+ val endTime = SystemClock.elapsedRealtime()
+
+ // Assert
+ assertThat((endTime - startTime))
+ .isIn(
+ Range.closed(
+ MIN_DURATION_FOR_ENABLE_DISABLE_SESSION,
+ MIN_DURATION_FOR_ENABLE_DISABLE_SESSION + TOLERANCE
+ ))
+ }
+
+ @Test
+ fun enabled_doNotWaitExtraIfDurationExceeds() {
+ // 1. Arrange
+ val check = OnEnableDisableSessionDurationCheck(/* enabledMinimumDuration */true)
+
+ // make the duration of onEnable to onDisable to be the minimal duration.
+ val duration = MIN_DURATION_FOR_ENABLE_DISABLE_SESSION
+
+ // 2. Act
+ val startTime = SystemClock.elapsedRealtime()
+ check.onEnableSessionInvoked()
+ // make the duration of onEnable to onDisable to be the minimal duration.
+ Thread.sleep(duration)
+ check.onDisableSessionInvoked()
+ val endTime = SystemClock.elapsedRealtime()
+
+ // 3. Assert: no extra time waited.
+ assertThat((endTime - startTime))
+ .isLessThan(
+ duration + TOLERANCE
+ )
+ }
+
+ @Test
+ fun disabled_doNotWait() {
+ // 1. Arrange
+ val check = OnEnableDisableSessionDurationCheck(/* enabledMinimumDuration */ false)
+
+ // 2. Act
+ val startTime = SystemClock.elapsedRealtime()
+ check.onEnableSessionInvoked()
+ check.onDisableSessionInvoked()
+ val endTime = SystemClock.elapsedRealtime()
+
+ // 3. Assert
+ assertThat((endTime - startTime))
+ .isLessThan(TOLERANCE)
+ }
+}
\ No newline at end of file
diff --git a/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/quirk/CrashWhenOnDisableTooSoon.java b/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/quirk/CrashWhenOnDisableTooSoon.java
new file mode 100644
index 0000000..5ed2ce16
--- /dev/null
+++ b/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/quirk/CrashWhenOnDisableTooSoon.java
@@ -0,0 +1,37 @@
+/*
+ * Copyright 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package androidx.camera.extensions.internal.compat.quirk;
+
+import android.os.Build;
+
+import androidx.annotation.RequiresApi;
+import androidx.camera.core.impl.Quirk;
+
+/**
+ * <p>QuirkSummary
+ * Bug Id: b/245226085,
+ * Description: When enabling Extensions on devices that implement the Basic Extender. If
+ * onDisableSession is invoked within 50ms after onEnableSession, It could cause crashes like
+ * surface abandoned.
+ * Device(s): All Samsung devices
+ */
+@RequiresApi(21) // TODO(b/200306659): Remove and replace with annotation on package-info.java
+public class CrashWhenOnDisableTooSoon implements Quirk {
+ static boolean load() {
+ return Build.BRAND.equalsIgnoreCase("SAMSUNG");
+ }
+}
diff --git a/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/quirk/DeviceQuirksLoader.java b/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/quirk/DeviceQuirksLoader.java
index bd85647..82bb2e7 100644
--- a/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/quirk/DeviceQuirksLoader.java
+++ b/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/quirk/DeviceQuirksLoader.java
@@ -44,6 +44,10 @@
quirks.add(new ExtensionDisabledQuirk());
}
+ if (CrashWhenOnDisableTooSoon.load()) {
+ quirks.add(new CrashWhenOnDisableTooSoon());
+ }
+
return quirks;
}
}
diff --git a/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/quirk/ExtensionDisabledQuirk.java b/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/quirk/ExtensionDisabledQuirk.java
index 992962c..c1ba5c4 100644
--- a/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/quirk/ExtensionDisabledQuirk.java
+++ b/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/quirk/ExtensionDisabledQuirk.java
@@ -40,17 +40,16 @@
*/
@RequiresApi(21) // TODO(b/200306659): Remove and replace with annotation on package-info.java
public class ExtensionDisabledQuirk implements Quirk {
- private final boolean mIsAdvancedInterface = isAdvancedExtenderSupported();
static boolean load() {
- return isPixel5() || isMoto() || isAdvancedExtenderSupported();
+ return isPixel5() || isMoto();
}
/**
* Checks whether extension should be disabled.
*/
public boolean shouldDisableExtension() {
- if (isPixel5() && !mIsAdvancedInterface) {
+ if (isPixel5() && !isAdvancedExtenderSupported()) {
// 1. Disables Pixel 5's Basic Extender capability.
return true;
} else if (isMoto() && ExtensionVersion.isMaximumCompatibleVersion(Version.VERSION_1_1)) {
diff --git a/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/workaround/OnEnableDisableSessionDurationCheck.java b/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/workaround/OnEnableDisableSessionDurationCheck.java
new file mode 100644
index 0000000..ffc0791
--- /dev/null
+++ b/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/compat/workaround/OnEnableDisableSessionDurationCheck.java
@@ -0,0 +1,90 @@
+/*
+ * Copyright 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package androidx.camera.extensions.internal.compat.workaround;
+
+import android.os.SystemClock;
+
+import androidx.annotation.RequiresApi;
+import androidx.annotation.VisibleForTesting;
+import androidx.camera.core.Logger;
+import androidx.camera.extensions.internal.compat.quirk.CrashWhenOnDisableTooSoon;
+import androidx.camera.extensions.internal.compat.quirk.DeviceQuirks;
+
+/**
+ * A workaround to ensure the duration of onEnableSession to onDisableSession is long enough.
+ */
+@RequiresApi(21) // TODO(b/200306659): Remove and replace with annotation on package-info.java
+public class OnEnableDisableSessionDurationCheck {
+ private static final String TAG = "OnEnableDisableSessionDurationCheck";
+ private final boolean mEnabledMinimumDuration;
+ private long mOnEnableSessionTimeStamp = 0;
+ @VisibleForTesting
+ static final long MIN_DURATION_FOR_ENABLE_DISABLE_SESSION = 100L;
+
+ public OnEnableDisableSessionDurationCheck() {
+ mEnabledMinimumDuration = DeviceQuirks.get(CrashWhenOnDisableTooSoon.class) != null;
+ }
+
+ @VisibleForTesting
+ OnEnableDisableSessionDurationCheck(boolean enabledMinimalDuration) {
+ mEnabledMinimumDuration = enabledMinimalDuration;
+ }
+
+ /**
+ * Notify onEnableSession is invoked.
+ */
+ public void onEnableSessionInvoked() {
+ if (mEnabledMinimumDuration) {
+ mOnEnableSessionTimeStamp = SystemClock.elapsedRealtime();
+ }
+ }
+
+ /**
+ * Notify onDisableSession is invoked.
+ */
+ public void onDisableSessionInvoked() {
+ if (mEnabledMinimumDuration) {
+ ensureMinDurationAfterOnEnableSession();
+ }
+ }
+
+ /**
+ * Ensures onDisableSession not invoked too soon after onDisableSession. OEMs usually
+ * releases resources at onDisableSession. Invoking onDisableSession too soon might cause
+ * some crash during the initialization triggered by onEnableSession or onInit.
+ *
+ * It will ensure the duration is at least 100 ms after onEnabledSession is invoked. If the
+ * camera is opened more than 100ms, then it won't add any extra delay. Since a regular
+ * camera session will take more than 100ms, this change shouldn't cause any real impact for
+ * the user. It only affects the auto testing by increasing a little bit delay which
+ * should be okay.
+ */
+ private void ensureMinDurationAfterOnEnableSession() {
+ long timeAfterOnEnableSession =
+ SystemClock.elapsedRealtime() - mOnEnableSessionTimeStamp;
+ if (timeAfterOnEnableSession < MIN_DURATION_FOR_ENABLE_DISABLE_SESSION) {
+ try {
+ long timeToWait =
+ MIN_DURATION_FOR_ENABLE_DISABLE_SESSION - timeAfterOnEnableSession;
+ Logger.d(TAG, "onDisableSession too soon, wait " + timeToWait + " ms");
+ Thread.sleep(timeToWait);
+ } catch (InterruptedException e) {
+ Logger.e(TAG, "sleep interrupted");
+ }
+ }
+ }
+}
diff --git a/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/sessionprocessor/BasicExtenderSessionProcessor.java b/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/sessionprocessor/BasicExtenderSessionProcessor.java
index 5049106..5452a8a 100644
--- a/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/sessionprocessor/BasicExtenderSessionProcessor.java
+++ b/camera/camera-extensions/src/main/java/androidx/camera/extensions/internal/sessionprocessor/BasicExtenderSessionProcessor.java
@@ -49,6 +49,7 @@
import androidx.camera.extensions.impl.PreviewExtenderImpl;
import androidx.camera.extensions.impl.PreviewImageProcessorImpl;
import androidx.camera.extensions.impl.RequestUpdateProcessorImpl;
+import androidx.camera.extensions.internal.compat.workaround.OnEnableDisableSessionDurationCheck;
import androidx.core.util.Preconditions;
import java.util.ArrayList;
@@ -90,6 +91,8 @@
static AtomicInteger sLastOutputConfigId = new AtomicInteger(0);
@GuardedBy("mLock")
private final Map<CaptureRequest.Key<?>, Object> mParameters = new LinkedHashMap<>();
+ private OnEnableDisableSessionDurationCheck mOnEnableDisableSessionDurationCheck =
+ new OnEnableDisableSessionDurationCheck();
public BasicExtenderSessionProcessor(@NonNull PreviewExtenderImpl previewExtenderImpl,
@NonNull ImageCaptureExtenderImpl imageCaptureExtenderImpl,
@@ -252,6 +255,7 @@
if (captureStage2 != null) {
captureStages.add(captureStage2);
}
+ mOnEnableDisableSessionDurationCheck.onEnableSessionInvoked();
if (!captureStages.isEmpty()) {
submitRequestByCaptureStages(requestProcessor, captureStages);
@@ -323,6 +327,7 @@
@Override
public void onCaptureSessionEnd() {
+ mOnEnableDisableSessionDurationCheck.onDisableSessionInvoked();
List<CaptureStageImpl> captureStages = new ArrayList<>();
CaptureStageImpl captureStage1 = mPreviewExtenderImpl.onDisableSession();
Logger.d(TAG, "preview onDisableSession: " + captureStage1);
