| Message ID | 20241218072025.2932205-1-qi.hou@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Hou, What is the difference between v3 and v4 ? When posting a new version, please include a local changelog below the --- line. See below. On Wed, Dec 18, 2024 at 04:20:25PM +0900, Hou Qi wrote: > When libcamerasrc is negotiating with downstream element, it first > extracts colorimetry field from downstream supported caps, then set > this colorimetry to its stream configuration and propagates the > colorimetry downstream. > > Currently libamerasrc only considers the case there is one colorimetry > in colorimetry field of downstream caps. But the issue is that > downstream caps may report a list of supported colorimetry, which > causes libcamerasrc to set unknown colorimetry to stream configuration > and negotiate fail with downstream element. > > In order to fix the issue, need to fixate colorimetry field before > getting colorimetry string. > > Signed-off-by: Hou Qi <qi.hou@nxp.com> Please remember to pick the tags you have previously received (Reviewed-by, Tested-by, Acked-by, ...) and include them in new versions. > --- Here you can add --- Changes since v3: - ... - ... --- There's no need to resubmit this patch, I'll merge it with the tags from v3. > src/gstreamer/gstlibcamera-utils.cpp | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > index a466b305..7b34c759 100644 > --- a/src/gstreamer/gstlibcamera-utils.cpp > +++ b/src/gstreamer/gstlibcamera-utils.cpp > @@ -493,9 +493,12 @@ void gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg, > > /* Configure colorimetry */ > if (gst_structure_has_field(s, "colorimetry")) { > - const gchar *colorimetry_str = gst_structure_get_string(s, "colorimetry"); > + const gchar *colorimetry_str; > GstVideoColorimetry colorimetry; > > + gst_structure_fixate_field(s, "colorimetry"); > + colorimetry_str = gst_structure_get_string(s, "colorimetry"); > + > if (!gst_video_colorimetry_from_string(&colorimetry, colorimetry_str)) > g_critical("Invalid colorimetry %s", colorimetry_str); >
Hi Laurent Pinchart, Sorry for replying to this mail so late as I missed it. I will pick the tags and include the changelog between new and old version next time. There is no code difference between v3 and v4. I just changed the commit title from " gstreamer: Read correct colorimetry from caps into the stream configuration" to " gstreamer: Fixate colorimetry field during caps negotiation". Please help merge it. Thank you so much. Regards, Qi Hou -----Original Message----- From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Sent: 2024年12月18日 20:05 To: Qi Hou <qi.hou@nxp.com> Cc: libcamera-devel@lists.libcamera.org; Jared Hu <jared.hu@nxp.com>; Julien Vuillaumier <julien.vuillaumier@nxp.com> Subject: [EXT] Re: [PATCH v4] gstreamer: Fixate colorimetry field during caps negotiation 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 Hou, What is the difference between v3 and v4 ? When posting a new version, please include a local changelog below the --- line. See below. On Wed, Dec 18, 2024 at 04:20:25PM +0900, Hou Qi wrote: > When libcamerasrc is negotiating with downstream element, it first > extracts colorimetry field from downstream supported caps, then set > this colorimetry to its stream configuration and propagates the > colorimetry downstream. > > Currently libamerasrc only considers the case there is one colorimetry > in colorimetry field of downstream caps. But the issue is that > downstream caps may report a list of supported colorimetry, which > causes libcamerasrc to set unknown colorimetry to stream configuration > and negotiate fail with downstream element. > > In order to fix the issue, need to fixate colorimetry field before > getting colorimetry string. > > Signed-off-by: Hou Qi <qi.hou@nxp.com> Please remember to pick the tags you have previously received (Reviewed-by, Tested-by, Acked-by, ...) and include them in new versions. > --- Here you can add --- Changes since v3: - ... - ... --- There's no need to resubmit this patch, I'll merge it with the tags from v3. > src/gstreamer/gstlibcamera-utils.cpp | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp > b/src/gstreamer/gstlibcamera-utils.cpp > index a466b305..7b34c759 100644 > --- a/src/gstreamer/gstlibcamera-utils.cpp > +++ b/src/gstreamer/gstlibcamera-utils.cpp > @@ -493,9 +493,12 @@ void > gst_libcamera_configure_stream_from_caps(StreamConfiguration > &stream_cfg, > > /* Configure colorimetry */ > if (gst_structure_has_field(s, "colorimetry")) { > - const gchar *colorimetry_str = gst_structure_get_string(s, "colorimetry"); > + const gchar *colorimetry_str; > GstVideoColorimetry colorimetry; > > + gst_structure_fixate_field(s, "colorimetry"); > + colorimetry_str = gst_structure_get_string(s, > + "colorimetry"); > + > if (!gst_video_colorimetry_from_string(&colorimetry, colorimetry_str)) > g_critical("Invalid colorimetry %s", > colorimetry_str); > -- Regards, Laurent Pinchart
diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp index a466b305..7b34c759 100644 --- a/src/gstreamer/gstlibcamera-utils.cpp +++ b/src/gstreamer/gstlibcamera-utils.cpp @@ -493,9 +493,12 @@ void gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg, /* Configure colorimetry */ if (gst_structure_has_field(s, "colorimetry")) { - const gchar *colorimetry_str = gst_structure_get_string(s, "colorimetry"); + const gchar *colorimetry_str; GstVideoColorimetry colorimetry; + gst_structure_fixate_field(s, "colorimetry"); + colorimetry_str = gst_structure_get_string(s, "colorimetry"); + if (!gst_video_colorimetry_from_string(&colorimetry, colorimetry_str)) g_critical("Invalid colorimetry %s", colorimetry_str);
When libcamerasrc is negotiating with downstream element, it first extracts colorimetry field from downstream supported caps, then set this colorimetry to its stream configuration and propagates the colorimetry downstream. Currently libamerasrc only considers the case there is one colorimetry in colorimetry field of downstream caps. But the issue is that downstream caps may report a list of supported colorimetry, which causes libcamerasrc to set unknown colorimetry to stream configuration and negotiate fail with downstream element. In order to fix the issue, need to fixate colorimetry field before getting colorimetry string. Signed-off-by: Hou Qi <qi.hou@nxp.com> --- src/gstreamer/gstlibcamera-utils.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)