libcamera: meson: Fix libyuv detection
diff mbox series

Message ID 20250114211935.715028-1-kieran.bingham@ideasonboard.com
State New
Headers show
Series
  • libcamera: meson: Fix libyuv detection
Related show

Commit Message

Kieran Bingham Jan. 14, 2025, 9:19 p.m. UTC
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>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/meson.build | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Kieran Bingham Jan. 14, 2025, 9:24 p.m. UTC | #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>

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
>
Kieran Bingham Jan. 14, 2025, 9:26 p.m. UTC | #2
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
>
Barnabás Pőcze Jan. 14, 2025, 9:39 p.m. UTC | #3
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
> >
>
Kieran Bingham Jan. 14, 2025, 9:50 p.m. UTC | #4
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
> > >
> >
Barnabás Pőcze Jan. 14, 2025, 10:19 p.m. UTC | #5
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
> > > >
> > >
>

Patch
diff mbox series

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()