On Mon, Jun 17, 2024 at 10:38:39AM +0200, Hans Verkuil wrote:
> On 17/06/2024 09:51, sakari.ailus@xxxxxx 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. :-(
> >
> > 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 quickly looked at the API and I think it needs work before it is suitable
> for a generic library API. It is was never meant as a public API.
>
> Making it a proper library is a lot more work and needs someone to maintain
> it. I'm not maintaining this code, so it is not up to me, but for what it is
> worth, I would recommend against it.
I hack on that code occasionally when I need new features, but I'm not
sure that makes me a maintainer. In any case, I won't have time to turn
this into a public API in the near future. If someone wants to volunteer
to do that work, I could at least try to review the proposals.
> >> 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@xxxxxxx>
> >>>>> ---
> >>>>> 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
