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

[Laurent Pinchart]

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 at 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')),

-- 
Regards,

Laurent Pinchart