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

[Nicolas Dufresne]

Le jeudi 26 août 2021 à 15:33 +0100, Kieran Bingham a écrit :
> 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."

That is fine with me. 

> 
> 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"
> >