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

[Umang Jain]

Hi Laurent,

On 6/30/22 05:49, Laurent Pinchart wrote:
> 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.


Ah yes, that's correct. I didn't come across request_new_pad() earlier.

>
>> 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.


Submitting v1.1 with commit message change.

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