| Message ID | 20250616084733.18707-2-mzamazal@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Milan, Thank you for the patch. On Mon, Jun 16, 2025 at 10:47:19AM +0200, Milan Zamazal wrote: > Global configuration, which uses objects from the YAML parser, will be > accessed from base. Thus YAML parser must be moved to base too. > > Indentation is changed in one place of the moved file to make the > autoformatter and checkstyle.py happy. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/base/meson.build | 14 ++++++++++++++ > src/libcamera/{ => base}/yaml_parser.cpp | 13 ++++++------- You need to also move yaml_parser.h. And you will run into an issue, the file uses <libcamera/geometry.h>. I don't think geometry.h should be moved to base, so we may need to see how to implement the global configuration file without moving yaml_parser.cpp to libcamera-base. > src/libcamera/meson.build | 14 -------------- > 3 files changed, 20 insertions(+), 21 deletions(-) > rename src/libcamera/{ => base}/yaml_parser.cpp (98%) > > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build > index a742dfdfe..94843eb95 100644 > --- a/src/libcamera/base/meson.build > +++ b/src/libcamera/base/meson.build > @@ -24,10 +24,12 @@ libcamera_base_internal_sources = files([ > 'thread.cpp', > 'timer.cpp', > 'utils.cpp', > + 'yaml_parser.cpp', > ]) > > libdw = dependency('libdw', required : false) > libunwind = dependency('libunwind', required : false) > +libyaml = dependency('yaml-0.1', required : false) > > if cc.has_header_symbol('execinfo.h', 'backtrace') > config_h.set('HAVE_BACKTRACE', 1) > @@ -41,11 +43,23 @@ if libunwind.found() > config_h.set('HAVE_UNWIND', 1) > endif > > +# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP. > +if not libyaml.found() > + cmake = import('cmake') > + > + libyaml_vars = cmake.subproject_options() > + libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'}) > + libyaml_vars.append_compile_args('c', '-Wno-unused-value') > + libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars) > + libyaml = libyaml_wrap.dependency('yaml') > +endif > + > libcamera_base_deps = [ > libatomic, > libdw, > libthreads, > libunwind, > + libyaml, > ] > > # Internal components must use the libcamera_base_private dependency to enable > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/base/yaml_parser.cpp > similarity index 98% > rename from src/libcamera/yaml_parser.cpp > rename to src/libcamera/base/yaml_parser.cpp > index a5e424615..13111faaa 100644 > --- a/src/libcamera/yaml_parser.cpp > +++ b/src/libcamera/base/yaml_parser.cpp > @@ -149,13 +149,12 @@ YamlObject::Getter<bool>::get(const YamlObject &obj) const > > template<typename T> > struct YamlObject::Getter<T, std::enable_if_t< > - std::is_same_v<int8_t, T> || > - std::is_same_v<uint8_t, T> || > - std::is_same_v<int16_t, T> || > - std::is_same_v<uint16_t, T> || > - std::is_same_v<int32_t, T> || > - std::is_same_v<uint32_t, T>>> > -{ > + std::is_same_v<int8_t, T> || > + std::is_same_v<uint8_t, T> || > + std::is_same_v<int16_t, T> || > + std::is_same_v<uint16_t, T> || > + std::is_same_v<int32_t, T> || > + std::is_same_v<uint32_t, T>>> { This looks weird. You could write struct YamlObject::Getter<T, std::enable_if_t<std::is_same_v<int8_t, T> || std::is_same_v<uint8_t, T> || std::is_same_v<int16_t, T> || std::is_same_v<uint16_t, T> || std::is_same_v<int32_t, T> || std::is_same_v<uint32_t, T>>> { but I would probably keep the existing indentation. > std::optional<T> get(const YamlObject &obj) const > { > if (obj.type_ != Type::Value) > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 202db1efe..24fcfcf57 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -55,7 +55,6 @@ libcamera_internal_sources = files([ > 'v4l2_subdevice.cpp', > 'v4l2_videodevice.cpp', > 'vector.cpp', > - 'yaml_parser.cpp', > ]) > > includes = [ > @@ -83,7 +82,6 @@ if not cc.has_function('dlopen') > libdl = cc.find_library('dl') > endif > libudev = dependency('libudev', required : get_option('udev')) > -libyaml = dependency('yaml-0.1', required : false) > > # Use one of gnutls or libcrypto (provided by OpenSSL), trying gnutls first. > libcrypto = dependency('gnutls', required : false) > @@ -119,17 +117,6 @@ if libudev.found() > ]) > endif > > -# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP. > -if not libyaml.found() > - cmake = import('cmake') > - > - libyaml_vars = cmake.subproject_options() > - libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'}) > - libyaml_vars.append_compile_args('c', '-Wno-unused-value') > - libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars) > - libyaml = libyaml_wrap.dependency('yaml') > -endif > - > control_sources = [] > > controls_mode_files = { > @@ -190,7 +177,6 @@ libcamera_deps += [ > libdl, > liblttng, > libudev, > - libyaml, > ] > > # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.
Hi 2025. 06. 16. 22:38 keltezéssel, Laurent Pinchart írta: > Hi Milan, > > Thank you for the patch. > > On Mon, Jun 16, 2025 at 10:47:19AM +0200, Milan Zamazal wrote: >> Global configuration, which uses objects from the YAML parser, will be >> accessed from base. Thus YAML parser must be moved to base too. >> >> Indentation is changed in one place of the moved file to make the >> autoformatter and checkstyle.py happy. >> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> --- >> src/libcamera/base/meson.build | 14 ++++++++++++++ >> src/libcamera/{ => base}/yaml_parser.cpp | 13 ++++++------- > > You need to also move yaml_parser.h. And you will run into an issue, the > file uses <libcamera/geometry.h>. I don't think geometry.h should be > moved to base, so we may need to see how to implement the global > configuration file without moving yaml_parser.cpp to libcamera-base. I don't think this is needed anymore. At least I cannot see anything in the later patches. Logging is no longer touched. So `GlobalConfiguration` could be moved out of libcamera-base. Or am I missing something? Regards, Barnabás Pőcze > >> src/libcamera/meson.build | 14 -------------- >> 3 files changed, 20 insertions(+), 21 deletions(-) >> rename src/libcamera/{ => base}/yaml_parser.cpp (98%) >> >> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build >> index a742dfdfe..94843eb95 100644 >> --- a/src/libcamera/base/meson.build >> +++ b/src/libcamera/base/meson.build >> @@ -24,10 +24,12 @@ libcamera_base_internal_sources = files([ >> 'thread.cpp', >> 'timer.cpp', >> 'utils.cpp', >> + 'yaml_parser.cpp', >> ]) >> >> libdw = dependency('libdw', required : false) >> libunwind = dependency('libunwind', required : false) >> +libyaml = dependency('yaml-0.1', required : false) >> >> if cc.has_header_symbol('execinfo.h', 'backtrace') >> config_h.set('HAVE_BACKTRACE', 1) >> @@ -41,11 +43,23 @@ if libunwind.found() >> config_h.set('HAVE_UNWIND', 1) >> endif >> >> +# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP. >> +if not libyaml.found() >> + cmake = import('cmake') >> + >> + libyaml_vars = cmake.subproject_options() >> + libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'}) >> + libyaml_vars.append_compile_args('c', '-Wno-unused-value') >> + libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars) >> + libyaml = libyaml_wrap.dependency('yaml') >> +endif >> + >> libcamera_base_deps = [ >> libatomic, >> libdw, >> libthreads, >> libunwind, >> + libyaml, >> ] >> >> # Internal components must use the libcamera_base_private dependency to enable >> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/base/yaml_parser.cpp >> similarity index 98% >> rename from src/libcamera/yaml_parser.cpp >> rename to src/libcamera/base/yaml_parser.cpp >> index a5e424615..13111faaa 100644 >> --- a/src/libcamera/yaml_parser.cpp >> +++ b/src/libcamera/base/yaml_parser.cpp >> @@ -149,13 +149,12 @@ YamlObject::Getter<bool>::get(const YamlObject &obj) const >> >> template<typename T> >> struct YamlObject::Getter<T, std::enable_if_t< >> - std::is_same_v<int8_t, T> || >> - std::is_same_v<uint8_t, T> || >> - std::is_same_v<int16_t, T> || >> - std::is_same_v<uint16_t, T> || >> - std::is_same_v<int32_t, T> || >> - std::is_same_v<uint32_t, T>>> >> -{ >> + std::is_same_v<int8_t, T> || >> + std::is_same_v<uint8_t, T> || >> + std::is_same_v<int16_t, T> || >> + std::is_same_v<uint16_t, T> || >> + std::is_same_v<int32_t, T> || >> + std::is_same_v<uint32_t, T>>> { > > This looks weird. You could write > > struct YamlObject::Getter<T, std::enable_if_t<std::is_same_v<int8_t, T> || > std::is_same_v<uint8_t, T> || > std::is_same_v<int16_t, T> || > std::is_same_v<uint16_t, T> || > std::is_same_v<int32_t, T> || > std::is_same_v<uint32_t, T>>> { > > but I would probably keep the existing indentation. > >> std::optional<T> get(const YamlObject &obj) const >> { >> if (obj.type_ != Type::Value) >> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build >> index 202db1efe..24fcfcf57 100644 >> --- a/src/libcamera/meson.build >> +++ b/src/libcamera/meson.build >> @@ -55,7 +55,6 @@ libcamera_internal_sources = files([ >> 'v4l2_subdevice.cpp', >> 'v4l2_videodevice.cpp', >> 'vector.cpp', >> - 'yaml_parser.cpp', >> ]) >> >> includes = [ >> @@ -83,7 +82,6 @@ if not cc.has_function('dlopen') >> libdl = cc.find_library('dl') >> endif >> libudev = dependency('libudev', required : get_option('udev')) >> -libyaml = dependency('yaml-0.1', required : false) >> >> # Use one of gnutls or libcrypto (provided by OpenSSL), trying gnutls first. >> libcrypto = dependency('gnutls', required : false) >> @@ -119,17 +117,6 @@ if libudev.found() >> ]) >> endif >> >> -# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP. >> -if not libyaml.found() >> - cmake = import('cmake') >> - >> - libyaml_vars = cmake.subproject_options() >> - libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'}) >> - libyaml_vars.append_compile_args('c', '-Wno-unused-value') >> - libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars) >> - libyaml = libyaml_wrap.dependency('yaml') >> -endif >> - >> control_sources = [] >> >> controls_mode_files = { >> @@ -190,7 +177,6 @@ libcamera_deps += [ >> libdl, >> liblttng, >> libudev, >> - libyaml, >> ] >> >> # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag. >
diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build index a742dfdfe..94843eb95 100644 --- a/src/libcamera/base/meson.build +++ b/src/libcamera/base/meson.build @@ -24,10 +24,12 @@ libcamera_base_internal_sources = files([ 'thread.cpp', 'timer.cpp', 'utils.cpp', + 'yaml_parser.cpp', ]) libdw = dependency('libdw', required : false) libunwind = dependency('libunwind', required : false) +libyaml = dependency('yaml-0.1', required : false) if cc.has_header_symbol('execinfo.h', 'backtrace') config_h.set('HAVE_BACKTRACE', 1) @@ -41,11 +43,23 @@ if libunwind.found() config_h.set('HAVE_UNWIND', 1) endif +# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP. +if not libyaml.found() + cmake = import('cmake') + + libyaml_vars = cmake.subproject_options() + libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'}) + libyaml_vars.append_compile_args('c', '-Wno-unused-value') + libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars) + libyaml = libyaml_wrap.dependency('yaml') +endif + libcamera_base_deps = [ libatomic, libdw, libthreads, libunwind, + libyaml, ] # Internal components must use the libcamera_base_private dependency to enable diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/base/yaml_parser.cpp similarity index 98% rename from src/libcamera/yaml_parser.cpp rename to src/libcamera/base/yaml_parser.cpp index a5e424615..13111faaa 100644 --- a/src/libcamera/yaml_parser.cpp +++ b/src/libcamera/base/yaml_parser.cpp @@ -149,13 +149,12 @@ YamlObject::Getter<bool>::get(const YamlObject &obj) const template<typename T> struct YamlObject::Getter<T, std::enable_if_t< - std::is_same_v<int8_t, T> || - std::is_same_v<uint8_t, T> || - std::is_same_v<int16_t, T> || - std::is_same_v<uint16_t, T> || - std::is_same_v<int32_t, T> || - std::is_same_v<uint32_t, T>>> -{ + std::is_same_v<int8_t, T> || + std::is_same_v<uint8_t, T> || + std::is_same_v<int16_t, T> || + std::is_same_v<uint16_t, T> || + std::is_same_v<int32_t, T> || + std::is_same_v<uint32_t, T>>> { std::optional<T> get(const YamlObject &obj) const { if (obj.type_ != Type::Value) diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 202db1efe..24fcfcf57 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -55,7 +55,6 @@ libcamera_internal_sources = files([ 'v4l2_subdevice.cpp', 'v4l2_videodevice.cpp', 'vector.cpp', - 'yaml_parser.cpp', ]) includes = [ @@ -83,7 +82,6 @@ if not cc.has_function('dlopen') libdl = cc.find_library('dl') endif libudev = dependency('libudev', required : get_option('udev')) -libyaml = dependency('yaml-0.1', required : false) # Use one of gnutls or libcrypto (provided by OpenSSL), trying gnutls first. libcrypto = dependency('gnutls', required : false) @@ -119,17 +117,6 @@ if libudev.found() ]) endif -# Fallback to a subproject if libyaml isn't found, as it's not packaged in AOSP. -if not libyaml.found() - cmake = import('cmake') - - libyaml_vars = cmake.subproject_options() - libyaml_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'}) - libyaml_vars.append_compile_args('c', '-Wno-unused-value') - libyaml_wrap = cmake.subproject('libyaml', options : libyaml_vars) - libyaml = libyaml_wrap.dependency('yaml') -endif - control_sources = [] controls_mode_files = { @@ -190,7 +177,6 @@ libcamera_deps += [ libdl, liblttng, libudev, - libyaml, ] # We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.