[{"id":36195,"web_url":"https://patchwork.libcamera.org/comment/36195/","msgid":"<c24843792a37f1a5b26746baa342045afde32b68.camel@collabora.com>","date":"2025-10-10T15:19:57","subject":"Re: [PATCH 2/3] gstreamer: Associate libcamera::Stream with GstPad","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Hi,\n\nLe vendredi 10 octobre 2025 à 19:48 +0530, Umang Jain a écrit :\n> Instead of trying to figure out which libcamera::Stream, the GstPad\n> is configured with (from the libcamera pool), associate the Stream\n> with the GstPad directly using gst_pad_set_element_private().\n> \n> The libcamera::Stream can now be obtained using\n> gst_pad_get_element_private() hence, drop the getter\n> gst_libcamera_pad_get_stream().\n> \n> Additionally, drop retrieving the stream configuration\n> using the same index, as of the srcpads_ vector in processRequest().\n> This can break in future, if CameraConfiguration moves away\n> from vector for storing stream configurations. Hence, retrieve\n> the stream configuration correctly, from GstPad's configured stream.\n> \n> Signed-off-by: Umang Jain <uajain@igalia.com>\n> ---\n>  src/gstreamer/gstlibcamerapad.cpp | 11 -----------\n>  src/gstreamer/gstlibcamerapad.h   |  2 --\n>  src/gstreamer/gstlibcamerasrc.cpp | 10 +++++++---\n>  3 files changed, 7 insertions(+), 16 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp\n> index 22b96719..7855ef22 100644\n> --- a/src/gstreamer/gstlibcamerapad.cpp\n> +++ b/src/gstreamer/gstlibcamerapad.cpp\n> @@ -189,17 +189,6 @@ void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info)\n>  \tself->info = *info;\n>  }\n>  \n> -Stream *\n> -gst_libcamera_pad_get_stream(GstPad *pad)\n> -{\n> -\tauto *self = GST_LIBCAMERA_PAD(pad);\n> -\n> -\tif (self->pool)\n> -\t\treturn gst_libcamera_pool_get_stream(self->pool);\n> -\n> -\treturn nullptr;\n> -}\n> -\n>  void\n>  gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency)\n>  {\n> diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h\n> index f98b8a7f..5f355aa9 100644\n> --- a/src/gstreamer/gstlibcamerapad.h\n> +++ b/src/gstreamer/gstlibcamerapad.h\n> @@ -31,6 +31,4 @@ GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad);\n>  \n>  void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info);\n>  \n> -libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);\n> -\n>  void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency);\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 9ff90d9a..a5dbdfd4 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -195,7 +195,7 @@ int GstLibcameraSrcState::queueRequest()\n>  \t\tstd::make_unique<RequestWrap>(std::move(request));\n>  \n>  \tfor (GstPad *srcpad : srcpads_) {\n> -\t\tStream *stream = gst_libcamera_pad_get_stream(srcpad);\n> +\t\tStream *stream = (Stream *)gst_pad_get_element_private(srcpad);\n\nNo C cast please. I would suggest to keep gst_libcamera_pad_get_stream() (can be\nmade inline) and implement the cast in it instead.\n\n>  \t\tGstLibcameraPool *pool = gst_libcamera_pad_get_pool(srcpad);\n>  \t\tGstBuffer *buffer;\n>  \t\tGstFlowReturn ret;\n> @@ -359,11 +359,11 @@ int GstLibcameraSrcState::processRequest()\n>  \n>  \tfor (gsize i = 0; i < srcpads_.size(); i++) {\n>  \t\tGstPad *srcpad = srcpads_[i];\n> -\t\tStream *stream = gst_libcamera_pad_get_stream(srcpad);\n> +\t\tStream *stream = (Stream *)gst_pad_get_element_private(srcpad);\n>  \t\tGstBuffer *buffer = wrap->detachBuffer(stream);\n>  \n>  \t\tFrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer);\n> -\t\tconst StreamConfiguration &stream_cfg = config_->at(i);\n> +\t\tconst StreamConfiguration &stream_cfg = stream->configuration();\n\nHow can it break in the future ? With that claim, I would expect\ngst_libcamera_src_negotiate() to be impacted. One thing to remember, we don't\ndeny pad request while streaming.\n\n>  \t\tGstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad);\n>  \n>  \t\tif (video_pool) {\n> @@ -698,6 +698,9 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n>  \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n>  \t\tgst_libcamera_pad_set_video_pool(srcpad, video_pool);\n>  \n> +\t\t/* Associate the configured stream with the src pad. */\n> +\t\tgst_pad_set_element_private(srcpad, (gpointer)stream_cfg.stream());\n> +\n>  \t\t/* Clear all reconfigure flags. */\n>  \t\tgst_pad_check_reconfigure(srcpad);\n>  \t}\n> @@ -894,6 +897,7 @@ gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,\n>  \t\tfor (GstPad *srcpad : state->srcpads_) {\n>  \t\t\tgst_libcamera_pad_set_latency(srcpad, GST_CLOCK_TIME_NONE);\n>  \t\t\tgst_libcamera_pad_set_pool(srcpad, nullptr);\n> +\t\t\tgst_pad_set_element_private(srcpad, nullptr);\n>  \t\t}\n>  \t}\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E74C0BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 10 Oct 2025 15:20:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 326376B60F;\n\tFri, 10 Oct 2025 17:20:02 +0200 (CEST)","from bali.collaboradmins.com (bali.collaboradmins.com\n\t[IPv6:2a01:4f8:201:9162::2])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7CA856B599\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 10 Oct 2025 17:20:00 +0200 (CEST)","from [IPv6:2606:6d00:17:ebd3::c41] (unknown\n\t[IPv6:2606:6d00:17:ebd3::c41])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256)\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bali.collaboradmins.com (Postfix) with ESMTPSA id 7533817E02B0;\n\tFri, 10 Oct 2025 17:19:59 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=collabora.com header.i=@collabora.com\n\theader.b=\"aZbuTYFq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1760109599;\n\tbh=pmA8HmWG9/KJHXq7fvuVJFLIjQVjY+13ARShbDQPHxU=;\n\th=Subject:From:To:Date:In-Reply-To:References:From;\n\tb=aZbuTYFqAvUDOtWAcWDf7++twzgqRGiJTOXHddg7COalldcIConojte8inEzc6Y9u\n\tq5C2FDqAoY/yZZRasVxzERSo/zh0PwvUwAc53iPDK24ckYV/95vMpI8aAaDBUSBFJK\n\t4Pn9A4YISjojnYU5dA3mioMJz3GUehqFYDlqyfPlb/8NY48fFXPw0djMDF0roxtNbc\n\tJpb/JqFwqYO6UlrRaXyBz/TOhs9/yMps9ODp2PeHDxV+886WaI7xbn2hYbkC55KGEc\n\t/U3eBN7wOW61HK9rqDSWUqCc+5RBXYOGzL+uPWNk/U3XiaPGvhBOGgVzgCCGSLOIgB\n\thDcGxh/43sVYA==","Message-ID":"<c24843792a37f1a5b26746baa342045afde32b68.camel@collabora.com>","Subject":"Re: [PATCH 2/3] gstreamer: Associate libcamera::Stream with GstPad","From":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","To":"Umang Jain <uajain@igalia.com>, libcamera-devel@lists.libcamera.org","Date":"Fri, 10 Oct 2025 11:19:57 -0400","In-Reply-To":"<20251010141826.42995-3-uajain@igalia.com>","References":"<20251010141826.42995-1-uajain@igalia.com>\n\t<20251010141826.42995-3-uajain@igalia.com>","Autocrypt":"addr=nicolas.dufresne@collabora.com; prefer-encrypt=mutual;\n\tkeydata=mDMEaCN2ixYJKwYBBAHaRw8BAQdAM0EHepTful3JOIzcPv6ekHOenE1u0vDG1gdHFrChD\n\t/e0J05pY29sYXMgRHVmcmVzbmUgPG5pY29sYXNAbmR1ZnJlc25lLmNhPoicBBMWCgBEAhsDBQsJCA\n\tcCAiICBhUKCQgLAgQWAgMBAh4HAheABQkJZfd1FiEE7w1SgRXEw8IaBG8S2UGUUSlgcvQFAmibrjo\n\tCGQEACgkQ2UGUUSlgcvQlQwD/RjpU1SZYcKG6pnfnQ8ivgtTkGDRUJ8gP3fK7+XUjRNIA/iXfhXMN\n\tabIWxO2oCXKf3TdD7aQ4070KO6zSxIcxgNQFtDFOaWNvbGFzIER1ZnJlc25lIDxuaWNvbGFzLmR1Z\n\tnJlc25lQGNvbGxhYm9yYS5jb20+iJkEExYKAEECGwMFCwkIBwICIgIGFQoJCAsCBBYCAwECHgcCF4\n\tAWIQTvDVKBFcTDwhoEbxLZQZRRKWBy9AUCaCyyxgUJCWX3dQAKCRDZQZRRKWBy9ARJAP96pFmLffZ\n\tsmBUpkyVBfFAf+zq6BJt769R0al3kHvUKdgD9G7KAHuioxD2v6SX7idpIazjzx8b8rfzwTWyOQWHC\n\tAAS0LU5pY29sYXMgRHVmcmVzbmUgPG5pY29sYXMuZHVmcmVzbmVAZ21haWwuY29tPoiZBBMWCgBBF\n\tiEE7w1SgRXEw8IaBG8S2UGUUSlgcvQFAmibrGYCGwMFCQll93UFCwkIBwICIgIGFQoJCAsCBBYCAw\n\tECHgcCF4AACgkQ2UGUUSlgcvRObgD/YnQjfi4+L8f4fI7p1pPMTwRTcaRdy6aqkKEmKsCArzQBAK8\n\tbRLv9QjuqsE6oQZra/RB4widZPvphs78H0P6NmpIJ","Organization":"Collabora Canada","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-vqN6EhqO2QDtZSA8KZ25\"","User-Agent":"Evolution 3.56.2 (3.56.2-2.fc42) ","MIME-Version":"1.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36211,"web_url":"https://patchwork.libcamera.org/comment/36211/","msgid":"<26755dqbmm73q6rjjdkphi4surlbpod62czazesgwvrgkqijct@hk2rr74lsef6>","date":"2025-10-13T05:16:47","subject":"Re: [PATCH 2/3] gstreamer: Associate libcamera::Stream with GstPad","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On Fri, Oct 10, 2025 at 11:19:57AM -0400, Nicolas Dufresne wrote:\n> Hi,\n> \n> Le vendredi 10 octobre 2025 à 19:48 +0530, Umang Jain a écrit :\n> > Instead of trying to figure out which libcamera::Stream, the GstPad\n> > is configured with (from the libcamera pool), associate the Stream\n> > with the GstPad directly using gst_pad_set_element_private().\n> > \n> > The libcamera::Stream can now be obtained using\n> > gst_pad_get_element_private() hence, drop the getter\n> > gst_libcamera_pad_get_stream().\n> > \n> > Additionally, drop retrieving the stream configuration\n> > using the same index, as of the srcpads_ vector in processRequest().\n> > This can break in future, if CameraConfiguration moves away\n> > from vector for storing stream configurations. Hence, retrieve\n> > the stream configuration correctly, from GstPad's configured stream.\n> > \n> > Signed-off-by: Umang Jain <uajain@igalia.com>\n> > ---\n> >  src/gstreamer/gstlibcamerapad.cpp | 11 -----------\n> >  src/gstreamer/gstlibcamerapad.h   |  2 --\n> >  src/gstreamer/gstlibcamerasrc.cpp | 10 +++++++---\n> >  3 files changed, 7 insertions(+), 16 deletions(-)\n> > \n> > diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp\n> > index 22b96719..7855ef22 100644\n> > --- a/src/gstreamer/gstlibcamerapad.cpp\n> > +++ b/src/gstreamer/gstlibcamerapad.cpp\n> > @@ -189,17 +189,6 @@ void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info)\n> >  \tself->info = *info;\n> >  }\n> >  \n> > -Stream *\n> > -gst_libcamera_pad_get_stream(GstPad *pad)\n> > -{\n> > -\tauto *self = GST_LIBCAMERA_PAD(pad);\n> > -\n> > -\tif (self->pool)\n> > -\t\treturn gst_libcamera_pool_get_stream(self->pool);\n> > -\n> > -\treturn nullptr;\n> > -}\n> > -\n> >  void\n> >  gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency)\n> >  {\n> > diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h\n> > index f98b8a7f..5f355aa9 100644\n> > --- a/src/gstreamer/gstlibcamerapad.h\n> > +++ b/src/gstreamer/gstlibcamerapad.h\n> > @@ -31,6 +31,4 @@ GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad);\n> >  \n> >  void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info);\n> >  \n> > -libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);\n> > -\n> >  void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency);\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index 9ff90d9a..a5dbdfd4 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -195,7 +195,7 @@ int GstLibcameraSrcState::queueRequest()\n> >  \t\tstd::make_unique<RequestWrap>(std::move(request));\n> >  \n> >  \tfor (GstPad *srcpad : srcpads_) {\n> > -\t\tStream *stream = gst_libcamera_pad_get_stream(srcpad);\n> > +\t\tStream *stream = (Stream *)gst_pad_get_element_private(srcpad);\n> \n> No C cast please. I would suggest to keep gst_libcamera_pad_get_stream() (can be\n> made inline) and implement the cast in it instead.\n> \n> >  \t\tGstLibcameraPool *pool = gst_libcamera_pad_get_pool(srcpad);\n> >  \t\tGstBuffer *buffer;\n> >  \t\tGstFlowReturn ret;\n> > @@ -359,11 +359,11 @@ int GstLibcameraSrcState::processRequest()\n> >  \n> >  \tfor (gsize i = 0; i < srcpads_.size(); i++) {\n> >  \t\tGstPad *srcpad = srcpads_[i];\n> > -\t\tStream *stream = gst_libcamera_pad_get_stream(srcpad);\n> > +\t\tStream *stream = (Stream *)gst_pad_get_element_private(srcpad);\n> >  \t\tGstBuffer *buffer = wrap->detachBuffer(stream);\n> >  \n> >  \t\tFrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer);\n> > -\t\tconst StreamConfiguration &stream_cfg = config_->at(i);\n> > +\t\tconst StreamConfiguration &stream_cfg = stream->configuration();\n> \n> How can it break in the future ? With that claim, I would expect\n> gst_libcamera_src_negotiate() to be impacted. One thing to remember, we don't\n> deny pad request while streaming.\n\nAny direct usage of config_ will break and need to be fixed up, but we\nshould look out for any indirect usages like there. We should be easily get\nthe Stream from pad - at streaming time, without going to config_ again.\n\nI'll address your comment above and adjust the commit message\naccordingly.\n\n\n> \n> >  \t\tGstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad);\n> >  \n> >  \t\tif (video_pool) {\n> > @@ -698,6 +698,9 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n> >  \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> >  \t\tgst_libcamera_pad_set_video_pool(srcpad, video_pool);\n> >  \n> > +\t\t/* Associate the configured stream with the src pad. */\n> > +\t\tgst_pad_set_element_private(srcpad, (gpointer)stream_cfg.stream());\n> > +\n> >  \t\t/* Clear all reconfigure flags. */\n> >  \t\tgst_pad_check_reconfigure(srcpad);\n> >  \t}\n> > @@ -894,6 +897,7 @@ gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,\n> >  \t\tfor (GstPad *srcpad : state->srcpads_) {\n> >  \t\t\tgst_libcamera_pad_set_latency(srcpad, GST_CLOCK_TIME_NONE);\n> >  \t\t\tgst_libcamera_pad_set_pool(srcpad, nullptr);\n> > +\t\t\tgst_pad_set_element_private(srcpad, nullptr);\n> >  \t\t}\n> >  \t}\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 98412BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 13 Oct 2025 05:16:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E2D526045B;\n\tMon, 13 Oct 2025 07:16:40 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A62D603DF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 13 Oct 2025 07:16:37 +0200 (CEST)","from [49.36.127.56] (helo=uajain)\n\tby fanzine2.igalia.com with esmtpsa \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1v8AvD-008nE3-Hc; Mon, 13 Oct 2025 07:16:36 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"rh15d6WF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=In-Reply-To:Content-Transfer-Encoding:Content-Type:MIME-Version\n\t:References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=AT2m81Zxfh+XdR+oqhf73dibbLufkXpvpZqxqoZOCYk=;\n\tb=rh15d6WFExcBRn8JJbaek8Aohg\n\tKngx4scbgdLHGnCVFrsW4HsbeaaZDMP6WgOqOQTuMBD9HqktQxvW4lrtkpMpn7vJHCXLQQ7NqrVk/\n\t1lWgBHY4bQY5/JwjpzUabArFITNCQaUJOIaJeOQ5gPndUiQq65ThTOgJ+qOPoxaQWx1MO8/DjMGWz\n\tW3Al1g5+rHWsq9iIXPwGiUw9MLwr2bu0N7F0aflnIDknjvn7ILxFY5N7P0yJ6qXZ1UPopmS4Fj7bV\n\teopkEi/6VsP9vVuGvc05QLZxgg5tdnDRv6ais1K6QcauB38+paHXV3QGc2uVt3/K75x8O2zIBKUld\n\thj3tq2uw==;","Date":"Mon, 13 Oct 2025 10:46:47 +0530","From":"Umang Jain <uajain@igalia.com>","To":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/3] gstreamer: Associate libcamera::Stream with GstPad","Message-ID":"<26755dqbmm73q6rjjdkphi4surlbpod62czazesgwvrgkqijct@hk2rr74lsef6>","References":"<20251010141826.42995-1-uajain@igalia.com>\n\t<20251010141826.42995-3-uajain@igalia.com>\n\t<c24843792a37f1a5b26746baa342045afde32b68.camel@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<c24843792a37f1a5b26746baa342045afde32b68.camel@collabora.com>","User-Agent":"NeoMutt/20250905-dirty","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36244,"web_url":"https://patchwork.libcamera.org/comment/36244/","msgid":"<6055f633632d4e8923337447299f51cb9669ecd6.camel@collabora.com>","date":"2025-10-14T12:17:19","subject":"Re: [PATCH 2/3] gstreamer: Associate libcamera::Stream with GstPad","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le lundi 13 octobre 2025 à 10:46 +0530, Umang Jain a écrit :\n> On Fri, Oct 10, 2025 at 11:19:57AM -0400, Nicolas Dufresne wrote:\n> > Hi,\n> > \n> > Le vendredi 10 octobre 2025 à 19:48 +0530, Umang Jain a écrit :\n> > > Instead of trying to figure out which libcamera::Stream, the GstPad\n> > > is configured with (from the libcamera pool), associate the Stream\n> > > with the GstPad directly using gst_pad_set_element_private().\n> > > \n> > > The libcamera::Stream can now be obtained using\n> > > gst_pad_get_element_private() hence, drop the getter\n> > > gst_libcamera_pad_get_stream().\n> > > \n> > > Additionally, drop retrieving the stream configuration\n> > > using the same index, as of the srcpads_ vector in processRequest().\n> > > This can break in future, if CameraConfiguration moves away\n> > > from vector for storing stream configurations. Hence, retrieve\n> > > the stream configuration correctly, from GstPad's configured stream.\n> > > \n> > > Signed-off-by: Umang Jain <uajain@igalia.com>\n> > > ---\n> > >  src/gstreamer/gstlibcamerapad.cpp | 11 -----------\n> > >  src/gstreamer/gstlibcamerapad.h   |  2 --\n> > >  src/gstreamer/gstlibcamerasrc.cpp | 10 +++++++---\n> > >  3 files changed, 7 insertions(+), 16 deletions(-)\n> > > \n> > > diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp\n> > > index 22b96719..7855ef22 100644\n> > > --- a/src/gstreamer/gstlibcamerapad.cpp\n> > > +++ b/src/gstreamer/gstlibcamerapad.cpp\n> > > @@ -189,17 +189,6 @@ void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info)\n> > >  \tself->info = *info;\n> > >  }\n> > >  \n> > > -Stream *\n> > > -gst_libcamera_pad_get_stream(GstPad *pad)\n> > > -{\n> > > -\tauto *self = GST_LIBCAMERA_PAD(pad);\n> > > -\n> > > -\tif (self->pool)\n> > > -\t\treturn gst_libcamera_pool_get_stream(self->pool);\n> > > -\n> > > -\treturn nullptr;\n> > > -}\n> > > -\n> > >  void\n> > >  gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency)\n> > >  {\n> > > diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h\n> > > index f98b8a7f..5f355aa9 100644\n> > > --- a/src/gstreamer/gstlibcamerapad.h\n> > > +++ b/src/gstreamer/gstlibcamerapad.h\n> > > @@ -31,6 +31,4 @@ GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad);\n> > >  \n> > >  void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info);\n> > >  \n> > > -libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);\n> > > -\n> > >  void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency);\n> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > > index 9ff90d9a..a5dbdfd4 100644\n> > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > @@ -195,7 +195,7 @@ int GstLibcameraSrcState::queueRequest()\n> > >  \t\tstd::make_unique<RequestWrap>(std::move(request));\n> > >  \n> > >  \tfor (GstPad *srcpad : srcpads_) {\n> > > -\t\tStream *stream = gst_libcamera_pad_get_stream(srcpad);\n> > > +\t\tStream *stream = (Stream *)gst_pad_get_element_private(srcpad);\n> > \n> > No C cast please. I would suggest to keep gst_libcamera_pad_get_stream() (can be\n> > made inline) and implement the cast in it instead.\n> > \n> > >  \t\tGstLibcameraPool *pool = gst_libcamera_pad_get_pool(srcpad);\n> > >  \t\tGstBuffer *buffer;\n> > >  \t\tGstFlowReturn ret;\n> > > @@ -359,11 +359,11 @@ int GstLibcameraSrcState::processRequest()\n> > >  \n> > >  \tfor (gsize i = 0; i < srcpads_.size(); i++) {\n> > >  \t\tGstPad *srcpad = srcpads_[i];\n> > > -\t\tStream *stream = gst_libcamera_pad_get_stream(srcpad);\n> > > +\t\tStream *stream = (Stream *)gst_pad_get_element_private(srcpad);\n> > >  \t\tGstBuffer *buffer = wrap->detachBuffer(stream);\n> > >  \n> > >  \t\tFrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer);\n> > > -\t\tconst StreamConfiguration &stream_cfg = config_->at(i);\n> > > +\t\tconst StreamConfiguration &stream_cfg = stream->configuration();\n> > \n> > How can it break in the future ? With that claim, I would expect\n> > gst_libcamera_src_negotiate() to be impacted. One thing to remember, we don't\n> > deny pad request while streaming.\n> \n> Any direct usage of config_ will break and need to be fixed up, but we\n> should look out for any indirect usages like there. We should be easily get\n> the Stream from pad - at streaming time, without going to config_ again.\n\nPlease notice my typo, we DO deny pad request while streaming. Now, leave it\nalone if you don't have a plan here, since mixing up is going to create\nconfusion. Currently, nothing ever get added or removed at run-time from\nconfig_, and there is not concurrent thread running while doing the setup.\n\nNicolas\n\n> \n> I'll address your comment above and adjust the commit message\n> accordingly.\n> \n> \n> > \n> > >  \t\tGstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad);\n> > >  \n> > >  \t\tif (video_pool) {\n> > > @@ -698,6 +698,9 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n> > >  \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> > >  \t\tgst_libcamera_pad_set_video_pool(srcpad, video_pool);\n> > >  \n> > > +\t\t/* Associate the configured stream with the src pad. */\n> > > +\t\tgst_pad_set_element_private(srcpad, (gpointer)stream_cfg.stream());\n> > > +\n> > >  \t\t/* Clear all reconfigure flags. */\n> > >  \t\tgst_pad_check_reconfigure(srcpad);\n> > >  \t}\n> > > @@ -894,6 +897,7 @@ gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,\n> > >  \t\tfor (GstPad *srcpad : state->srcpads_) {\n> > >  \t\t\tgst_libcamera_pad_set_latency(srcpad, GST_CLOCK_TIME_NONE);\n> > >  \t\t\tgst_libcamera_pad_set_pool(srcpad, nullptr);\n> > > +\t\t\tgst_pad_set_element_private(srcpad, nullptr);\n> > >  \t\t}\n> > >  \t}\n> > >  \n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B4664BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 14 Oct 2025 12:17:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C6D05605F2;\n\tTue, 14 Oct 2025 14:17:23 +0200 (CEST)","from bali.collaboradmins.com (bali.collaboradmins.com\n\t[148.251.105.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9FB40605DF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 14 Oct 2025 14:17:22 +0200 (CEST)","from [IPv6:2606:6d00:17:ebd3::c41] (unknown\n\t[IPv6:2606:6d00:17:ebd3::c41])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256)\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bali.collaboradmins.com (Postfix) with ESMTPSA id B615417E04DA;\n\tTue, 14 Oct 2025 14:17:21 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=collabora.com header.i=@collabora.com\n\theader.b=\"h6g0qW2n\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1760444242;\n\tbh=NoXVQktV3tgPE1nOZjAv+4vPJrEc9pwEhyjSthTryus=;\n\th=Subject:From:To:Cc:Date:In-Reply-To:References:From;\n\tb=h6g0qW2n28hB0E9n1bEZu0Len7jeFxn2DWPnhkQihR0Al/Bf3ZE9JkF5ZG/ZiGtQL\n\tlBrwbgoghy0yZabY8vtf+9f/ukCC3TRbibQF1H3ChuNtUqSQVyrGtTK1gmW7vHfock\n\tyBrXfCrSgtImauK9ih6N0BsDHHA6y0Y4nkJkaDqOHAAAjrk98wKXK0sWM9V2vaHakA\n\tO9tQQwGyWPuRgyKHlP5fa/qyv1BIE7cP8mcvWru7S4OTwFbw6MpREF/1Dvrh0rHMrq\n\tiU6VlrF2aUDVAqzmHpdgECmUKMsQHA4JQje3Tf1kOjGurz+3E8ISaznfDITJtaetfp\n\taXhgb42QMTueA==","Message-ID":"<6055f633632d4e8923337447299f51cb9669ecd6.camel@collabora.com>","Subject":"Re: [PATCH 2/3] gstreamer: Associate libcamera::Stream with GstPad","From":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","To":"Umang Jain <uajain@igalia.com>","Cc":"libcamera-devel@lists.libcamera.org","Date":"Tue, 14 Oct 2025 08:17:19 -0400","In-Reply-To":"<26755dqbmm73q6rjjdkphi4surlbpod62czazesgwvrgkqijct@hk2rr74lsef6>","References":"<20251010141826.42995-1-uajain@igalia.com>\n\t<20251010141826.42995-3-uajain@igalia.com>\n\t<c24843792a37f1a5b26746baa342045afde32b68.camel@collabora.com>\n\t<26755dqbmm73q6rjjdkphi4surlbpod62czazesgwvrgkqijct@hk2rr74lsef6>","Autocrypt":"addr=nicolas.dufresne@collabora.com; prefer-encrypt=mutual;\n\tkeydata=mDMEaCN2ixYJKwYBBAHaRw8BAQdAM0EHepTful3JOIzcPv6ekHOenE1u0vDG1gdHFrChD\n\t/e0J05pY29sYXMgRHVmcmVzbmUgPG5pY29sYXNAbmR1ZnJlc25lLmNhPoicBBMWCgBEAhsDBQsJCA\n\tcCAiICBhUKCQgLAgQWAgMBAh4HAheABQkJZfd1FiEE7w1SgRXEw8IaBG8S2UGUUSlgcvQFAmibrjo\n\tCGQEACgkQ2UGUUSlgcvQlQwD/RjpU1SZYcKG6pnfnQ8ivgtTkGDRUJ8gP3fK7+XUjRNIA/iXfhXMN\n\tabIWxO2oCXKf3TdD7aQ4070KO6zSxIcxgNQFtDFOaWNvbGFzIER1ZnJlc25lIDxuaWNvbGFzLmR1Z\n\tnJlc25lQGNvbGxhYm9yYS5jb20+iJkEExYKAEECGwMFCwkIBwICIgIGFQoJCAsCBBYCAwECHgcCF4\n\tAWIQTvDVKBFcTDwhoEbxLZQZRRKWBy9AUCaCyyxgUJCWX3dQAKCRDZQZRRKWBy9ARJAP96pFmLffZ\n\tsmBUpkyVBfFAf+zq6BJt769R0al3kHvUKdgD9G7KAHuioxD2v6SX7idpIazjzx8b8rfzwTWyOQWHC\n\tAAS0LU5pY29sYXMgRHVmcmVzbmUgPG5pY29sYXMuZHVmcmVzbmVAZ21haWwuY29tPoiZBBMWCgBBF\n\tiEE7w1SgRXEw8IaBG8S2UGUUSlgcvQFAmibrGYCGwMFCQll93UFCwkIBwICIgIGFQoJCAsCBBYCAw\n\tECHgcCF4AACgkQ2UGUUSlgcvRObgD/YnQjfi4+L8f4fI7p1pPMTwRTcaRdy6aqkKEmKsCArzQBAK8\n\tbRLv9QjuqsE6oQZra/RB4widZPvphs78H0P6NmpIJ","Organization":"Collabora Canada","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-ZD8dDdbVFfPV+voqz9yQ\"","User-Agent":"Evolution 3.56.2 (3.56.2-2.fc42) ","MIME-Version":"1.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]