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

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

[Neel Gandhi]

Install mediactl and v4l2subdev header and library
files, which may be required by 3rd party applications
to populate and control v4l2subdev device node tree

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)
 
-- 
2.34.1

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

[Laurent Pinchart]

Hi Neel,

Thank you for the patch.

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.

> 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

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

[Gandhi, Neel]

Hi Laurent,

Thank you for your quick response. Please see my below inline response.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Sent: Friday, June 14, 2024 3:35 PM
> To: Gandhi, Neel <neel.gandhi@amd.com>
> Cc: linux-media@vger.kernel.org; hverkuil@xs4all.nl; sakari.ailus@iki.fi; Hatle,
> Mark <mark.hatle@amd.com>; Allagadapa, Varunkumar
> <varunkumar.allagadapa@amd.com>; Sagar, Vishal <vishal.sagar@amd.com>
> Subject: Re: [v4l-utils] utils: media-ctl: Install media-ctl header and library files
> 
> Hi Neel,
> 
> Thank you for the patch.
> 
> 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?

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

Thanks & Regards,

Neel Gandhi

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

[Laurent Pinchart]

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 ?

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

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

[sakari.ailus]

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.

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

-- 
Kind regards,

Sakari Ailus

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

[Hans Verkuil]

On 17/06/2024 09:51, sakari.ailus@iki.fi 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@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)
>>>>>
>>
> 

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

[Laurent Pinchart]

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

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

[Laurent Pinchart]

On Mon, Jun 17, 2024 at 10:38:39AM +0200, Hans Verkuil wrote:
> On 17/06/2024 09:51, sakari.ailus@iki.fi 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@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