<div dir="ltr"><div dir="ltr">Hi David,<div><br></div><div>Thank you for your review.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 3 Oct 2022 at 10:41, David Plowman <<a href="mailto:david.plowman@raspberrypi.com">david.plowman@raspberrypi.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush<br>
<br>
Thanks for the patch!<br>
<br>
On Thu, 29 Sept 2022 at 08:35, Jacopo Mondi via libcamera-devel<br>
<<a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a>> wrote:<br>
><br>
> Hi Naush<br>
><br>
> On Thu, Sep 29, 2022 at 08:23:21AM +0100, Naushir Patuck via libcamera-devel wrote:<br>
> > Append "_mono" to the sensor name when generating the tuning filename for<br>
> > monochrome sensor variants. So the new naming convention is as follows:<br>
> ><br>
> > <sensor_name>.json - Standard colour sensor variant<br>
> > <sensor_name>_mono.json - Monochrom sensor variant<br>
> ><br>
> > Rename the existing imx296.json file to imx296_mono.json as this tuning file<br>
> > is based on the monochrome variant of the IMX296.<br>
> ><br>
> > Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
><br>
> Reviewed-by: Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>><br>
><br>
> Thanks<br>
>   j<br>
><br>
> > ---<br>
> >  .../data/{imx296.json => imx296_mono.json}    |  0<br>
> >  src/ipa/raspberrypi/data/meson.build          |  2 +-<br>
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 22 ++++++++++++++-----<br>
> >  3 files changed, 18 insertions(+), 6 deletions(-)<br>
> >  rename src/ipa/raspberrypi/data/{imx296.json => imx296_mono.json} (100%)<br>
> ><br>
> > diff --git a/src/ipa/raspberrypi/data/imx296.json b/src/ipa/raspberrypi/data/imx296_mono.json<br>
> > similarity index 100%<br>
> > rename from src/ipa/raspberrypi/data/imx296.json<br>
> > rename to src/ipa/raspberrypi/data/imx296_mono.json<br>
> > diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build<br>
> > index 211811cfa915..465a7a17ce6f 100644<br>
> > --- a/src/ipa/raspberrypi/data/meson.build<br>
> > +++ b/src/ipa/raspberrypi/data/meson.build<br>
> > @@ -4,7 +4,7 @@ conf_files = files([<br>
> >      'imx219.json',<br>
> >      'imx219_noir.json',<br>
> >      'imx290.json',<br>
> > -    'imx296.json',<br>
> > +    'imx296_mono.json',<br>
> >      'imx378.json',<br>
> >      'imx477.json',<br>
> >      'imx477_noir.json',<br>
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > index dcd81650c32d..dab17c34d370 100644<br>
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > @@ -67,6 +67,14 @@ SensorFormats populateSensorFormats(std::unique_ptr<CameraSensor> &sensor)<br>
> >       return formats;<br>
> >  }<br>
> ><br>
> > +bool isMonoSensor(std::unique_ptr<CameraSensor> &sensor)<br>
> > +{<br>
> > +     unsigned int mbusCode = sensor->mbusCodes()[0];<br>
> > +     const BayerFormat &bayer = BayerFormat::fromMbusCode(mbusCode);<br>
> > +<br>
> > +     return bayer.order == BayerFormat::Order::MONO;<br>
> > +}<br>
> > +<br>
> >  PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code,<br>
> >                                 BayerFormat::Packing packingReq)<br>
> >  {<br>
> > @@ -1551,12 +1559,16 @@ int RPiCameraData::loadIPA(ipa::RPi::IPAInitResult *result)<br>
> >        */<br>
> >       std::string configurationFile;<br>
> >       char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_TUNING_FILE");<br>
> > -     if (!configFromEnv || *configFromEnv == '\0')<br>
> > -             configurationFile = ipa_->configurationFile(sensor_->model() + ".json");<br>
> > -     else<br>
> > -             configurationFile = std::string(configFromEnv);<br>
> > +     std::string model = sensor_->model();<br>
> > +     if (!configFromEnv || *configFromEnv == '\0') {<br>
> > +             if (isMonoSensor(sensor_))<br>
> > +                     model += "_mono";<br>
> > +             configurationFile = ipa_->configurationFile(model + ".json");<br>
> > +     } else {<br>
> > +             configurationFile = model;<br>
<br>
Doesn't this line want to be "configurationFile =<br>
std::string(configFromEnv);" ??<br></blockquote><div><br></div><div>Oops, you are right!  I'll fix that up now.</div><div><br></div><div>Naush</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks<br>
David<br>
<br>
> > +     }<br>
> ><br>
> > -     IPASettings settings(configurationFile, sensor_->model());<br>
> > +     IPASettings settings(configurationFile, model);<br>
> ><br>
> >       return ipa_->init(settings, result);<br>
> >  }<br>
> > --<br>
> > 2.25.1<br>
> ><br>
</blockquote></div></div>