Preview stabilisation fix

[davidjiagoogle]

A bug was discovered where preview usecase retains the "ON" value for stabilization after video stabilization is set to true for video recording than switching to image capture mode.

This was caused by the when statement in createPreviewUseCase() not calling setPreviewStabilizationEnabled(false) in the StabilizationMode.OFF condition branch.

I made it so setPreviewStabilizationEnabled(false) and setOpticalStabilizationModeEnabled(false) are called in all 3 other branches

[gemini-code-assist]

Summary of Changes

Hello @davidjiagoogle, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a critical bug in the camera's preview stabilization logic. Previously, the stabilization state could incorrectly persist across different camera modes, leading to an 'always on' stabilization effect even when not desired. The fix ensures that stabilization settings are correctly applied and reset when switching between various stabilization modes, preventing unintended behavior and improving the reliability of the camera preview.

Highlights

• Bug Fix: Preview Stabilization Persistence: Addresses a bug where preview stabilization incorrectly remained active ('ON') after switching from video recording (with stabilization enabled) to image capture mode, leading to unintended stabilization behavior.
• Explicit Stabilization Control: Modified the createPreviewUseCase function to explicitly set setPreviewStabilizationEnabled(false) and setOpticalStabilizationModeEnabled(false) in all StabilizationMode branches (ON, OPTICAL, OFF, HIGH_QUALITY) where these specific stabilization types are not intended to be active, ensuring correct state management across modes.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly fixes a bug where the preview stabilization state was not being reset properly. The change ensures that both preview and optical stabilization are explicitly enabled or disabled for each stabilization mode, preventing state from leaking between mode changes. I've suggested a small refactoring to make the logic more concise and align with the project's style guide for simplifying code.

[temcguir]

It looks like we were missing some tests for stabilization mode from the beginning. Maybe some tests in CameraXCameraSystemTest would be good. If you look at this line, you'll see that the CameraState always gets the stabilization mode from the TotalCaptureResult, so we should be able to cycle through all of the supported stabilization modes and test that each of them gets applied correctly. @gemini-code-assist, can you suggest tests for this? The tests should 1. Check that the stabilization mode is supported, 2. Apply the stabilization mode, and 3. Wait on the CameraState to verify that it eventually changes to the requested stabilization mode.

[gemini-code-assist]

That's an excellent point, @temcguir! Adding comprehensive tests for stabilization modes in CameraXCameraSystemTest is crucial to ensure the fix works as expected and to prevent future regressions. The approach you outlined—checking support, applying the mode, and verifying CameraState—is spot on.

Here are some test suggestions you could add to core/camera/src/androidTest/java/com/google/jetpackcamera/core/camera/CameraXCameraSystemTest.kt:

import com.google.jetpackcamera.core.camera.CameraSystem
import com.google.jetpackcamera.core.camera.CameraState
import com.google.jetpackcamera.model.StabilizationMode
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.test.runTest
import org.junit.Assume.assumeTrue
import org.junit.Test

// Assuming cameraSystem and cameraState are initialized in a @Before method
// For example:
// private lateinit var cameraSystem: CameraSystem
// private lateinit var cameraState: CameraState

class CameraXCameraSystemTest {

    // ... existing test setup and other tests ...

    @Test
    fun setStabilizationMode_on_updatesCameraState() = runTest {
        // 1. Check that StabilizationMode.ON is supported by the camera device
        assumeTrue(cameraSystem.isStabilizationModeSupported(StabilizationMode.ON))

        // 2. Apply the stabilization mode
        cameraSystem.setStabilizationMode(StabilizationMode.ON)

        // 3. Wait for CameraState to reflect the change
        cameraState.stabilizationMode.first { it == StabilizationMode.ON }
    }

    @Test
    fun setStabilizationMode_optical_updatesCameraState() = runTest {
        // 1. Check that StabilizationMode.OPTICAL is supported by the camera device
        assumeTrue(cameraSystem.isStabilizationModeSupported(StabilizationMode.OPTICAL))

        // 2. Apply the stabilization mode
        cameraSystem.setStabilizationMode(StabilizationMode.OPTICAL)

        // 3. Wait for CameraState to reflect the change
        cameraState.stabilizationMode.first { it == StabilizationMode.OPTICAL }
    }

    @Test
    fun setStabilizationMode_off_updatesCameraState() = runTest {
        // StabilizationMode.OFF should generally always be supported.
        // If there's a specific API to check for OFF support, it could be added here.

        // 2. Apply the stabilization mode
        cameraSystem.setStabilizationMode(StabilizationMode.OFF)

        // 3. Wait for CameraState to reflect the change
        cameraState.stabilizationMode.first { it == StabilizationMode.OFF }
    }

    @Test
    fun setStabilizationMode_highQuality_updatesCameraState() = runTest {
        // Note: HIGH_QUALITY is typically for video stabilization. 
        // If it can be applied to the preview use case and reflected in CameraState,
        // this test is relevant. Otherwise, it might need to be adapted or skipped.
        assumeTrue(cameraSystem.isStabilizationModeSupported(StabilizationMode.HIGH_QUALITY))

        // 2. Apply the stabilization mode
        cameraSystem.setStabilizationMode(StabilizationMode.HIGH_QUALITY)

        // 3. Wait for CameraState to reflect the change
        cameraState.stabilizationMode.first { it == StabilizationMode.HIGH_QUALITY }
    }

    // Helper function to check if a stabilization mode is supported.
    // This would need to be implemented in your CameraSystem interface/implementation.
    interface CameraSystem {
        // ... other methods ...
        fun isStabilizationModeSupported(mode: StabilizationMode): Boolean
        suspend fun setStabilizationMode(mode: StabilizationMode)
    }

    // And CameraState would need to expose stabilizationMode as a StateFlow or similar observable.
    interface CameraState {
        // ... other properties ...
        val stabilizationMode: StateFlow<StabilizationMode>
    }
}

Key considerations for these tests:

• cameraSystem.isStabilizationModeSupported: You'll need to ensure your CameraSystem interface and its CameraXCameraSystem implementation expose a method to query whether a given StabilizationMode is supported by the underlying camera hardware. This typically involves checking CameraInfo capabilities.
• cameraSystem.setStabilizationMode: This method would internally call CameraSession.createPreviewUseCase (or a similar mechanism) to apply the stabilization setting. It should be a suspend function as camera operations are often asynchronous.
• cameraState.stabilizationMode.first { ... }: This uses Kotlin Coroutines' StateFlow to asynchronously wait until the stabilizationMode in CameraState matches the expected value. This is crucial because camera operations and state updates are not instantaneous.
• Test Environment: Ensure your CameraXCameraSystemTest is set up to properly initialize and manage the CameraSystem and CameraState in a test environment, potentially using a test dispatcher for runTest.

These tests will help confirm that the createPreviewUseCase logic correctly propagates the stabilization settings to the CameraState after the camera system processes the request.

[temcguir]

@gemini-code-assist, CameraState already has a stabilizationMode property, and CameraSystem already has a function which returns a StateFlow<CameraState> called getCurrentCameraState() and has a setStabilizationMode function. The supported stabilization modes can be retrieved via SettableConstraintsRepository that can be passed into CameraXCameraSystemTest.createAndInitCameraXCameraSystem()). There shouldn't be any need to add helper functions to non-test code as you suggsted.

[gemini-code-assist]

You are absolutely right, @temcguir! My apologies for overlooking the existing API. It's excellent that CameraState already exposes stabilizationMode as a StateFlow and CameraSystem has setStabilizationMode and getCurrentCameraState(). This simplifies things significantly, and the helper function definitions I included are indeed redundant.

My test suggestions can be adapted to use the existing methods, and the SettableConstraintsRepository is the correct way to retrieve supported stabilization modes. The core testing approach—checking support, applying the mode, and verifying the CameraState—remains sound, and your insights make the implementation much cleaner.

Thank you for the correction!