[{"id":25705,"web_url":"https://patchwork.libcamera.org/comment/25705/","msgid":"","date":"2022-11-01T12:01:12","subject":"Re: [libcamera-devel] [PATCH v1 04/10] pipeline: raspberrypi: Read\n\tconfig parameters from a file","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the patch!\n\nOn Fri, 14 Oct 2022 at 14:19, Naushir Patuck via libcamera-devel\n wrote:\n>\n> Read the platform configuration parameters from default.json config file. Use\n> the PipelineHandler::configurationFile() helper to determine the full path of\n> the file. The default.json filename can be overridden by the user setting the\n> LIBCAMERA_RPI_CONFIG_FILE environment variable giving the full path of the\n> custom file.\n>\n> Add config parameter validation to ensure bad parameters cannot cause the\n> pipeline handler to stop working.\n>\n> Currently three parameters are available through the json file:\n>\n> \"min_unicam_buffers\"\n> The minimum number of internal Unicam buffers to allocate.\n>\n> \"min_total_unicam_buffers\"\n> The minimum number of internal + external Unicam buffers that must be allocated.\n>\n> \"num_output0_buffers\"\n> Number of internal ISP Output0 buffers to allocate.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/data/default.json | 20 +++++++\n> .../pipeline/raspberrypi/data/meson.build | 8 +++\n> .../pipeline/raspberrypi/meson.build | 2 +\n> .../pipeline/raspberrypi/raspberrypi.cpp | 56 +++++++++++++++++++\n> 4 files changed, 86 insertions(+)\n> create mode 100644 src/libcamera/pipeline/raspberrypi/data/default.json\n> create mode 100644 src/libcamera/pipeline/raspberrypi/data/meson.build\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/data/default.json b/src/libcamera/pipeline/raspberrypi/data/default.json\n> new file mode 100644\n> index 000000000000..d709e31850af\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/raspberrypi/data/default.json\n> @@ -0,0 +1,20 @@\n> +{\n> + \"version\": 1.0,\n> + \"target\": \"bcm2835\",\n> +\n> + \"pipeline_handler\":\n> + {\n> + # The minimum number of internal buffers to be allocated for Unicam.\n> + # This value must be greater than 0, but less than or equal to min_total_unicam_buffers.\n\nAre we allowed comments in json files now? That's nice! Perhaps we\nshould put some in our tuning files...\n\n> + \"min_unicam_buffers\": 2,\n> +\n> + # The minimum total (internal + external) buffer count used for Unicam.\n> + # The number of internal buffers allocated for Unicam is given by:\n> + # internal buffer count = max(min_unicam_buffers,\n> + # min_total_unicam_buffers - external buffer count)\n> + \"min_total_unicam_buffers\": 4,\n> +\n> + # The number of internal buffers used for ISP Output0. This must be set to 1.\n> + \"num_output0_buffers\": 1\n> + }\n> +}\n> diff --git a/src/libcamera/pipeline/raspberrypi/data/meson.build b/src/libcamera/pipeline/raspberrypi/data/meson.build\n> new file mode 100644\n> index 000000000000..232f8b43c5fd\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/raspberrypi/data/meson.build\n> @@ -0,0 +1,8 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +conf_files = files([\n> + 'default.json',\n> +])\n> +\n> +install_data(conf_files,\n> + install_dir : libcamera_datadir / 'pipeline/raspberrypi')\n> diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/raspberrypi/meson.build\n> index f1a2f5ee72c2..48235f887501 100644\n> --- a/src/libcamera/pipeline/raspberrypi/meson.build\n> +++ b/src/libcamera/pipeline/raspberrypi/meson.build\n> @@ -5,3 +5,5 @@ libcamera_sources += files([\n> 'raspberrypi.cpp',\n> 'rpi_stream.cpp',\n> ])\n> +\n> +subdir('data')\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 450029197b11..bac7a66ba900 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n>\n> +#include \n> #include \n> #include \n>\n> @@ -40,6 +41,7 @@\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> +#include \"libcamera/internal/yaml_parser.h\"\n>\n> #include \"dma_heaps.h\"\n> #include \"rpi_stream.h\"\n> @@ -301,6 +303,7 @@ public:\n> };\n>\n> Config config_;\n> + unsigned int numUnicamBuffers;\n>\n> private:\n> void checkRequestCompleted();\n> @@ -1140,6 +1143,19 @@ int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)\n> */\n> stream->setExternalBuffer(buffer);\n> }\n> +\n> + if (!buffer) {\n> + if (stream == &data->isp_[Isp::Output0] && !data->config_.numOutput0Buffers) {\n> + LOG(RPI, Error) << \"Output0 buffer must be provided in the request.\";\n> + return -EINVAL;\n> + }\n> +\n> + if (stream == &data->unicam_[Unicam::Image] && !data->numUnicamBuffers) {\n> + LOG(RPI, Error) << \"Unicam Image buffer must be provided in the request.\";\n> + return -EINVAL;\n> + }\n> + }\n> +\n> /*\n> * If no buffer is provided by the request for this stream, we\n> * queue a nullptr to the stream to signify that it must use an\n> @@ -1398,6 +1414,7 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me\n> int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)\n> {\n> RPiCameraData::Config &config = data->config_;\n> + std::string filename;\n>\n> config = {\n> .minUnicamBuffers = 2,\n> @@ -1405,6 +1422,44 @@ int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)\n> .numOutput0Buffers = 1,\n> };\n>\n> + char const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> + if (!configFromEnv || *configFromEnv == '\\0')\n> + filename = configurationFile(\"raspberrypi/default.json\");\n\nI was a teeny bit nervous that in the non-installed case it might come\nback with \"libcamera/src/ipa/raspberrypi/data/raspberrypi/default.json\"\nwhich would be the wrong thing? Maybe you could just put my mind at\nease here...\n\n> + else\n> + filename = std::string(configFromEnv);\n> +\n> + if (filename.empty())\n> + return -EINVAL;\n> +\n> + File file(filename);\n> + if (!file.open(File::OpenModeFlag::ReadOnly)) {\n> + LOG(RPI, Error) << \"Failed to open configuration file '\" << filename << \"'\";\n> + return -EIO;\n> + }\n> +\n> + LOG(RPI, Info) << \"Using configuration file '\" << filename << \"'\";\n> +\n> + std::unique_ptr root = YamlParser::parse(file);\n> + if (!root) {\n> + LOG(RPI, Error) << \"Failed to parse configuration file, using defaults!\";\n> + return -EINVAL;\n> + }\n> +\n> + ASSERT((*root)[\"version\"].get() == 1.0);\n> +\n> + const YamlObject &phConfig = (*root)[\"pipeline_handler\"];\n> + config.minUnicamBuffers =\n> + phConfig[\"min_unicam_buffers\"].get(config.minUnicamBuffers);\n> + config.minTotalUnicamBuffers =\n> + phConfig[\"min_total_unicam_buffers\"].get(config.minTotalUnicamBuffers);\n> + config.numOutput0Buffers =\n> + phConfig[\"num_output0_buffers\"].get(config.numOutput0Buffers);\n> +\n> + if (config.minTotalUnicamBuffers < config.minUnicamBuffers || config.minTotalUnicamBuffers < 1) {\n\nI wonder a little bit about splitting up all these little checks so\nthat we can be really specific in the error message? But I'm not too\nfussed.\n\nSubject to a check on that pathname question I had:\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> + LOG(RPI, Error) << \"Invalid Unicam buffer configuration used!\";\n> + return -EINVAL;\n> + }\n> +\n> return 0;\n> }\n>\n> @@ -1471,6 +1526,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> */\n> numBuffers = std::max(data->config_.minUnicamBuffers,\n> minBuffers - numRawBuffers);\n> + data->numUnicamBuffers = numBuffers;\n> } else if (stream == &data->isp_[Isp::Input]) {\n> /*\n> * ISP input buffers are imported from Unicam, so follow\n> --\n> 2.25.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4389DBDB16\n\tfor ;\n\tTue, 1 Nov 2022 12:01:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA23663031;\n\tTue, 1 Nov 2022 13:01:26 +0100 (CET)","from mail-pj1-x1032.google.com (mail-pj1-x1032.google.com\n\t[IPv6:2607:f8b0:4864:20::1032])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0E98C63009\n\tfor ;\n\tTue, 1 Nov 2022 13:01:25 +0100 (CET)","by mail-pj1-x1032.google.com with SMTP id\n\tb1-20020a17090a7ac100b00213fde52d49so3036599pjl.3\n\tfor ;\n\tTue, 01 Nov 2022 05:01:24 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1667304086;\n\tbh=oU800Q01N5+l/ZXqQAAWwmJHMn+x6KsGeZLWtmV4QZ8=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=tmLyUtkuQwGeQCvz1vAQLzVWtAQc8O9Pfi/51+HaOVqSaPZqAEDbrwdkTuUqdZOCX\n\tnynd43ScxNkqU8FmDKtVuLmdbBLGyeJREiuLiOPcg2SBYIhZ02kpFFlxfyFu9OkBLr\n\tcTa5JgiZy5yQyb9Os+PZ9Nd5uC8aeSjXatm/9xic9BIsoMlC36RaL61IxeERY1g8ZP\n\tPSR7DyzXG41QVvTdiDdGyeffbn0c7E3Z9ddKUUaKKCfP6JHvSEDKDiJnJ7c4H35qkY\n\tNYmtQ3i3tWgJNXE/6cVWJztwdj95r2e5iFzpz4VUONLpqEPEij5G9vwyYwWt6MsnCG\n\t4KyMuvJ0evNLw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=5DUrkUXB6kl/KSfBkbEsuKRt2RI6l0gshwF6LmScCV8=;\n\tb=HDCO94oT54eBeH00Oye+30lb6Dz9gSqNDAYlPNHlJOpGloFtWXKtrdist6XBQtDiwZ\n\tL/O1Y/oMCXVSctnFiD4CzPTZZ3+TaOX4QSnoPwKoiC3mj7LPsQxHqFpGxMBZ9X+TTjvQ\n\tvWP/rhhBMxto8jAUfzXQJhQLjn4fbuHCp4WLrWpqiBKykDr0FgOMa73Kc+CS924AfPUm\n\tYsRL9IwExoySJhCvCuyaE4JVeToJYOGP9zVGoQmfajrhjObIRk1A7VU6uDWHQrCJj6x4\n\tpecwvxddy3NkfXu4XQvgR8PHx14/4TPS3/eHtfj4khGGSqiMB79m0NvVWc2joDXKe11J\n\tKyCA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"HDCO94oT\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=5DUrkUXB6kl/KSfBkbEsuKRt2RI6l0gshwF6LmScCV8=;\n\tb=zsnZ6PKmElFWW0lks0ssXvSgfxkHjqttjAZ/JJw8hx9Ii9wD133vTNUkugCV3wh3iu\n\tLik57xCBBc2yUlZnQqFHpCfhpo6WFoWEomS+K+n3+eIqyqH3SNq8cHFNt/4Ib9wwIP36\n\tbO4I1d4iIx9ywp4fuU1opiE7BWN/g+sNY41P3BNaWOQSnH6kT1IwT8kvZXB6bwcxoMqa\n\tcJIHKfdHL4Ll/wW1PAYgjhWXixbLCcjQARpDKilMQvrtep/Jk3u5sPJOVxZsA+RNfiNb\n\tdQ5iNjo51KQ5H3D73z2qwOK125LC2E0xzME7dzpAoylj3kKyn9HkRsQPwNBintYPPQyw\n\tNVlA==","X-Gm-Message-State":"ACrzQf3cBwsZXErEbRBHNFGhMpG70165598t+uPfXPEBolXrVVdh3cBB\n\tO4oo/fbnA/FSd5xox24FL3Bwrt+1V7DsjRrt3MK5tTz0JqA=","X-Google-Smtp-Source":"AMsMyM7SBLBjJEiqhrCmxeIFtuvM0qdx6oB+k/dpTMPfJ941bAkVhzrFAzIeuQxY+4u14T7jhQLJm5w5ZcwTpmgPqCA=","X-Received":"by 2002:a17:902:720a:b0:181:150c:fcc4 with SMTP id\n\tba10-20020a170902720a00b00181150cfcc4mr19603209plb.109.1667304083463;\n\tTue, 01 Nov 2022 05:01:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20221014131846.27169-1-naush@raspberrypi.com>\n\t<20221014131846.27169-5-naush@raspberrypi.com>","In-Reply-To":"<20221014131846.27169-5-naush@raspberrypi.com>","Date":"Tue, 1 Nov 2022 12:01:12 +0000","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 04/10] pipeline: raspberrypi: Read\n\tconfig parameters from a file","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25923,"web_url":"https://patchwork.libcamera.org/comment/25923/","msgid":"","date":"2022-11-29T10:44:18","subject":"Re: [libcamera-devel] [PATCH v1 04/10] pipeline: raspberrypi: Read\n\tconfig parameters from a file","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for your feedback\n\nOn Tue, 1 Nov 2022 at 12:01, David Plowman \nwrote:\n\n> Hi Naush\n>\n> Thanks for the patch!\n>\n> On Fri, 14 Oct 2022 at 14:19, Naushir Patuck via libcamera-devel\n> wrote:\n> >\n> > Read the platform configuration parameters from default.json config\n> file. Use\n> > the PipelineHandler::configurationFile() helper to determine the full\n> path of\n> > the file. The default.json filename can be overridden by the user\n> setting the\n> > LIBCAMERA_RPI_CONFIG_FILE environment variable giving the full path of\n> the\n> > custom file.\n> >\n> > Add config parameter validation to ensure bad parameters cannot cause the\n> > pipeline handler to stop working.\n> >\n> > Currently three parameters are available through the json file:\n> >\n> > \"min_unicam_buffers\"\n> > The minimum number of internal Unicam buffers to allocate.\n> >\n> > \"min_total_unicam_buffers\"\n> > The minimum number of internal + external Unicam buffers that must be\n> allocated.\n> >\n> > \"num_output0_buffers\"\n> > Number of internal ISP Output0 buffers to allocate.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/raspberrypi/data/default.json | 20 +++++++\n> > .../pipeline/raspberrypi/data/meson.build | 8 +++\n> > .../pipeline/raspberrypi/meson.build | 2 +\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 56 +++++++++++++++++++\n> > 4 files changed, 86 insertions(+)\n> > create mode 100644 src/libcamera/pipeline/raspberrypi/data/default.json\n> > create mode 100644 src/libcamera/pipeline/raspberrypi/data/meson.build\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/data/default.json\n> b/src/libcamera/pipeline/raspberrypi/data/default.json\n> > new file mode 100644\n> > index 000000000000..d709e31850af\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/raspberrypi/data/default.json\n> > @@ -0,0 +1,20 @@\n> > +{\n> > + \"version\": 1.0,\n> > + \"target\": \"bcm2835\",\n> > +\n> > + \"pipeline_handler\":\n> > + {\n> > + # The minimum number of internal buffers to be\n> allocated for Unicam.\n> > + # This value must be greater than 0, but less than or\n> equal to min_total_unicam_buffers.\n>\n> Are we allowed comments in json files now? That's nice! Perhaps we\n> should put some in our tuning files...\n>\n> > + \"min_unicam_buffers\": 2,\n> > +\n> > + # The minimum total (internal + external) buffer count\n> used for Unicam.\n> > + # The number of internal buffers allocated for Unicam\n> is given by:\n> > + # internal buffer count = max(min_unicam_buffers,\n> > + # min_total_unicam_buffers\n> - external buffer count)\n> > + \"min_total_unicam_buffers\": 4,\n> > +\n> > + # The number of internal buffers used for ISP Output0.\n> This must be set to 1.\n> > + \"num_output0_buffers\": 1\n> > + }\n> > +}\n> > diff --git a/src/libcamera/pipeline/raspberrypi/data/meson.build\n> b/src/libcamera/pipeline/raspberrypi/data/meson.build\n> > new file mode 100644\n> > index 000000000000..232f8b43c5fd\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/raspberrypi/data/meson.build\n> > @@ -0,0 +1,8 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +\n> > +conf_files = files([\n> > + 'default.json',\n> > +])\n> > +\n> > +install_data(conf_files,\n> > + install_dir : libcamera_datadir / 'pipeline/raspberrypi')\n> > diff --git a/src/libcamera/pipeline/raspberrypi/meson.build\n> b/src/libcamera/pipeline/raspberrypi/meson.build\n> > index f1a2f5ee72c2..48235f887501 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/meson.build\n> > +++ b/src/libcamera/pipeline/raspberrypi/meson.build\n> > @@ -5,3 +5,5 @@ libcamera_sources += files([\n> > 'raspberrypi.cpp',\n> > 'rpi_stream.cpp',\n> > ])\n> > +\n> > +subdir('data')\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 450029197b11..bac7a66ba900 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -14,6 +14,7 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > #include \n> > #include \n> >\n> > @@ -40,6 +41,7 @@\n> > #include \"libcamera/internal/media_device.h\"\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> > #include \"libcamera/internal/v4l2_videodevice.h\"\n> > +#include \"libcamera/internal/yaml_parser.h\"\n> >\n> > #include \"dma_heaps.h\"\n> > #include \"rpi_stream.h\"\n> > @@ -301,6 +303,7 @@ public:\n> > };\n> >\n> > Config config_;\n> > + unsigned int numUnicamBuffers;\n> >\n> > private:\n> > void checkRequestCompleted();\n> > @@ -1140,6 +1143,19 @@ int PipelineHandlerRPi::queueRequestDevice(Camera\n> *camera, Request *request)\n> > */\n> > stream->setExternalBuffer(buffer);\n> > }\n> > +\n> > + if (!buffer) {\n> > + if (stream == &data->isp_[Isp::Output0] &&\n> !data->config_.numOutput0Buffers) {\n> > + LOG(RPI, Error) << \"Output0 buffer must\n> be provided in the request.\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + if (stream == &data->unicam_[Unicam::Image] &&\n> !data->numUnicamBuffers) {\n> > + LOG(RPI, Error) << \"Unicam Image buffer\n> must be provided in the request.\";\n> > + return -EINVAL;\n> > + }\n> > + }\n> > +\n> > /*\n> > * If no buffer is provided by the request for this\n> stream, we\n> > * queue a nullptr to the stream to signify that it must\n> use an\n> > @@ -1398,6 +1414,7 @@ int PipelineHandlerRPi::registerCamera(MediaDevice\n> *unicam, MediaDevice *isp, Me\n> > int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)\n> > {\n> > RPiCameraData::Config &config = data->config_;\n> > + std::string filename;\n> >\n> > config = {\n> > .minUnicamBuffers = 2,\n> > @@ -1405,6 +1422,44 @@ int\n> PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)\n> > .numOutput0Buffers = 1,\n> > };\n> >\n> > + char const *configFromEnv =\n> utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> > + if (!configFromEnv || *configFromEnv == '\\0')\n> > + filename = configurationFile(\"raspberrypi/default.json\");\n>\n> I was a teeny bit nervous that in the non-installed case it might come\n> back with \"libcamera/src/ipa/raspberrypi/data/raspberrypi/default.json\"\n> which would be the wrong thing? Maybe you could just put my mind at\n> ease here...\n>\n\nIt should load things from\n\"libcamera/src/libcamera/pipeline/data/raspberrypi/default.json\",\nbut I ought to check this again to make sure!\n\nHowever, that does lead to some awkwardness, perhaps it should be read from\n \"libcamera/src/libcamera/pipeline/raspberrypi/data/default.json\" to be\nconsistent with\nthe ipa config files...?\n\n\n\n>\n> > + else\n> > + filename = std::string(configFromEnv);\n> > +\n> > + if (filename.empty())\n> > + return -EINVAL;\n> > +\n> > + File file(filename);\n> > + if (!file.open(File::OpenModeFlag::ReadOnly)) {\n> > + LOG(RPI, Error) << \"Failed to open configuration file '\"\n> << filename << \"'\";\n> > + return -EIO;\n> > + }\n> > +\n> > + LOG(RPI, Info) << \"Using configuration file '\" << filename <<\n> \"'\";\n> > +\n> > + std::unique_ptr root = YamlParser::parse(file);\n> > + if (!root) {\n> > + LOG(RPI, Error) << \"Failed to parse configuration file,\n> using defaults!\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + ASSERT((*root)[\"version\"].get() == 1.0);\n> > +\n> > + const YamlObject &phConfig = (*root)[\"pipeline_handler\"];\n> > + config.minUnicamBuffers =\n> > + phConfig[\"min_unicam_buffers\"].get int>(config.minUnicamBuffers);\n> > + config.minTotalUnicamBuffers =\n> > + phConfig[\"min_total_unicam_buffers\"].get int>(config.minTotalUnicamBuffers);\n> > + config.numOutput0Buffers =\n> > + phConfig[\"num_output0_buffers\"].get int>(config.numOutput0Buffers);\n> > +\n> > + if (config.minTotalUnicamBuffers < config.minUnicamBuffers ||\n> config.minTotalUnicamBuffers < 1) {\n>\n> I wonder a little bit about splitting up all these little checks so\n> that we can be really specific in the error message? But I'm not too\n> fussed.\n>\n\nGood point, I will split this up.\n\nRegards,\nNaush\n\n\n>\n> Subject to a check on that pathname question I had:\n>\n> Reviewed-by: David Plowman \n>\n> Thanks!\n> David\n>\n> > + LOG(RPI, Error) << \"Invalid Unicam buffer configuration\n> used!\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > return 0;\n> > }\n> >\n> > @@ -1471,6 +1526,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera\n> *camera)\n> > */\n> > numBuffers =\n> std::max(data->config_.minUnicamBuffers,\n> > minBuffers -\n> numRawBuffers);\n> > + data->numUnicamBuffers = numBuffers;\n> > } else if (stream == &data->isp_[Isp::Input]) {\n> > /*\n> > * ISP input buffers are imported from Unicam,\n> so follow\n> > --\n> > 2.25.1\n> >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0A2ABBE08B\n\tfor ;\n\tTue, 29 Nov 2022 10:44:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 410156333A;\n\tTue, 29 Nov 2022 11:44:36 +0100 (CET)","from mail-io1-xd31.google.com (mail-io1-xd31.google.com\n\t[IPv6:2607:f8b0:4864:20::d31])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3F5B063314\n\tfor ;\n\tTue, 29 Nov 2022 11:44:34 +0100 (CET)","by mail-io1-xd31.google.com with SMTP id r81so9734113iod.2\n\tfor ;\n\tTue, 29 Nov 2022 02:44:34 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669718676;\n\tbh=PN0foAVEXiBTeGyiArtjIld8lrkCzxsv4lvr7eBdxeg=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=zhr+XXITPjFmIdL9gZBdFVXz6Czzijdy1yG9rzDUZCOd/E55dPG/Kzg4bPq9NGkJ9\n\tygVqRmyfxqgyeRFgxaCFRgsMJhpEqaQL619az8hPnu8fNDu7eqvOQsE4B11ZZKLPSR\n\tv6mABboAcOjwjKwt6JcM8PMjgJHcmsgLlX2itZNL4djVLMh2n+hUJvmikYlF3Q383y\n\tlOK6G/e5yHYYw7oGUUJlW97iQ3qmVI3WKVsZ3IcRDXQ3ijoag/RAyoM3VMfCSyjoSS\n\tSMo5q4HZgL9VQ9mCPXYtl/yGmul/evYmYxhI806K8GtesJOOmq2O8rDtv5TrMWBO5C\n\tZq/BinKUFqKnA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=lL5WHXB0Zqo3wWlqLPVSEjk8QuC7WKQ2gNTSBLWDTOA=;\n\tb=s/eRbCQ8GqNODP4dePvnU9UZC5ZLMMrW45qHxMEr1vajMCQSl3ok0vorRywCgQBDFL\n\t7HVSk+89jgLmTs9XFcJ74xnMGhnt4SEy9XuG/t8Go0/o48l2A6Pab8Mu+jdKBTH6cGeZ\n\t0lyazwJWr6v3b3BGfxg7GxTf5FHBL6Z2EFJBdso8CwzX1Kz1P1NdqYgOG7GWummrfM85\n\twWCaIiEjodWT9HbmM5e0/Pu2lMm0VsruICHMRSCZcEh5ieUg7owX5Ukj3IeYIG1SZCOW\n\tIoXVrJkJkR2T4ImEQlbLGkeqae5kqX+qwOvxV451otgFry4P8TkPh7cgW5nIhNrRG2lG\n\tQr3Q=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"s/eRbCQ8\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=lL5WHXB0Zqo3wWlqLPVSEjk8QuC7WKQ2gNTSBLWDTOA=;\n\tb=CgxARL5jYlSqKMxOjrN6Svaot82lozN4AjRv+H/Wri6MMTj0Lb50C3M1oJcFdODXlX\n\tMTe/a+czus3V+zIwlhPx2FWYkK55VLBCQugYFTAbshOJZ9hFFMl3cW+SsZHCewxM76Rw\n\tqF2qKgY72ks+CH/dgRSUcrc2ohodxT1MDwibT2oAdTxORASjd7oOj0KaDFAtEHRJDhkd\n\tNhhPnUgoDAPXlmi7jSkAV+ngTZ+zz3jg2hfYLZMowNSWRD2GcSeJx3cDMf1LPgL4cP9Q\n\tBFo3mWfs+4xCzfQUGbMUxTO87isXvZXkNDr1TUedSLNcIUBx2wA7jckCfn4gwQ4MbFSW\n\t16qg==","X-Gm-Message-State":"ANoB5pkz+OTC7TiFHjrGgxGZeotnm17eXvmOdSwjyxNkc5DHLoNOXe4F\n\tg+JZc1WiJaZPQLZBqBenP4ao6FFi1hjW/d29qj9G0LzSJc+CKg==","X-Google-Smtp-Source":"AA0mqf65ooNVL5fLXfHUg3tCN4qnEVDUqfRN+E5R3yuqo745x6ooPT8ZDDJtfYQFiIN1GsdRasGFj9kUvetN64W0Egk=","X-Received":"by 2002:a05:6638:3786:b0:363:b82d:d510 with SMTP id\n\tw6-20020a056638378600b00363b82dd510mr25503885jal.112.1669718673009;\n\tTue, 29 Nov 2022 02:44:33 -0800 (PST)","MIME-Version":"1.0","References":"<20221014131846.27169-1-naush@raspberrypi.com>\n\t<20221014131846.27169-5-naush@raspberrypi.com>\n\t","In-Reply-To":"","Date":"Tue, 29 Nov 2022 10:44:18 +0000","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"00000000000061419505ee99ac4e\"","Subject":"Re: [libcamera-devel] [PATCH v1 04/10] pipeline: raspberrypi: Read\n\tconfig parameters from a file","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]