[libcamera-devel] [PATCH v1 04/23] gst: utils: Add a macro to use a GMutexLocker

[Nicolas Dufresne]

On mar, 2020-02-11 at 20:00 +0200, Laurent Pinchart wrote:
> Hi Nicolas,
> 
> Thank you for the patch.
> 
> On Tue, Jan 28, 2020 at 10:31:51PM -0500, Nicolas Dufresne wrote:
> > From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> > 
> > The GMutexLocker uses GCC/CLANG C feature to implement a smart lock,
> 
> s/smart/scoped/ ?
> 
> > that unlocks the mutex when the GMutexLocker goes out of scope. This
> > could have been implemented in C++, but as this is already implemented
> > I decided to just reuse it. This is particularly handy to avoid gotos
> > in error handling cases and to prevent unbalanced locking.
> > 
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> > ---
> >  src/gstreamer/gstlibcamera-utils.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/src/gstreamer/gstlibcamera-utils.h
> > b/src/gstreamer/gstlibcamera-utils.h
> > index 4545512..528dc2c 100644
> > --- a/src/gstreamer/gstlibcamera-utils.h
> > +++ b/src/gstreamer/gstlibcamera-utils.h
> > @@ -13,6 +13,8 @@
> >  
> >  #ifndef __GST_LIBCAMERA_UTILS_H_
> >  
> > +#define GST_OBJECT_LOCKER(obj) g_autoptr(GMutexLocker) locker =
> > g_mutex_locker_new(GST_OBJECT_GET_LOCK(obj))
> > +
> 
> The name of the macro would lead me to think it's provided by gstreamer.
> Should it be renamed to GST_LIBCAMERA_OBJECT_LOCKER() ? Or would that
> be too long ?

Sorry for the overhead, as these locker are only present in very recent GLib, it
was agreed to implement C++ helper instead. I agree with you here, sometimes I
use GST_ namespace for helper I think maybe be worth moving into core of
GStreamer, but it make little sense for this one since this is not portable (not
supported by MSVC), so unlikely to be added.

About the proposal, let's give the general idea, maybe you have better ideas.

class GLibLocker {
	GLibLocker(GMutex *mutex) : mutex_(mutex)
	{
    		g_mutex_lock(mutex_);
	}

	GLibLocker(GstObject *object) : mutex_(GST_OBJECT_GET_LOCK(gst_object), rec_mutex_(rec_mutex)
	{
    		g_mutex_lock(mutex_);
	}

	
	~GLibLocker()
	{
		g_mutex_unlock(mutex_);
	}
};

some_code()
{
	GLibLocker(GST_OBJECT(my_object));
} /* unlocked here */

> 
> >  GstCaps *gst_libcamera_stream_formats_to_caps(const
> > libcamera::StreamFormats &formats);
> >  
> >  #endif /* __GST_LIBCAMERA_UTILS_H_ */