[v2] gstreamer: Reconfigure when peer caps gain extra fields
diff mbox series

Message ID 20260818031046.2791348-1-elliot.chen@oss.nxp.com
State Superseded
Headers show
Series
  • [v2] gstreamer: Reconfigure when peer caps gain extra fields
Related show

Commit Message

Elliot Chen (OSS) Aug. 18, 2026, 3:10 a.m. UTC 
From: Elliot Chen <elliot.chen@nxp.com>

After initial negotiation, a downstream element may append extra
fields to the caps (e.g. colorimetry). The existing check only
handles empty peer caps. Add a structure comparison to detect such
changes and trigger reconfiguration with caps renegotiation.

Signed-off-by: Elliot Chen <elliot.chen@nxp.com>

---
Changes in v2:
- remove redundant checks both for caps and peercaps

Changes in v1:
- Initial submission
---
 src/gstreamer/gstlibcamerasrc.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Barnabás Pőcze Aug. 18, 2026, 8:35 a.m. UTC | #1 
Hi

2026. 08. 18. 5:10 keltezéssel, elliot.chen@oss.nxp.com írta:
> From: Elliot Chen <elliot.chen@nxp.com>
> 
> After initial negotiation, a downstream element may append extra
> fields to the caps (e.g. colorimetry). The existing check only
> handles empty peer caps. Add a structure comparison to detect such
> changes and trigger reconfiguration with caps renegotiation.
> 
> Signed-off-by: Elliot Chen <elliot.chen@nxp.com>
> 
> ---
> Changes in v2:
> - remove redundant checks both for caps and peercaps
> 
> Changes in v1:
> - Initial submission
> ---
>   src/gstreamer/gstlibcamerasrc.cpp | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 9061f9163..24a4293a5 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -744,6 +744,18 @@ gst_libcamera_src_task_run(gpointer user_data)
>   				reconfigure = true;
>   				break;
>   			}
> +
> +			/*
> +			 * Caps may gain extra fields after the initial negotiation (e.g.
> +			 * colorimetry added by a downstream element). Detect such changes
> +			 * and trigger reconfiguration.
> +			 */
> +			const GstStructure *CapsStruct = gst_caps_get_structure(caps, 0);

CapsStruct -> capsStruct

Or I feel like I might just call them `caps0` and `peercaps0`.


> +			const GstStructure *peerCapsStruct = gst_caps_get_structure(peercaps, 0);

Do I understand it correctly that these `GstCaps` will only have a 1 structure each?
So only index 0 needs to be checked?


> +			if (!gst_structure_is_equal(CapsStruct, peerCapsStruct)) {
> +				reconfigure = true;
> +				break;
> +			}
>   		}
>   	}
>
Elliot Chen Aug. 18, 2026, 8:56 a.m. UTC | #2 
Hi Barnabás Pőcze,
1. Yes. I'll update the name in the patch.
2. peercaps may not only have one structure.  I think we only need to check the first one of them.


NXP Confidential
-----Original Message-----
From: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Sent: Tuesday, August 18, 2026 4:35 PM
To: Elliot Chen (OSS) <elliot.chen@oss.nxp.com>; libcamera-devel@lists.libcamera.org
Cc: Elliot Chen <elliot.chen@nxp.com>
Subject: [EXT] Re: [PATCH v2] gstreamer: Reconfigure when peer caps gain extra fields

[You don't often get email from barnabas.pocze@ideasonboard.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button


Hi

2026. 08. 18. 5:10 keltezéssel, elliot.chen@oss.nxp.com írta:
> From: Elliot Chen <elliot.chen@nxp.com>
>
> After initial negotiation, a downstream element may append extra
> fields to the caps (e.g. colorimetry). The existing check only handles
> empty peer caps. Add a structure comparison to detect such changes and
> trigger reconfiguration with caps renegotiation.
>
> Signed-off-by: Elliot Chen <elliot.chen@nxp.com>
>
> ---
> Changes in v2:
> - remove redundant checks both for caps and peercaps
>
> Changes in v1:
> - Initial submission
> ---
>   src/gstreamer/gstlibcamerasrc.cpp | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp
> b/src/gstreamer/gstlibcamerasrc.cpp
> index 9061f9163..24a4293a5 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -744,6 +744,18 @@ gst_libcamera_src_task_run(gpointer user_data)
>                               reconfigure = true;
>                               break;
>                       }
> +
> +                     /*
> +                      * Caps may gain extra fields after the initial negotiation (e.g.
> +                      * colorimetry added by a downstream element). Detect such changes
> +                      * and trigger reconfiguration.
> +                      */
> +                     const GstStructure *CapsStruct =
> + gst_caps_get_structure(caps, 0);

CapsStruct -> capsStruct

Or I feel like I might just call them `caps0` and `peercaps0`.


> +                     const GstStructure *peerCapsStruct =
> + gst_caps_get_structure(peercaps, 0);

Do I understand it correctly that these `GstCaps` will only have a 1 structure each?
So only index 0 needs to be checked?


> +                     if (!gst_structure_is_equal(CapsStruct, peerCapsStruct)) {
> +                             reconfigure = true;
> +                             break;
> +                     }
>               }
>       }
>
Jacopo Mondi Aug. 18, 2026, 9:48 a.m. UTC | #3 
Hi Elliot,


On Tue, Aug 18, 2026 at 08:56:43AM +0000, Elliot Chen wrote:
> Hi Barnabás Pőcze,
> 1. Yes. I'll update the name in the patch.
> 2. peercaps may not only have one structure.  I think we only need to check the first one of them.
>
>
> NXP Confidential
> -----Original Message-----
> From: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> Sent: Tuesday, August 18, 2026 4:35 PM
> To: Elliot Chen (OSS) <elliot.chen@oss.nxp.com>; libcamera-devel@lists.libcamera.org
> Cc: Elliot Chen <elliot.chen@nxp.com>
> Subject: [EXT] Re: [PATCH v2] gstreamer: Reconfigure when peer caps gain extra fields
>
> [You don't often get email from barnabas.pocze@ideasonboard.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
>
>
> Hi
>
> 2026. 08. 18. 5:10 keltezéssel, elliot.chen@oss.nxp.com írta:
> > From: Elliot Chen <elliot.chen@nxp.com>
> >
> > After initial negotiation, a downstream element may append extra
> > fields to the caps (e.g. colorimetry). The existing check only handles
> > empty peer caps. Add a structure comparison to detect such changes and
> > trigger reconfiguration with caps renegotiation.

could you please add the following tag in your next version ?

Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/344

> >
> > Signed-off-by: Elliot Chen <elliot.chen@nxp.com>

Thanks
  j

> >
> > ---
> > Changes in v2:
> > - remove redundant checks both for caps and peercaps
> >
> > Changes in v1:
> > - Initial submission
> > ---
> >   src/gstreamer/gstlibcamerasrc.cpp | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> >
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp
> > b/src/gstreamer/gstlibcamerasrc.cpp
> > index 9061f9163..24a4293a5 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -744,6 +744,18 @@ gst_libcamera_src_task_run(gpointer user_data)
> >                               reconfigure = true;
> >                               break;
> >                       }
> > +
> > +                     /*
> > +                      * Caps may gain extra fields after the initial negotiation (e.g.
> > +                      * colorimetry added by a downstream element). Detect such changes
> > +                      * and trigger reconfiguration.
> > +                      */
> > +                     const GstStructure *CapsStruct =
> > + gst_caps_get_structure(caps, 0);
>
> CapsStruct -> capsStruct
>
> Or I feel like I might just call them `caps0` and `peercaps0`.
>
>
> > +                     const GstStructure *peerCapsStruct =
> > + gst_caps_get_structure(peercaps, 0);
>
> Do I understand it correctly that these `GstCaps` will only have a 1 structure each?
> So only index 0 needs to be checked?
>
>
> > +                     if (!gst_structure_is_equal(CapsStruct, peerCapsStruct)) {
> > +                             reconfigure = true;
> > +                             break;
> > +                     }
> >               }
> >       }
> >
>
Elliot Chen (OSS) Aug. 18, 2026, 9:53 a.m. UTC | #4 
Hi Jacopo Mondi,
I submitted patch v3 a few minutes ago, link: https://patchwork.libcamera.org/patch/27836/. I'll add this part for the next version.


NXP Confidential
-----Original Message-----
From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Sent: Tuesday, August 18, 2026 5:49 PM
To: Elliot Chen <elliot.chen@nxp.com>
Cc: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>; Elliot Chen (OSS) <elliot.chen@oss.nxp.com>; libcamera-devel@lists.libcamera.org
Subject: Re: [EXT] Re: [PATCH v2] gstreamer: Reconfigure when peer caps gain extra fields

[You don't often get email from jacopo.mondi@ideasonboard.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

Hi Elliot,


On Tue, Aug 18, 2026 at 08:56:43AM +0000, Elliot Chen wrote:
> Hi Barnabás Pőcze,
> 1. Yes. I'll update the name in the patch.
> 2. peercaps may not only have one structure.  I think we only need to check the first one of them.
>
>
> NXP Confidential
> -----Original Message-----
> From: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> Sent: Tuesday, August 18, 2026 4:35 PM
> To: Elliot Chen (OSS) <elliot.chen@oss.nxp.com>;
> libcamera-devel@lists.libcamera.org
> Cc: Elliot Chen <elliot.chen@nxp.com>
> Subject: [EXT] Re: [PATCH v2] gstreamer: Reconfigure when peer caps
> gain extra fields
>
> [You don't often get email from barnabas.pocze@ideasonboard.com. Learn
> why this is important at https://aka.ms/LearnAboutSenderIdentification
> ]
>
> Caution: This is an external email. Please take care when clicking
> links or opening attachments. When in doubt, report the message using
> the 'Report this email' button
>
>
> Hi
>
> 2026. 08. 18. 5:10 keltezéssel, elliot.chen@oss.nxp.com írta:
> > From: Elliot Chen <elliot.chen@nxp.com>
> >
> > After initial negotiation, a downstream element may append extra
> > fields to the caps (e.g. colorimetry). The existing check only
> > handles empty peer caps. Add a structure comparison to detect such
> > changes and trigger reconfiguration with caps renegotiation.

could you please add the following tag in your next version ?

Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/344

> >
> > Signed-off-by: Elliot Chen <elliot.chen@nxp.com>

Thanks
  j

> >
> > ---
> > Changes in v2:
> > - remove redundant checks both for caps and peercaps
> >
> > Changes in v1:
> > - Initial submission
> > ---
> >   src/gstreamer/gstlibcamerasrc.cpp | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> >
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp
> > b/src/gstreamer/gstlibcamerasrc.cpp
> > index 9061f9163..24a4293a5 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -744,6 +744,18 @@ gst_libcamera_src_task_run(gpointer user_data)
> >                               reconfigure = true;
> >                               break;
> >                       }
> > +
> > +                     /*
> > +                      * Caps may gain extra fields after the initial negotiation (e.g.
> > +                      * colorimetry added by a downstream element). Detect such changes
> > +                      * and trigger reconfiguration.
> > +                      */
> > +                     const GstStructure *CapsStruct =
> > + gst_caps_get_structure(caps, 0);
>
> CapsStruct -> capsStruct
>
> Or I feel like I might just call them `caps0` and `peercaps0`.
>
>
> > +                     const GstStructure *peerCapsStruct =
> > + gst_caps_get_structure(peercaps, 0);
>
> Do I understand it correctly that these `GstCaps` will only have a 1 structure each?
> So only index 0 needs to be checked?
>
>
> > +                     if (!gst_structure_is_equal(CapsStruct, peerCapsStruct)) {
> > +                             reconfigure = true;
> > +                             break;
> > +                     }
> >               }
> >       }
> >
>

Patch
diff mbox series 

diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index 9061f9163..24a4293a5 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -744,6 +744,18 @@  gst_libcamera_src_task_run(gpointer user_data)
 				reconfigure = true;
 				break;
 			}
+
+			/*
+			 * Caps may gain extra fields after the initial negotiation (e.g.
+			 * colorimetry added by a downstream element). Detect such changes
+			 * and trigger reconfiguration.
+			 */
+			const GstStructure *CapsStruct = gst_caps_get_structure(caps, 0);
+			const GstStructure *peerCapsStruct = gst_caps_get_structure(peercaps, 0);
+			if (!gst_structure_is_equal(CapsStruct, peerCapsStruct)) {
+				reconfigure = true;
+				break;
+			}
 		}
 	}