[libcamera-devel] [PATCH 2/2] v4l2: Move the v4l2-compat to libcamera/

[Laurent Pinchart]

Hi Javier and Kieran,

On Wed, May 03, 2023 at 02:51:38PM +0200, Javier Martinez Canillas via libcamera-devel wrote:
> On 5/1/23 17:55, Kieran Bingham wrote:
> > Move the v4l2-compat.so shared library installation to the libcamera
> > library directory. This is the same location that IPA modules live and
> > will allow the 'generically sounding' v4l2-compat.so to be more clearly
> > identified as a libcamera compatibility layer.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> > We could also rename this to libcamera-v4l2-compat.so, and/or we could

I think libcamera/v4l2-compat.so is enough from a namespacing point of
view.

> > also re-consider whether the libcamera-hal.so file should also live
> > under the libcamera/ libdir by default.

That's a question specific to Android (and Chrome OS), I don't recall
what the Android rules are.

> >  src/v4l2/meson.build | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build
> > index f132103cb503..114fddf7302a 100644
> > --- a/src/v4l2/meson.build
> > +++ b/src/v4l2/meson.build
> > @@ -31,6 +31,7 @@ v4l2_compat = shared_library('v4l2-compat',
> >                               v4l2_compat_sources,
> >                               name_prefix : '',
> >                               install : true,
> > +                             install_dir : libcamera_libdir,
> >                               dependencies : [libcamera_private, libdl],
> >                               cpp_args : v4l2_compat_cpp_args)
> >  
> > @@ -38,7 +39,7 @@ v4l2_compat = shared_library('v4l2-compat',
> >  # adaptation layer.
> >  
> >  cdata = configuration_data()
> > -cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so')
> > +cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / libcamera_libdir / 'v4l2-compat.so')
> 
> As Laurent mentioned in another thread, I think that:
> 
> +cdata.set('LIBCAMERA_V4L2_SO', get_option('libexecdir') / 'libcamera' / 'v4l2-compat.so')

I think you need

	get_option('prefix') / get_option('libexecdir') / 'libcamera' / 'v4l2-compat.so'

> would be better and more consistent with how other projects install
> shared objects that are not meant to be public.

According to
https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s07.html,

"/usr/libexec includes internal binaries that are not intended to be
executed directly by users or shell scripts. Applications may use a
single subdirectory under /usr/libexec."

This being said, valgrind installs LD_PRELOAD-able .so files in libexec,
and there are some other .so from different packages there too. I thus
think it makes sense to store v4l2-compat.so in libexec.

If we go for libexec, the first patch in this series could also be
dropped. We could keep it for additional safety, but until we store
other files in that directory, it's a bit pointless.

> That would also solve the issue I had when trying to enable the v4l2
> compatibility layer and ship it in a separate libcamera-v4l2 sub-package.
> 
> If you post a v2 with that change, feel free to add my:
> 
> Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>

-- 
Regards,

Laurent Pinchart