[libcamera-devel] [PATCH] gstreamer: Implement EOS handling

[Barnabás Pőcze]

Hi


I am not too familiar with gstreamer, so I cannot comment on that part,
but I believe using a single `std::atomic` object would be better. Please see below.


2023. november 6., hétfő 17:52 keltezéssel, Jaslo Ziska via libcamera-devel <libcamera-devel at lists.libcamera.org> írta:

> ---
> Hi,
> 
> I recently implemented basic EOS handling for the libcamerasrc gstreamer element.
> 
> I basically looked at how GstBaseSrc does it and tried to do it similarly.
> 
> I have no idea whether the locking is correct or if this is thread safe but so far it worked without any issues for my purposes.
> 
> You can also now run the following command and receive a working video file:
> 
> gst-launch-1.0 -e libcamerasrc ! videoconvert ! x264enc ! h264parse ! mp4mux ! filesink location=test.mp4
> 
> Looking forward for feedback!
> 
> Cheers,
> 
> Jaslo
> 
>  src/gstreamer/gstlibcamerasrc.cpp | 47 +++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 63d99571..a4681e1e 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -144,6 +144,9 @@ struct _GstLibcameraSrc {
>  	gchar *camera_name;
>  	controls::AfModeEnum auto_focus_mode = controls::AfModeManual;
> 
> +	GstEvent *pending_eos;
> +	int has_pending_eos;

std::atomic<GstEvent *> instead of the above two?


> +
>  	GstLibcameraSrcState *state;
>  	GstLibcameraAllocator *allocator;
>  	GstFlowCombiner *flow_combiner;
> @@ -397,6 +400,21 @@ gst_libcamera_src_task_run(gpointer user_data)
> 
>  	bool doResume = false;
> 
> +	if (g_atomic_int_get(&self->has_pending_eos)) {
> +		g_atomic_int_set(&self->has_pending_eos, FALSE);

Then you could just do

  if (auto *event = self->pending_eos.exchange(nullptr)) {
    ...
  }

> +
> +		GST_OBJECT_LOCK(self);
> +		GstEvent *event = self->pending_eos;
> +		self->pending_eos = NULL;
> +		GST_OBJECT_UNLOCK(self);
> +
> +		for (GstPad *srcpad : state->srcpads_)
> +			gst_pad_push_event(srcpad, gst_event_ref(event));
> +		gst_event_unref(event);
> +
> +		return;
> +	}
> +
>  	/*
>  	 * Create and queue one request. If no buffers are available the
>  	 * function returns -ENOBUFS, which we ignore here as that's not a
> @@ -747,6 +765,32 @@ gst_libcamera_src_change_state(GstElement *element, GstStateChange transition)
>  	return ret;
>  }
> 
> +static gboolean
> +gst_libcamera_src_send_event(GstElement *element, GstEvent *event)
> +{
> +	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);
> +	gboolean ret = FALSE;
> +
> +	switch (GST_EVENT_TYPE(event)) {
> +	case GST_EVENT_EOS: {
> +		GST_OBJECT_LOCK(self);
> +		g_atomic_int_set(&self->has_pending_eos, TRUE);
> +		if (self->pending_eos)
> +			gst_event_unref(self->pending_eos);
> +		self->pending_eos = event;
> +		GST_OBJECT_UNLOCK(self);

And you could do

  if (auto *old_event = self->pending_eos.exchange(event))
    gst_event_unref(old_event);


> +
> +		ret = TRUE;
> +		break;
> +	}
> +	default:
> +		gst_event_unref(event);
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
>  static void
>  gst_libcamera_src_finalize(GObject *object)
>  {
> @@ -779,6 +823,8 @@ gst_libcamera_src_init(GstLibcameraSrc *self)
>  	state->srcpads_.push_back(gst_pad_new_from_template(templ, "src"));
>  	gst_element_add_pad(GST_ELEMENT(self), state->srcpads_.back());
> 
> +	GST_OBJECT_FLAG_SET(self, GST_ELEMENT_FLAG_SOURCE);
> +
>  	/* C-style friend. */
>  	state->src_ = self;
>  	self->state = state;
> @@ -844,6 +890,7 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>  	element_class->request_new_pad = gst_libcamera_src_request_new_pad;
>  	element_class->release_pad = gst_libcamera_src_release_pad;
>  	element_class->change_state = gst_libcamera_src_change_state;
> +	element_class->send_event = gst_libcamera_src_send_event;
> 
>  	gst_element_class_set_metadata(element_class,
>  				       "libcamera Source", "Source/Video",
> --
> 2.42.1


Regards,
Barnabás Pőcze