[v3] gstreamer: Read correct colorimetry from caps into the stream configuration
diff mbox series

Message ID 20241217022422.4054343-1-qi.hou@nxp.com
State New
Headers show
Series
  • [v3] gstreamer: Read correct colorimetry from caps into the stream configuration
Related show

Commit Message

Hou Qi Dec. 17, 2024, 2:24 a.m. UTC
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(-)

Comments

Nicolas Dufresne Dec. 17, 2024, 6:37 p.m. UTC | #1
Hi,

The object should perhaps be:

gstreamer: Fixate colorimetry field during caps negotiation

Le mardi 17 décembre 2024 à 11:24 +0900, Hou Qi a écrit :
> 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>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  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);
>
Laurent Pinchart Dec. 17, 2024, 7:40 p.m. UTC | #2
Hi Hou,

Thank you for the patch.

On Tue, Dec 17, 2024 at 11:24:22AM +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>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  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);
>

Patch
diff mbox series

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);