| Message ID | 20220523092435.475510-2-fsylvestre@baylibre.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Florian, Thank you for the patch. Nitpicking a bit on the naming, I'd write "Support IPA tuning file" in the subject line, as the tuning isn't just about the camera sensor. On Mon, May 23, 2022 at 11:24:31AM +0200, Florian Sylvestre via libcamera-devel wrote: > Allow the usage of a Yaml configuration file that is either defined by Likewise, here, you can write "YAML IPA tuning file". > the sensor name (sensor_name.yaml) or defined by > LIBCAMERA_RKISP1_TUNING_FILE environment variable. > > Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com> > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 4b3d2cf7..865c4ab0 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -322,7 +322,18 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision) > ipa_->paramsBufferReady.connect(this, &RkISP1CameraData::paramFilled); > ipa_->metadataReady.connect(this, &RkISP1CameraData::metadataReady); > > - int ret = ipa_->init(IPASettings{ "", sensor_->model() }, hwRevision); > + /* > + * The configuration (tuning file) is made from the sensor name unless And "The IPA tuning file is made ...". > + * the environment variable overrides it. > + */ > + std::string configurationFile; I'll let you continue :-) With this small issue addressed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RKISP1_TUNING_FILE"); > + if (!configFromEnv || *configFromEnv == '\0') > + configurationFile = ipa_->configurationFile(sensor_->model() + ".yaml"); > + else > + configurationFile = std::string(configFromEnv); > + > + int ret = ipa_->init({ configurationFile, sensor_->model() }, hwRevision); > if (ret < 0) { > LOG(RkISP1, Error) << "IPA initialization failure"; > return ret;
Hi Florian, On Mon, May 23, 2022 at 11:24:31AM +0200, Florian Sylvestre via libcamera-devel wrote: > Allow the usage of a Yaml configuration file that is either defined by > the sensor name (sensor_name.yaml) or defined by > LIBCAMERA_RKISP1_TUNING_FILE environment variable. > > Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com> Looks good. With the changes suggested by Laurent, Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 4b3d2cf7..865c4ab0 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -322,7 +322,18 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision) > ipa_->paramsBufferReady.connect(this, &RkISP1CameraData::paramFilled); > ipa_->metadataReady.connect(this, &RkISP1CameraData::metadataReady); > > - int ret = ipa_->init(IPASettings{ "", sensor_->model() }, hwRevision); > + /* > + * The configuration (tuning file) is made from the sensor name unless > + * the environment variable overrides it. > + */ > + std::string configurationFile; > + char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RKISP1_TUNING_FILE"); > + if (!configFromEnv || *configFromEnv == '\0') > + configurationFile = ipa_->configurationFile(sensor_->model() + ".yaml"); > + else > + configurationFile = std::string(configFromEnv); > + > + int ret = ipa_->init({ configurationFile, sensor_->model() }, hwRevision); > if (ret < 0) { > LOG(RkISP1, Error) << "IPA initialization failure"; > return ret; > -- > 2.34.1 >
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 4b3d2cf7..865c4ab0 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -322,7 +322,18 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision) ipa_->paramsBufferReady.connect(this, &RkISP1CameraData::paramFilled); ipa_->metadataReady.connect(this, &RkISP1CameraData::metadataReady); - int ret = ipa_->init(IPASettings{ "", sensor_->model() }, hwRevision); + /* + * The configuration (tuning file) is made from the sensor name unless + * the environment variable overrides it. + */ + std::string configurationFile; + char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RKISP1_TUNING_FILE"); + if (!configFromEnv || *configFromEnv == '\0') + configurationFile = ipa_->configurationFile(sensor_->model() + ".yaml"); + else + configurationFile = std::string(configFromEnv); + + int ret = ipa_->init({ configurationFile, sensor_->model() }, hwRevision); if (ret < 0) { LOG(RkISP1, Error) << "IPA initialization failure"; return ret;
Allow the usage of a Yaml configuration file that is either defined by the sensor name (sensor_name.yaml) or defined by LIBCAMERA_RKISP1_TUNING_FILE environment variable. Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com> --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)