[PATCH] android: camera_device: The first valid settings should be saved

[Jacopo Mondi]

Hi Fang Hui

In subject:

android: camera_device: Save capture settings unconditionally

On Mon, Feb 26, 2024 at 11:08:12AM +0800, Fang Hui wrote:
> If not, it will be deferred to the next frame.

This commit message is really too short and does not explain why this
is an issue. With your ack I would change it to:

------------------------------------------------------------------------------
As the Android framework sends to the camera device settings
incrementally (only the ones that change are updated), the CameraDevice
class in the Android camera HAL keeps a copy of the last received
settings to be able to apply controls to the libcamera Camera and to
populate metadata correctly.

When a valid 'camera3Request->settings' is provided, it gets saved to
'lastSettings_' but 'descriptor->settings_' is not initialized until
the next frame (assuming it does not contain more settings).

Fix this by assigning to 'descriptor->settings_' the last saved
settings unconditionally.
------------------------------------------------------------------------------

And now that I wrote the last sentence, I realized lastSettings_ is
not re-initialized between streaming sessions.

You will likely need:

--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1108,6 +1108,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
        }

        if (state_ == State::Stopped) {
+               lastSettings_ = {};
+
                ret = camera_->start();
                if (ret) {
                        LOG(HAL, Error) << "Failed to start camera";

Which I can again add when applying with your ack.

Thanks
   j

>
> Signed-off-by: Fang Hui <hui.fang at nxp.com>
> ---
>  src/android/camera_device.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 25cedd44..d45ed1a5 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -952,8 +952,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	 */
>  	if (camera3Request->settings)
>  		lastSettings_ = camera3Request->settings;
> -	else
> -		descriptor->settings_ = lastSettings_;
> +
> +	descriptor->settings_ = lastSettings_;
>
>  	LOG(HAL, Debug) << "Queueing request " << descriptor->request_->cookie()
>  			<< " with " << descriptor->buffers_.size() << " streams";
> --
> 2.25.1
>