[libcamera-devel] gst: Fix GLib detection and warning disabling

3036 diff mbox series

Commit 17cccc68a88f ("Add GStreamer plugin and element skeleton") has
gained a last minute fix for a clang compilation error with GLib prior
to v2.63.0. The fix wasn't properly tested, and not only did it try to
silence the affected compiler warning for C files only, it also failed
to check the GLib dependency correctly. This resulted in compilation of
the GStreamer element to always be disabled.

Fix this by changing the GLib package name from 'glib' to 'glib-2.0',
and silence the compiler warning for C++ files.

Fixes: 17cccc68a88f ("Add GStreamer plugin and element skeleton")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/gstreamer/meson.build | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Le samedi 07 mars 2020 à 02:33 +0200, Laurent Pinchart a écrit :
> Commit 17cccc68a88f ("Add GStreamer plugin and element skeleton") has
> gained a last minute fix for a clang compilation error with GLib prior
> to v2.63.0. The fix wasn't properly tested, and not only did it try to
> silence the affected compiler warning for C files only, it also failed
> to check the GLib dependency correctly. This resulted in compilation of
> the GStreamer element to always be disabled.
> 
> Fix this by changing the GLib package name from 'glib' to 'glib-2.0',
> and silence the compiler warning for C++ files.
> 
> Fixes: 17cccc68a88f ("Add GStreamer plugin and element skeleton")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/gstreamer/meson.build | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build
> index 1965b5041132..1b9dedd2528e 100644
> --- a/src/gstreamer/meson.build
> +++ b/src/gstreamer/meson.build
> @@ -12,8 +12,9 @@ libcamera_gst_c_args = [
>      '-DVERSION="@0@"'.format(libcamera_git_version),
>      '-DPACKAGE="@0@"'.format(meson.project_name()),
>  ]
> +libcamera_gst_cpp_args = []
>  
> -glib_dep = dependency('glib', required : get_option('gstreamer'))
> +glib_dep = dependency('glib-2.0', required : get_option('gstreamer'))
>  
>  gst_dep_version = '>=1.14.0'
>  gstvideo_dep = dependency('gstreamer-video-1.0', version : gst_dep_version,
> @@ -27,12 +28,13 @@ if glib_dep.found() and gstvideo_dep.found() and gstallocator_dep.found()
>      # complain about the ones we are not using. Silence the -Wunused-function
>      # warning in that case.
>      if cc.get_id() == 'clang' and glib_dep.version().version_compare('<2.63.0')
> -        libcamera_gst_c_args += [ '-Wno-unused-function' ]
> +        libcamera_gst_cpp_args += [ '-Wno-unused-function' ]

We have gstlibcamera.c that includes gstlibcameraprovider.h and
gstlibcamerasrc.h, so shouldn't this be setting both C and CPP args ? 

If I may, maybe we could simply move gstlibcamera.c to gstlibcamera.cpp
and drop the C args. The generated code by GST_DEFINE_PLUGIN is inside
a scope of:

  extern "C" {
    
  }

>      endif
>  
>      libcamera_gst = shared_library('gstlibcamera',
>          libcamera_gst_sources,
>          c_args : libcamera_gst_c_args,
> +        cpp_args : libcamera_gst_cpp_args,
>          dependencies : [libcamera_dep, gstvideo_dep, gstallocator_dep],
>          install: true,
>          install_dir : '@0@/gstreamer-1.0'.format(get_option('libdir')),

Hi Nicolas,

On Sat, Mar 07, 2020 at 01:33:44PM -0500, Nicolas Dufresne wrote:
> Le samedi 07 mars 2020 à 02:33 +0200, Laurent Pinchart a écrit :
> > Commit 17cccc68a88f ("Add GStreamer plugin and element skeleton") has
> > gained a last minute fix for a clang compilation error with GLib prior
> > to v2.63.0. The fix wasn't properly tested, and not only did it try to
> > silence the affected compiler warning for C files only, it also failed
> > to check the GLib dependency correctly. This resulted in compilation of
> > the GStreamer element to always be disabled.
> > 
> > Fix this by changing the GLib package name from 'glib' to 'glib-2.0',
> > and silence the compiler warning for C++ files.
> > 
> > Fixes: 17cccc68a88f ("Add GStreamer plugin and element skeleton")
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/gstreamer/meson.build | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build
> > index 1965b5041132..1b9dedd2528e 100644
> > --- a/src/gstreamer/meson.build
> > +++ b/src/gstreamer/meson.build
> > @@ -12,8 +12,9 @@ libcamera_gst_c_args = [
> >      '-DVERSION="@0@"'.format(libcamera_git_version),
> >      '-DPACKAGE="@0@"'.format(meson.project_name()),
> >  ]
> > +libcamera_gst_cpp_args = []
> >  
> > -glib_dep = dependency('glib', required : get_option('gstreamer'))
> > +glib_dep = dependency('glib-2.0', required : get_option('gstreamer'))
> >  
> >  gst_dep_version = '>=1.14.0'
> >  gstvideo_dep = dependency('gstreamer-video-1.0', version : gst_dep_version,
> > @@ -27,12 +28,13 @@ if glib_dep.found() and gstvideo_dep.found() and gstallocator_dep.found()
> >      # complain about the ones we are not using. Silence the -Wunused-function
> >      # warning in that case.
> >      if cc.get_id() == 'clang' and glib_dep.version().version_compare('<2.63.0')
> > -        libcamera_gst_c_args += [ '-Wno-unused-function' ]
> > +        libcamera_gst_cpp_args += [ '-Wno-unused-function' ]
> 
> We have gstlibcamera.c that includes gstlibcameraprovider.h and
> gstlibcamerasrc.h, so shouldn't this be setting both C and CPP args ? 

I agree, I wonder why the compiler didn't complain for gstlibcamera.c.

> If I may, maybe we could simply move gstlibcamera.c to gstlibcamera.cpp
> and drop the C args. The generated code by GST_DEFINE_PLUGIN is inside
> a scope of:
> 
>   extern "C" {
>     
>   }

I think that would be better. I'll post a v2.

> >      endif
> >  
> >      libcamera_gst = shared_library('gstlibcamera',
> >          libcamera_gst_sources,
> >          c_args : libcamera_gst_c_args,
> > +        cpp_args : libcamera_gst_cpp_args,
> >          dependencies : [libcamera_dep, gstvideo_dep, gstallocator_dep],
> >          install: true,
> >          install_dir : '@0@/gstreamer-1.0'.format(get_option('libdir')),