[{"id":34498,"web_url":"https://patchwork.libcamera.org/comment/34498/","msgid":"<20250616203815.GF32454@pendragon.ideasonboard.com>","date":"2025-06-16T20:38:15","subject":"Re: [PATCH v10 01/13] yaml: Move yaml_parser.cpp to base","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nThank you for the patch.\n\nOn Mon, Jun 16, 2025 at 10:47:19AM +0200, Milan Zamazal wrote:\n> Global configuration, which uses objects from the YAML parser, will be\n> accessed from base. Thus YAML parser must be moved to base too.\n> \n> Indentation is changed in one place of the moved file to make the\n> autoformatter and checkstyle.py happy.\n> \n> Signed-off-by: Milan Zamazal \n> Reviewed-by: Kieran Bingham \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/base/meson.build | 14 ++++++++++++++\n> src/libcamera/{ => base}/yaml_parser.cpp | 13 ++++++-------\n\nYou need to also move yaml_parser.h. And you will run into an issue, the\nfile uses . I don't think geometry.h should be\nmoved to base, so we may need to see how to implement the global\nconfiguration file without moving yaml_parser.cpp to libcamera-base.\n\n> src/libcamera/meson.build | 14 --------------\n> 3 files changed, 20 insertions(+), 21 deletions(-)\n> rename src/libcamera/{ => base}/yaml_parser.cpp (98%)\n> \n> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n> index a742dfdfe..94843eb95 100644\n> --- a/src/libcamera/base/meson.build\n> +++ b/src/libcamera/base/meson.build\n> @@ -24,10 +24,12 @@ libcamera_base_internal_sources = files([\n> 'thread.cpp',\n> 'timer.cpp',\n> 'utils.cpp',\n> + 'yaml_parser.cpp',\n> ])\n> \n> libdw = dependency('libdw', required : false)\n> libunwind = dependency('libunwind', required : false)\n> +libyaml = dependency('yaml-0.1', required : false)\n> \n> if cc.has_header_symbol('execinfo.h', 'backtrace')\n> config_h.set('HAVE_BACKTRACE', 1)\n> @@ -41,11 +43,23 @@ if libunwind.found()\n> config_h.set('HAVE_UNWIND', 1)\n> endif\n> \n> +# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP.\n> +if not libyaml.found()\n> + cmake = import('cmake')\n> +\n> + libyaml_vars = cmake.subproject_options()\n> + libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'})\n> + libyaml_vars.append_compile_args('c', '-Wno-unused-value')\n> + libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars)\n> + libyaml = libyaml_wrap.dependency('yaml')\n> +endif\n> +\n> libcamera_base_deps = [\n> libatomic,\n> libdw,\n> libthreads,\n> libunwind,\n> + libyaml,\n> ]\n> \n> # Internal components must use the libcamera_base_private dependency to enable\n> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/base/yaml_parser.cpp\n> similarity index 98%\n> rename from src/libcamera/yaml_parser.cpp\n> rename to src/libcamera/base/yaml_parser.cpp\n> index a5e424615..13111faaa 100644\n> --- a/src/libcamera/yaml_parser.cpp\n> +++ b/src/libcamera/base/yaml_parser.cpp\n> @@ -149,13 +149,12 @@ YamlObject::Getter::get(const YamlObject &obj) const\n> \n> template\n> struct YamlObject::Getter -\tstd::is_same_v ||\n> -\tstd::is_same_v ||\n> -\tstd::is_same_v ||\n> -\tstd::is_same_v ||\n> -\tstd::is_same_v ||\n> -\tstd::is_same_v>>\n> -{\n> +\t\t\t\t std::is_same_v ||\n> +\t\t\t\t std::is_same_v ||\n> +\t\t\t\t std::is_same_v ||\n> +\t\t\t\t std::is_same_v ||\n> +\t\t\t\t std::is_same_v ||\n> +\t\t\t\t std::is_same_v>> {\n\nThis looks weird. You could write\n\nstruct YamlObject::Getter ||\n\t\t\t\t\t std::is_same_v ||\n\t\t\t\t\t std::is_same_v ||\n\t\t\t\t\t std::is_same_v ||\n\t\t\t\t\t std::is_same_v ||\n\t\t\t\t\t std::is_same_v>> {\n\nbut I would probably keep the existing indentation.\n\n> \tstd::optional get(const YamlObject &obj) const\n> \t{\n> \t\tif (obj.type_ != Type::Value)\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 202db1efe..24fcfcf57 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -55,7 +55,6 @@ libcamera_internal_sources = files([\n> 'v4l2_subdevice.cpp',\n> 'v4l2_videodevice.cpp',\n> 'vector.cpp',\n> - 'yaml_parser.cpp',\n> ])\n> \n> includes = [\n> @@ -83,7 +82,6 @@ if not cc.has_function('dlopen')\n> libdl = cc.find_library('dl')\n> endif\n> libudev = dependency('libudev', required : get_option('udev'))\n> -libyaml = dependency('yaml-0.1', required : false)\n> \n> # Use one of gnutls or libcrypto (provided by OpenSSL), trying gnutls first.\n> libcrypto = dependency('gnutls', required : false)\n> @@ -119,17 +117,6 @@ if libudev.found()\n> ])\n> endif\n> \n> -# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP.\n> -if not libyaml.found()\n> - cmake = import('cmake')\n> -\n> - libyaml_vars = cmake.subproject_options()\n> - libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'})\n> - libyaml_vars.append_compile_args('c', '-Wno-unused-value')\n> - libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars)\n> - libyaml = libyaml_wrap.dependency('yaml')\n> -endif\n> -\n> control_sources = []\n> \n> controls_mode_files = {\n> @@ -190,7 +177,6 @@ libcamera_deps += [\n> libdl,\n> liblttng,\n> libudev,\n> - libyaml,\n> ]\n> \n> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.","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 6A027BDE6B\n\tfor ;\n\tMon, 16 Jun 2025 20:38:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 55AF168DC7;\n\tMon, 16 Jun 2025 22:38:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ED4EC68DC0\n\tfor ;\n\tMon, 16 Jun 2025 22:38:31 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E203AC6D;\n\tMon, 16 Jun 2025 22:38:19 +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=\"iQKgK9QI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750106300;\n\tbh=pQOmvSSd/ntSktX1+OyIgBBinL5MuAk6up4DvEP3b8s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iQKgK9QIGlBhR/jLicgYy5h3rCHYVbevxT/prEuIdBSMDuzMHkhzDq99LOpCMnuPe\n\tWWAdmH4iOQSojXCEBpZyFxpmnLPpp4Xr/NgqhtTjGalWaFRWG8HWWZCtcuA5SbX8dM\n\tHCDFyVU1+IBSmgwXPJGGMuyyPKCdqbnkKi7mcjsI=","Date":"Mon, 16 Jun 2025 23:38:15 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t","Subject":"Re: [PATCH v10 01/13] yaml: Move yaml_parser.cpp to base","Message-ID":"<20250616203815.GF32454@pendragon.ideasonboard.com>","References":"<20250616084733.18707-1-mzamazal@redhat.com>\n\t<20250616084733.18707-2-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250616084733.18707-2-mzamazal@redhat.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34539,"web_url":"https://patchwork.libcamera.org/comment/34539/","msgid":"<55bea688-43a0-4ac7-b308-33959c21091e@ideasonboard.com>","date":"2025-06-18T10:44:48","subject":"Re: [PATCH v10 01/13] yaml: Move yaml_parser.cpp to base","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 06. 16. 22:38 keltezéssel, Laurent Pinchart írta:\n> Hi Milan,\n> \n> Thank you for the patch.\n> \n> On Mon, Jun 16, 2025 at 10:47:19AM +0200, Milan Zamazal wrote:\n>> Global configuration, which uses objects from the YAML parser, will be\n>> accessed from base. Thus YAML parser must be moved to base too.\n>>\n>> Indentation is changed in one place of the moved file to make the\n>> autoformatter and checkstyle.py happy.\n>>\n>> Signed-off-by: Milan Zamazal \n>> Reviewed-by: Kieran Bingham \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/libcamera/base/meson.build | 14 ++++++++++++++\n>> src/libcamera/{ => base}/yaml_parser.cpp | 13 ++++++-------\n> \n> You need to also move yaml_parser.h. And you will run into an issue, the\n> file uses . I don't think geometry.h should be\n> moved to base, so we may need to see how to implement the global\n> configuration file without moving yaml_parser.cpp to libcamera-base.\n\nI don't think this is needed anymore. At least I cannot see\nanything in the later patches. Logging is no longer touched.\nSo `GlobalConfiguration` could be moved out of libcamera-base.\nOr am I missing something?\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>> src/libcamera/meson.build | 14 --------------\n>> 3 files changed, 20 insertions(+), 21 deletions(-)\n>> rename src/libcamera/{ => base}/yaml_parser.cpp (98%)\n>>\n>> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n>> index a742dfdfe..94843eb95 100644\n>> --- a/src/libcamera/base/meson.build\n>> +++ b/src/libcamera/base/meson.build\n>> @@ -24,10 +24,12 @@ libcamera_base_internal_sources = files([\n>> 'thread.cpp',\n>> 'timer.cpp',\n>> 'utils.cpp',\n>> + 'yaml_parser.cpp',\n>> ])\n>> \n>> libdw = dependency('libdw', required : false)\n>> libunwind = dependency('libunwind', required : false)\n>> +libyaml = dependency('yaml-0.1', required : false)\n>> \n>> if cc.has_header_symbol('execinfo.h', 'backtrace')\n>> config_h.set('HAVE_BACKTRACE', 1)\n>> @@ -41,11 +43,23 @@ if libunwind.found()\n>> config_h.set('HAVE_UNWIND', 1)\n>> endif\n>> \n>> +# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP.\n>> +if not libyaml.found()\n>> + cmake = import('cmake')\n>> +\n>> + libyaml_vars = cmake.subproject_options()\n>> + libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'})\n>> + libyaml_vars.append_compile_args('c', '-Wno-unused-value')\n>> + libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars)\n>> + libyaml = libyaml_wrap.dependency('yaml')\n>> +endif\n>> +\n>> libcamera_base_deps = [\n>> libatomic,\n>> libdw,\n>> libthreads,\n>> libunwind,\n>> + libyaml,\n>> ]\n>> \n>> # Internal components must use the libcamera_base_private dependency to enable\n>> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/base/yaml_parser.cpp\n>> similarity index 98%\n>> rename from src/libcamera/yaml_parser.cpp\n>> rename to src/libcamera/base/yaml_parser.cpp\n>> index a5e424615..13111faaa 100644\n>> --- a/src/libcamera/yaml_parser.cpp\n>> +++ b/src/libcamera/base/yaml_parser.cpp\n>> @@ -149,13 +149,12 @@ YamlObject::Getter::get(const YamlObject &obj) const\n>> \n>> template\n>> struct YamlObject::Getter> -\tstd::is_same_v ||\n>> -\tstd::is_same_v ||\n>> -\tstd::is_same_v ||\n>> -\tstd::is_same_v ||\n>> -\tstd::is_same_v ||\n>> -\tstd::is_same_v>>\n>> -{\n>> +\t\t\t\t std::is_same_v ||\n>> +\t\t\t\t std::is_same_v ||\n>> +\t\t\t\t std::is_same_v ||\n>> +\t\t\t\t std::is_same_v ||\n>> +\t\t\t\t std::is_same_v ||\n>> +\t\t\t\t std::is_same_v>> {\n> \n> This looks weird. You could write\n> \n> struct YamlObject::Getter ||\n> \t\t\t\t\t std::is_same_v ||\n> \t\t\t\t\t std::is_same_v ||\n> \t\t\t\t\t std::is_same_v ||\n> \t\t\t\t\t std::is_same_v ||\n> \t\t\t\t\t std::is_same_v>> {\n> \n> but I would probably keep the existing indentation.\n> \n>> \tstd::optional get(const YamlObject &obj) const\n>> \t{\n>> \t\tif (obj.type_ != Type::Value)\n>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>> index 202db1efe..24fcfcf57 100644\n>> --- a/src/libcamera/meson.build\n>> +++ b/src/libcamera/meson.build\n>> @@ -55,7 +55,6 @@ libcamera_internal_sources = files([\n>> 'v4l2_subdevice.cpp',\n>> 'v4l2_videodevice.cpp',\n>> 'vector.cpp',\n>> - 'yaml_parser.cpp',\n>> ])\n>> \n>> includes = [\n>> @@ -83,7 +82,6 @@ if not cc.has_function('dlopen')\n>> libdl = cc.find_library('dl')\n>> endif\n>> libudev = dependency('libudev', required : get_option('udev'))\n>> -libyaml = dependency('yaml-0.1', required : false)\n>> \n>> # Use one of gnutls or libcrypto (provided by OpenSSL), trying gnutls first.\n>> libcrypto = dependency('gnutls', required : false)\n>> @@ -119,17 +117,6 @@ if libudev.found()\n>> ])\n>> endif\n>> \n>> -# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP.\n>> -if not libyaml.found()\n>> - cmake = import('cmake')\n>> -\n>> - libyaml_vars = cmake.subproject_options()\n>> - libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'})\n>> - libyaml_vars.append_compile_args('c', '-Wno-unused-value')\n>> - libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars)\n>> - libyaml = libyaml_wrap.dependency('yaml')\n>> -endif\n>> -\n>> control_sources = []\n>> \n>> controls_mode_files = {\n>> @@ -190,7 +177,6 @@ libcamera_deps += [\n>> libdl,\n>> liblttng,\n>> libudev,\n>> - libyaml,\n>> ]\n>> \n>> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\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 61EE5C3237\n\tfor ;\n\tWed, 18 Jun 2025 10:44:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6B99A68DCC;\n\tWed, 18 Jun 2025 12:44:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 33D7768DC1\n\tfor ;\n\tWed, 18 Jun 2025 12:44:53 +0200 (CEST)","from [192.168.33.16] (185.221.143.107.nat.pool.zt.hu\n\t[185.221.143.107])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CBFF9593;\n\tWed, 18 Jun 2025 12:44:39 +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=\"YL+y2S/W\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750243480;\n\tbh=8O/lIvxXS3htkAPlLVzon0vmuD3MCyvzhCwDs2C4VBA=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=YL+y2S/Wwn/sOvmsXofLD60L8ZIdC3ei8EymVaaCQ6dzXzr+02I2v/9Zoi9x8AlK6\n\tuMitxoUAcCKrOZLIQVhA4gslBgggVg04C5RaJ+RzbzrIJH5clvD/vJ+8CmL9XPOZry\n\tA/2wO2evkQvOr6fsnbXb50wurRRTP6ACrdj8XW20=","Message-ID":"<55bea688-43a0-4ac7-b308-33959c21091e@ideasonboard.com>","Date":"Wed, 18 Jun 2025 12:44:48 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v10 01/13] yaml: Move yaml_parser.cpp to base","To":"Laurent Pinchart ,\n\tMilan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ","References":"<20250616084733.18707-1-mzamazal@redhat.com>\n\t<20250616084733.18707-2-mzamazal@redhat.com>\n\t<20250616203815.GF32454@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250616203815.GF32454@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]