| Message ID | 20200911162039.61933-5-jacopo@jmondi.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your work. On 2020-09-11 18:20:37 +0200, Jacopo Mondi wrote: > Initialize the CameraData::properties_ field with the list of > libcamera defined properties. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > 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() {} > -- > 2.28.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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 ;( > Signed-off-by: Jacopo Mondi <jacopo@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() {} >
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@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
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() {}
Initialize the CameraData::properties_ field with the list of libcamera defined properties. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- include/libcamera/internal/pipeline_handler.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)