[libcamera-devel] [RFC 5/6] libcamera: ipu3: Initialize draft properties

[Jacopo Mondi]

Hi Kieran,

On Wed, Oct 07, 2020 at 07:36:41PM +0100, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 13/09/2020 12:39, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2020-09-11 18:20:38 +0200, Jacopo Mondi wrote:
> >> Initialize three draft properties for the IPU3 platform.
> >>
> >> IPU3 reports a maximum of three processing stages: exposure, capture and
> >> ISP processing and does not support noise reduction and color
> >> aberration.
> >>
> >> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> >
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >
> >> ---
> >>  src/libcamera/pipeline/ipu3/ipu3.cpp | 10 +++++++++-
> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> index 2d881fe28f98..9ce329a83f5d 100644
> >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> @@ -13,6 +13,7 @@
> >>
> >>  #include <libcamera/camera.h>
> >>  #include <libcamera/formats.h>
> >> +#include <libcamera/property_ids.h>
> >>  #include <libcamera/request.h>
> >>  #include <libcamera/stream.h>
> >>
> >> @@ -776,7 +777,14 @@ int PipelineHandlerIPU3::registerCameras()
> >>  			continue;
> >>
> >>  		/* Initialize the camera properties. */
> >> -		data->properties_ = cio2->sensor()->properties();
> >> +		for (const auto &sensorProperty : cio2->sensor()->properties())
> >> +			data->properties_.set(sensorProperty.first,
> >> +					      sensorProperty.second);
>
> Why does this need to change here?
>
> Perhaps that's because of the previous patch... I guess the previous
> patch has set data->properties to be initialised with all properties and
> their defaults? (I'm still confused if that's needed.) - so this only
> updates properties that are in the sensor...

No the previous patch initialized properties_ with an idmap, but the
'content' is empty until you don't add Control and ControlValues to
the list by calling ControlList::set().

On this change: I could have kept the original line, i felt it was
clearer to add sensor controls one by one to the properties list. I
can drop this line and just add the pipeline controls has it's done
here below.

Thanks
  j

>
>
>
>
> >> +		data->properties_.set(properties::DraftPipelineMaxDepth, 3);
> >> +		data->properties_.set(properties::DraftAvailableNoiseReductionModes,
> >> +				      { static_cast<int32_t>(properties::DRAFT_NOISE_REDUCTION_MODE_OFF) });
>
> Is the cast necessary here?
>
> >> +		data->properties_.set(properties::DraftAvailableColorCorrectionAberrationModes,
> >> +				      { static_cast<int32_t>(properties::DRAFT_COLOR_CORRECTION_ABERRATION_OFF) });
> >>
> >>  		/**
> >>  		 * \todo Dynamically assign ImgU and output devices to each
> >> --
> >> 2.28.0
> >>
> >> _______________________________________________
> >> libcamera-devel mailing list
> >> libcamera-devel at lists.libcamera.org
> >> https://lists.libcamera.org/listinfo/libcamera-devel
> >
>
> --
> Regards
> --
> Kieran