Message ID | 20250114211935.715028-1-kieran.bingham@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
Quoting Kieran Bingham (2025-01-14 21:19:35) > From: Dylan Aïssi <daissi@debian.org> > > We already fall back to a subproject to support the libyuv package when > it can not be discovered through the usual dependency() mechanism. > > Unfortunately libyuv may be packaged without any corresponding > pkg-config support as can be seen at [0], so further extend the > dependency search by using an explicit cc.find_library() call. > > [0] https://packages.debian.org/bookworm/amd64/libyuv-dev/filelist > > Signed-off-by: Dylan Aïssi <dylan.aissi@collabora.com> I've posted this patch on Dylan's behalf from https://salsa.debian.org/multimedia-team/libcamera/-/commit/685482827a0e6fe5dfe19778e633ab95b703cef4 though I have reworded the commit message and comment inline, but the core functional change is direct from Dylan's patch... On Ubuntu 22.04.5 LTS: $ sudo apt install libyuv-dev ... libcamera$ meson setup ... ... Found pkg-config: YES (/usr/bin/pkg-config) 0.29.2 Found CMake: /usr/bin/cmake (3.22.1) Run-time dependency libyuv found: NO (tried pkgconfig and cmake) Library yuv found: YES Library atomic found: YES ... (Based on the second Library yuv found: YES) Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/meson.build | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/src/meson.build b/src/meson.build > index 76198e9535db..0a9cdff8e7dc 100644 > --- a/src/meson.build > +++ b/src/meson.build > @@ -27,10 +27,15 @@ else > ipa_sign_module = false > endif > > -# libyuv, used by the Android adaptation layer and the virtual pipeline handler. > -# Fallback to a subproject if libyuv isn't found, as it's typically not provided > -# by distributions. > +# libyuv, used by the Android adaptation layer and the virtual pipeline > +# handler. Fallback to a subproject if libyuv isn't found, as it's typically > +# not provided by distributions. Where libyuv is provided by a distribution, it > +# may not always supply a pkg-config implementation, requiring cc.find_library() > +# to search for it. > libyuv_dep = dependency('libyuv', required : false) > +if not libyuv_dep.found() > + libyuv_dep = cc.find_library('yuv', required: false) > +endif > > if (pipelines.contains('virtual') or get_option('android').allowed()) and \ > not libyuv_dep.found() > -- > 2.47.1 >
Quoting Kieran Bingham (2025-01-14 21:19:35) > From: Dylan Aïssi <daissi@debian.org> > > We already fall back to a subproject to support the libyuv package when > it can not be discovered through the usual dependency() mechanism. > > Unfortunately libyuv may be packaged without any corresponding > pkg-config support as can be seen at [0], so further extend the > dependency search by using an explicit cc.find_library() call. > > [0] https://packages.debian.org/bookworm/amd64/libyuv-dev/filelist > > Signed-off-by: Dylan Aïssi <dylan.aissi@collabora.com> Dylan, I've taken this tag from https://salsa.debian.org/multimedia-team/libcamera/-/commit/685482827a0e6fe5dfe19778e633ab95b703cef4 However it doesn't match the author of that patch so it will fail our CI. Can you confirm who should be the author+signoff for this please? If daissi@debian.org is correct, just replying with the correct SoB will let patchwork pick up the tag. -- Kieran > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/meson.build | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/src/meson.build b/src/meson.build > index 76198e9535db..0a9cdff8e7dc 100644 > --- a/src/meson.build > +++ b/src/meson.build > @@ -27,10 +27,15 @@ else > ipa_sign_module = false > endif > > -# libyuv, used by the Android adaptation layer and the virtual pipeline handler. > -# Fallback to a subproject if libyuv isn't found, as it's typically not provided > -# by distributions. > +# libyuv, used by the Android adaptation layer and the virtual pipeline > +# handler. Fallback to a subproject if libyuv isn't found, as it's typically > +# not provided by distributions. Where libyuv is provided by a distribution, it > +# may not always supply a pkg-config implementation, requiring cc.find_library() > +# to search for it. > libyuv_dep = dependency('libyuv', required : false) > +if not libyuv_dep.found() > + libyuv_dep = cc.find_library('yuv', required: false) > +endif > > if (pipelines.contains('virtual') or get_option('android').allowed()) and \ > not libyuv_dep.found() > -- > 2.47.1 >
Hi 2025. január 14., kedd 22:24 keltezéssel, Kieran Bingham <kieran.bingham@ideasonboard.com> írta: > Quoting Kieran Bingham (2025-01-14 21:19:35) > > From: Dylan Aïssi <daissi@debian.org> > > > > We already fall back to a subproject to support the libyuv package when > > it can not be discovered through the usual dependency() mechanism. > > > > Unfortunately libyuv may be packaged without any corresponding > > pkg-config support as can be seen at [0], so further extend the > > dependency search by using an explicit cc.find_library() call. > > > > [0] https://packages.debian.org/bookworm/amd64/libyuv-dev/filelist > > > > Signed-off-by: Dylan Aïssi <dylan.aissi@collabora.com> > > I've posted this patch on Dylan's behalf from > https://salsa.debian.org/multimedia-team/libcamera/-/commit/685482827a0e6fe5dfe19778e633ab95b703cef4 > though I have reworded the commit message and comment inline, but the > core functional change is direct from Dylan's patch... > > > On Ubuntu 22.04.5 LTS: > > $ sudo apt install libyuv-dev > > ... > libcamera$ meson setup ... > ... > Found pkg-config: YES (/usr/bin/pkg-config) 0.29.2 > Found CMake: /usr/bin/cmake (3.22.1) > Run-time dependency libyuv found: NO (tried pkgconfig and cmake) > Library yuv found: YES > Library atomic found: YES > ... > > (Based on the second Library yuv found: YES) > > Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> This is the change I have locally to achieve the same: diff --git a/src/meson.build b/src/meson.build index 76198e953..8cb4af61a 100644 --- a/src/meson.build +++ b/src/meson.build @@ -30,7 +30,20 @@ endif # libyuv, used by the Android adaptation layer and the virtual pipeline handler. # Fallback to a subproject if libyuv isn't found, as it's typically not provided # by distributions. -libyuv_dep = dependency('libyuv', required : false) + +if not get_option('force_fallback_for').contains('libyuv') + libyuv_dep = dependency('libyuv', required : false) + + if not libyuv_dep.found() + libyuv_dep = cxx.find_library( + 'yuv', + required : false, + has_headers : 'libyuv.h', + ) + endif +else + libyuv_dep = dependency('', required : false) +endif if (pipelines.contains('virtual') or get_option('android').allowed()) and \ not libyuv_dep.found() This has two advantages in my opinion: * the `force_fallback_for=libyuv` meson option works; * the presence of at least `libyuv.h` is checked (I imagine this may be relevant for distributions where the development files are placed in different packages). `meson` has recently added experimental cmake wraps: https://mesonbuild.com/Wrap-dependency-system-manual.html#cmake-wraps I think this could make the dependency lookup simpler, but it is probably too new. Or potentially `libyuv` could be added to the meson wrapdb, which could also enable some simplification. Regards, Barnabás Pőcze > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > src/meson.build | 11 ++++++++--- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/src/meson.build b/src/meson.build > > index 76198e9535db..0a9cdff8e7dc 100644 > > --- a/src/meson.build > > +++ b/src/meson.build > > @@ -27,10 +27,15 @@ else > > ipa_sign_module = false > > endif > > > > -# libyuv, used by the Android adaptation layer and the virtual pipeline handler. > > -# Fallback to a subproject if libyuv isn't found, as it's typically not provided > > -# by distributions. > > +# libyuv, used by the Android adaptation layer and the virtual pipeline > > +# handler. Fallback to a subproject if libyuv isn't found, as it's typically > > +# not provided by distributions. Where libyuv is provided by a distribution, it > > +# may not always supply a pkg-config implementation, requiring cc.find_library() > > +# to search for it. > > libyuv_dep = dependency('libyuv', required : false) > > +if not libyuv_dep.found() > > + libyuv_dep = cc.find_library('yuv', required: false) > > +endif > > > > if (pipelines.contains('virtual') or get_option('android').allowed()) and \ > > not libyuv_dep.found() > > -- > > 2.47.1 > > >
Quoting Barnabás Pőcze (2025-01-14 21:39:40) > Hi > > > 2025. január 14., kedd 22:24 keltezéssel, Kieran Bingham <kieran.bingham@ideasonboard.com> írta: > > > Quoting Kieran Bingham (2025-01-14 21:19:35) > > > From: Dylan Aïssi <daissi@debian.org> > > > > > > We already fall back to a subproject to support the libyuv package when > > > it can not be discovered through the usual dependency() mechanism. > > > > > > Unfortunately libyuv may be packaged without any corresponding > > > pkg-config support as can be seen at [0], so further extend the > > > dependency search by using an explicit cc.find_library() call. > > > > > > [0] https://packages.debian.org/bookworm/amd64/libyuv-dev/filelist > > > > > > Signed-off-by: Dylan Aïssi <dylan.aissi@collabora.com> > > > > I've posted this patch on Dylan's behalf from > > https://salsa.debian.org/multimedia-team/libcamera/-/commit/685482827a0e6fe5dfe19778e633ab95b703cef4 > > though I have reworded the commit message and comment inline, but the > > core functional change is direct from Dylan's patch... > > > > > > On Ubuntu 22.04.5 LTS: > > > > $ sudo apt install libyuv-dev > > > > ... > > libcamera$ meson setup ... > > ... > > Found pkg-config: YES (/usr/bin/pkg-config) 0.29.2 > > Found CMake: /usr/bin/cmake (3.22.1) > > Run-time dependency libyuv found: NO (tried pkgconfig and cmake) > > Library yuv found: YES > > Library atomic found: YES > > ... > > > > (Based on the second Library yuv found: YES) > > > > Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > This is the change I have locally to achieve the same: > > diff --git a/src/meson.build b/src/meson.build > index 76198e953..8cb4af61a 100644 > --- a/src/meson.build > +++ b/src/meson.build > @@ -30,7 +30,20 @@ endif > # libyuv, used by the Android adaptation layer and the virtual pipeline handler. > # Fallback to a subproject if libyuv isn't found, as it's typically not provided > # by distributions. > -libyuv_dep = dependency('libyuv', required : false) > + > +if not get_option('force_fallback_for').contains('libyuv') > + libyuv_dep = dependency('libyuv', required : false) > + > + if not libyuv_dep.found() > + libyuv_dep = cxx.find_library( I guess using cxx is the toolchain we're compiling the rest of the project with so that likely makes sense. > + 'yuv', > + required : false, > + has_headers : 'libyuv.h', > + ) > + endif > +else > + libyuv_dep = dependency('', required : false) is the '' intentional ? What does that do or how does it work ? > +endif > > if (pipelines.contains('virtual') or get_option('android').allowed()) and \ > not libyuv_dep.found() > > This has two advantages in my opinion: > * the `force_fallback_for=libyuv` meson option works; Do we need to repeat all of this for gtest and libyaml too as subprojects? Or are they already handled correctly ? > * the presence of at least `libyuv.h` is checked the has_headers check is a very good addition also. > (I imagine this may be relevant for distributions where the development files > are placed in different packages). > > `meson` has recently added experimental cmake wraps: > https://mesonbuild.com/Wrap-dependency-system-manual.html#cmake-wraps > I think this could make the dependency lookup simpler, but it is probably too new. > > Or potentially `libyuv` could be added to the meson wrapdb, > which could also enable some simplification. Perhaps adding it to the wrapdb sounds good, but I don't know how to go about that. Our subprojects/libyuv.wrap probably needs an update too - it's still set at a revision by 9109bcf22c7f ("subprojects: Add libyuv and built if -Dandroid=enabled") in 2021 ... so must be quite dated! > > > Regards, > Barnabás Pőcze > > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > > src/meson.build | 11 ++++++++--- > > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > > > diff --git a/src/meson.build b/src/meson.build > > > index 76198e9535db..0a9cdff8e7dc 100644 > > > --- a/src/meson.build > > > +++ b/src/meson.build > > > @@ -27,10 +27,15 @@ else > > > ipa_sign_module = false > > > endif > > > > > > -# libyuv, used by the Android adaptation layer and the virtual pipeline handler. > > > -# Fallback to a subproject if libyuv isn't found, as it's typically not provided > > > -# by distributions. > > > +# libyuv, used by the Android adaptation layer and the virtual pipeline > > > +# handler. Fallback to a subproject if libyuv isn't found, as it's typically > > > +# not provided by distributions. Where libyuv is provided by a distribution, it > > > +# may not always supply a pkg-config implementation, requiring cc.find_library() > > > +# to search for it. > > > libyuv_dep = dependency('libyuv', required : false) > > > +if not libyuv_dep.found() > > > + libyuv_dep = cc.find_library('yuv', required: false) > > > +endif > > > > > > if (pipelines.contains('virtual') or get_option('android').allowed()) and \ > > > not libyuv_dep.found() > > > -- > > > 2.47.1 > > > > >
2025. január 14., kedd 22:50 keltezéssel, Kieran Bingham <kieran.bingham@ideasonboard.com> írta: > Quoting Barnabás Pőcze (2025-01-14 21:39:40) > > Hi > > > > > > 2025. január 14., kedd 22:24 keltezéssel, Kieran Bingham <kieran.bingham@ideasonboard.com> írta: > > > > > Quoting Kieran Bingham (2025-01-14 21:19:35) > > > > From: Dylan Aïssi <daissi@debian.org> > > > > > > > > We already fall back to a subproject to support the libyuv package when > > > > it can not be discovered through the usual dependency() mechanism. > > > > > > > > Unfortunately libyuv may be packaged without any corresponding > > > > pkg-config support as can be seen at [0], so further extend the > > > > dependency search by using an explicit cc.find_library() call. > > > > > > > > [0] https://packages.debian.org/bookworm/amd64/libyuv-dev/filelist > > > > > > > > Signed-off-by: Dylan Aïssi <dylan.aissi@collabora.com> > > > > > > I've posted this patch on Dylan's behalf from > > > https://salsa.debian.org/multimedia-team/libcamera/-/commit/685482827a0e6fe5dfe19778e633ab95b703cef4 > > > though I have reworded the commit message and comment inline, but the > > > core functional change is direct from Dylan's patch... > > > > > > > > > On Ubuntu 22.04.5 LTS: > > > > > > $ sudo apt install libyuv-dev > > > > > > ... > > > libcamera$ meson setup ... > > > ... > > > Found pkg-config: YES (/usr/bin/pkg-config) 0.29.2 > > > Found CMake: /usr/bin/cmake (3.22.1) > > > Run-time dependency libyuv found: NO (tried pkgconfig and cmake) > > > Library yuv found: YES > > > Library atomic found: YES > > > ... > > > > > > (Based on the second Library yuv found: YES) > > > > > > Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > This is the change I have locally to achieve the same: > > > > diff --git a/src/meson.build b/src/meson.build > > index 76198e953..8cb4af61a 100644 > > --- a/src/meson.build > > +++ b/src/meson.build > > @@ -30,7 +30,20 @@ endif > > # libyuv, used by the Android adaptation layer and the virtual pipeline handler. > > # Fallback to a subproject if libyuv isn't found, as it's typically not provided > > # by distributions. > > -libyuv_dep = dependency('libyuv', required : false) > > + > > +if not get_option('force_fallback_for').contains('libyuv') > > + libyuv_dep = dependency('libyuv', required : false) > > + > > + if not libyuv_dep.found() > > + libyuv_dep = cxx.find_library( > > I guess using cxx is the toolchain we're compiling the rest of the > project with so that likely makes sense. > > > + 'yuv', > > + required : false, > > + has_headers : 'libyuv.h', > > + ) > > + endif > > +else > > + libyuv_dep = dependency('', required : false) > > is the '' intentional ? What does that do or how does it work ? It is a never found dependency: https://mesonbuild.com/Reference-manual_functions.html#dependency If dependency_name is '', the dependency is always not found. It is used to make the `not libyuv_dep.found()` part of the condition below true. > > > +endif > > > > if (pipelines.contains('virtual') or get_option('android').allowed()) and \ > > not libyuv_dep.found() > > > > This has two advantages in my opinion: > > * the `force_fallback_for=libyuv` meson option works; > > Do we need to repeat all of this for gtest and libyaml too as > subprojects? Or are they already handled correctly ? `gtest` is from the wrapdb, so no. But `libyaml` seems to be the same situation, however `libyaml` is available in the wrapdb, so switching to that may not be unreasonable. > > > * the presence of at least `libyuv.h` is checked > > the has_headers check is a very good addition also. > > > (I imagine this may be relevant for distributions where the development files > > are placed in different packages). > > > > `meson` has recently added experimental cmake wraps: > > https://mesonbuild.com/Wrap-dependency-system-manual.html#cmake-wraps > > I think this could make the dependency lookup simpler, but it is probably too new. > > > > Or potentially `libyuv` could be added to the meson wrapdb, > > which could also enable some simplification. > > Perhaps adding it to the wrapdb sounds good, but I don't know how to go > about that. Essentially one needs to write meson build files for the project and submit them to https://github.com/mesonbuild/wrapdb . ( https://mesonbuild.com/Adding-new-projects-to-wrapdb.html ) > > Our subprojects/libyuv.wrap probably needs an update too - it's still > set at a revision by 9109bcf22c7f ("subprojects: Add libyuv and built if > -Dandroid=enabled") in 2021 ... so must be quite dated! Unfortunately, as things stand, libyuv breaks since a6135cfe0fc8a3102273b83f129c8175fb5e4d88 ( https://chromium.googlesource.com/libyuv/libyuv.git/+/a6135cfe0fc8a3102273b83f129c8175fb5e4d88 ) because of an issue in meson's cmake module: https://github.com/mesonbuild/meson/issues/10764 Regards, Barnabás Pőcze > > > > > > > Regards, > > Barnabás Pőcze > > > > > > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > --- > > > > src/meson.build | 11 ++++++++--- > > > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/src/meson.build b/src/meson.build > > > > index 76198e9535db..0a9cdff8e7dc 100644 > > > > --- a/src/meson.build > > > > +++ b/src/meson.build > > > > @@ -27,10 +27,15 @@ else > > > > ipa_sign_module = false > > > > endif > > > > > > > > -# libyuv, used by the Android adaptation layer and the virtual pipeline handler. > > > > -# Fallback to a subproject if libyuv isn't found, as it's typically not provided > > > > -# by distributions. > > > > +# libyuv, used by the Android adaptation layer and the virtual pipeline > > > > +# handler. Fallback to a subproject if libyuv isn't found, as it's typically > > > > +# not provided by distributions. Where libyuv is provided by a distribution, it > > > > +# may not always supply a pkg-config implementation, requiring cc.find_library() > > > > +# to search for it. > > > > libyuv_dep = dependency('libyuv', required : false) > > > > +if not libyuv_dep.found() > > > > + libyuv_dep = cc.find_library('yuv', required: false) > > > > +endif > > > > > > > > if (pipelines.contains('virtual') or get_option('android').allowed()) and \ > > > > not libyuv_dep.found() > > > > -- > > > > 2.47.1 > > > > > > > >
diff --git a/src/meson.build b/src/meson.build index 76198e9535db..0a9cdff8e7dc 100644 --- a/src/meson.build +++ b/src/meson.build @@ -27,10 +27,15 @@ else ipa_sign_module = false endif -# libyuv, used by the Android adaptation layer and the virtual pipeline handler. -# Fallback to a subproject if libyuv isn't found, as it's typically not provided -# by distributions. +# libyuv, used by the Android adaptation layer and the virtual pipeline +# handler. Fallback to a subproject if libyuv isn't found, as it's typically +# not provided by distributions. Where libyuv is provided by a distribution, it +# may not always supply a pkg-config implementation, requiring cc.find_library() +# to search for it. libyuv_dep = dependency('libyuv', required : false) +if not libyuv_dep.found() + libyuv_dep = cc.find_library('yuv', required: false) +endif if (pipelines.contains('virtual') or get_option('android').allowed()) and \ not libyuv_dep.found()