[libcamera-devel] [RFC PATCH 1/4] pipeline: ipu3: Support IPA tuning file

[Laurent Pinchart]

Hi Kieran,

On Mon, Aug 08, 2022 at 11:53:52PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart via libcamera-devel (2022-08-02 11:09:52)
> > Pass the path name of the YAML IPA tuning file to the IPA module. The
> > file name is derived from the sensor name ("${sensor_name}.yaml"), with
> > a fallback to "uncalibrated.yaml".
> > 
> > The tuning file name can be manually overridden with the
> > LIBCAMERA_IPU3_TUNING_FILE environment variable.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 22 ++++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 4fe52f74a68c..5c4a0a567ff2 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -1243,8 +1243,26 @@ int IPU3CameraData::loadIPA()
> >         if (ret)
> >                 return ret;
> >  
> > -       ret = ipa_->init(IPASettings{ "", sensor->model() }, sensorInfo,
> > -                        sensor->controls(), &ipaControls_);
> > +       /*
> > +        * The API tuning file is made from the sensor name unless the
> > +        * environment variable overrides it. If
> 
> If ?

Good question :-)

> > +        */
> > +       std::string ipaTuningFile;
> > +       char const *configFromEnv = utils::secure_getenv("LIBCAMERA_IPU3_TUNING_FILE");
> 
> This is just about OK for RPi where there is usually only a single
> camera, but this doesn't really work when there are more such as most IPU3
> devices.
> 
> I would be tempted to drop this - and move towards having a libcamera
> yaml config file that could override this on a per-camera id basis.

You're right, I'll drop it.

> > +       if (!configFromEnv || *configFromEnv == '\0') {
> > +               ipaTuningFile = ipa_->configurationFile(sensor->model() + ".yaml");
> > +               /*
> > +                * If the tuning file isn't found, fall back to the
> > +                * 'uncalibrated' configuration file.
> > +                */
> > +               if (ipaTuningFile.empty())
> > +                       ipaTuningFile = ipa_->configurationFile("uncalibrated.yaml");
> > +       } else {
> > +               ipaTuningFile = std::string(configFromEnv);
> > +       }
> > +
> > +       ret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },
> > +                        sensorInfo, sensor->controls(), &ipaControls_);
> >         if (ret) {
> >                 LOG(IPU3, Error) << "Failed to initialise the IPU3 IPA";
> >                 return ret;

-- 
Regards,

Laurent Pinchart