[RFC,07/11] config: Look up rpi config path in configuration file

19808 diff mbox series

The configuration snippet:

  configuration:
    pipeline:
      rpi:
        config_file: FILENAME

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Hi Milan,

Thank you for this patch.

On Tue, 26 Mar 2024 at 11:26, Milan Zamazal <mzamazal@redhat.com> wrote:
>
> The configuration snippet:
>
>   configuration:
>     pipeline:
>       rpi:
>         config_file: FILENAME
>
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---

I'm not sure if the pipeline specific config belongs in the global
config file.  However, I am ok for this to be the case.  If it does,
you probably also want to do the same for the
"LIBCAMERA_RPI_TUNING_FILE" config item in the same file.

Regards,
Naush


>  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 7e420b3f..8260a3ff 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -20,6 +20,7 @@
>  #include <libcamera/property_ids.h>
>
>  #include "libcamera/internal/camera_lens.h"
> +#include "libcamera/internal/global_configuration.h"
>  #include "libcamera/internal/ipa_manager.h"
>  #include "libcamera/internal/v4l2_subdevice.h"
>
> @@ -1086,7 +1087,12 @@ int CameraData::loadPipelineConfiguration()
>         /* Initial configuration of the platform, in case no config file is present */
>         platformPipelineConfigure({});
>
> -       char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");
> +       std::optional<std::string> configFile =
> +               GlobalConfiguration::envOption("LIBCAMERA_RPI_CONFIG_FILE",
> +                                              "pipeline.rpi.config_file");
> +       if (!configFile.has_value())
> +               return 0;
> +       char const *configFromEnv = configFile.value().c_str();
>         if (!configFromEnv || *configFromEnv == '\0')
>                 return 0;
>
> --
> 2.42.0
>

Hi Naush,

thank you for your comment.

Naushir Patuck <naush@raspberrypi.com> writes:

> Hi Milan,
>
> Thank you for this patch.
>
> On Tue, 26 Mar 2024 at 11:26, Milan Zamazal <mzamazal@redhat.com> wrote:
>>
>> The configuration snippet:
>>
>>   configuration:
>>     pipeline:
>>       rpi:
>>         config_file: FILENAME
>>
>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
>> ---
>
> I'm not sure if the pipeline specific config belongs in the global
> config file.  However, I am ok for this to be the case.

I'd say if it is configurable via environment variable then it belongs to the
global config file.  But I don't have a strong opinion about it.

> If it does, you probably also want to do the same for the
> "LIBCAMERA_RPI_TUNING_FILE" config item in the same file.

Ah, right, will add it in v2.

Thanks,
Milan

> Regards,
> Naush
>
>
>>  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>> index 7e420b3f..8260a3ff 100644
>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>> @@ -20,6 +20,7 @@
>>  #include <libcamera/property_ids.h>
>>
>>  #include "libcamera/internal/camera_lens.h"
>> +#include "libcamera/internal/global_configuration.h"
>>  #include "libcamera/internal/ipa_manager.h"
>>  #include "libcamera/internal/v4l2_subdevice.h"
>>
>> @@ -1086,7 +1087,12 @@ int CameraData::loadPipelineConfiguration()
>>         /* Initial configuration of the platform, in case no config file is present */
>>         platformPipelineConfigure({});
>>
>> -       char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");
>> +       std::optional<std::string> configFile =
>> +               GlobalConfiguration::envOption("LIBCAMERA_RPI_CONFIG_FILE",
>> +                                              "pipeline.rpi.config_file");
>> +       if (!configFile.has_value())
>> +               return 0;
>> +       char const *configFromEnv = configFile.value().c_str();
>>         if (!configFromEnv || *configFromEnv == '\0')
>>                 return 0;
>>
>> --
>> 2.42.0
>>