Re: [v4l-utils] utils: media-ctl: Install media-ctl header and library files

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Mon, Jun 17, 2024 at 07:51:52AM +0000, Sakari Ailus wrote:
> On Sat, Jun 15, 2024 at 03:35:25AM +0300, Laurent Pinchart wrote:
> > On Fri, Jun 14, 2024 at 01:07:24PM +0000, Gandhi, Neel wrote:
> > > Hi Laurent,
> > > 
> > > Thank you for your quick response. Please see my below inline response.
> > > 
> > > On Friday, June 14, 2024 3:35 PM, Laurent Pinchart wrote:
> > > > On Fri, Jun 14, 2024 at 12:21:20PM +0530, Neel Gandhi wrote:
> > > > > Install mediactl and v4l2subdev header and library files, which may be
> > > > > required by 3rd party applications to populate and control v4l2subdev
> > > > > device node tree
> > > > 
> > > > We haven't done so because the API of those libraries is currently internal, and
> > > > not guaranteed to be stable.
> > > 
> > > So, if one's application is developed based on those libraries, then
> > > is there any alternative solution for it?
> > 
> > Good question. I personally have no issue with applications using those
> > libraries, as long as they don't expect a stable API. This means that
> > any future version of v4l-utils may ship incompatible libraries, and
> > that won't be considered as a regression, and won't be addressed. If we
> > want to go that way, I think we should install static versions of the
> > libraries only, as dynamic linking will really be asking for trouble.
> > 
> > A better option, of course, would be to offer a stable API. That will
> > require work, the current API will need to be reviewed and improved to
> > make sure we can then extend it in a backward-compatible way whenever
> > the kernel APIs get extended. It's not work that I could commit to, so
> > we would need a volunteer for work on that and then maintain the
> > libraries.
> > 
> > Hans, Sakari, any opinion ?
> 
> Given it's been around for a decade without much changes, it wouldn't seem
> unreasonable to consider the API stable. A lot of projects copy the code in
> verbatim which makes fixing bugs very hard. :-(

Well, I've pushed API-breaking changes not later than last week :-)
Having to preserve the API would have been annoying.

> I think the API could be improved on and the library could use G_TOPOLOGY
> IOCTL. Neither has happened but these don't much affect the value of the
> librarisation.
> 
> I've already proposed making this a proper library previously and I still
> believe it should be one.

I agree it would be nice. Someone would have to step up as a maintainer,
and start by desining the API in a way to would allow for future
extensions.

> > Neel, out of curiosity, what do you use those libraries for (if you can
> > tell publicly) ?
> > 
> > > As in the prior versions of v4l-utils package, those libraries are
> > > available to use but it was removed from v1.25+ onwards (from meson
> > > related changes in c2b91b9c3853b2cbcbe170a542864a3147d179ee commitID).
> > > We're using yocto scarthgap, which is using v1.26.
> > > 
> > > > > Signed-off-by: Neel Gandhi <neel.gandhi@amd.com>
> > > > > ---
> > > > >  utils/media-ctl/meson.build | 28 +++++++++++++++++++++-------
> > > > >  1 file changed, 21 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/utils/media-ctl/meson.build b/utils/media-ctl/meson.build
> > > > > index 3a7b0c9a..40669b4c 100644
> > > > > --- a/utils/media-ctl/meson.build
> > > > > +++ b/utils/media-ctl/meson.build
> > > > > @@ -3,14 +3,24 @@ libmediactl_sources = files(
> > > > >      'mediactl-priv.h',
> > > > >  )
> > > > >
> > > > > +libmediactl_api = files(
> > > > > +    'mediactl.h',
> > > > > +    'v4l2subdev.h',
> > > > > +)
> > > > > +
> > > > > +install_headers(libmediactl_api, subdir: 'mediactl')
> > > > > +
> > > > >  libmediactl_deps = [
> > > > >      dep_libudev,
> > > > >  ]
> > > > >
> > > > > -libmediactl = static_library('mediactl',
> > > > > -                             libmediactl_sources,
> > > > > -                             dependencies : libmediactl_deps,
> > > > > -                             include_directories : v4l2_utils_incdir)
> > > > > +libmediactl = library('mediactl',
> > > > > +                      libmediactl_sources,
> > > > > +                      soversion: '0',
> > > > > +                      version: '0.0.0',
> > > > > +                      install : true,
> > > > > +                      dependencies : libmediactl_deps,
> > > > > +                      include_directories : v4l2_utils_incdir)
> > > > >
> > > > >  dep_libmediactl = declare_dependency(link_with : libmediactl)
> > > > >
> > > > > @@ -18,9 +28,13 @@ libv4l2subdev_sources = files('libv4l2subdev.c')
> > > > > libv4l2subdev_sources += media_bus_format_names_h
> > > > > libv4l2subdev_sources += media_bus_format_codes_h
> > > > >
> > > > > -libv4l2subdev = static_library('v4l2subdev',
> > > > > -                               libv4l2subdev_sources,
> > > > > -                               include_directories : v4l2_utils_incdir)
> > > > > +libv4l2subdev = library('v4l2subdev',
> > > > > +                        libv4l2subdev_sources,
> > > > > +                        soversion: '0',
> > > > > +                        version: '0.0.0',
> > > > > +                        install : true,
> > > > > +                        dependencies : dep_libmediactl,
> > > > > +                        include_directories : v4l2_utils_incdir)
> > > > >
> > > > >  dep_libv4l2subdev = declare_dependency(link_with : libv4l2subdev)
> > > > >

-- 
Regards,

Laurent Pinchart