[libcamera-devel,v2,2/4] pipeline: raspberrypi: Update naming convention for tuning files
diff mbox series

Message ID 20220929072323.7400-3-naush@raspberrypi.com
State Superseded
Headers show
Series
  • imx296 colour tuning
Related show

Commit Message

Naushir Patuck Sept. 29, 2022, 7:23 a.m. UTC 
Append "_mono" to the sensor name when generating the tuning filename for
monochrome sensor variants. So the new naming convention is as follows:

<sensor_name>.json - Standard colour sensor variant
<sensor_name>_mono.json - Monochrom sensor variant

Rename the existing imx296.json file to imx296_mono.json as this tuning file
is based on the monochrome variant of the IMX296.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 .../data/{imx296.json => imx296_mono.json}    |  0
 src/ipa/raspberrypi/data/meson.build          |  2 +-
 .../pipeline/raspberrypi/raspberrypi.cpp      | 22 ++++++++++++++-----
 3 files changed, 18 insertions(+), 6 deletions(-)
 rename src/ipa/raspberrypi/data/{imx296.json => imx296_mono.json} (100%)

Comments

Jacopo Mondi Sept. 29, 2022, 7:35 a.m. UTC | #1 
Hi Naush

On Thu, Sep 29, 2022 at 08:23:21AM +0100, Naushir Patuck via libcamera-devel wrote:
> Append "_mono" to the sensor name when generating the tuning filename for
> monochrome sensor variants. So the new naming convention is as follows:
>
> <sensor_name>.json - Standard colour sensor variant
> <sensor_name>_mono.json - Monochrom sensor variant
>
> Rename the existing imx296.json file to imx296_mono.json as this tuning file
> is based on the monochrome variant of the IMX296.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> ---
>  .../data/{imx296.json => imx296_mono.json}    |  0
>  src/ipa/raspberrypi/data/meson.build          |  2 +-
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 22 ++++++++++++++-----
>  3 files changed, 18 insertions(+), 6 deletions(-)
>  rename src/ipa/raspberrypi/data/{imx296.json => imx296_mono.json} (100%)
>
> diff --git a/src/ipa/raspberrypi/data/imx296.json b/src/ipa/raspberrypi/data/imx296_mono.json
> similarity index 100%
> rename from src/ipa/raspberrypi/data/imx296.json
> rename to src/ipa/raspberrypi/data/imx296_mono.json
> diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> index 211811cfa915..465a7a17ce6f 100644
> --- a/src/ipa/raspberrypi/data/meson.build
> +++ b/src/ipa/raspberrypi/data/meson.build
> @@ -4,7 +4,7 @@ conf_files = files([
>      'imx219.json',
>      'imx219_noir.json',
>      'imx290.json',
> -    'imx296.json',
> +    'imx296_mono.json',
>      'imx378.json',
>      'imx477.json',
>      'imx477_noir.json',
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index dcd81650c32d..dab17c34d370 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -67,6 +67,14 @@ SensorFormats populateSensorFormats(std::unique_ptr<CameraSensor> &sensor)
>  	return formats;
>  }
>
> +bool isMonoSensor(std::unique_ptr<CameraSensor> &sensor)
> +{
> +	unsigned int mbusCode = sensor->mbusCodes()[0];
> +	const BayerFormat &bayer = BayerFormat::fromMbusCode(mbusCode);
> +
> +	return bayer.order == BayerFormat::Order::MONO;
> +}
> +
>  PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code,
>  				  BayerFormat::Packing packingReq)
>  {
> @@ -1551,12 +1559,16 @@ int RPiCameraData::loadIPA(ipa::RPi::IPAInitResult *result)
>  	 */
>  	std::string configurationFile;
>  	char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_TUNING_FILE");
> -	if (!configFromEnv || *configFromEnv == '\0')
> -		configurationFile = ipa_->configurationFile(sensor_->model() + ".json");
> -	else
> -		configurationFile = std::string(configFromEnv);
> +	std::string model = sensor_->model();
> +	if (!configFromEnv || *configFromEnv == '\0') {
> +		if (isMonoSensor(sensor_))
> +			model += "_mono";
> +		configurationFile = ipa_->configurationFile(model + ".json");
> +	} else {
> +		configurationFile = model;
> +	}
>
> -	IPASettings settings(configurationFile, sensor_->model());
> +	IPASettings settings(configurationFile, model);
>
>  	return ipa_->init(settings, result);
>  }
> --
> 2.25.1
>
David Plowman Oct. 3, 2022, 9:41 a.m. UTC | #2 
Hi Naush

Thanks for the patch!

On Thu, 29 Sept 2022 at 08:35, Jacopo Mondi via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> Hi Naush
>
> On Thu, Sep 29, 2022 at 08:23:21AM +0100, Naushir Patuck via libcamera-devel wrote:
> > Append "_mono" to the sensor name when generating the tuning filename for
> > monochrome sensor variants. So the new naming convention is as follows:
> >
> > <sensor_name>.json - Standard colour sensor variant
> > <sensor_name>_mono.json - Monochrom sensor variant
> >
> > Rename the existing imx296.json file to imx296_mono.json as this tuning file
> > is based on the monochrome variant of the IMX296.
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Thanks
>   j
>
> > ---
> >  .../data/{imx296.json => imx296_mono.json}    |  0
> >  src/ipa/raspberrypi/data/meson.build          |  2 +-
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 22 ++++++++++++++-----
> >  3 files changed, 18 insertions(+), 6 deletions(-)
> >  rename src/ipa/raspberrypi/data/{imx296.json => imx296_mono.json} (100%)
> >
> > diff --git a/src/ipa/raspberrypi/data/imx296.json b/src/ipa/raspberrypi/data/imx296_mono.json
> > similarity index 100%
> > rename from src/ipa/raspberrypi/data/imx296.json
> > rename to src/ipa/raspberrypi/data/imx296_mono.json
> > diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
> > index 211811cfa915..465a7a17ce6f 100644
> > --- a/src/ipa/raspberrypi/data/meson.build
> > +++ b/src/ipa/raspberrypi/data/meson.build
> > @@ -4,7 +4,7 @@ conf_files = files([
> >      'imx219.json',
> >      'imx219_noir.json',
> >      'imx290.json',
> > -    'imx296.json',
> > +    'imx296_mono.json',
> >      'imx378.json',
> >      'imx477.json',
> >      'imx477_noir.json',
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index dcd81650c32d..dab17c34d370 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -67,6 +67,14 @@ SensorFormats populateSensorFormats(std::unique_ptr<CameraSensor> &sensor)
> >       return formats;
> >  }
> >
> > +bool isMonoSensor(std::unique_ptr<CameraSensor> &sensor)
> > +{
> > +     unsigned int mbusCode = sensor->mbusCodes()[0];
> > +     const BayerFormat &bayer = BayerFormat::fromMbusCode(mbusCode);
> > +
> > +     return bayer.order == BayerFormat::Order::MONO;
> > +}
> > +
> >  PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code,
> >                                 BayerFormat::Packing packingReq)
> >  {
> > @@ -1551,12 +1559,16 @@ int RPiCameraData::loadIPA(ipa::RPi::IPAInitResult *result)
> >        */
> >       std::string configurationFile;
> >       char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_TUNING_FILE");
> > -     if (!configFromEnv || *configFromEnv == '\0')
> > -             configurationFile = ipa_->configurationFile(sensor_->model() + ".json");
> > -     else
> > -             configurationFile = std::string(configFromEnv);
> > +     std::string model = sensor_->model();
> > +     if (!configFromEnv || *configFromEnv == '\0') {
> > +             if (isMonoSensor(sensor_))
> > +                     model += "_mono";
> > +             configurationFile = ipa_->configurationFile(model + ".json");
> > +     } else {
> > +             configurationFile = model;

Doesn't this line want to be "configurationFile =
std::string(configFromEnv);" ??

Thanks
David

> > +     }
> >
> > -     IPASettings settings(configurationFile, sensor_->model());
> > +     IPASettings settings(configurationFile, model);
> >
> >       return ipa_->init(settings, result);
> >  }
> > --
> > 2.25.1
> >
Naushir Patuck Oct. 3, 2022, 9:50 a.m. UTC | #3 
Hi David,

Thank you for your review.

On Mon, 3 Oct 2022 at 10:41, David Plowman <david.plowman@raspberrypi.com>
wrote:

> Hi Naush
>
> Thanks for the patch!
>
> On Thu, 29 Sept 2022 at 08:35, Jacopo Mondi via libcamera-devel
> <libcamera-devel@lists.libcamera.org> wrote:
> >
> > Hi Naush
> >
> > On Thu, Sep 29, 2022 at 08:23:21AM +0100, Naushir Patuck via
> libcamera-devel wrote:
> > > Append "_mono" to the sensor name when generating the tuning filename
> for
> > > monochrome sensor variants. So the new naming convention is as follows:
> > >
> > > <sensor_name>.json - Standard colour sensor variant
> > > <sensor_name>_mono.json - Monochrom sensor variant
> > >
> > > Rename the existing imx296.json file to imx296_mono.json as this
> tuning file
> > > is based on the monochrome variant of the IMX296.
> > >
> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> >
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> >
> > Thanks
> >   j
> >
> > > ---
> > >  .../data/{imx296.json => imx296_mono.json}    |  0
> > >  src/ipa/raspberrypi/data/meson.build          |  2 +-
> > >  .../pipeline/raspberrypi/raspberrypi.cpp      | 22 ++++++++++++++-----
> > >  3 files changed, 18 insertions(+), 6 deletions(-)
> > >  rename src/ipa/raspberrypi/data/{imx296.json => imx296_mono.json}
> (100%)
> > >
> > > diff --git a/src/ipa/raspberrypi/data/imx296.json
> b/src/ipa/raspberrypi/data/imx296_mono.json
> > > similarity index 100%
> > > rename from src/ipa/raspberrypi/data/imx296.json
> > > rename to src/ipa/raspberrypi/data/imx296_mono.json
> > > diff --git a/src/ipa/raspberrypi/data/meson.build
> b/src/ipa/raspberrypi/data/meson.build
> > > index 211811cfa915..465a7a17ce6f 100644
> > > --- a/src/ipa/raspberrypi/data/meson.build
> > > +++ b/src/ipa/raspberrypi/data/meson.build
> > > @@ -4,7 +4,7 @@ conf_files = files([
> > >      'imx219.json',
> > >      'imx219_noir.json',
> > >      'imx290.json',
> > > -    'imx296.json',
> > > +    'imx296_mono.json',
> > >      'imx378.json',
> > >      'imx477.json',
> > >      'imx477_noir.json',
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > index dcd81650c32d..dab17c34d370 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > @@ -67,6 +67,14 @@ SensorFormats
> populateSensorFormats(std::unique_ptr<CameraSensor> &sensor)
> > >       return formats;
> > >  }
> > >
> > > +bool isMonoSensor(std::unique_ptr<CameraSensor> &sensor)
> > > +{
> > > +     unsigned int mbusCode = sensor->mbusCodes()[0];
> > > +     const BayerFormat &bayer = BayerFormat::fromMbusCode(mbusCode);
> > > +
> > > +     return bayer.order == BayerFormat::Order::MONO;
> > > +}
> > > +
> > >  PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code,
> > >                                 BayerFormat::Packing packingReq)
> > >  {
> > > @@ -1551,12 +1559,16 @@ int
> RPiCameraData::loadIPA(ipa::RPi::IPAInitResult *result)
> > >        */
> > >       std::string configurationFile;
> > >       char const *configFromEnv =
> utils::secure_getenv("LIBCAMERA_RPI_TUNING_FILE");
> > > -     if (!configFromEnv || *configFromEnv == '\0')
> > > -             configurationFile =
> ipa_->configurationFile(sensor_->model() + ".json");
> > > -     else
> > > -             configurationFile = std::string(configFromEnv);
> > > +     std::string model = sensor_->model();
> > > +     if (!configFromEnv || *configFromEnv == '\0') {
> > > +             if (isMonoSensor(sensor_))
> > > +                     model += "_mono";
> > > +             configurationFile = ipa_->configurationFile(model +
> ".json");
> > > +     } else {
> > > +             configurationFile = model;
>
> Doesn't this line want to be "configurationFile =
> std::string(configFromEnv);" ??
>

Oops, you are right!  I'll fix that up now.

Naush



>
> Thanks
> David
>
> > > +     }
> > >
> > > -     IPASettings settings(configurationFile, sensor_->model());
> > > +     IPASettings settings(configurationFile, model);
> > >
> > >       return ipa_->init(settings, result);
> > >  }
> > > --
> > > 2.25.1
> > >
>

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/data/imx296.json b/src/ipa/raspberrypi/data/imx296_mono.json
similarity index 100%
rename from src/ipa/raspberrypi/data/imx296.json
rename to src/ipa/raspberrypi/data/imx296_mono.json
diff --git a/src/ipa/raspberrypi/data/meson.build b/src/ipa/raspberrypi/data/meson.build
index 211811cfa915..465a7a17ce6f 100644
--- a/src/ipa/raspberrypi/data/meson.build
+++ b/src/ipa/raspberrypi/data/meson.build
@@ -4,7 +4,7 @@  conf_files = files([
     'imx219.json',
     'imx219_noir.json',
     'imx290.json',
-    'imx296.json',
+    'imx296_mono.json',
     'imx378.json',
     'imx477.json',
     'imx477_noir.json',
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index dcd81650c32d..dab17c34d370 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -67,6 +67,14 @@  SensorFormats populateSensorFormats(std::unique_ptr<CameraSensor> &sensor)
 	return formats;
 }
 
+bool isMonoSensor(std::unique_ptr<CameraSensor> &sensor)
+{
+	unsigned int mbusCode = sensor->mbusCodes()[0];
+	const BayerFormat &bayer = BayerFormat::fromMbusCode(mbusCode);
+
+	return bayer.order == BayerFormat::Order::MONO;
+}
+
 PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code,
 				  BayerFormat::Packing packingReq)
 {
@@ -1551,12 +1559,16 @@  int RPiCameraData::loadIPA(ipa::RPi::IPAInitResult *result)
 	 */
 	std::string configurationFile;
 	char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_TUNING_FILE");
-	if (!configFromEnv || *configFromEnv == '\0')
-		configurationFile = ipa_->configurationFile(sensor_->model() + ".json");
-	else
-		configurationFile = std::string(configFromEnv);
+	std::string model = sensor_->model();
+	if (!configFromEnv || *configFromEnv == '\0') {
+		if (isMonoSensor(sensor_))
+			model += "_mono";
+		configurationFile = ipa_->configurationFile(model + ".json");
+	} else {
+		configurationFile = model;
+	}
 
-	IPASettings settings(configurationFile, sensor_->model());
+	IPASettings settings(configurationFile, model);
 
 	return ipa_->init(settings, result);
 }