[libcamera-devel] [PATCH] gstreamer: Be pedantic on srcpads access

Laurent Pinchart laurent.pinchart at ideasonboard.com
Thu Jun 30 02:19:47 CEST 2022


Hi Umang,

Thank you for the patch.

On Wed, Jun 29, 2022 at 06:25:51PM +0530, Umang Jain via libcamera-devel wrote:
> While the "src" pad is added to the element, it is accessed
> via a index number. If multiple pads are added(in future)
> and tracked in state->srcpads_, the index might need re-adjusting.

Don't we already support multiple pads ? I don't foresee
libcamera_src_init() adding more than one pad, the other ones should be
added dynamically by gst_libcamera_src_request_new_pad() as far as I
understand.

> Use the std::vector::back() instead of index, which corresponds
> to std::vector::push_back() for tracking of pads. It also slightly
> helps with readability.
> 
> No functional changes intended.
> 
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 46fd02d2..4813ab96 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -631,7 +631,7 @@ gst_libcamera_src_init(GstLibcameraSrc *self)
>  	gst_task_set_lock(self->task, &self->stream_lock);
>  
>  	state->srcpads_.push_back(gst_pad_new_from_template(templ, "src"));
> -	gst_element_add_pad(GST_ELEMENT(self), state->srcpads_[0]);
> +	gst_element_add_pad(GST_ELEMENT(self), state->srcpads_.back());

I'm fine with the code change, but the commit message should be reworded
if my understanding is right and this function will never add more than
one pad.

>  
>  	/* C-style friend. */
>  	state->src_ = self;

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list