[libcamera-devel] [PATCH v2] gstreamer: Fix usage of default size for fixation

[Kieran Bingham]

On 26/08/2021 15:16, Nicolas Dufresne wrote:
> From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> 
> Pipeline managers sets a default value to StreamConfiguration::size. 

If you're happy, I'd change this to

"Pipeline handlers provide a default StreamConfiguration when given a
StreamRole."

Eitherway, we can merge this now I think.
--
Kieran

> The
> original fixation code was attempting to use it, but as it was truncating
> the caps to its first structure it would never actually find a best match.
> 
> In this patch, instead of truncating, we weight various matches using the
> product of the width and height delta. We also split delta from ranges
> apart and prefer fixed size over them as ranges are not reliable.
> 
> This patch also removes the related todo, as it seems that libcamera core
> won't go further then providing this default value and won't be sorting the
> format and size lists.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> Tested-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/gstreamer/gstlibcamera-utils.cpp | 51 +++++++++++++++++++++++++---
>  src/gstreamer/gstlibcamerasrc.cpp    |  4 ---
>  2 files changed, 47 insertions(+), 8 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index 61370d5f..723c4fa3 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -136,14 +136,57 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
>  					 GstCaps *caps)
>  {
>  	GstVideoFormat gst_format = pixel_format_to_gst_format(stream_cfg.pixelFormat);
> +	guint i;
> +	gint best_fixed = -1, best_in_range = -1;
> +	GstStructure *s;
> +
> +	/*
> +	 * These are delta weight computed from:
> +	 *   ABS(width - stream_cfg.size.width) * ABS(height - stream_cfg.size.height)
> +	 */
> +	guint best_fixed_delta = G_MAXUINT;
> +	guint best_in_range_delta = G_MAXUINT;
>  
>  	/* First fixate the caps using default configuration value. */
>  	g_assert(gst_caps_is_writable(caps));
> -	caps = gst_caps_truncate(caps);
> -	GstStructure *s = gst_caps_get_structure(caps, 0);
>  
> -	gst_structure_fixate_field_nearest_int(s, "width", stream_cfg.size.width);
> -	gst_structure_fixate_field_nearest_int(s, "height", stream_cfg.size.height);
> +	/* Lookup the structure for a close match to the stream_cfg.size */
> +	for (i = 0; i < gst_caps_get_size(caps); i++) {
> +		s = gst_caps_get_structure(caps, i);
> +		gint width, height;
> +		guint delta;
> +
> +		if (gst_structure_has_field_typed(s, "width", G_TYPE_INT) &&
> +		    gst_structure_has_field_typed(s, "height", G_TYPE_INT)) {
> +			gst_structure_get_int(s, "width", &width);
> +			gst_structure_get_int(s, "height", &height);
> +
> +			delta = ABS(width - (gint)stream_cfg.size.width) * ABS(height - (gint)stream_cfg.size.height);
> +
> +			if (delta < best_fixed_delta) {
> +				best_fixed_delta = delta;
> +				best_fixed = i;
> +			}
> +		} else {
> +			gst_structure_fixate_field_nearest_int(s, "width", stream_cfg.size.width);
> +			gst_structure_fixate_field_nearest_int(s, "height", stream_cfg.size.height);
> +			gst_structure_get_int(s, "width", &width);
> +			gst_structure_get_int(s, "height", &height);
> +
> +			delta = ABS(width - (gint)stream_cfg.size.width) * ABS(height - (gint)stream_cfg.size.height);
> +
> +			if (delta < best_in_range_delta) {
> +				best_in_range_delta = delta;
> +				best_in_range = i;
> +			}
> +		}
> +	}
> +
> +	/* Prefer reliable fixed value over ranges */
> +	if (best_fixed >= 0)
> +		s = gst_caps_get_structure(caps, best_fixed);
> +	else
> +		s = gst_caps_get_structure(caps, best_in_range);
>  
>  	if (gst_structure_has_name(s, "video/x-raw")) {
>  		const gchar *format = gst_video_format_to_string(gst_format);
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index b243aeb3..4c7b36ae 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -25,10 +25,6 @@
>   *  - Add timestamp support
>   *  - Use unique names to select the camera devices
>   *  - Add GstVideoMeta support (strides and offsets)
> - *
> - * \todo libcamera UVC drivers picks the lowest possible resolution first, this
> - * should be fixed so that we get a decent resolution and framerate for the
> - * role by default.
>   */
>  
>  #include "gstlibcamerasrc.h"
>