Implement LowLightBoost Capture #236

yasith · 2024-06-25T17:09:42Z

This PR contains the following.

Low Light Boost(LLB) feature toggle is enabled, if the device supports LLB
When LLB toggle is turned on, switch to multi-stream mode, and enable LLB capture with Camera2Interop. Show the outlined LLB indicator with 50% opacity
Query for the LLB active status CONTROL_LOW_LIGHT_BOOST_STATE_ACTIVE, if active change the outlined LLB indicator to a filled indicator with full opacity

temcguir · 2024-06-26T17:23:58Z

domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraXCameraUseCase.kt

+    @androidx.annotation.OptIn(ExperimentalCamera2Interop::class)
+    @OptIn(BuildCompat.PrereleaseSdkCheck::class)
+    private fun getLowLightBoostDeviceSupport() = when (BuildCompat.isAtLeastV()) {
+        true -> cameraProvider.availableCameraInfos.map { cameraInfo ->
+            Camera2CameraInfo
+                .from(cameraInfo)
+                .getCameraCharacteristic(CameraCharacteristics.CONTROL_AE_AVAILABLE_MODES)
+                ?.contains(
+                    CameraMetadata.CONTROL_AE_MODE_ON_LOW_LIGHT_BOOST_BRIGHTNESS_PRIORITY
+                )
+        }.any()
+        false -> false
+    }
+


We should be checking for support on a per-lens basis. This can be rewritten as an extension property to make it easier:

private val CameraInfo.isLowLightBoostSupported: Boolean @androidx.annotation.OptIn(ExperimentalCamera2Interop::class) @OptIn(BuildCompat.PrereleaseSdkCheck::class) get() = BuildCompat.isAtLeastV() && Camera2CameraInfo .from(this) .getCameraCharacteristic(CameraCharacteristics.CONTROL_AE_AVAILABLE_MODES) ?.contains( CameraMetadata.CONTROL_AE_MODE_ON_LOW_LIGHT_BOOST_BRIGHTNESS_PRIORITY ) ?: false

Then in the CameraConstraints (line 197), we can say:

lowLightBoostSupport = camInfo.isLowLightBoostSupported

temcguir · 2024-06-26T17:28:38Z

domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraXCameraUseCase.kt

+                currentSettings.update { old ->
+                    old?.copy(
+                        lowLightBoost = lowLightBoost,
+                        captureMode = CaptureMode.MULTI_STREAM,


What if the CaptureMode was SINGLE_STREAM before disabling low-light boost?

If we can only support LLB in SINGLE_STREAM mode, then we should do something similar to what we did with CameraConstraints.supportedImageFormatsMap where we have a Map<CaptureMode, Boolean>. We can then determine in the UI whether we should allow users to enable LLB depending on which capture mode is enabled (otherwise we disable the button).

temcguir · 2024-06-26T17:38:30Z

domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraXCameraUseCase.kt

+                camera2Interop.setSessionCaptureCallback(object : CaptureCallback() {
+                    override fun onCaptureCompleted(
+                        session: CameraCaptureSession,
+                        request: CaptureRequest,
+                        result: TotalCaptureResult
+                    ) {
+                        super.onCaptureCompleted(session, request, result)
+                        val boostState = result.get(CaptureResult.CONTROL_LOW_LIGHT_BOOST_STATE)
+                        if (boostState == CameraMetadata.CONTROL_LOW_LIGHT_BOOST_STATE_ACTIVE) {
+                            Log.d(TAG, "LLB Active")
+                            _lowLightBoostActiveStatus.value = true
+                        } else {
+                            Log.d(TAG, "LLB Inactive")
+                            _lowLightBoostActiveStatus.value = false
+                        }
+                    }
+                })


We should add this callback regardless of whether LLB is enabled. Technically the CONTROL_AE_MODE can be set at any time (without needing to rebind use cases). So it could go from the DISABLED -> ENABLED state without needing to recreate / rebind Preview.

temcguir · 2024-06-26T17:40:43Z

domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraXCameraUseCase.kt

+                camera2Interop.setCaptureRequestOption(
+                    CaptureRequest.CONTROL_AE_MODE,
+                    CameraMetadata.CONTROL_AE_MODE_ON_LOW_LIGHT_BOOST_BRIGHTNESS_PRIORITY
+                )


This should be done using Camera2CameraControl rather than adding it to the Preview.Builder. It's similar to the CameraControl.setZoomRatio or setFlashModeInternal that we set in the cameraProvider.runWith {} block.

temcguir · 2024-06-26T17:42:43Z

domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraXCameraUseCase.kt

@@ -834,6 +883,34 @@ constructor(
            getResolutionSelector(sessionSettings.aspectRatio)
        )

+        if (currentSettings.value?.lowLightBoost == LowLightBoost.ENABLED) {


Rather than reading this from currentSettings, lowLightBoost should be added to TransientSessionSettings and pulled out of settings in the the currentSettings Flow in runCamera(). That way we know we have a valid snapshot whenever we rebind the camera.

temcguir · 2024-06-26T17:47:01Z

feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt

+                cameraUseCase.getZoomScale(),
+                cameraUseCase.getLowLightBoostActiveStatus()


We're going to start to run out of the ability to combine these flows if we keep adding more. combine only has overloads for up to 5 flows (with unique types). Since zoomScale and LLBActiveStatus are both descriptive camera state, maybe we can combine them into a single data class called DescriptiveCameraState? Then we can have a single flow for both from CameraUseCase.

temcguir · 2024-06-26T17:49:11Z

...preview/src/main/java/com/google/jetpackcamera/feature/preview/ui/PreviewScreenComponents.kt

                contentDescription =
                stringResource(id = R.string.quick_settings_lowlightboost_enabled),
-                modifier = modifier.alpha(0.5f)
+                modifier = modifier.alpha(if (active) 1.0f else 0.5f)


Disabled states should use an alpha of 0.38f. See https://m3.material.io/foundations/interaction/states/applying-states#4aff9c51-d20f-4580-a510-862d2e25e931

jsaund · 2024-10-06T15:02:39Z

domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraUseCase.kt

@@ -73,6 +73,8 @@ interface CameraUseCase {

    fun getZoomScale(): StateFlow<Float>

+    fun getLowLightBoostActiveStatus(): StateFlow<Boolean>


Have you considered defining an enum for this? Maybe UNSUPPORTED, ACTIVE, INACTIVE ?
That way the client only needs to observe this and not isLowLightBoostSupport and the default can be UNSUPPORTED rather than true

jsaund · 2024-10-06T15:04:26Z

domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraXCameraUseCase.kt

@@ -541,6 +566,10 @@ constructor(
    private val _zoomScale = MutableStateFlow(1f)
    override fun getZoomScale(): StateFlow<Float> = _zoomScale.asStateFlow()

+    private val _lowLightBoostActiveStatus = MutableStateFlow(true)


This would mean that low light is active?
a) maybe the enum approach I suggested above might be better?
b) maybe starting as inactive might be better?
c) if I got the active / inactive wrong then it might be better to type it to an enum to avoid confusion

jsaund · 2024-10-06T15:09:00Z

domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraXCameraUseCase.kt

+        if (currentSettings.value?.lowLightBoost == LowLightBoost.ENABLED) {
+            Log.d(TAG, "Adding Control AE_MODE_ON_LOW_LIGHT_BOOST_BRIGHTNESS_PRIORITY")
+            val camera2Interop = Camera2Interop.Extender(previewUseCaseBuilder)
+            if (Build.VERSION.SDK_INT >= 35) {


nit: should we use BuildCompat.isAtLeastV() instead?

jsaund · 2024-10-06T15:18:46Z

domain/camera/src/main/java/com/google/jetpackcamera/domain/camera/CameraXCameraUseCase.kt

+                        result: TotalCaptureResult
+                    ) {
+                        super.onCaptureCompleted(session, request, result)
+                        val boostState = result.get(CaptureResult.CONTROL_LOW_LIGHT_BOOST_STATE)


Not sure if this will change anything for your use case but sharing for context:

LOW_LIGHT_BOOST_STATE is only active / inactive if CaptureResult.CONTROL_AE_MODE is AE_ON_LOW_LIGHT_BOOST_BRIGHTNESS_PRIORITY

So if the current camera feature combination wasn't supported when LLB was requested, the result may switch back to AE_MODE_ON

Or if the AE_MODE was not LLB, then maybe hide the moon icon?

Unless if the UI keeps the moon icon there but in an INACTIVE state? When pressed it will turn on LLB? But that might not be intuitive because the scene could be well lit and still be inactive so it wouldn't be clear to the user when AE_MODE is LLB or auto.

yasith added 4 commits June 25, 2024 12:46

Create CameraConstraints based on LowLightBoost feature availability

6773fe8

Switch to low light boost mode, when enabled

8ab19d2

Show low light boost active status through the low light boost indicator

e89d9d5

Apply spotless

4015e66

yasith requested review from temcguir and Kimblebee June 25, 2024 17:09

temcguir requested changes Jun 26, 2024

View reviewed changes

Check LLB compatibility only for the selected lens

73c0e55

temcguir mentioned this pull request Aug 20, 2024

Add dual concurrent camera for devices that support it #258

Merged

yasith requested a review from jsaund October 3, 2024 20:48

jsaund reviewed Oct 6, 2024

View reviewed changes

yasith mentioned this pull request Oct 9, 2024

Add LLB as a FlashMode, and remove existing LLB toggle related settings #269

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement LowLightBoost Capture #236

Implement LowLightBoost Capture #236

yasith commented Jun 25, 2024

temcguir Jun 26, 2024

temcguir Jun 26, 2024

temcguir Jun 26, 2024

temcguir Jun 26, 2024

temcguir Jun 26, 2024

temcguir Jun 26, 2024

temcguir Jun 26, 2024

jsaund Oct 6, 2024

jsaund Oct 6, 2024

jsaund Oct 6, 2024

jsaund Oct 6, 2024

		cameraUseCase.getZoomScale(),
		cameraUseCase.getLowLightBoostActiveStatus()

		@@ -73,6 +73,8 @@ interface CameraUseCase {

		fun getZoomScale(): StateFlow<Float>

		fun getLowLightBoostActiveStatus(): StateFlow<Boolean>

Implement LowLightBoost Capture #236

Are you sure you want to change the base?

Implement LowLightBoost Capture #236

Conversation

yasith commented Jun 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment