The first valid settings should be saved

[Jacopo Mondi]

Hello Fang Hui
   thanks for your patch

The patch does not seem to be in a valid format, it is being
copied into an email ? Could you have a read to https://git-send-email.io/
and try it ? :)

On Fri, Feb 23, 2024 at 11:26:29AM +0800, hui fang wrote:
> commit 4c220580fc2c71a978dc768619b6e65f3a5aea4a (HEAD -> master)
> Author: Fang Hui <hui.fang at nxp.com>
> Date:   Thu Jan 25 02:11:04 2024 +0800
>
>     android: camera_device: The first valid settings should be saved
>
>     If not, it will be deferred to the next frame.

On the patch itself, if I get the issue right, the problem is that the
very first frame won't have descriptor->settings_ initialized and you
patch indeed fixes this. How did you detect the problem, did you get run-time
errors or was that just from looking at the code (I'm asking to know
what test I could run to reproduce the issue).

Thanks
  j

>
>     Signed-off-by: Fang Hui <hui.fang at nxp.com>
>
> 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";
>
>
> BRs,
> Fang Hui
>
> Laurent Pinchart <laurent.pinchart at ideasonboard.com> 于2024年2月21日周三 21:13写道：
>
> > Hi Hui,
> >
> > Thank you for the patch.
> >
> > On Wed, Feb 21, 2024 at 01:38:00PM +0800, hui fang wrote:
> > > The first valid settings should be saved, or will be deferred to the
> > next frame.
> >
> > This is missing a Signed-off-by tag, and the subject is missing a
> > prefix. Both issues should have been caught by the checkstyle.py
> > utility part of libcamera.
> >
> > Please see https://libcamera.org/contributing.html#submitting-patches
> > for more information about Signed-off-by, and
> > https://libcamera.org/coding-style.html#tools for more information about
> > the coding style and checkstyle.py utility.
> >
> > > 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_;
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >