[{"id":31602,"web_url":"https://patchwork.libcamera.org/comment/31602/","msgid":"<20241007213912.GA30699@pendragon.ideasonboard.com>","date":"2024-10-07T21:39:12","subject":"Re: [PATCH v2 2/2] apps: cam: Add support for loading configuration\n\tfrom capture script","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Fri, Oct 04, 2024 at 09:05:17PM +0900, Paul Elder wrote:\n> Add support to the cam application for loading the camera configuration\n> from a capture script. These can be written manually, or dumped from a\n> capture session via the LIBCAMERA_DUMP_CAPTURE_SCRIPT environment\n> variable.\n> \n> If any configuration options are specified by command line parameters,\n> those will take precedence.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> \n> ---\n> Changes in v2:\n> - clean up code\n> ---\n>  src/apps/cam/camera_session.cpp |  26 +++--\n>  src/apps/cam/capture_script.cpp | 163 ++++++++++++++++++++++++++++++++\n>  src/apps/cam/capture_script.h   |   9 ++\n>  src/apps/cam/main.cpp           |   4 +-\n>  4 files changed, 191 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> index edc49b875450..695b313c3779 100644\n> --- a/src/apps/cam/camera_session.cpp\n> +++ b/src/apps/cam/camera_session.cpp\n> @@ -70,6 +70,22 @@ CameraSession::CameraSession(CameraManager *cm,\n>  \t\treturn;\n>  \t}\n>  \n> +\t/*\n> +\t * Parse the capture script first to populate the configuration, and\n> +\t * let command line arguments take precedence.\n> +\t */\n> +\tif (options_.isSet(OptCaptureScript)) {\n> +\t\tstd::string scriptName = options_[OptCaptureScript].toString();\n> +\t\tscript_ = std::make_unique<CaptureScript>(camera_, scriptName);\n> +\t\tif (!script_->valid()) {\n> +\t\t\tstd::cerr << \"Invalid capture script '\" << scriptName\n> +\t\t\t\t  << \"'\" << std::endl;\n> +\t\t\treturn;\n> +\t\t}\n> +\n> +\t\tscript_->populateConfiguration(*config);\n> +\t}\n> +\n>  \tif (options_.isSet(OptOrientation)) {\n>  \t\tstd::string orientOpt = options_[OptOrientation].toString();\n>  \t\tstatic const std::map<std::string, libcamera::Orientation> orientations{\n> @@ -119,16 +135,6 @@ CameraSession::CameraSession(CameraManager *cm,\n>  \t}\n>  #endif\n>  \n> -\tif (options_.isSet(OptCaptureScript)) {\n> -\t\tstd::string scriptName = options_[OptCaptureScript].toString();\n> -\t\tscript_ = std::make_unique<CaptureScript>(camera_, scriptName);\n> -\t\tif (!script_->valid()) {\n> -\t\t\tstd::cerr << \"Invalid capture script '\" << scriptName\n> -\t\t\t\t  << \"'\" << std::endl;\n> -\t\t\treturn;\n> -\t\t}\n> -\t}\n> -\n>  \tswitch (config->validate()) {\n>  \tcase CameraConfiguration::Valid:\n>  \t\tbreak;\n> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n> index fc1dfa75f2d4..a4298f99c4d8 100644\n> --- a/src/apps/cam/capture_script.cpp\n> +++ b/src/apps/cam/capture_script.cpp\n> @@ -7,6 +7,7 @@\n>  \n>  #include \"capture_script.h\"\n>  \n> +#include <algorithm>\n>  #include <iostream>\n>  #include <stdio.h>\n>  #include <stdlib.h>\n> @@ -158,6 +159,10 @@ int CaptureScript::parseScript(FILE *script)\n>  \t\t\tret = parseProperties();\n>  \t\t\tif (ret)\n>  \t\t\t\treturn ret;\n> +\t\t} else if (section == \"configuration\") {\n> +\t\t\tret = parseConfiguration();\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n>  \t\t} else if (section == \"frames\") {\n>  \t\t\tret = parseFrames();\n>  \t\t\tif (ret)\n> @@ -229,6 +234,156 @@ int CaptureScript::parseProperties()\n>  \treturn 0;\n>  }\n>  \n> +int CaptureScript::parseConfiguration()\n> +{\n> +\tint ret;\n> +\n> +\tEventPtr event = nextEvent(YAML_MAPPING_START_EVENT);\n> +\tif (!event)\n> +\t\treturn -EINVAL;\n> +\n> +\twhile (1) {\n> +\t\tevent = nextEvent();\n> +\t\tif (!event)\n> +\t\t\treturn -EINVAL;\n> +\n> +\t\tif (event->type == YAML_MAPPING_END_EVENT)\n> +\t\t\tbreak;\n> +\n> +\t\tstd::string key = eventScalarValue(event);\n> +\n> +\t\tevent = nextEvent();\n> +\t\tif (!event)\n> +\t\t\treturn -EINVAL;\n> +\t\tif (event->type == YAML_MAPPING_END_EVENT)\n> +\t\t\tbreak;\n> +\n> +\t\t/* TODO Load sensor configuration */\n\n\\todo\n\n> +\t\tif (key == \"orientation\") {\n> +\t\t\tret = parseOrientation(std::move(event));\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\t\t} else if (key == \"streams\") {\n> +\t\t\tret = parseStreams(std::move(event));\n> +\t\t\tif (ret)\n> +\t\t\t\treturn ret;\n> +\t\t}\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int CaptureScript::parseOrientation(EventPtr event)\n> +{\n> +\tstatic const std::map<std::string, libcamera::Orientation> orientations{\n> +\t\t{ \"Rotate0\", libcamera::Orientation::Rotate0 },\n> +\t\t{ \"Rotate0Mirror\", libcamera::Orientation::Rotate0Mirror },\n> +\t\t{ \"Rotate180\", libcamera::Orientation::Rotate180 },\n> +\t\t{ \"Rotate180Mirror\", libcamera::Orientation::Rotate180Mirror },\n> +\t\t{ \"Rotate90Mirror\", libcamera::Orientation::Rotate90Mirror },\n> +\t\t{ \"Rotate270\", libcamera::Orientation::Rotate270 },\n> +\t\t{ \"Rotate270Mirror\", libcamera::Orientation::Rotate270Mirror },\n> +\t\t{ \"Rotate90\", libcamera::Orientation::Rotate90 },\n> +\t};\n> +\n> +\tstd::string orientation = eventScalarValue(event);\n> +\n> +\tauto it = orientations.find(orientation);\n> +\tif (it == orientations.end()) {\n> +\t\tstd::cerr << \"Invalid orientation '\" << orientation\n> +\t\t\t  << \"' in capture script\" << std::endl;\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\torientation_ = it->second;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int CaptureScript::parseStreams(EventPtr event)\n> +{\n> +\tint ret;\n> +\n> +\tif (!checkEvent(event, YAML_SEQUENCE_START_EVENT))\n> +\t\treturn -EINVAL;\n> +\n> +\tunsigned int index = 0;\n> +\n> +\twhile (1) {\n> +\t\tevent = nextEvent();\n> +\t\tif (!event)\n> +\t\t\treturn -EINVAL;\n> +\n> +\t\tif (event->type == YAML_SEQUENCE_END_EVENT)\n> +\t\t\treturn 0;\n> +\n> +\t\tif (event->type == YAML_MAPPING_START_EVENT) {\n> +\t\t\tif ((ret = parseStream(std::move(event), index++)) < 0)\n\n\t\t\tint ret = parseStream(std::move(event), index++));\n\t\t\tif (ret < 0)\n\n> +\t\t\t\treturn ret;\n> +\t\t} else {\n> +\t\t\tstd::cerr << \"Unknown type\" << std::endl;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n\nPlease also see my last reply on v1.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int CaptureScript::parseStream(EventPtr event, unsigned int index)\n> +{\n> +\tif (!checkEvent(event, YAML_MAPPING_START_EVENT))\n> +\t\treturn -EINVAL;\n> +\n> +\tStreamConfiguration config;\n> +\twhile (1) {\n> +\t\tevent = nextEvent();\n> +\t\tif (!event)\n> +\t\t\treturn -EINVAL;\n> +\n> +\t\tif (event->type == YAML_MAPPING_END_EVENT)\n> +\t\t\tbreak;\n> +\n> +\t\tstd::string key = eventScalarValue(event);\n> +\n> +\t\tstd::string value = parseScalar();\n> +\t\tif (value.empty())\n> +\t\t\treturn -EINVAL;\n> +\n> +\t\tif (key == \"pixelFormat\") {\n> +\t\t\tconfig.pixelFormat = libcamera::PixelFormat::fromString(value);\n> +\t\t} else if (key == \"size\") {\n> +\t\t\tunsigned int split = value.find(\"x\");\n> +\t\t\tif (split == std::string::npos) {\n> +\t\t\t\tstd::cerr << \"Invalid size '\" << value\n> +\t\t\t\t\t  << \"' in stream configuration \"\n> +\t\t\t\t\t  << index << std::endl;\n> +\t\t\t}\n> +\n> +\t\t\tstd::string width = value.substr(0, split);\n> +\t\t\tstd::string height = value.substr(split + 1);\n> +\t\t\tconfig.size = Size(std::stoi(width), std::stoi(height));\n> +\t\t} else if (key == \"stride\") {\n> +\t\t\tconfig.stride = std::stoi(value);\n> +\t\t} else if (key == \"frameSize\") {\n> +\t\t\tconfig.frameSize = std::stoi(value);\n> +\t\t} else if (key == \"bufferCount\") {\n> +\t\t\tconfig.bufferCount = std::stoi(value);\n> +\t\t} else if (key == \"colorSpace\") {\n> +\t\t\tconfig.colorSpace = libcamera::ColorSpace::fromString(value);\n> +\t\t} else {\n> +\t\t\tstd::cerr << \"Unknown key-value pair '\"\n> +\t\t\t\t  << key << \"': '\" << value\n> +\t\t\t\t  << \"' in stream configuration \"\n> +\t\t\t\t  << index << std::endl;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t}\n> +\n> +\tstreamConfigs_.push_back(config);\n> +\n> +\treturn 0;\n> +}\n> +\n>  int CaptureScript::parseFrames()\n>  {\n>  \tEventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);\n> @@ -322,6 +477,14 @@ int CaptureScript::parseControl(EventPtr event, ControlList &controls)\n>  \treturn 0;\n>  }\n>  \n> +void CaptureScript::populateConfiguration(CameraConfiguration &configuration) const\n> +{\n> +\tconfiguration.orientation = orientation_;\n> +\n> +\tfor (unsigned int i = 0; i < streamConfigs_.size(); i++)\n> +\t\tconfiguration[i] = streamConfigs_[i];\n> +}\n> +\n>  std::string CaptureScript::parseScalar()\n>  {\n>  \tEventPtr event = nextEvent(YAML_SCALAR_EVENT);\n> diff --git a/src/apps/cam/capture_script.h b/src/apps/cam/capture_script.h\n> index 294b920363ba..4fa3447d156f 100644\n> --- a/src/apps/cam/capture_script.h\n> +++ b/src/apps/cam/capture_script.h\n> @@ -26,6 +26,8 @@ public:\n>  \n>  \tconst libcamera::ControlList &frameControls(unsigned int frame);\n>  \n> +\tvoid populateConfiguration(libcamera::CameraConfiguration &configuration) const;\n> +\n>  private:\n>  \tstruct EventDeleter {\n>  \t\tvoid operator()(yaml_event_t *event) const\n> @@ -43,6 +45,9 @@ private:\n>  \tunsigned int loop_;\n>  \tbool valid_;\n>  \n> +\tlibcamera::Orientation orientation_;\n> +\tstd::vector<libcamera::StreamConfiguration> streamConfigs_;\n> +\n>  \tEventPtr nextEvent(yaml_event_type_t expectedType = YAML_NO_EVENT);\n>  \tbool checkEvent(const EventPtr &event, yaml_event_type_t expectedType) const;\n>  \tstatic std::string eventScalarValue(const EventPtr &event);\n> @@ -52,6 +57,10 @@ private:\n>  \n>  \tint parseProperties();\n>  \tint parseProperty();\n> +\tint parseConfiguration();\n> +\tint parseOrientation(EventPtr event);\n> +\tint parseStreams(EventPtr event);\n> +\tint parseStream(EventPtr event, unsigned int index);\n>  \tint parseFrames();\n>  \tint parseFrame(EventPtr event);\n>  \tint parseControl(EventPtr event, libcamera::ControlList &controls);\n> diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp\n> index 460dbc813060..4291b64e250d 100644\n> --- a/src/apps/cam/main.cpp\n> +++ b/src/apps/cam/main.cpp\n> @@ -175,7 +175,9 @@ int CamApp::parseOptions(int argc, char *argv[])\n>  \t\t\t \"metadata\", ArgumentNone, nullptr, false,\n>  \t\t\t OptCamera);\n>  \tparser.addOption(OptCaptureScript, OptionString,\n> -\t\t\t \"Load a capture session configuration script from a file\",\n> +\t\t\t \"Load a capture session configuration script from a file.\\n\"\n> +\t\t\t \"Configuration options specified in the capture script will be\\n\"\n> +\t\t\t \"overwritten by --stream and --orientation.\",\n>  \t\t\t \"script\", ArgumentRequired, \"script\", false,\n>  \t\t\t OptCamera);\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 277DABD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  7 Oct 2024 21:39:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 58DE86353A;\n\tMon,  7 Oct 2024 23:39:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8A3AD6351F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  7 Oct 2024 23:39:18 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [132.205.230.14])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D886555;\n\tMon,  7 Oct 2024 23:37:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"orTiOFFn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728337062;\n\tbh=L7FyIgMpiNNwLIRU6q71Zde968gwaoFzpI2zmOHNlcs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=orTiOFFng5TAPyyU9Xkx+bdyR8ZEePHkJc4jcWIappd2TiwOM2zbn9WenLXeBZBIL\n\tTTzUHjWWA8zAT4VuBNbWfAE5Y1TE4OzfOTo3L+K9CZtMDDgg/+ZCaLzW5pCiOPQ4sk\n\tmkUzqBqSo0us1WgTvjcqwqX9wsN+yEsuvKqA6NKw=","Date":"Tue, 8 Oct 2024 00:39:12 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 2/2] apps: cam: Add support for loading configuration\n\tfrom capture script","Message-ID":"<20241007213912.GA30699@pendragon.ideasonboard.com>","References":"<20241004120517.3572281-1-paul.elder@ideasonboard.com>\n\t<20241004120517.3572281-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241004120517.3572281-3-paul.elder@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]