[libcamera-devel] [PATCH v1] gstreamer: Added virtual functions needed to support request pads

[Nicolas Dufresne]

p.s. Add RFC in front of PATCH for in progress work.

Le lundi 07 juin 2021 à 22:31 +0530, Vedant Paranjape a écrit :
> Using request pads needs virtual functions to create new request pads and
> release the allocated pads.
> 
> It defines the functions gst_libcamera_src_request_new_pad() and
> gst_libcamera_src_release_pad() which handle, creating and releasing
> request pads. It also assigned these functions to their callback
> handlers in GstLibcameraSrcClass.
> 
> This doesn't enable request pad support in gstreamer element. This is
> one of the series of changes needed to make it work.
> 
> Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 53 +++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index ccc61590..f0a4fbf0 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -640,6 +640,57 @@ gst_libcamera_src_init(GstLibcameraSrc *self)
>  	self->state = state;
>  }
>  
> +static GstPad*
> +gst_libcamera_src_request_new_pad(GstElement *element, GstPadTemplate *templ,
> +					const gchar *name, [[maybe_unused]] const GstCaps *caps)
> +{
> +	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);
> +	g_autoptr(GstPad) pad = NULL;
> +
> +	pad = gst_pad_new_from_template(templ, name);
> +	if (G_IS_INITIALLY_UNOWNED (pad))

This is not needed, as it's always true for pads.

> +	{
> +		g_object_ref_sink(pad);
> +	}
> +	
> +	{
> +		GLibLocker lock(GST_OBJECT(self->state));
> +		self->state->srcpads_.push_back((GstPad*)g_object_ref(&pad));

Oops, & in front of pad will send a GstPad** to object_ref, the memory will
likely be corrupted after that.

> +	}
> +
> +	if (!gst_element_add_pad(element, pad))
> +	{
> +		GST_ELEMENT_ERROR (element, STREAM, FAILED,
> +			("Internal data stream error."), ("Could not add pad to element"));
> +		{
> +			GLibLocker lock(GST_OBJECT(self->state));
> +			self->state->srcpads_.pop_back();

This virtual function could be called concurrently, so you cannot assume that
it's the first one.

> +		}
> +		return NULL;		
> +	}
> +
> +	return (GstPad*)g_steal_pointer(&pad);

Use reinterpret_cast.

> +}
> +
> +static void
> +gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)
> +{
> +	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);
> +	
> +	GST_DEBUG_OBJECT (self, "Pad %" GST_PTR_FORMAT " being released", pad);
> +
> +	{
> +		GLibLocker lock(GST_OBJECT(self->state));
> +		auto iterator = std::find(self->state->srcpads_.begin(), self->state->srcpads_.end(), pad);
> +		if (iterator != self->state->srcpads_.end())

There is in C++ nicer way to iterate vectors. Perhaps have a look at other code.

> +		{
> +			g_object_unref(self->state->srcpads_[std::distance(self->state->srcpads_.begin(), iterator)]);
> +			self->state->srcpads_.erase(iterator);
> +		}
> +	}
> +	gst_element_remove_pad(element, pad);
> +}
> +
>  static void
>  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>  {
> @@ -650,6 +701,8 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>  	object_class->get_property = gst_libcamera_src_get_property;
>  	object_class->finalize = gst_libcamera_src_finalize;
>  
> +	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;
>  
>  	gst_element_class_set_metadata(element_class,