Fix flaky PreviewViewFragmentTest
Use the DeferrableSurface from the SurfaceRequest directly fixes the problem. This is also consistent with VideoCapture.
However we still don't understand why it happend in the first place. Closing the parent DeferrableSurface(SurfaceEdge.SettableSurface) should close the SurfaceRequest's DeferrableSurface too. The difference might be that there is a delay waiting for the camera to release the Surface. We will fix the failing tests first before investigating further.
Bug: 286817690
Test: manual test and ./gradlew bOS
Change-Id: Ic1252294a481df65f2fe1f7e53ace646192e69bf
diff --git a/camera/camera-core/src/main/java/androidx/camera/core/Preview.java b/camera/camera-core/src/main/java/androidx/camera/core/Preview.java
index 52c6e0d..6283a37 100644
--- a/camera/camera-core/src/main/java/androidx/camera/core/Preview.java
+++ b/camera/camera-core/src/main/java/androidx/camera/core/Preview.java
@@ -252,12 +252,13 @@
SurfaceEdge appEdge = requireNonNull(nodeOutput.get(outConfig));
appEdge.addOnInvalidatedListener(() -> onAppEdgeInvalidated(appEdge, camera));
mCurrentSurfaceRequest = appEdge.createSurfaceRequest(camera);
+ mSessionDeferrableSurface = mCameraEdge.getDeferrableSurface();
} else {
mCameraEdge.addOnInvalidatedListener(this::notifyReset);
mCurrentSurfaceRequest = mCameraEdge.createSurfaceRequest(camera);
+ mSessionDeferrableSurface = mCurrentSurfaceRequest.getDeferrableSurface();
}
- // Send the app Surface to the app.
- mSessionDeferrableSurface = mCameraEdge.getDeferrableSurface();
+
if (mSurfaceProvider != null) {
// Only send surface request if the provider is set.
sendSurfaceRequest();
diff --git a/camera/camera-core/src/main/java/androidx/camera/core/processing/SurfaceEdge.java b/camera/camera-core/src/main/java/androidx/camera/core/processing/SurfaceEdge.java
index d237eda..3f8d2b2 100644
--- a/camera/camera-core/src/main/java/androidx/camera/core/processing/SurfaceEdge.java
+++ b/camera/camera-core/src/main/java/androidx/camera/core/processing/SurfaceEdge.java
@@ -43,6 +43,7 @@
import androidx.annotation.RequiresApi;
import androidx.annotation.VisibleForTesting;
import androidx.camera.core.CameraEffect;
+import androidx.camera.core.Preview;
import androidx.camera.core.SurfaceOutput;
import androidx.camera.core.SurfaceProcessor;
import androidx.camera.core.SurfaceRequest;
@@ -54,6 +55,7 @@
import androidx.camera.core.impl.SessionConfig;
import androidx.camera.core.impl.StreamSpec;
import androidx.camera.core.impl.utils.futures.Futures;
+import androidx.camera.core.streamsharing.StreamSharing;
import androidx.concurrent.futures.CallbackToFutureAdapter;
import com.google.common.util.concurrent.ListenableFuture;
@@ -262,6 +264,8 @@
try {
DeferrableSurface deferrableSurface = surfaceRequest.getDeferrableSurface();
if (mSettableSurface.setProvider(deferrableSurface)) {
+ // TODO(b/286817690): consider close the deferrableSurface directly when the
+ // SettableSurface is closed. The delay might cause issues on legacy devices.
mSettableSurface.getTerminationFuture().addListener(deferrableSurface::close,
directExecutor());
}
@@ -572,6 +576,11 @@
*
* <p>This class provides mechanisms to link an {@link DeferrableSurface}, and propagates
* Surface releasing/closure to the {@link DeferrableSurface}.
+ *
+ * <p>Closing the parent {@link SettableSurface} does not close the linked
+ * {@link DeferrableSurface}. This is by design. The lifecycle of the child
+ * {@link DeferrableSurface} will be managed by the owner of the child. For example, the
+ * parent could be {@link StreamSharing} and the child could be a {@link Preview}.
*/
static class SettableSurface extends DeferrableSurface {
diff --git a/camera/camera-core/src/test/java/androidx/camera/core/PreviewTest.kt b/camera/camera-core/src/test/java/androidx/camera/core/PreviewTest.kt
index b2692bc..768b8bb 100644
--- a/camera/camera-core/src/test/java/androidx/camera/core/PreviewTest.kt
+++ b/camera/camera-core/src/test/java/androidx/camera/core/PreviewTest.kt
@@ -176,6 +176,15 @@
}
@Test
+ fun createPreview_deferrableSurfaceIsTheSurfaceRequestSurface() {
+ // Act: Create a preview use case.
+ val preview = createPreview()
+ // Assert: The preview's deferrable surface is the surface request surface.
+ assertThat(preview.sessionConfig.surfaces.single())
+ .isEqualTo(preview.mCurrentSurfaceRequest!!.deferrableSurface)
+ }
+
+ @Test
fun verifySupportedEffects() {
val preview = Preview.Builder().build()
assertThat(preview.isEffectTargetsSupported(PREVIEW)).isTrue()
