[v5,10/15] config: Look up RkISP1 tuning file in configuration file

21457 diff mbox series

The configuration snippet:

  configuration:
    pipeline:
      rkisp1:
        tuning_file: FILE

This environment variable has not been documented.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

Hi


2024. október 1., kedd 12:28 keltezéssel, Milan Zamazal <mzamazal@redhat.com> írta:

> The configuration snippet:
>
>   configuration:
>     pipeline:
>       rkisp1:
>         tuning_file: FILE
>
> This environment variable has not been documented.
>
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index c02c7cf37..da94e41d5 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -35,6 +35,7 @@
>  #include "libcamera/internal/delayed_controls.h"
>  #include "libcamera/internal/device_enumerator.h"
>  #include "libcamera/internal/framebuffer.h"
> +#include "libcamera/internal/global_configuration.h"
>  #include "libcamera/internal/ipa_manager.h"
>  #include "libcamera/internal/media_device.h"
>  #include "libcamera/internal/pipeline_handler.h"
> @@ -348,12 +349,21 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>  	 * environment variable overrides it.
>  	 */
>  	std::string ipaTuningFile;
> -	char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RKISP1_TUNING_FILE");
> -	if (!configFromEnv || *configFromEnv == '\0') {
> -		ipaTuningFile =
> -			ipa_->configurationFile(sensor_->model() + ".yaml", "uncalibrated.yaml");
> +	const std::string confPath =
> +		std::string("pipelines.rkisp1.cameras.") + sensor_->id() + std::string(".tuning_file");

Just `+ ".tuning_file"` should work.


> +	const auto confTuningFile =
> +		GlobalConfiguration::envOption(
> +			"LIBCAMERA_RKISP1_TUNING_FILE", confPath.c_str());
> +	if (!confTuningFile.has_value() || confTuningFile.value() == "") {
> +		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");


Does

  ipaTuningFile = ipa_->configurationFile(sensor_->model() + ".yaml", "uncalibrated.yaml")

not work?


>  	} else {
> -		ipaTuningFile = std::string(configFromEnv);
> +		ipaTuningFile = confTuningFile.value();
>  	}
>
>  	IPACameraSensorInfo sensorInfo{};
> --
> 2.44.1


Regards,
Barnabás Pőcze

Hi Barnabás,

Barnabás Pőcze <pobrn@protonmail.com> writes:

> Hi
>
>
> 2024. október 1., kedd 12:28 keltezéssel, Milan Zamazal <mzamazal@redhat.com> írta:
>
>> The configuration snippet:
>>
>>   configuration:
>>     pipeline:
>>       rkisp1:
>>         tuning_file: FILE
>>
>> This environment variable has not been documented.
>>
>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
>> ---
>>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 +++++++++++++++-----
>>  1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> index c02c7cf37..da94e41d5 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> @@ -35,6 +35,7 @@
>>  #include "libcamera/internal/delayed_controls.h"
>>  #include "libcamera/internal/device_enumerator.h"
>>  #include "libcamera/internal/framebuffer.h"
>> +#include "libcamera/internal/global_configuration.h"
>>  #include "libcamera/internal/ipa_manager.h"
>>  #include "libcamera/internal/media_device.h"
>>  #include "libcamera/internal/pipeline_handler.h"
>> @@ -348,12 +349,21 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>>  	 * environment variable overrides it.
>>  	 */
>>  	std::string ipaTuningFile;
>> -	char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RKISP1_TUNING_FILE");
>> -	if (!configFromEnv || *configFromEnv == '\0') {
>> -		ipaTuningFile =
>> -			ipa_->configurationFile(sensor_->model() + ".yaml", "uncalibrated.yaml");
>> +	const std::string confPath =
>> +		std::string("pipelines.rkisp1.cameras.") + sensor_->id() + std::string(".tuning_file");
>
> Just `+ ".tuning_file"` should work.

Indeed, it does.

>> +	const auto confTuningFile =
>> +		GlobalConfiguration::envOption(
>> +			"LIBCAMERA_RKISP1_TUNING_FILE", confPath.c_str());
>> +	if (!confTuningFile.has_value() || confTuningFile.value() == "") {
>> +		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");
>
>
> Does
>
>   ipaTuningFile = ipa_->configurationFile(sensor_->model() + ".yaml", "uncalibrated.yaml")
>
> not work?

It does, a piece of old code slipped in back in some rebase.

>>  	} else {
>> -		ipaTuningFile = std::string(configFromEnv);
>> +		ipaTuningFile = confTuningFile.value();
>>  	}
>>
>>  	IPACameraSensorInfo sensorInfo{};
>> --
>> 2.44.1
>
>
> Regards,
> Barnabás Pőcze