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

[Qi Hou]

Hi @Olivier Crête,

Thank you very much for pointing out the corner case. I refined the patch based on your kind suggestion and it works.
But what should I do next? To resend a review mail or continue using this mail?

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


Regards,
Qi Hou

-----Original Message-----
From: Olivier Crête <olivier.crete at collabora.com> 
Sent: 2024年7月5日 0:04
To: Qi Hou <qi.hou at nxp.com>; libcamera-devel at lists.libcamera.org
Cc: Jared Hu <jared.hu at nxp.com>; Julien Vuillaumier <julien.vuillaumier at nxp.com>
Subject: [EXT] Re: [PATCH] gstreamer: Fix critical warning "gst_value_set_int_range_step: assertion 'start < end' failed"

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,

On Thu, 2024-06-27 at 10:22 +0900, Hou Qi wrote:
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -354,7 +354,7 @@ gst_libcamera_stream_formats_to_caps(const StreamFormats &formats)
>               }
>
>               const SizeRange &range = formats.range(pixelformat);
> -             if (range.hStep && range.vStep) {
> +             if (range.hStep && range.vStep && range.min != 
> + range.max) {
>                       GstStructure *s = gst_structure_copy(bare_s);
>                       GValue val = G_VALUE_INIT;

What if range.min.width == range.max.widht, but range.min.height != range.max.height ?

I think a more foolproof implementation is:

g_value_init(&val, GST_TYPE_INT_RANGE);
if (range.min.width == range.max.width) {
  gst_structure_set_int (s, "width", G_TYPE_INT, range.min.width); } 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_int (s, "height", G_TYPE_INT, range.min.height); } else {
  gst_value_set_int_range_step(&val, range.min.height, range.max.height, range.vStep);
  gst_structure_set_value(s, "height", &val); }

gst_caps_merge_structure(caps, s); // Using merge to avoid duplicating existing structures from size)



--
Olivier Crête
olivier.crete at collabora.com
Multimedia Lead