[2/3] gstreamer: Associate libcamera::Stream with GstPad

24583 diff mbox series

Instead of trying to figure out which libcamera::Stream, the GstPad
is configured with (from the libcamera pool), associate the Stream
with the GstPad directly using gst_pad_set_element_private().

The libcamera::Stream can now be obtained using
gst_pad_get_element_private() hence, drop the getter
gst_libcamera_pad_get_stream().

Additionally, drop retrieving the stream configuration
using the same index, as of the srcpads_ vector in processRequest().
This can break in future, if CameraConfiguration moves away
from vector for storing stream configurations. Hence, retrieve
the stream configuration correctly, from GstPad's configured stream.

Signed-off-by: Umang Jain <uajain@igalia.com>
---
 src/gstreamer/gstlibcamerapad.cpp | 11 -----------
 src/gstreamer/gstlibcamerapad.h   |  2 --
 src/gstreamer/gstlibcamerasrc.cpp | 10 +++++++---
 3 files changed, 7 insertions(+), 16 deletions(-)

Hi,

Le vendredi 10 octobre 2025 à 19:48 +0530, Umang Jain a écrit :
> Instead of trying to figure out which libcamera::Stream, the GstPad
> is configured with (from the libcamera pool), associate the Stream
> with the GstPad directly using gst_pad_set_element_private().
> 
> The libcamera::Stream can now be obtained using
> gst_pad_get_element_private() hence, drop the getter
> gst_libcamera_pad_get_stream().
> 
> Additionally, drop retrieving the stream configuration
> using the same index, as of the srcpads_ vector in processRequest().
> This can break in future, if CameraConfiguration moves away
> from vector for storing stream configurations. Hence, retrieve
> the stream configuration correctly, from GstPad's configured stream.
> 
> Signed-off-by: Umang Jain <uajain@igalia.com>
> ---
>  src/gstreamer/gstlibcamerapad.cpp | 11 -----------
>  src/gstreamer/gstlibcamerapad.h   |  2 --
>  src/gstreamer/gstlibcamerasrc.cpp | 10 +++++++---
>  3 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp
> index 22b96719..7855ef22 100644
> --- a/src/gstreamer/gstlibcamerapad.cpp
> +++ b/src/gstreamer/gstlibcamerapad.cpp
> @@ -189,17 +189,6 @@ void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info)
>  	self->info = *info;
>  }
>  
> -Stream *
> -gst_libcamera_pad_get_stream(GstPad *pad)
> -{
> -	auto *self = GST_LIBCAMERA_PAD(pad);
> -
> -	if (self->pool)
> -		return gst_libcamera_pool_get_stream(self->pool);
> -
> -	return nullptr;
> -}
> -
>  void
>  gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency)
>  {
> diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h
> index f98b8a7f..5f355aa9 100644
> --- a/src/gstreamer/gstlibcamerapad.h
> +++ b/src/gstreamer/gstlibcamerapad.h
> @@ -31,6 +31,4 @@ GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad);
>  
>  void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info);
>  
> -libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);
> -
>  void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency);
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 9ff90d9a..a5dbdfd4 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -195,7 +195,7 @@ int GstLibcameraSrcState::queueRequest()
>  		std::make_unique<RequestWrap>(std::move(request));
>  
>  	for (GstPad *srcpad : srcpads_) {
> -		Stream *stream = gst_libcamera_pad_get_stream(srcpad);
> +		Stream *stream = (Stream *)gst_pad_get_element_private(srcpad);

No C cast please. I would suggest to keep gst_libcamera_pad_get_stream() (can be
made inline) and implement the cast in it instead.

>  		GstLibcameraPool *pool = gst_libcamera_pad_get_pool(srcpad);
>  		GstBuffer *buffer;
>  		GstFlowReturn ret;
> @@ -359,11 +359,11 @@ int GstLibcameraSrcState::processRequest()
>  
>  	for (gsize i = 0; i < srcpads_.size(); i++) {
>  		GstPad *srcpad = srcpads_[i];
> -		Stream *stream = gst_libcamera_pad_get_stream(srcpad);
> +		Stream *stream = (Stream *)gst_pad_get_element_private(srcpad);
>  		GstBuffer *buffer = wrap->detachBuffer(stream);
>  
>  		FrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer);
> -		const StreamConfiguration &stream_cfg = config_->at(i);
> +		const StreamConfiguration &stream_cfg = stream->configuration();

How can it break in the future ? With that claim, I would expect
gst_libcamera_src_negotiate() to be impacted. One thing to remember, we don't
deny pad request while streaming.

>  		GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad);
>  
>  		if (video_pool) {
> @@ -698,6 +698,9 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
>  		gst_libcamera_pad_set_pool(srcpad, pool);
>  		gst_libcamera_pad_set_video_pool(srcpad, video_pool);
>  
> +		/* Associate the configured stream with the src pad. */
> +		gst_pad_set_element_private(srcpad, (gpointer)stream_cfg.stream());
> +
>  		/* Clear all reconfigure flags. */
>  		gst_pad_check_reconfigure(srcpad);
>  	}
> @@ -894,6 +897,7 @@ gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,
>  		for (GstPad *srcpad : state->srcpads_) {
>  			gst_libcamera_pad_set_latency(srcpad, GST_CLOCK_TIME_NONE);
>  			gst_libcamera_pad_set_pool(srcpad, nullptr);
> +			gst_pad_set_element_private(srcpad, nullptr);
>  		}
>  	}
>