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

[Kieran Bingham]

Quoting Laurent Pinchart (2022-08-09 15:38:45)
> 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 :-)

With this answered,

> 
> > > +        */
> > > +       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.

And this dropped...


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

> 
> > > +       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