[PATCH v2] gstreamer: Fix critical warning "gst_value_set_int_range_step: assertion 'start < end' failed"

[Jacopo Mondi]

Hi Hou

On Thu, Jul 25, 2024 at 06:26:22PM GMT, Hou Qi wrote:
> This changes is fixing critical error message
> "gst_value_set_int_range_step: assertion 'start < end' failed" observed
> when building GStreamer caps from a stream configuration whose size
> range holds a single size.
>
> GStreamer range step definition requires distinct min and max values
> definitions, otherwise above error message is output.
>
> libcamera SizeRange instance may return a single size leading to
> identical min and max values. Add a conditional check where the min and
> max of the range are distinct during iterating the supported sizes for
> each pixelformat.
>
> Signed-off-by: Hou Qi <qi.hou at nxp.com>
> ---
>  src/gstreamer/gstlibcamera-utils.cpp | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index ec4da435..79f71246 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -359,13 +359,21 @@ gst_libcamera_stream_formats_to_caps(const StreamFormats &formats)
>  			GValue val = G_VALUE_INIT;
>
>  			g_value_init(&val, GST_TYPE_INT_RANGE);
> -			gst_value_set_int_range_step(&val, range.min.width, range.max.width, range.hStep);
> -			gst_structure_set_value(s, "width", &val);
> -			gst_value_set_int_range_step(&val, range.min.height, range.max.height, range.vStep);
> -			gst_structure_set_value(s, "height", &val);
> +			if (range.min.width == range.max.width) {
> +				gst_structure_set(s, "width", G_TYPE_INT, range.min.width, nullptr);
> +			} else {
> +				gst_value_set_int_range_step(&val, range.min.width, range.max.width, range.hStep);
> +				gst_structure_set_value(s, "width", &val);
> +			}
> +			if (range.min.height == range.max.height) {
> +				gst_structure_set(s, "height", G_TYPE_INT, range.min.height, nullptr);
> +			} else {
> +				gst_value_set_int_range_step(&val, range.min.height, range.max.height, range.vStep);
> +				gst_structure_set_value(s, "height", &val);
> +			}
>  			g_value_unset(&val);
>
> -			gst_caps_append_structure(caps, s);
> +			caps = gst_caps_merge_structure(caps, s);

The patch seems logically correct, but I defer the call on the
gstreamer api usage to people with more knwoledge in that area.

With the commit message updated as Kieran's suggested
Acked-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

>  		}
>  	}
>
> --
> 2.34.1
>