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

[Jacopo Mondi]

Hello

On Wed, Feb 28, 2024 at 05:30:25AM +0000, Hui Fang wrote:
> commit 3a5da02954c7ab76a0c939eb1f806c3d529c7ec9 (HEAD -> master)
> Author: Fang Hui <hui.fang at nxp.com>
> Date:   Thu Jan 25 02:11:04 2024 +0800
>
>     android: camera_device: Save capture settings unconditionally
>
>     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.
>
>     Bug: https://bugs.libcamera.org/show_bug.cgi?id=210
>
>     Signed-off-by: Fang Hui <hui.fang at nxp.com>

Now tested with CTS, no regressions detected

Tested-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

I'll merge it!

Thanks
  j

>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 25cedd44..d2679a97 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";
> @@ -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";
> ________________________________
> From: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> Sent: Monday, February 26, 2024 10:21 PM
> To: Hui Fang <hui.fang at nxp.com>
> Cc: libcamera-devel at lists.libcamera.org <libcamera-devel at lists.libcamera.org>; biomifang118 at gmail.com <biomifang118 at gmail.com>
> Subject: [EXT] Re: [PATCH] android: camera_device: The first valid settings should be saved
>
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
>
>
> Also,
>   as there's a bug assigned to this issue
>
> On Mon, Feb 26, 2024 at 11:08:12AM +0800, Fang Hui wrote:
> > If not, it will be deferred to the next frame.
> >
>
> Bug: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.libcamera.org%2Fshow_bug.cgi%3Fid%3D210&data=05%7C02%7Chui.fang%40nxp.com%7Cc9341f6fe9624af769f108dc36d64722%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638445541153242800%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=2JewcBFhgU4K%2BDofrUZLQUduptmwAwDcomPEXwQ3Ybw%3D&reserved=0<https://bugs.libcamera.org/show_bug.cgi?id=210>
>
> > 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
> >