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

[Javier Martinez Canillas]

Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:

> 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.
>

Agreed.

>> > 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'
>

Right, sorry I missed the 'prefix' when typed the above.

>> 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.
>

Agreed.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat