gstreamer: Fix critical warning "gst_value_set_int_range_step: assertion 'start < end' failed"
diff mbox series

Message ID 20240627012204.1949820-1-qi.hou@nxp.com
State New
Headers show
Series
  • gstreamer: Fix critical warning "gst_value_set_int_range_step: assertion 'start < end' failed"
Related show

Commit Message

Qi Hou June 27, 2024, 1:22 a.m. UTC
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 define a single size leading to
identical min and max values. Add a test to avoid building GStreamer
range step from a single-size SizeRange to avoid such error.

Signed-off-by: Hou Qi <qi.hou@nxp.com>
---
 src/gstreamer/gstlibcamera-utils.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Umang Jain June 29, 2024, 6:29 a.m. UTC | #1
Hi Hou Qi,

Thank you for the patch.

On 27/06/24 6:52 am, 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.

Ah, seems legit.
> Libcamera SizeRange instance may define a single size leading to

s/Libcamera/libcamera

s/may define/may return/
> identical min and max values. Add a test to avoid building GStreamer

This can be rephrased better.

Add a conditional check where the min and max of the range are distinct 
during iterating the supported sizes for each pixelformat.

> range step from a single-size SizeRange to avoid such error.
>
> Signed-off-by: Hou Qi <qi.hou@nxp.com>

I can take care of the commit message while applying the patch,


Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>   src/gstreamer/gstlibcamera-utils.cpp | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index ec4da435..b6672b9f 100644
> --- 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;
>
Qi Hou July 1, 2024, 1:11 a.m. UTC | #2
Hi Umang Jain,

Thanks for your review.

Regards,
Qi Hou

-----Original Message-----
From: Umang Jain <umang.jain@ideasonboard.com> 
Sent: 2024年6月29日 14:29
To: Qi Hou <qi.hou@nxp.com>; libcamera-devel@lists.libcamera.org
Cc: Jared Hu <jared.hu@nxp.com>; Julien Vuillaumier <julien.vuillaumier@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 Hou Qi,

Thank you for the patch.

On 27/06/24 6:52 am, 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.

Ah, seems legit.
> Libcamera SizeRange instance may define a single size leading to

s/Libcamera/libcamera

s/may define/may return/
> identical min and max values. Add a test to avoid building GStreamer

This can be rephrased better.

Add a conditional check where the min and max of the range are distinct during iterating the supported sizes for each pixelformat.

> range step from a single-size SizeRange to avoid such error.
>
> Signed-off-by: Hou Qi <qi.hou@nxp.com>

I can take care of the commit message while applying the patch,


Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>   src/gstreamer/gstlibcamera-utils.cpp | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp 
> b/src/gstreamer/gstlibcamera-utils.cpp
> index ec4da435..b6672b9f 100644
> --- 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;
>

Patch
diff mbox series

diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
index ec4da435..b6672b9f 100644
--- 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;