[libcamera-devel] gstreamer: Drop libcamera_private dependency

18763 diff mbox series

Drop libcamera_private dependency entirely as to avoid libcamerasrc
getting more dependent on it. In order to achieve that, one of the
mutex locks in GstLibcameraSrcState needs to be replaced with GMutex.

However doing so, this won't let us to use the clang's thread annotation
macros in libcamera (which should be fine as libcamerasrc would move
out of libcamera repo once matured).

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/gstreamer/gstlibcamerasrc.cpp | 23 +++++++++++------------
 src/gstreamer/meson.build         |  2 +-
 2 files changed, 12 insertions(+), 13 deletions(-)

Hi,

Le mardi 27 juin 2023 à 13:52 +0200, Umang Jain a écrit :
> Drop libcamera_private dependency entirely as to avoid libcamerasrc
> getting more dependent on it. In order to achieve that, one of the
> mutex locks in GstLibcameraSrcState needs to be replaced with GMutex.
> 
> However doing so, this won't let us to use the clang's thread annotation
> macros in libcamera (which should be fine as libcamerasrc would move
> out of libcamera repo once matured).
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 23 +++++++++++------------
>  src/gstreamer/meson.build         |  2 +-
>  2 files changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 11f15068..b5c6e9ab 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -32,8 +32,6 @@
>  #include <queue>
>  #include <vector>
>  
> -#include <libcamera/base/mutex.h>
> -
>  #include <libcamera/camera.h>
>  #include <libcamera/camera_manager.h>
>  #include <libcamera/control_ids.h>
> @@ -125,11 +123,9 @@ struct GstLibcameraSrcState {
>  	 * be held while calling into other graph elements (e.g. when calling
>  	 * gst_pad_query()).
>  	 */
> -	Mutex lock_;
> -	std::queue<std::unique_ptr<RequestWrap>> queuedRequests_
> -		LIBCAMERA_TSA_GUARDED_BY(lock_);
> -	std::queue<std::unique_ptr<RequestWrap>> completedRequests_
> -		LIBCAMERA_TSA_GUARDED_BY(lock_);
> +	GMutex lock_;
> +	std::queue<std::unique_ptr<RequestWrap>> queuedRequests_;
> +	std::queue<std::unique_ptr<RequestWrap>> completedRequests_;
>  
>  	ControlList initControls_;
>  	guint group_id_;
> @@ -208,7 +204,7 @@ int GstLibcameraSrcState::queueRequest()
>  	cam_->queueRequest(wrap->request_.get());
>  
>  	{
> -		MutexLocker locker(lock_);
> +		GLibLocker locker(&lock_);
>  		queuedRequests_.push(std::move(wrap));
>  	}
>  
> @@ -224,7 +220,7 @@ GstLibcameraSrcState::requestCompleted(Request *request)
>  	std::unique_ptr<RequestWrap> wrap;
>  
>  	{
> -		MutexLocker locker(lock_);
> +		GLibLocker locker(&lock_);
>  		wrap = std::move(queuedRequests_.front());
>  		queuedRequests_.pop();
>  	}
> @@ -251,7 +247,7 @@ GstLibcameraSrcState::requestCompleted(Request *request)
>  	}
>  
>  	{
> -		MutexLocker locker(lock_);
> +		GLibLocker locker(&lock_);
>  		completedRequests_.push(std::move(wrap));
>  	}
>  
> @@ -265,7 +261,7 @@ int GstLibcameraSrcState::processRequest()
>  	int err = 0;
>  
>  	{
> -		MutexLocker locker(lock_);
> +		GLibLocker locker(&lock_);
>  
>  		if (!completedRequests_.empty()) {
>  			wrap = std::move(completedRequests_.front());
> @@ -639,7 +635,7 @@ gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,
>  	state->cam_->stop();
>  
>  	{
> -		MutexLocker locker(state->lock_);
> +		GLibLocker locker(&state->lock_);
>  		state->completedRequests_ = {};
>  	}
>  
> @@ -776,6 +772,7 @@ gst_libcamera_src_finalize(GObject *object)
>  
>  	g_rec_mutex_clear(&self->stream_lock);
>  	g_clear_object(&self->task);
> +	g_mutex_clear(&self->state->lock_);
>  	g_free(self->camera_name);
>  	delete self->controls;
>  	delete self->state;
> @@ -795,6 +792,8 @@ gst_libcamera_src_init(GstLibcameraSrc *self)
>  	gst_task_set_leave_callback(self->task, gst_libcamera_src_task_leave, self, nullptr);
>  	gst_task_set_lock(self->task, &self->stream_lock);
>  
> +	g_mutex_init(&state->lock_);
> +
>  	state->srcpads_.push_back(gst_pad_new_from_template(templ, "src"));
>  	gst_element_add_pad(GST_ELEMENT(self), state->srcpads_.back());
>  
> diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build
> index eda246d7..77c79140 100644
> --- a/src/gstreamer/meson.build
> +++ b/src/gstreamer/meson.build
> @@ -42,7 +42,7 @@ endif
>  libcamera_gst = shared_library('gstlibcamera',
>      libcamera_gst_sources,
>      cpp_args : libcamera_gst_cpp_args,
> -    dependencies : [libcamera_private, gstvideo_dep, gstallocator_dep],
> +    dependencies : [libcamera_public, gstvideo_dep, gstallocator_dep],
>      install: true,
>      install_dir : '@0@/gstreamer-1.0'.format(get_option('libdir')),
>  )