[{"id":3659,"web_url":"https://patchwork.libcamera.org/comment/3659/","msgid":"<20200211180026.GN22612@pendragon.ideasonboard.com>","date":"2020-02-11T18:00:26","subject":"Re: [libcamera-devel] [PATCH v1 04/23] gst: utils: Add a macro to\n\tuse a GMutexLocker","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nThank you for the patch.\n\nOn Tue, Jan 28, 2020 at 10:31:51PM -0500, Nicolas Dufresne wrote:\n> From: Nicolas Dufresne \n> \n> The GMutexLocker uses GCC/CLANG C feature to implement a smart lock,\n\ns/smart/scoped/ ?\n\n> that unlocks the mutex when the GMutexLocker goes out of scope. This\n> could have been implemented in C++, but as this is already implemented\n> I decided to just reuse it. This is particularly handy to avoid gotos\n> in error handling cases and to prevent unbalanced locking.\n> \n> Signed-off-by: Nicolas Dufresne \n> ---\n> src/gstreamer/gstlibcamera-utils.h | 2 ++\n> 1 file changed, 2 insertions(+)\n> \n> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> index 4545512..528dc2c 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -13,6 +13,8 @@\n> \n> #ifndef __GST_LIBCAMERA_UTILS_H_\n> \n> +#define GST_OBJECT_LOCKER(obj) g_autoptr(GMutexLocker) locker = g_mutex_locker_new(GST_OBJECT_GET_LOCK(obj))\n> +\n\nThe name of the macro would lead me to think it's provided by gstreamer.\nShould it be renamed to GST_LIBCAMERA_OBJECT_LOCKER() ? Or would that\nbe too long ?\n\n> GstCaps *gst_libcamera_stream_formats_to_caps(const libcamera::StreamFormats &formats);\n> \n> #endif /* __GST_LIBCAMERA_UTILS_H_ */","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C9AA60F3C\n\tfor ;\n\tTue, 11 Feb 2020 19:00:44 +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 2FFBF9DA;\n\tTue, 11 Feb 2020 19:00:42 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1581444044;\n\tbh=VgotgXIaSX2fUi3kemxNeP34CX+M1AVn+si9muwTNNU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=B/ERbR+ZS472vA+XCDX3MeFCoKQ4z4H+u2Yd6pD90+ZmZabhzxAxxCmxHZqYghIIb\n\tDLawgfBtoqcJzpwAioRELM4hNEBHMoHNOHiDC/AoehVOaQ/oGUcm3rpZuegFk9I8Gb\n\tBPAVJiINdyaAKH1GeI8Q25ZKVVuETUGTnxZnUuPw=","Date":"Tue, 11 Feb 2020 20:00:26 +0200","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org,\n\tNicolas Dufresne ","Message-ID":"<20200211180026.GN22612@pendragon.ideasonboard.com>","References":"<20200129033210.278800-1-nicolas@ndufresne.ca>\n\t<20200129033210.278800-5-nicolas@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200129033210.278800-5-nicolas@ndufresne.ca>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v1 04/23] gst: utils: Add a macro to\n\tuse a GMutexLocker","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":"Tue, 11 Feb 2020 18:00:44 -0000"}},{"id":3660,"web_url":"https://patchwork.libcamera.org/comment/3660/","msgid":"","date":"2020-02-11T18:50:54","subject":"Re: [libcamera-devel] [PATCH v1 04/23] gst: utils: Add a macro to\n\tuse a GMutexLocker","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"On mar, 2020-02-11 at 20:00 +0200, Laurent Pinchart wrote:\n> Hi Nicolas,\n> \n> Thank you for the patch.\n> \n> On Tue, Jan 28, 2020 at 10:31:51PM -0500, Nicolas Dufresne wrote:\n> > From: Nicolas Dufresne \n> > \n> > The GMutexLocker uses GCC/CLANG C feature to implement a smart lock,\n> \n> s/smart/scoped/ ?\n> \n> > that unlocks the mutex when the GMutexLocker goes out of scope. This\n> > could have been implemented in C++, but as this is already implemented\n> > I decided to just reuse it. This is particularly handy to avoid gotos\n> > in error handling cases and to prevent unbalanced locking.\n> > \n> > Signed-off-by: Nicolas Dufresne \n> > ---\n> > src/gstreamer/gstlibcamera-utils.h | 2 ++\n> > 1 file changed, 2 insertions(+)\n> > \n> > diff --git a/src/gstreamer/gstlibcamera-utils.h\n> > b/src/gstreamer/gstlibcamera-utils.h\n> > index 4545512..528dc2c 100644\n> > --- a/src/gstreamer/gstlibcamera-utils.h\n> > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > @@ -13,6 +13,8 @@\n> > \n> > #ifndef __GST_LIBCAMERA_UTILS_H_\n> > \n> > +#define GST_OBJECT_LOCKER(obj) g_autoptr(GMutexLocker) locker =\n> > g_mutex_locker_new(GST_OBJECT_GET_LOCK(obj))\n> > +\n> \n> The name of the macro would lead me to think it's provided by gstreamer.\n> Should it be renamed to GST_LIBCAMERA_OBJECT_LOCKER() ? Or would that\n> be too long ?\n\nSorry for the overhead, as these locker are only present in very recent GLib, it\nwas agreed to implement C++ helper instead. I agree with you here, sometimes I\nuse GST_ namespace for helper I think maybe be worth moving into core of\nGStreamer, but it make little sense for this one since this is not portable (not\nsupported by MSVC), so unlikely to be added.\n\nAbout the proposal, let's give the general idea, maybe you have better ideas.\n\nclass GLibLocker {\n\tGLibLocker(GMutex *mutex) : mutex_(mutex)\n\t{\n \t\tg_mutex_lock(mutex_);\n\t}\n\n\tGLibLocker(GstObject *object) : mutex_(GST_OBJECT_GET_LOCK(gst_object), rec_mutex_(rec_mutex)\n\t{\n \t\tg_mutex_lock(mutex_);\n\t}\n\n\t\n\t~GLibLocker()\n\t{\n\t\tg_mutex_unlock(mutex_);\n\t}\n};\n\nsome_code()\n{\n\tGLibLocker(GST_OBJECT(my_object));\n} /* unlocked here */\n\n> \n> > GstCaps *gst_libcamera_stream_formats_to_caps(const\n> > libcamera::StreamFormats &formats);\n> > \n> > #endif /* __GST_LIBCAMERA_UTILS_H_ */","headers":{"Return-Path":"","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7818060F3C\n\tfor ;\n\tTue, 11 Feb 2020 19:51:03 +0100 (CET)","from nicolas-tpx395.localdomain (unknown [IPv6:2610:98:8005::527])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id CED452914CE;\n\tTue, 11 Feb 2020 18:51:02 +0000 (GMT)"],"Message-ID":"","From":"Nicolas Dufresne ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Date":"Tue, 11 Feb 2020 13:50:54 -0500","In-Reply-To":"<20200211180026.GN22612@pendragon.ideasonboard.com>","References":"<20200129033210.278800-1-nicolas@ndufresne.ca>\n\t<20200129033210.278800-5-nicolas@ndufresne.ca>\n\t<20200211180026.GN22612@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.34.3 (3.34.3-1.fc31) ","MIME-Version":"1.0","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v1 04/23] gst: utils: Add a macro to\n\tuse a GMutexLocker","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":"Tue, 11 Feb 2020 18:51:03 -0000"}},{"id":3661,"web_url":"https://patchwork.libcamera.org/comment/3661/","msgid":"<20200211191906.GA20823@pendragon.ideasonboard.com>","date":"2020-02-11T19:19:06","subject":"Re: [libcamera-devel] [PATCH v1 04/23] gst: utils: Add a macro to\n\tuse a GMutexLocker","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nOn Tue, Feb 11, 2020 at 01:50:54PM -0500, Nicolas Dufresne wrote:\n> On mar, 2020-02-11 at 20:00 +0200, Laurent Pinchart wrote:\n> > On Tue, Jan 28, 2020 at 10:31:51PM -0500, Nicolas Dufresne wrote:\n> > > From: Nicolas Dufresne \n> > > \n> > > The GMutexLocker uses GCC/CLANG C feature to implement a smart lock,\n> > \n> > s/smart/scoped/ ?\n> > \n> > > that unlocks the mutex when the GMutexLocker goes out of scope. This\n> > > could have been implemented in C++, but as this is already implemented\n> > > I decided to just reuse it. This is particularly handy to avoid gotos\n> > > in error handling cases and to prevent unbalanced locking.\n> > > \n> > > Signed-off-by: Nicolas Dufresne \n> > > ---\n> > > src/gstreamer/gstlibcamera-utils.h | 2 ++\n> > > 1 file changed, 2 insertions(+)\n> > > \n> > > diff --git a/src/gstreamer/gstlibcamera-utils.h\n> > > b/src/gstreamer/gstlibcamera-utils.h\n> > > index 4545512..528dc2c 100644\n> > > --- a/src/gstreamer/gstlibcamera-utils.h\n> > > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > > @@ -13,6 +13,8 @@\n> > > \n> > > #ifndef __GST_LIBCAMERA_UTILS_H_\n> > > \n> > > +#define GST_OBJECT_LOCKER(obj) g_autoptr(GMutexLocker) locker =\n> > > g_mutex_locker_new(GST_OBJECT_GET_LOCK(obj))\n> > > +\n> > \n> > The name of the macro would lead me to think it's provided by gstreamer.\n> > Should it be renamed to GST_LIBCAMERA_OBJECT_LOCKER() ? Or would that\n> > be too long ?\n> \n> Sorry for the overhead, as these locker are only present in very recent GLib, it\n> was agreed to implement C++ helper instead. I agree with you here, sometimes I\n> use GST_ namespace for helper I think maybe be worth moving into core of\n> GStreamer, but it make little sense for this one since this is not portable (not\n> supported by MSVC), so unlikely to be added.\n> \n> About the proposal, let's give the general idea, maybe you have better ideas.\n> \n> class GLibLocker {\n> \tGLibLocker(GMutex *mutex) : mutex_(mutex)\n> \t{\n> \t\tg_mutex_lock(mutex_);\n> \t}\n> \n> \tGLibLocker(GstObject *object) : mutex_(GST_OBJECT_GET_LOCK(gst_object), rec_mutex_(rec_mutex)\n\nI assume s/gst_object/object/. Not sure what rec_mutex is used for :-)\n\n> \t{\n> \t\tg_mutex_lock(mutex_);\n> \t}\n> \n> \t\n> \t~GLibLocker()\n> \t{\n> \t\tg_mutex_unlock(mutex_);\n> \t}\n> };\n> \n> some_code()\n> {\n> \tGLibLocker(GST_OBJECT(my_object));\n> } /* unlocked here */\n\nThis looks good to me.\n\n> > > GstCaps *gst_libcamera_stream_formats_to_caps(const\n> > > libcamera::StreamFormats &formats);\n> > > \n> > > #endif /* __GST_LIBCAMERA_UTILS_H_ */","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 E644B60F3C\n\tfor ;\n\tTue, 11 Feb 2020 20:19:22 +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 E1DFF9DA;\n\tTue, 11 Feb 2020 20:19:21 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1581448762;\n\tbh=ATdfDE+vTmrnpb8GAHPP5tp6IEwX1GfzkLkj6zrbzB0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZMZpyqfilh+omdz5MGjVVHxTYEHQqxErRomtwzHgEaQKmuUOyU6FS2On4+FM47GP9\n\tQjSDb1BDnAg4UP3rD0nLukX3o5e3uCkTdYHaumPpWj6qnmwgnXnz15utwDThey9FNc\n\t3+JNzGkPa8U3Tbz5OBz4+/OqQRWKFofejjDxg5qY=","Date":"Tue, 11 Feb 2020 21:19:06 +0200","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200211191906.GA20823@pendragon.ideasonboard.com>","References":"<20200129033210.278800-1-nicolas@ndufresne.ca>\n\t<20200129033210.278800-5-nicolas@ndufresne.ca>\n\t<20200211180026.GN22612@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v1 04/23] gst: utils: Add a macro to\n\tuse a GMutexLocker","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":"Tue, 11 Feb 2020 19:19:23 -0000"}},{"id":3669,"web_url":"https://patchwork.libcamera.org/comment/3669/","msgid":"","date":"2020-02-11T21:51:30","subject":"Re: [libcamera-devel] [PATCH v1 04/23] gst: utils: Add a macro to\n\tuse a GMutexLocker","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"On mar, 2020-02-11 at 21:19 +0200, Laurent Pinchart wrote:\n> Hi Nicolas,\n> \n> On Tue, Feb 11, 2020 at 01:50:54PM -0500, Nicolas Dufresne wrote:\n> > On mar, 2020-02-11 at 20:00 +0200, Laurent Pinchart wrote:\n> > > On Tue, Jan 28, 2020 at 10:31:51PM -0500, Nicolas Dufresne wrote:\n> > > > From: Nicolas Dufresne \n> > > > \n> > > > The GMutexLocker uses GCC/CLANG C feature to implement a smart lock,\n> > > \n> > > s/smart/scoped/ ?\n> > > \n> > > > that unlocks the mutex when the GMutexLocker goes out of scope. This\n> > > > could have been implemented in C++, but as this is already implemented\n> > > > I decided to just reuse it. This is particularly handy to avoid gotos\n> > > > in error handling cases and to prevent unbalanced locking.\n> > > > \n> > > > Signed-off-by: Nicolas Dufresne \n> > > > ---\n> > > > src/gstreamer/gstlibcamera-utils.h | 2 ++\n> > > > 1 file changed, 2 insertions(+)\n> > > > \n> > > > diff --git a/src/gstreamer/gstlibcamera-utils.h\n> > > > b/src/gstreamer/gstlibcamera-utils.h\n> > > > index 4545512..528dc2c 100644\n> > > > --- a/src/gstreamer/gstlibcamera-utils.h\n> > > > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > > > @@ -13,6 +13,8 @@\n> > > > \n> > > > #ifndef __GST_LIBCAMERA_UTILS_H_\n> > > > \n> > > > +#define GST_OBJECT_LOCKER(obj) g_autoptr(GMutexLocker) locker =\n> > > > g_mutex_locker_new(GST_OBJECT_GET_LOCK(obj))\n> > > > +\n> > > \n> > > The name of the macro would lead me to think it's provided by gstreamer.\n> > > Should it be renamed to GST_LIBCAMERA_OBJECT_LOCKER() ? Or would that\n> > > be too long ?\n> > \n> > Sorry for the overhead, as these locker are only present in very recent\n> > GLib, it\n> > was agreed to implement C++ helper instead. I agree with you here, sometimes\n> > I\n> > use GST_ namespace for helper I think maybe be worth moving into core of\n> > GStreamer, but it make little sense for this one since this is not portable\n> > (not\n> > supported by MSVC), so unlikely to be added.\n> > \n> > About the proposal, let's give the general idea, maybe you have better\n> > ideas.\n> > \n> > class GLibLocker {\n> > \tGLibLocker(GMutex *mutex) : mutex_(mutex)\n> > \t{\n> > \t\tg_mutex_lock(mutex_);\n> > \t}\n> > \n> > \tGLibLocker(GstObject *object) : mutex_(GST_OBJECT_GET_LOCK(gst_object),\n> > rec_mutex_(rec_mutex)\n> \n> I assume s/gst_object/object/. Not sure what rec_mutex is used for :-)\n\nIndeed, I just wrote this quickly, I initially thought of having both recursive\nmutex and mutex in the same object, but then retracted and didn't cleanup\nproperly. Will have two class, it's cleaner.\n\n> \n> > \t{\n> > \t\tg_mutex_lock(mutex_);\n> > \t}\n> > \n> > \t\n> > \t~GLibLocker()\n> > \t{\n> > \t\tg_mutex_unlock(mutex_);\n> > \t}\n> > };\n> > \n> > some_code()\n> > {\n> > \tGLibLocker(GST_OBJECT(my_object));\n> > } /* unlocked here */\n> \n> This looks good to me.\n\nOk, and that should bring us back to GLib 2.44, I'll test this time before V2,\njust to make sure. Thanks for the feedback.\n\n> \n> > > > GstCaps *gst_libcamera_stream_formats_to_caps(const\n> > > > libcamera::StreamFormats &formats);\n> > > > \n> > > > #endif /* __GST_LIBCAMERA_UTILS_H_ */","headers":{"Return-Path":"","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D6C4160F3C\n\tfor ;\n\tTue, 11 Feb 2020 22:51:39 +0100 (CET)","from nicolas-tpx395.localdomain (unknown [IPv6:2610:98:8005::527])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id 13AD52943DC;\n\tTue, 11 Feb 2020 21:51:38 +0000 (GMT)"],"Message-ID":"","From":"Nicolas Dufresne ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Date":"Tue, 11 Feb 2020 16:51:30 -0500","In-Reply-To":"<20200211191906.GA20823@pendragon.ideasonboard.com>","References":"<20200129033210.278800-1-nicolas@ndufresne.ca>\n\t<20200129033210.278800-5-nicolas@ndufresne.ca>\n\t<20200211180026.GN22612@pendragon.ideasonboard.com>\n\t\n\t<20200211191906.GA20823@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.34.3 (3.34.3-1.fc31) ","MIME-Version":"1.0","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v1 04/23] gst: utils: Add a macro to\n\tuse a GMutexLocker","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":"Tue, 11 Feb 2020 21:51:40 -0000"}},{"id":3678,"web_url":"https://patchwork.libcamera.org/comment/3678/","msgid":"<20200211231837.GL20823@pendragon.ideasonboard.com>","date":"2020-02-11T23:18:37","subject":"Re: [libcamera-devel] [PATCH v1 04/23] gst: utils: Add a macro to\n\tuse a GMutexLocker","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nOn Tue, Feb 11, 2020 at 04:51:30PM -0500, Nicolas Dufresne wrote:\n> On mar, 2020-02-11 at 21:19 +0200, Laurent Pinchart wrote:\n> > On Tue, Feb 11, 2020 at 01:50:54PM -0500, Nicolas Dufresne wrote:\n> > > On mar, 2020-02-11 at 20:00 +0200, Laurent Pinchart wrote:\n> > > > On Tue, Jan 28, 2020 at 10:31:51PM -0500, Nicolas Dufresne wrote:\n> > > > > From: Nicolas Dufresne \n> > > > > \n> > > > > The GMutexLocker uses GCC/CLANG C feature to implement a smart lock,\n> > > > \n> > > > s/smart/scoped/ ?\n> > > > \n> > > > > that unlocks the mutex when the GMutexLocker goes out of scope. This\n> > > > > could have been implemented in C++, but as this is already implemented\n> > > > > I decided to just reuse it. This is particularly handy to avoid gotos\n> > > > > in error handling cases and to prevent unbalanced locking.\n> > > > > \n> > > > > Signed-off-by: Nicolas Dufresne \n> > > > > ---\n> > > > > src/gstreamer/gstlibcamera-utils.h | 2 ++\n> > > > > 1 file changed, 2 insertions(+)\n> > > > > \n> > > > > diff --git a/src/gstreamer/gstlibcamera-utils.h\n> > > > > b/src/gstreamer/gstlibcamera-utils.h\n> > > > > index 4545512..528dc2c 100644\n> > > > > --- a/src/gstreamer/gstlibcamera-utils.h\n> > > > > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > > > > @@ -13,6 +13,8 @@\n> > > > > \n> > > > > #ifndef __GST_LIBCAMERA_UTILS_H_\n> > > > > \n> > > > > +#define GST_OBJECT_LOCKER(obj) g_autoptr(GMutexLocker) locker =\n> > > > > g_mutex_locker_new(GST_OBJECT_GET_LOCK(obj))\n> > > > > +\n> > > > \n> > > > The name of the macro would lead me to think it's provided by gstreamer.\n> > > > Should it be renamed to GST_LIBCAMERA_OBJECT_LOCKER() ? Or would that\n> > > > be too long ?\n> > > \n> > > Sorry for the overhead, as these locker are only present in very recent\n> > > GLib, it\n> > > was agreed to implement C++ helper instead. I agree with you here, sometimes\n> > > I\n> > > use GST_ namespace for helper I think maybe be worth moving into core of\n> > > GStreamer, but it make little sense for this one since this is not portable\n> > > (not\n> > > supported by MSVC), so unlikely to be added.\n> > > \n> > > About the proposal, let's give the general idea, maybe you have better\n> > > ideas.\n> > > \n> > > class GLibLocker {\n> > > \tGLibLocker(GMutex *mutex) : mutex_(mutex)\n> > > \t{\n> > > \t\tg_mutex_lock(mutex_);\n> > > \t}\n> > > \n> > > \tGLibLocker(GstObject *object) : mutex_(GST_OBJECT_GET_LOCK(gst_object),\n> > > rec_mutex_(rec_mutex)\n> > \n> > I assume s/gst_object/object/. Not sure what rec_mutex is used for :-)\n> \n> Indeed, I just wrote this quickly, I initially thought of having both recursive\n> mutex and mutex in the same object, but then retracted and didn't cleanup\n> properly. Will have two class, it's cleaner.\n\nSeems like a good idea. By the way, if you want to extend the API of\nyour lockers, could you try to do so in a way that mimicks the C++\nlockers (https://en.cppreference.com/w/cpp/thread/lock_guard,\nhttps://en.cppreference.com/w/cpp/thread/unique_lock or\nhttps://en.cppreference.com/w/cpp/thread/scoped_lock) ?\n\n> > > \t{\n> > > \t\tg_mutex_lock(mutex_);\n> > > \t}\n> > > \n> > > \t\n> > > \t~GLibLocker()\n> > > \t{\n> > > \t\tg_mutex_unlock(mutex_);\n> > > \t}\n> > > };\n> > > \n> > > some_code()\n> > > {\n> > > \tGLibLocker(GST_OBJECT(my_object));\n> > > } /* unlocked here */\n> > \n> > This looks good to me.\n> \n> Ok, and that should bring us back to GLib 2.44, I'll test this time before V2,\n> just to make sure. Thanks for the feedback.\n\nThank you for making our life easier with glib 2.44 :-)\n\n> > > > > GstCaps *gst_libcamera_stream_formats_to_caps(const\n> > > > > libcamera::StreamFormats &formats);\n> > > > > \n> > > > > #endif /* __GST_LIBCAMERA_UTILS_H_ */","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 385C360990\n\tfor ;\n\tWed, 12 Feb 2020 00:18:53 +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 9AB9C9DA;\n\tWed, 12 Feb 2020 00:18:52 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1581463132;\n\tbh=0ujnEVqNEJavvSAzDNmTNhHGA4kCBmrvi6yDeIvAej0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Hmw0M1mqQk2I5+9BjllZNtqdJUSHogNEAbLxbDE8w85+sSQqdddmvAMYYVz/lWK6p\n\td3Ed7sgxw+kLwb1YbsHubu81e1EammZjyeTLnMKCmaVFE28nf0ZiZWeSd09zg0WvqD\n\tVwjy2P5gZkef6dW/hU2XJpMZb1X451h3p9D6/tuU=","Date":"Wed, 12 Feb 2020 01:18:37 +0200","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200211231837.GL20823@pendragon.ideasonboard.com>","References":"<20200129033210.278800-1-nicolas@ndufresne.ca>\n\t<20200129033210.278800-5-nicolas@ndufresne.ca>\n\t<20200211180026.GN22612@pendragon.ideasonboard.com>\n\t\n\t<20200211191906.GA20823@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v1 04/23] gst: utils: Add a macro to\n\tuse a GMutexLocker","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":"Tue, 11 Feb 2020 23:18:53 -0000"}}]