[libcamera-devel] [PATCH v1 04/10] pipeline: raspberrypi: Read config parameters from a file
David Plowman
david.plowman at raspberrypi.com
Tue Nov 1 13:01:12 CET 2022
Hi Naush
Thanks for the patch!
On Fri, 14 Oct 2022 at 14:19, Naushir Patuck via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Read the platform configuration parameters from default.json config file. Use
> the PipelineHandler::configurationFile() helper to determine the full path of
> the file. The default.json filename can be overridden by the user setting the
> LIBCAMERA_RPI_CONFIG_FILE environment variable giving the full path of the
> custom file.
>
> Add config parameter validation to ensure bad parameters cannot cause the
> pipeline handler to stop working.
>
> Currently three parameters are available through the json file:
>
> "min_unicam_buffers"
> The minimum number of internal Unicam buffers to allocate.
>
> "min_total_unicam_buffers"
> The minimum number of internal + external Unicam buffers that must be allocated.
>
> "num_output0_buffers"
> Number of internal ISP Output0 buffers to allocate.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> .../pipeline/raspberrypi/data/default.json | 20 +++++++
> .../pipeline/raspberrypi/data/meson.build | 8 +++
> .../pipeline/raspberrypi/meson.build | 2 +
> .../pipeline/raspberrypi/raspberrypi.cpp | 56 +++++++++++++++++++
> 4 files changed, 86 insertions(+)
> create mode 100644 src/libcamera/pipeline/raspberrypi/data/default.json
> create mode 100644 src/libcamera/pipeline/raspberrypi/data/meson.build
>
> diff --git a/src/libcamera/pipeline/raspberrypi/data/default.json b/src/libcamera/pipeline/raspberrypi/data/default.json
> new file mode 100644
> index 000000000000..d709e31850af
> --- /dev/null
> +++ b/src/libcamera/pipeline/raspberrypi/data/default.json
> @@ -0,0 +1,20 @@
> +{
> + "version": 1.0,
> + "target": "bcm2835",
> +
> + "pipeline_handler":
> + {
> + # The minimum number of internal buffers to be allocated for Unicam.
> + # This value must be greater than 0, but less than or equal to min_total_unicam_buffers.
Are we allowed comments in json files now? That's nice! Perhaps we
should put some in our tuning files...
> + "min_unicam_buffers": 2,
> +
> + # The minimum total (internal + external) buffer count used for Unicam.
> + # The number of internal buffers allocated for Unicam is given by:
> + # internal buffer count = max(min_unicam_buffers,
> + # min_total_unicam_buffers - external buffer count)
> + "min_total_unicam_buffers": 4,
> +
> + # The number of internal buffers used for ISP Output0. This must be set to 1.
> + "num_output0_buffers": 1
> + }
> +}
> diff --git a/src/libcamera/pipeline/raspberrypi/data/meson.build b/src/libcamera/pipeline/raspberrypi/data/meson.build
> new file mode 100644
> index 000000000000..232f8b43c5fd
> --- /dev/null
> +++ b/src/libcamera/pipeline/raspberrypi/data/meson.build
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +conf_files = files([
> + 'default.json',
> +])
> +
> +install_data(conf_files,
> + install_dir : libcamera_datadir / 'pipeline/raspberrypi')
> diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/raspberrypi/meson.build
> index f1a2f5ee72c2..48235f887501 100644
> --- a/src/libcamera/pipeline/raspberrypi/meson.build
> +++ b/src/libcamera/pipeline/raspberrypi/meson.build
> @@ -5,3 +5,5 @@ libcamera_sources += files([
> 'raspberrypi.cpp',
> 'rpi_stream.cpp',
> ])
> +
> +subdir('data')
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 450029197b11..bac7a66ba900 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -14,6 +14,7 @@
> #include <unordered_set>
> #include <utility>
>
> +#include <libcamera/base/file.h>
> #include <libcamera/base/shared_fd.h>
> #include <libcamera/base/utils.h>
>
> @@ -40,6 +41,7 @@
> #include "libcamera/internal/media_device.h"
> #include "libcamera/internal/pipeline_handler.h"
> #include "libcamera/internal/v4l2_videodevice.h"
> +#include "libcamera/internal/yaml_parser.h"
>
> #include "dma_heaps.h"
> #include "rpi_stream.h"
> @@ -301,6 +303,7 @@ public:
> };
>
> Config config_;
> + unsigned int numUnicamBuffers;
>
> private:
> void checkRequestCompleted();
> @@ -1140,6 +1143,19 @@ int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)
> */
> stream->setExternalBuffer(buffer);
> }
> +
> + if (!buffer) {
> + if (stream == &data->isp_[Isp::Output0] && !data->config_.numOutput0Buffers) {
> + LOG(RPI, Error) << "Output0 buffer must be provided in the request.";
> + return -EINVAL;
> + }
> +
> + if (stream == &data->unicam_[Unicam::Image] && !data->numUnicamBuffers) {
> + LOG(RPI, Error) << "Unicam Image buffer must be provided in the request.";
> + return -EINVAL;
> + }
> + }
> +
> /*
> * If no buffer is provided by the request for this stream, we
> * queue a nullptr to the stream to signify that it must use an
> @@ -1398,6 +1414,7 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me
> int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)
> {
> RPiCameraData::Config &config = data->config_;
> + std::string filename;
>
> config = {
> .minUnicamBuffers = 2,
> @@ -1405,6 +1422,44 @@ int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)
> .numOutput0Buffers = 1,
> };
>
> + char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");
> + if (!configFromEnv || *configFromEnv == '\0')
> + filename = configurationFile("raspberrypi/default.json");
I was a teeny bit nervous that in the non-installed case it might come
back with "libcamera/src/ipa/raspberrypi/data/raspberrypi/default.json"
which would be the wrong thing? Maybe you could just put my mind at
ease here...
> + else
> + filename = std::string(configFromEnv);
> +
> + if (filename.empty())
> + return -EINVAL;
> +
> + File file(filename);
> + if (!file.open(File::OpenModeFlag::ReadOnly)) {
> + LOG(RPI, Error) << "Failed to open configuration file '" << filename << "'";
> + return -EIO;
> + }
> +
> + LOG(RPI, Info) << "Using configuration file '" << filename << "'";
> +
> + std::unique_ptr<YamlObject> root = YamlParser::parse(file);
> + if (!root) {
> + LOG(RPI, Error) << "Failed to parse configuration file, using defaults!";
> + return -EINVAL;
> + }
> +
> + ASSERT((*root)["version"].get<double>() == 1.0);
> +
> + const YamlObject &phConfig = (*root)["pipeline_handler"];
> + config.minUnicamBuffers =
> + phConfig["min_unicam_buffers"].get<unsigned int>(config.minUnicamBuffers);
> + config.minTotalUnicamBuffers =
> + phConfig["min_total_unicam_buffers"].get<unsigned int>(config.minTotalUnicamBuffers);
> + config.numOutput0Buffers =
> + phConfig["num_output0_buffers"].get<unsigned int>(config.numOutput0Buffers);
> +
> + if (config.minTotalUnicamBuffers < config.minUnicamBuffers || config.minTotalUnicamBuffers < 1) {
I wonder a little bit about splitting up all these little checks so
that we can be really specific in the error message? But I'm not too
fussed.
Subject to a check on that pathname question I had:
Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
Thanks!
David
> + LOG(RPI, Error) << "Invalid Unicam buffer configuration used!";
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> @@ -1471,6 +1526,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> */
> numBuffers = std::max<int>(data->config_.minUnicamBuffers,
> minBuffers - numRawBuffers);
> + data->numUnicamBuffers = numBuffers;
> } else if (stream == &data->isp_[Isp::Input]) {
> /*
> * ISP input buffers are imported from Unicam, so follow
> --
> 2.25.1
>
More information about the libcamera-devel
mailing list