[libcamera-devel,v2,10/27] gst: libcamerasrc: Add a task for the streaming thread

Message ID 20200227200407.490616-11-nicolas.dufresne@collabora.com
State Accepted
Headers show
Series
  • GStreamer Element for libcamera
Related show

Commit Message

Nicolas Dufresne Feb. 27, 2020, 8:03 p.m. UTC
Use a GstTask as our internal streaming thread. Unlike GstBaseSrc, we
will be running a streaming thread at the element level rather than
per pad. This is needed to combine buffer request for multiple pads.

Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 src/gstreamer/gstlibcamerasrc.cpp | 49 +++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

Comments

Laurent Pinchart Feb. 29, 2020, 1:52 p.m. UTC | #1
Hi Nicolas,

Thank you for the patch.

On Thu, Feb 27, 2020 at 03:03:50PM -0500, Nicolas Dufresne wrote:
> Use a GstTask as our internal streaming thread. Unlike GstBaseSrc, we
> will be running a streaming thread at the element level rather than
> per pad. This is needed to combine buffer request for multiple pads.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 49 +++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 0c60478..53ece26 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -26,7 +26,11 @@ struct GstLibcameraSrcState {
>  
>  struct _GstLibcameraSrc {
>  	GstElement parent;
> +
> +	GRecMutex stream_lock;
> +	GstTask *task;
>  	GstPad *srcpad;
> +
>  	gchar *camera_name;
>  
>  	GstLibcameraSrcState *state;
> @@ -112,6 +116,30 @@ gst_libcamera_src_open(GstLibcameraSrc *self)
>  	return true;
>  }
>  
> +static void
> +gst_libcamera_src_task_run(gpointer user_data)
> +{
> +	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
> +
> +	GST_DEBUG_OBJECT(self, "Streaming thread is now capturing");
> +}
> +
> +static void
> +gst_libcamera_src_task_enter(GstTask *task, GThread *thread, gpointer user_data)
> +{
> +	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
> +
> +	GST_DEBUG_OBJECT(self, "Streaming thread has started");
> +}
> +
> +static void
> +gst_libcamera_src_task_leave(GstTask *task, GThread *thread, gpointer user_data)
> +{
> +	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
> +
> +	GST_DEBUG_OBJECT(self, "Streaming thread is about to stop");
> +}
> +
>  static void
>  gst_libcamera_src_close(GstLibcameraSrc *self)
>  {
> @@ -182,9 +210,22 @@ gst_libcamera_src_change_state(GstElement *element, GstStateChange transition)
>  			return GST_STATE_CHANGE_FAILURE;
>  		break;
>  	case GST_STATE_CHANGE_READY_TO_PAUSED:
> +		/* This needs to be called after pads activation.*/
> +		if (!gst_task_pause(self->task))
> +			return GST_STATE_CHANGE_FAILURE;
> +		ret = GST_STATE_CHANGE_NO_PREROLL;
> +		break;
>  	case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
>  		ret = GST_STATE_CHANGE_NO_PREROLL;
>  		break;
> +	case GST_STATE_CHANGE_PAUSED_TO_READY:
> +		/* \todo this might requires some thread unblocking in the future

s/requires/require/

> +		 * if the streaming thread starts doing any kind of blocking
> +		 * operations. If this was the case, we would need to do so
> +		 * before pad deactivation, so before chaining to the parent
> +		 * change_state function. */

		/*
		 * ...
		 */

here too.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +		gst_task_join(self->task);
> +		break;
>  	case GST_STATE_CHANGE_READY_TO_NULL:
>  		gst_libcamera_src_close(self);
>  		break;
> @@ -201,6 +242,8 @@ gst_libcamera_src_finalize(GObject *object)
>  	GObjectClass *klass = G_OBJECT_CLASS(gst_libcamera_src_parent_class);
>  	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(object);
>  
> +	g_rec_mutex_clear(&self->stream_lock);
> +	g_clear_object(&self->task);
>  	g_free(self->camera_name);
>  	delete self->state;
>  
> @@ -213,6 +256,12 @@ gst_libcamera_src_init(GstLibcameraSrc *self)
>  	GstLibcameraSrcState *state = new GstLibcameraSrcState();
>  	GstPadTemplate *templ = gst_element_get_pad_template(GST_ELEMENT(self), "src");
>  
> +	g_rec_mutex_init(&self->stream_lock);
> +	self->task = gst_task_new(gst_libcamera_src_task_run, self, nullptr);
> +	gst_task_set_enter_callback(self->task, gst_libcamera_src_task_enter, self, nullptr);
> +	gst_task_set_leave_callback(self->task, gst_libcamera_src_task_leave, self, nullptr);
> +	gst_task_set_lock(self->task, &self->stream_lock);
> +
>  	self->srcpad = gst_pad_new_from_template(templ, "src");
>  	gst_element_add_pad(GST_ELEMENT(self), self->srcpad);
>  	self->state = state;

Patch

diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index 0c60478..53ece26 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -26,7 +26,11 @@  struct GstLibcameraSrcState {
 
 struct _GstLibcameraSrc {
 	GstElement parent;
+
+	GRecMutex stream_lock;
+	GstTask *task;
 	GstPad *srcpad;
+
 	gchar *camera_name;
 
 	GstLibcameraSrcState *state;
@@ -112,6 +116,30 @@  gst_libcamera_src_open(GstLibcameraSrc *self)
 	return true;
 }
 
+static void
+gst_libcamera_src_task_run(gpointer user_data)
+{
+	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
+
+	GST_DEBUG_OBJECT(self, "Streaming thread is now capturing");
+}
+
+static void
+gst_libcamera_src_task_enter(GstTask *task, GThread *thread, gpointer user_data)
+{
+	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
+
+	GST_DEBUG_OBJECT(self, "Streaming thread has started");
+}
+
+static void
+gst_libcamera_src_task_leave(GstTask *task, GThread *thread, gpointer user_data)
+{
+	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
+
+	GST_DEBUG_OBJECT(self, "Streaming thread is about to stop");
+}
+
 static void
 gst_libcamera_src_close(GstLibcameraSrc *self)
 {
@@ -182,9 +210,22 @@  gst_libcamera_src_change_state(GstElement *element, GstStateChange transition)
 			return GST_STATE_CHANGE_FAILURE;
 		break;
 	case GST_STATE_CHANGE_READY_TO_PAUSED:
+		/* This needs to be called after pads activation.*/
+		if (!gst_task_pause(self->task))
+			return GST_STATE_CHANGE_FAILURE;
+		ret = GST_STATE_CHANGE_NO_PREROLL;
+		break;
 	case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
 		ret = GST_STATE_CHANGE_NO_PREROLL;
 		break;
+	case GST_STATE_CHANGE_PAUSED_TO_READY:
+		/* \todo this might requires some thread unblocking in the future
+		 * if the streaming thread starts doing any kind of blocking
+		 * operations. If this was the case, we would need to do so
+		 * before pad deactivation, so before chaining to the parent
+		 * change_state function. */
+		gst_task_join(self->task);
+		break;
 	case GST_STATE_CHANGE_READY_TO_NULL:
 		gst_libcamera_src_close(self);
 		break;
@@ -201,6 +242,8 @@  gst_libcamera_src_finalize(GObject *object)
 	GObjectClass *klass = G_OBJECT_CLASS(gst_libcamera_src_parent_class);
 	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(object);
 
+	g_rec_mutex_clear(&self->stream_lock);
+	g_clear_object(&self->task);
 	g_free(self->camera_name);
 	delete self->state;
 
@@ -213,6 +256,12 @@  gst_libcamera_src_init(GstLibcameraSrc *self)
 	GstLibcameraSrcState *state = new GstLibcameraSrcState();
 	GstPadTemplate *templ = gst_element_get_pad_template(GST_ELEMENT(self), "src");
 
+	g_rec_mutex_init(&self->stream_lock);
+	self->task = gst_task_new(gst_libcamera_src_task_run, self, nullptr);
+	gst_task_set_enter_callback(self->task, gst_libcamera_src_task_enter, self, nullptr);
+	gst_task_set_leave_callback(self->task, gst_libcamera_src_task_leave, self, nullptr);
+	gst_task_set_lock(self->task, &self->stream_lock);
+
 	self->srcpad = gst_pad_new_from_template(templ, "src");
 	gst_element_add_pad(GST_ELEMENT(self), self->srcpad);
 	self->state = state;