[RFC,2/2] libcamera: request: Create control list with Camera info map

25161 diff mbox series

The control lists associated with a Request is created with the global
libcamera::controls::controls id map.

This is fine as the idmap/info map used to contruct a ControlList are not
used for any control validation purposes by the libcamera code.

However creating a ControlList with the camera ControlInfoMap has two
advantages:

1) The idmap can be extracted from the info map, but not the other way
   around
2) The control list is constructed with a valid map of info to the
   controls it can actually supports, instead of the global map of
   libcamera controls

Initialize the ControlList part of a Request with the Camera control
info map, as the association between a Request and a Camera is permanent
and valid for the whole lifetime of a Request.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/libcamera/request.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Quoting Jacopo Mondi (2025-11-24 16:43:45)
> The control lists associated with a Request is created with the global
> libcamera::controls::controls id map.
> 
> This is fine as the idmap/info map used to contruct a ControlList are not
> used for any control validation purposes by the libcamera code.
> 
> However creating a ControlList with the camera ControlInfoMap has two
> advantages:
> 
> 1) The idmap can be extracted from the info map, but not the other way
>    around
> 2) The control list is constructed with a valid map of info to the
>    controls it can actually supports, instead of the global map of
>    libcamera controls
> 
> Initialize the ControlList part of a Request with the Camera control
> info map, as the association between a Request and a Camera is permanent
> and valid for the whole lifetime of a Request.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/libcamera/request.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 60565f5984971c7ab34e119a15b8141799f71071..2544a059f6984d930ec909c74e0b621c9fe82726 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -356,7 +356,7 @@ Request::Request(Camera *camera, uint64_t cookie)
>         : Extensible(std::make_unique<Private>(camera)),
>           cookie_(cookie), status_(RequestPending)
>  {
> -       controls_ = new ControlList(controls::controls,
> +       controls_ = new ControlList(camera->controls(),

I see what you mean, this does look so obvious it should have been like
this from the start ...

Does the Request now have a copy of the info map - or will it have an
internal pointer to the most up to date info map ? (As in - will it
always be up to date when things like exposure limits are updated?)

--
Kieran

>                                     camera->_d()->validator());
>  
>         /**
> 
> -- 
> 2.51.1
>

Hi Kieran

On Mon, Nov 24, 2025 at 05:13:00PM +0000, Kieran Bingham wrote:
> Quoting Jacopo Mondi (2025-11-24 16:43:45)
> > The control lists associated with a Request is created with the global
> > libcamera::controls::controls id map.
> >
> > This is fine as the idmap/info map used to contruct a ControlList are not
> > used for any control validation purposes by the libcamera code.
> >
> > However creating a ControlList with the camera ControlInfoMap has two
> > advantages:
> >
> > 1) The idmap can be extracted from the info map, but not the other way
> >    around
> > 2) The control list is constructed with a valid map of info to the
> >    controls it can actually supports, instead of the global map of
> >    libcamera controls
> >
> > Initialize the ControlList part of a Request with the Camera control
> > info map, as the association between a Request and a Camera is permanent
> > and valid for the whole lifetime of a Request.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  src/libcamera/request.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> > index 60565f5984971c7ab34e119a15b8141799f71071..2544a059f6984d930ec909c74e0b621c9fe82726 100644
> > --- a/src/libcamera/request.cpp
> > +++ b/src/libcamera/request.cpp
> > @@ -356,7 +356,7 @@ Request::Request(Camera *camera, uint64_t cookie)
> >         : Extensible(std::make_unique<Private>(camera)),
> >           cookie_(cookie), status_(RequestPending)
> >  {
> > -       controls_ = new ControlList(controls::controls,
> > +       controls_ = new ControlList(camera->controls(),
>
> I see what you mean, this does look so obvious it should have been like
> this from the start ...

Isn't it ? I feel like I'm overlooking something

>
> Does the Request now have a copy of the info map - or will it have an
> internal pointer to the most up to date info map ? (As in - will it
> always be up to date when things like exposure limits are updated?)

Granted we still don't have a reliable way to update ControlInfo as
per my latest discussions on wanting to updated ExposureTime limits on
FrameDurationLimits change, infoMap_ is a pointer in ControlList, so
no copies, just a reference.

Thanks
  j

>
> --
> Kieran
>
> >                                     camera->_d()->validator());
> >
> >         /**
> >
> > --
> > 2.51.1
> >