[libcamera-devel] [RFC 4/6] libcamera: pipeline_handler: Initialize properties_

[Jacopo Mondi]

Hi Kieran,

On Wed, Oct 07, 2020 at 08:38:17PM +0100, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 11/09/2020 17:20, Jacopo Mondi wrote:
> > Initialize the CameraData::properties_ field with the list of
> > libcamera defined properties.
> >
>
> Maybe it will be clearer in the following patches, but what is the
> purpose of this?
>
> This doesn't initialise it with any specific default values - just sets
> the properties are available. And perhaps not all properties will be
> available on every pipeline ? Or have I missed something obvious...
>
> This seems odd to me at the moment ;(

It is, it has puzzled me for the last hour or so.

My recollection was that every control list should be initialized with
an idMap_ to support the id-based interface. The id-based interface is
used by the CameraSensor to set/get controls on the subdevice and by
closed source IPAs that use custom controls.

I'm currently failing to find is where the idmap_ is used in the whole
ControlList or ControlSerializer.

I might have mixed up how the PipelineHandler::controls_ ControlInfoMap needs
to be initialized to specify control limits for application-modifiable
ControlList associated with a Request, and how
PipelineHandler::properties_, being immutable from application
perspective, just need to be filled in with ControlValues (and being
application visibile, it should only be inspected using libcamera
controls).

I've tested without this change, and it seems not required.
I'll keep looking as I've the feeling I'm really missing something
now, or got something mixed up at the time I implemented this.

>
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  include/libcamera/internal/pipeline_handler.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> > index a4e1b529c461..e579da711b33 100644
> > --- a/include/libcamera/internal/pipeline_handler.h
> > +++ b/include/libcamera/internal/pipeline_handler.h
> > @@ -17,6 +17,7 @@
> >
> >  #include <libcamera/controls.h>
> >  #include <libcamera/object.h>
> > +#include <libcamera/property_ids.h>
> >  #include <libcamera/stream.h>
> >
> >  #include "libcamera/internal/ipa_proxy.h"
> > @@ -37,7 +38,7 @@ class CameraData
> >  {
> >  public:
> >  	explicit CameraData(PipelineHandler *pipe)
> > -		: pipe_(pipe)
> > +		: pipe_(pipe), properties_(properties::properties)
> >  	{
> >  	}
> >  	virtual ~CameraData() {}
> >
>
> --
> Regards
> --
> Kieran