Rename VirtualCameraManager and put it behind an interface
Rename VirtualCameraManager to Camera2DeviceManagerImpl and put it
behind an interface, Camera2DeviceManager.
This is done in order to create fakes for the camera manager, enabling
future unit tests, as well as allow us to swap out the implementation in
the eventual camera manager rewrite.
Bug: 372258646
Test: CameraPipe sample app.
Change-Id: I40f81e6e54ff80461de1ecceaea43a0dde1d9953
diff --git a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2Backend.kt b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2Backend.kt
index b7eb8bb..79900402 100644
--- a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2Backend.kt
+++ b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2Backend.kt
@@ -39,7 +39,7 @@
constructor(
private val camera2DeviceCache: Camera2DeviceCache,
private val camera2MetadataCache: Camera2MetadataCache,
- private val virtualCameraManager: VirtualCameraManager,
+ private val camera2DeviceManager: Camera2DeviceManager,
private val camera2CameraControllerComponent: Camera2ControllerComponent.Builder,
) : CameraBackend {
override val id: CameraBackendId
@@ -59,31 +59,31 @@
camera2MetadataCache.awaitCameraMetadata(cameraId)
override fun disconnect(cameraId: CameraId) {
- virtualCameraManager.close(cameraId)
+ camera2DeviceManager.close(cameraId)
}
override fun disconnectAsync(cameraId: CameraId): Deferred<Unit> {
TODO(
- "b/324142928 - Add support in VirtualCameraManager for closing a camera " +
+ "b/324142928 - Add support in Camera2DeviceManager for closing a camera " +
"with a deferred result."
)
}
override fun disconnectAll() {
- return virtualCameraManager.closeAll()
+ return camera2DeviceManager.closeAll()
}
override fun disconnectAllAsync(): Deferred<Unit> {
TODO(
- "b/324142928 - Add support in VirtualCameraManager for closing a camera " +
+ "b/324142928 - Add support in Camera2DeviceManager for closing a camera " +
"with a deferred result."
)
}
override fun shutdownAsync(): Deferred<Unit> {
- // TODO: VirtualCameraManager needs to be extended to support a suspendable future that can
+ // TODO: Camera2DeviceManager needs to be extended to support a suspendable future that can
// be used to wait until close has been called on all camera devices.
- virtualCameraManager.closeAll()
+ camera2DeviceManager.closeAll()
return CompletableDeferred(Unit)
}
@@ -113,6 +113,6 @@
}
override fun prewarm(cameraId: CameraId) {
- virtualCameraManager.prewarm(cameraId)
+ camera2DeviceManager.prewarm(cameraId)
}
}
diff --git a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2CameraController.kt b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2CameraController.kt
index fd75f74..4023f2d 100644
--- a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2CameraController.kt
+++ b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2CameraController.kt
@@ -64,7 +64,7 @@
private val cameraStatusMonitor: CameraStatusMonitor,
private val captureSessionFactory: CaptureSessionFactory,
private val captureSequenceProcessorFactory: Camera2CaptureSequenceProcessorFactory,
- private val virtualCameraManager: VirtualCameraManager,
+ private val camera2DeviceManager: Camera2DeviceManager,
private val cameraSurfaceManager: CameraSurfaceManager,
private val timeSource: TimeSource,
override val cameraGraphId: CameraGraphId
@@ -162,10 +162,11 @@
}
lastCameraError = null
val camera =
- virtualCameraManager.open(
- graphConfig.camera,
- graphConfig.sharedCameraIds,
- graphListener,
+ camera2DeviceManager.open(
+ cameraId = graphConfig.camera,
+ sharedCameraIds = graphConfig.sharedCameraIds,
+ graphListener = graphListener,
+ isPrewarm = false,
) { _ ->
isForeground
}
@@ -292,7 +293,7 @@
disconnectSessionAndCamera(session, camera)
if (graphConfig.flags.closeCameraDeviceOnClose) {
Log.debug { "Quirk: Closing all camera devices" }
- virtualCameraManager.closeAll()
+ camera2DeviceManager.closeAll()
}
}
diff --git a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/VirtualCameraManager.kt b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2DeviceManager.kt
similarity index 80%
rename from camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/VirtualCameraManager.kt
rename to camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2DeviceManager.kt
index 8d0f429..8c932545 100644
--- a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/VirtualCameraManager.kt
+++ b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2DeviceManager.kt
@@ -51,8 +51,7 @@
* executed sequentially, as the camera may take a while to be fully opened, and RequestClose()
* might execute in parallel.
*/
-internal data class RequestClose(val activeCamera: VirtualCameraManager.ActiveCamera) :
- CameraRequest()
+internal data class RequestClose(val activeCamera: ActiveCamera) : CameraRequest()
internal data class RequestCloseById(val activeCameraId: CameraId) : CameraRequest()
@@ -68,35 +67,120 @@
override fun onGraphError(graphStateError: GraphState.GraphStateError) {}
}
-// A queue depth of 32 was deemed necessary in b/276051078 where a flood of requests can cause the
-// queue depth to go over 8. In the long run, we can perhaps look into refactoring and
-// reimplementing the request queue in a more robust way.
+internal interface Camera2DeviceManager {
+ /**
+ * Issue a request to open the specified camera. The camera will be delivered through
+ * [VirtualCamera.state] when opened, and the state will continue to provide updates to the
+ * state of the camera. If shared camera IDs are specified, the cameras won't be provided until
+ * all cameras are opened.
+ */
+ fun open(
+ cameraId: CameraId,
+ sharedCameraIds: List<CameraId>,
+ graphListener: GraphListener,
+ isPrewarm: Boolean,
+ isForegroundObserver: (Unit) -> Boolean,
+ ): VirtualCamera?
+
+ /**
+ * Connects and starts the underlying camera. Once the active camera timeout elapses and it
+ * hasn't been utilized, the camera is closed.
+ */
+ fun prewarm(cameraId: CameraId)
+
+ /** Submits a request to close the underlying camera. */
+ fun close(cameraId: CameraId)
+
+ /** Instructs Camera2DeviceManager to close all cameras. */
+ fun closeAll()
+}
+
+internal class ActiveCamera(
+ private val androidCameraState: AndroidCameraState,
+ internal val allCameraIds: Set<CameraId>,
+ scope: CoroutineScope,
+ channel: SendChannel<CameraRequest>
+) {
+ val cameraId: CameraId
+ get() = androidCameraState.cameraId
+
+ private val listenerJob: Job
+ private var current: VirtualCameraState? = null
+
+ private val wakelock =
+ WakeLock(
+ scope,
+ timeout = 1000,
+ callback = { channel.trySend(RequestClose(this)).isSuccess },
+ // Every ActiveCamera is associated with an opened camera. We should ensure that we
+ // issue a RequestClose eventually for every ActiveCamera created.
+ //
+ // A notable bug is b/264396089 where, because camera opens took too long, we didn't
+ // acquire a WakeLockToken, and thereby not issuing the request to close camera
+ // eventually.
+ startTimeoutOnCreation = true
+ )
+
+ init {
+ listenerJob =
+ scope.launch {
+ androidCameraState.state.collect {
+ if (it is CameraStateClosing || it is CameraStateClosed) {
+ wakelock.release()
+ this.cancel()
+ }
+ }
+ }
+ }
+
+ suspend fun connectTo(virtualCameraState: VirtualCameraState) {
+ val token = wakelock.acquire()
+ val previous = current
+ current = virtualCameraState
+
+ previous?.disconnect()
+ virtualCameraState.connect(androidCameraState.state, token)
+ }
+
+ fun close() {
+ wakelock.release()
+ androidCameraState.close()
+ }
+
+ suspend fun awaitClosed() {
+ androidCameraState.awaitClosed()
+ }
+}
+
+// TODO: b/307396261 - A queue depth of 64 was deemed necessary in b/276051078 and b/307396261 where
+// a flood of requests can cause the queue depth to grow larger than anticipated. Rewrite the
+// camera manager such that it handles these abnormal scenarios more robustly.
private const val requestQueueDepth = 64
@Suppress("EXPERIMENTAL_API_USAGE")
@Singleton
-internal class VirtualCameraManager
+internal class Camera2DeviceManagerImpl
@Inject
constructor(
private val permissions: Permissions,
private val retryingCameraStateOpener: RetryingCameraStateOpener,
private val camera2ErrorProcessor: Camera2ErrorProcessor,
private val threads: Threads
-) {
+) : Camera2DeviceManager {
// TODO: Consider rewriting this as a MutableSharedFlow
private val requestQueue: Channel<CameraRequest> = Channel(requestQueueDepth)
private val activeCameras: MutableSet<ActiveCamera> = mutableSetOf()
private val pendingRequestOpens = mutableListOf<RequestOpen>()
init {
- threads.globalScope.launch(CoroutineName("CXCP-VirtualCameraManager")) { requestLoop() }
+ threads.globalScope.launch(CoroutineName("CXCP-Camera2DeviceManager")) { requestLoop() }
}
- internal fun open(
+ override fun open(
cameraId: CameraId,
sharedCameraIds: List<CameraId>,
graphListener: GraphListener,
- isPrewarm: Boolean = false,
+ isPrewarm: Boolean,
isForegroundObserver: (Unit) -> Boolean,
): VirtualCamera? {
val result = VirtualCameraState(cameraId, graphListener, threads.globalScope)
@@ -105,7 +189,7 @@
RequestOpen(result, sharedCameraIds, graphListener, isPrewarm, isForegroundObserver)
)
) {
- Log.error { "Camera open request failed: VirtualCameraManager queue size exceeded" }
+ Log.error { "Camera open request failed: Camera2DeviceManagerImpl queue size exceeded" }
graphListener.onGraphError(
GraphState.GraphStateError(
CameraError.ERROR_CAMERA_OPENER,
@@ -117,20 +201,22 @@
return result
}
- /**
- * Connects and starts the underlying camera. Once the, ActiveCamera, timeout elapses and it
- * hasn't been utilized, the camera is closed.
- */
- internal fun prewarm(cameraId: CameraId) {
- open(cameraId, emptyList(), NoOpGraphListener, isPrewarm = true) { _ -> false }
+ override fun prewarm(cameraId: CameraId) {
+ open(
+ cameraId = cameraId,
+ sharedCameraIds = emptyList(),
+ graphListener = NoOpGraphListener,
+ isPrewarm = true,
+ ) { _ ->
+ false
+ }
}
- /** Submits a request to close the underlying camera */
- internal fun close(cameraId: CameraId) {
+ override fun close(cameraId: CameraId) {
offerChecked(RequestCloseById(cameraId))
}
- internal fun closeAll() {
+ override fun closeAll() {
if (!offerChecked(RequestCloseAll)) {
Log.warn { "Failed to close all cameras: Close request submission failed" }
return
@@ -367,63 +453,6 @@
}
}
- internal class ActiveCamera(
- private val androidCameraState: AndroidCameraState,
- internal val allCameraIds: Set<CameraId>,
- scope: CoroutineScope,
- channel: SendChannel<CameraRequest>
- ) {
- val cameraId: CameraId
- get() = androidCameraState.cameraId
-
- private val listenerJob: Job
- private var current: VirtualCameraState? = null
-
- private val wakelock =
- WakeLock(
- scope,
- timeout = 1000,
- callback = { channel.trySend(RequestClose(this)).isSuccess },
- // Every ActiveCamera is associated with an opened camera. We should ensure that we
- // issue a RequestClose eventually for every ActiveCamera created.
- //
- // A notable bug is b/264396089 where, because camera opens took too long, we didn't
- // acquire a WakeLockToken, and thereby not issuing the request to close camera
- // eventually.
- startTimeoutOnCreation = true
- )
-
- init {
- listenerJob =
- scope.launch {
- androidCameraState.state.collect {
- if (it is CameraStateClosing || it is CameraStateClosed) {
- wakelock.release()
- this.cancel()
- }
- }
- }
- }
-
- suspend fun connectTo(virtualCameraState: VirtualCameraState) {
- val token = wakelock.acquire()
- val previous = current
- current = virtualCameraState
-
- previous?.disconnect()
- virtualCameraState.connect(androidCameraState.state, token)
- }
-
- fun close() {
- wakelock.release()
- androidCameraState.close()
- }
-
- suspend fun awaitClosed() {
- androidCameraState.awaitClosed()
- }
- }
-
/**
* There are 3 possible scenarios with [OpenVirtualCameraResult]. Suppose we denote the values
* in pairs of ([activeCamera], [lastCameraError]):
diff --git a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2ErrorProcessor.kt b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2ErrorProcessor.kt
index 048d080..b1acb28 100644
--- a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2ErrorProcessor.kt
+++ b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/compat/Camera2ErrorProcessor.kt
@@ -26,7 +26,7 @@
/**
* A class responsible for reporting camera errors with a particular [CameraId]. When
- * [androidx.camera.camera2.pipe.compat.VirtualCameraManager] processes a camera open request, it
+ * [androidx.camera.camera2.pipe.compat.Camera2DeviceManager] processes a camera open request, it
* should update CameraErrorProcessor with the [VirtualCameraState] that came with the open request.
*/
@Singleton
diff --git a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/config/Camera2Component.kt b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/config/Camera2Component.kt
index 7f403c5..7684737 100644
--- a/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/config/Camera2Component.kt
+++ b/camera/camera-camera2-pipe/src/main/java/androidx/camera/camera2/pipe/config/Camera2Component.kt
@@ -33,6 +33,8 @@
import androidx.camera.camera2.pipe.compat.Camera2CaptureSessionsModule
import androidx.camera.camera2.pipe.compat.Camera2DeviceCloser
import androidx.camera.camera2.pipe.compat.Camera2DeviceCloserImpl
+import androidx.camera.camera2.pipe.compat.Camera2DeviceManager
+import androidx.camera.camera2.pipe.compat.Camera2DeviceManagerImpl
import androidx.camera.camera2.pipe.compat.Camera2ErrorProcessor
import androidx.camera.camera2.pipe.compat.Camera2MetadataCache
import androidx.camera.camera2.pipe.compat.Camera2MetadataProvider
@@ -59,6 +61,11 @@
@DefaultCameraBackend
abstract fun bindCameraPipeCameraBackend(camera2Backend: Camera2Backend): CameraBackend
+ @Binds
+ abstract fun bindCamera2DeviceManager(
+ camera2DeviceManager: Camera2DeviceManagerImpl
+ ): Camera2DeviceManager
+
@Binds abstract fun bindCameraOpener(camera2CameraOpener: Camera2CameraOpener): CameraOpener
@Binds
