[libcamera-devel] [PATCH 2/3] libcamera: v4l2_device: Add support for integer array controls

[David Plowman]

Hi Laurent

Thanks for the review!

On Fri, 24 Dec 2021 at 03:16, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> On Thu, Dec 23, 2021 at 08:01:09AM +0000, David Plowman wrote:
>
> A commit message would be nice.
>
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  src/libcamera/v4l2_device.cpp | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index 62c91779..6f9de8ad 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -316,6 +316,18 @@ int V4L2Device::setControls(ControlList *ctrls)
> >                       break;
> >               }
> >
> > +             case ControlTypeInteger32: {
>
> Could you move this above ControlTypeInteger32 ?

I'm guessing you mean this should go between ControlTypeInteger64 and
ControlTypeIntegerByte, like I see in updateControls()? Will do!

>
> > +                     if (value.isArray()) {
> > +                             Span<uint8_t> data = value.data();
> > +                             v4l2Ctrl.p_u32 = reinterpret_cast<uint32_t *>(data.data());
> > +                             v4l2Ctrl.size = data.size();
> > +                     } else {
> > +                             v4l2Ctrl.value = value.get<int32_t>();
> > +                     }
>
> This looks fine, but I think you also need to update
> V4L2Device::updateControls().

Just to check (my understanding of how ControlLists work has always
been a bit hazy...), the same logic will apply there as in the "Byte"
case, i.e. I just need to check for the array type and then do
nothing?

Thanks!
David

>
> > +
> > +                     break;
> > +             }
> > +
> >               default:
> >                       /* \todo To be changed to support strings. */
> >                       v4l2Ctrl.value = value.get<int32_t>();
>
> --
> Regards,
>
> Laurent Pinchart