[{"id":3985,"web_url":"https://patchwork.libcamera.org/comment/3985/","msgid":"<42562f43ce0bd7cae248f83863dc7add8d789af4.camel@collabora.com>","date":"2020-03-07T18:33:44","subject":"Re: [libcamera-devel] [PATCH] gst: Fix GLib detection and warning\n\tdisabling","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le samedi 07 mars 2020 à 02:33 +0200, Laurent Pinchart a écrit :\n> Commit 17cccc68a88f (\"Add GStreamer plugin and element skeleton\") has\n> gained a last minute fix for a clang compilation error with GLib prior\n> to v2.63.0. The fix wasn't properly tested, and not only did it try to\n> silence the affected compiler warning for C files only, it also failed\n> to check the GLib dependency correctly. This resulted in compilation of\n> the GStreamer element to always be disabled.\n> \n> Fix this by changing the GLib package name from 'glib' to 'glib-2.0',\n> and silence the compiler warning for C++ files.\n> \n> Fixes: 17cccc68a88f (\"Add GStreamer plugin and element skeleton\")\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/gstreamer/meson.build | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build\n> index 1965b5041132..1b9dedd2528e 100644\n> --- a/src/gstreamer/meson.build\n> +++ b/src/gstreamer/meson.build\n> @@ -12,8 +12,9 @@ libcamera_gst_c_args = [\n> '-DVERSION=\"@0@\"'.format(libcamera_git_version),\n> '-DPACKAGE=\"@0@\"'.format(meson.project_name()),\n> ]\n> +libcamera_gst_cpp_args = []\n> \n> -glib_dep = dependency('glib', required : get_option('gstreamer'))\n> +glib_dep = dependency('glib-2.0', required : get_option('gstreamer'))\n> \n> gst_dep_version = '>=1.14.0'\n> gstvideo_dep = dependency('gstreamer-video-1.0', version : gst_dep_version,\n> @@ -27,12 +28,13 @@ if glib_dep.found() and gstvideo_dep.found() and gstallocator_dep.found()\n> # complain about the ones we are not using. Silence the -Wunused-function\n> # warning in that case.\n> if cc.get_id() == 'clang' and glib_dep.version().version_compare('<2.63.0')\n> - libcamera_gst_c_args += [ '-Wno-unused-function' ]\n> + libcamera_gst_cpp_args += [ '-Wno-unused-function' ]\n\nWe have gstlibcamera.c that includes gstlibcameraprovider.h and\ngstlibcamerasrc.h, so shouldn't this be setting both C and CPP args ? \n\nIf I may, maybe we could simply move gstlibcamera.c to gstlibcamera.cpp\nand drop the C args. The generated code by GST_DEFINE_PLUGIN is inside\na scope of:\n\n extern \"C\" {\n \n }\n\n> endif\n> \n> libcamera_gst = shared_library('gstlibcamera',\n> libcamera_gst_sources,\n> c_args : libcamera_gst_c_args,\n> + cpp_args : libcamera_gst_cpp_args,\n> dependencies : [libcamera_dep, gstvideo_dep, gstallocator_dep],\n> install: true,\n> install_dir : '@0@/gstreamer-1.0'.format(get_option('libdir')),","headers":{"Return-Path":"","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CE6F960425\n\tfor ;\n\tSat, 7 Mar 2020 19:33:48 +0100 (CET)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: nicolas) with ESMTPSA id EBC2726B66A"],"Message-ID":"<42562f43ce0bd7cae248f83863dc7add8d789af4.camel@collabora.com>","From":"Nicolas Dufresne ","Reply-To":"Nicolas Dufresne ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Date":"Sat, 07 Mar 2020 13:33:44 -0500","In-Reply-To":"<20200307003310.29372-1-laurent.pinchart@ideasonboard.com>","References":"<20200307003310.29372-1-laurent.pinchart@ideasonboard.com>","Organization":"Collabora","Content-Type":"multipart/signed; micalg=\"pgp-sha1\";\n\tprotocol=\"application/pgp-signature\"; \n\tboundary=\"=-LsmGmlQ/aPGzVhNLFBbc\"","User-Agent":"Evolution 3.34.4 (3.34.4-1.fc31) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH] gst: Fix GLib detection and warning\n\tdisabling","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 07 Mar 2020 18:33:49 -0000"}},{"id":3986,"web_url":"https://patchwork.libcamera.org/comment/3986/","msgid":"<20200307184403.GB5021@pendragon.ideasonboard.com>","date":"2020-03-07T18:44:03","subject":"Re: [libcamera-devel] [PATCH] gst: Fix GLib detection and warning\n\tdisabling","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nOn Sat, Mar 07, 2020 at 01:33:44PM -0500, Nicolas Dufresne wrote:\n> Le samedi 07 mars 2020 à 02:33 +0200, Laurent Pinchart a écrit :\n> > Commit 17cccc68a88f (\"Add GStreamer plugin and element skeleton\") has\n> > gained a last minute fix for a clang compilation error with GLib prior\n> > to v2.63.0. The fix wasn't properly tested, and not only did it try to\n> > silence the affected compiler warning for C files only, it also failed\n> > to check the GLib dependency correctly. This resulted in compilation of\n> > the GStreamer element to always be disabled.\n> > \n> > Fix this by changing the GLib package name from 'glib' to 'glib-2.0',\n> > and silence the compiler warning for C++ files.\n> > \n> > Fixes: 17cccc68a88f (\"Add GStreamer plugin and element skeleton\")\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/gstreamer/meson.build | 6 ++++--\n> > 1 file changed, 4 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build\n> > index 1965b5041132..1b9dedd2528e 100644\n> > --- a/src/gstreamer/meson.build\n> > +++ b/src/gstreamer/meson.build\n> > @@ -12,8 +12,9 @@ libcamera_gst_c_args = [\n> > '-DVERSION=\"@0@\"'.format(libcamera_git_version),\n> > '-DPACKAGE=\"@0@\"'.format(meson.project_name()),\n> > ]\n> > +libcamera_gst_cpp_args = []\n> > \n> > -glib_dep = dependency('glib', required : get_option('gstreamer'))\n> > +glib_dep = dependency('glib-2.0', required : get_option('gstreamer'))\n> > \n> > gst_dep_version = '>=1.14.0'\n> > gstvideo_dep = dependency('gstreamer-video-1.0', version : gst_dep_version,\n> > @@ -27,12 +28,13 @@ if glib_dep.found() and gstvideo_dep.found() and gstallocator_dep.found()\n> > # complain about the ones we are not using. Silence the -Wunused-function\n> > # warning in that case.\n> > if cc.get_id() == 'clang' and glib_dep.version().version_compare('<2.63.0')\n> > - libcamera_gst_c_args += [ '-Wno-unused-function' ]\n> > + libcamera_gst_cpp_args += [ '-Wno-unused-function' ]\n> \n> We have gstlibcamera.c that includes gstlibcameraprovider.h and\n> gstlibcamerasrc.h, so shouldn't this be setting both C and CPP args ? \n\nI agree, I wonder why the compiler didn't complain for gstlibcamera.c.\n\n> If I may, maybe we could simply move gstlibcamera.c to gstlibcamera.cpp\n> and drop the C args. The generated code by GST_DEFINE_PLUGIN is inside\n> a scope of:\n> \n> extern \"C\" {\n> \n> }\n\nI think that would be better. I'll post a v2.\n\n> > endif\n> > \n> > libcamera_gst = shared_library('gstlibcamera',\n> > libcamera_gst_sources,\n> > c_args : libcamera_gst_c_args,\n> > + cpp_args : libcamera_gst_cpp_args,\n> > dependencies : [libcamera_dep, gstvideo_dep, gstallocator_dep],\n> > install: true,\n> > install_dir : '@0@/gstreamer-1.0'.format(get_option('libdir')),","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CAE7460425\n\tfor ;\n\tSat, 7 Mar 2020 19:44:07 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 241C75F;\n\tSat, 7 Mar 2020 19:44:07 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1583606647;\n\tbh=kQiGwZmC9m8SrDndvB7zTFC9B4yYYO4QtEQVBDVGVPo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=khk+RBqpkfxOxjoyoAPJywMRNcWhV45cCBjmOFzu2lxL0k+N1Fs4VSCWDQSsON2Bb\n\t/p57QEJSnv/mqb8JEYraIXj4bheYOIT2Wrtx46QPE70lcnSAk9isDXaGu2etNK7cf8\n\tzF53x+bQGsYwHUH3zclxIu98I8l5Uq4Ygn2ny6g8=","Date":"Sat, 7 Mar 2020 20:44:03 +0200","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200307184403.GB5021@pendragon.ideasonboard.com>","References":"<20200307003310.29372-1-laurent.pinchart@ideasonboard.com>\n\t<42562f43ce0bd7cae248f83863dc7add8d789af4.camel@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<42562f43ce0bd7cae248f83863dc7add8d789af4.camel@collabora.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] gst: Fix GLib detection and warning\n\tdisabling","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 07 Mar 2020 18:44:08 -0000"}}]