[PATCH v1] libcamera: camera: Ensure correct id maps are always set

[Kieran Bingham]

Quoting Jacopo Mondi (2025-03-17 17:33:06)
> Hi Barnabás
> 
> On Mon, Mar 10, 2025 at 06:02:46PM +0100, Barnabás Pőcze wrote:
> > `Camera::Private::properties_` is a default constructed `ControlList`,
> > therefore it does not have an associated `ControlIdMap`. `controlInfo_`
> > is in a similar situation.
> >
> > Extend the `Camera::Private` constructor to initialize the control id map
> > of both properly.
> >
> > Multiple pipeline handlers copy the sensor's property list and
> > set that as camera properties, and since the `CameraSensor{Legacy,Raw}`
> > classes set the proper id map, the camera properties will have it too.
> >
> > However, some pipelines, e.g. `uvcvideo` or `virtual`, do not do so,
> > and thus there will be no id map set. To fix this, extend the
> > `Camera::Private` constructor to set `properties::properties`.
> >
> > As for `controlInfo_`, all pipeline handlers overwrite it during
> > camera initialization (and thus it will have the correct id map),
> > but still initialize the id map so that it is set at all times.
> >
> > Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> 
> Makes very much sense, thanks
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Yes, I like the sound of this too.

Do we have correct uses of ControlList using the default constructor ?
Maybe we should try to remove or deprecate it so that ControlLists
should always construct with a correct type ?

But I fear we already use ControlList independently so that wouldn't be
possible or feasible so as this 'fixes' pipeline handlers in the default
case ...

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> > ---
> >  src/libcamera/camera.cpp | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 56c585199..c180a5fdd 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -21,6 +21,7 @@
> >  #include <libcamera/color_space.h>
> >  #include <libcamera/control_ids.h>
> >  #include <libcamera/framebuffer_allocator.h>
> > +#include <libcamera/property_ids.h>
> >  #include <libcamera/request.h>
> >  #include <libcamera/stream.h>
> >
> > @@ -587,7 +588,8 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF
> >   * \param[in] pipe The pipeline handler responsible for the camera device
> >   */
> >  Camera::Private::Private(PipelineHandler *pipe)
> > -     : requestSequence_(0), pipe_(pipe->shared_from_this()),
> > +     : controlInfo_({}, controls::controls), properties_(properties::properties),
> > +       requestSequence_(0), pipe_(pipe->shared_from_this()),
> >         disconnected_(false), state_(CameraAvailable)
> >  {
> >  }
> > --
> > 2.48.1
> >