On 17/06/2024 09:51, sakari.ailus@xxxxxx wrote: > Hi Laurent, > > On Sat, Jun 15, 2024 at 03:35:25AM +0300, Laurent Pinchart wrote: >> Hi Neel, >> >> 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. Regards, Hans > >> >> 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) >>>>> >> >