[PATCH i-g-t v4 0/2] Analogue/VGA frame comparison support

[PATCH i-g-t v4 0/2] Analogue/VGA frame comparison support

[Paul Kocialkowski]

Changes since v3:
* Squashed configure.ac changes in this series

Changes since v2:
* Changed analogue in favor of analog
* Integrated analog frame match fixup in the original commit
* Rebased on top of the new revisions of the series this depends on

Changes since v1:
* Split analogue frame comparison to igt_frame, using cairo surfaces
* Added a chamelium helper for analogue frame checking and frame dumping

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v4 1/2] lib/igt_frame: Add support for analog frame comparison testing

[Paul Kocialkowski]

This adds support for analog frame comparison check, as used in VGA.
Since VGA uses a DAC-ADC chain, its data cannot be expected to be pixel
perfect. Thus, it is impossible to uses a CRC check and full frames have
to be analyzed instead. Such an analysis is implemented, based on both
an absolute error threshold and a correlation with the expected error
trend for a DAC-ADC chain. It was tested with a couple encoders and
provides reliable error detection with few false positives.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 configure.ac         |   2 +
 lib/Makefile.am      |   9 ++++
 lib/Makefile.sources |   2 -
 lib/igt_frame.c      | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_frame.h      |   2 +
 5 files changed, 144 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7655df1c..7ea768e2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -181,6 +181,8 @@ PKG_CHECK_MODULES(GLIB, [glib-2.0], [glib=yes], [glib=no])
 if test x"$glib" = xyes; then
 	AC_DEFINE(HAVE_GLIB,1,[Enable glib support])
 fi
+PKG_CHECK_MODULES(GSL, [gsl], [gsl=yes], [gsl=no])
+AM_CONDITIONAL(HAVE_GSL, [test "x$gsl" = xyes])
 
 # for chamelium
 AC_ARG_ENABLE(chamelium, AS_HELP_STRING([--disable-chamelium],
diff --git a/lib/Makefile.am b/lib/Makefile.am
index d4f41128..9c932d6f 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -29,12 +29,20 @@ lib_source_list +=	 	\
 	$(NULL)
 endif
 
+if HAVE_GSL
+lib_source_list +=		\
+	igt_frame.c		\
+	igt_frame.h		\
+	$(NULL)
+endif
+
 AM_CPPFLAGS = -I$(top_srcdir)
 AM_CFLAGS = \
 	    $(CWARNFLAGS) \
 	    $(DRM_CFLAGS) \
 	    $(PCIACCESS_CFLAGS) \
 	    $(LIBUNWIND_CFLAGS) \
+	    $(GSL_CFLAGS) \
 	    $(KMOD_CFLAGS) \
 	    $(PROCPS_CFLAGS) \
 	    $(DEBUG_CFLAGS) \
@@ -54,6 +62,7 @@ libintel_tools_la_LIBADD = \
 	$(DRM_LIBS) \
 	$(PCIACCESS_LIBS) \
 	$(PROCPS_LIBS) \
+	$(GSL_LIBS) \
 	$(KMOD_LIBS) \
 	$(CAIRO_LIBS) \
 	$(LIBUDEV_LIBS) \
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index c2e58809..53fdb54c 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -83,8 +83,6 @@ lib_source_list =	 	\
 	uwildmat/uwildmat.c	\
 	igt_kmod.c		\
 	igt_kmod.h		\
-	igt_frame.c		\
-	igt_frame.h		\
 	$(NULL)
 
 .PHONY: version.h.tmp
diff --git a/lib/igt_frame.c b/lib/igt_frame.c
index dfafe53d..222a45f8 100644
--- a/lib/igt_frame.c
+++ b/lib/igt_frame.c
@@ -29,6 +29,8 @@
 #include <fcntl.h>
 #include <pixman.h>
 #include <cairo.h>
+#include <gsl/gsl_statistics_double.h>
+#include <gsl/gsl_fit.h>
 
 #include "igt.h"
 
@@ -135,3 +137,132 @@ void igt_write_compared_frames_to_png(cairo_surface_t *reference,
 
 	close(fd);
 }
+
+/**
+ * igt_check_analog_frame_match:
+ * @reference: The reference cairo surface
+ * @capture: The captured cairo surface
+ *
+ * Checks that the analog image contained in the chamelium frame dump matches
+ * the given framebuffer.
+ *
+ * In order to determine whether the frame matches the reference, the following
+ * reasoning is implemented:
+ * 1. The absolute error for each color value of the reference is collected.
+ * 2. The average absolute error is calculated for each color value of the
+ *    reference and must not go above 60 (23.5 % of the total range).
+ * 3. A linear fit for the average absolute error from the pixel value is
+ *    calculated, as a DAC-ADC chain is expected to have a linear error curve.
+ * 4. The linear fit is correlated with the actual average absolute error for
+ *    the frame and the correlation coefficient is checked to be > 0.985,
+ *    indicating a match with the expected error trend.
+ *
+ * Most errors (e.g. due to scaling, rotation, color space, etc) can be
+ * reliably detected this way, with a minimized number of false-positives.
+ * However, the brightest values (250 and up) are ignored as the error trend
+ * is often not linear there in practice due to clamping.
+ *
+ * Returns: a boolean indicating whether the frames match
+ */
+
+bool igt_check_analog_frame_match(cairo_surface_t *reference,
+				  cairo_surface_t *capture)
+{
+	pixman_image_t *reference_src, *capture_src;
+	int w, h;
+	int error_count[3][256][2] = { 0 };
+	double error_average[4][250];
+	double error_trend[250];
+	double c0, c1, cov00, cov01, cov11, sumsq;
+	double correlation;
+	unsigned char *reference_pixels, *capture_pixels;
+	unsigned char *p;
+	unsigned char *q;
+	bool match = true;
+	int diff;
+	int x, y;
+	int i, j;
+
+	w = cairo_image_surface_get_width(reference);
+	h = cairo_image_surface_get_height(reference);
+
+	reference_src = pixman_image_create_bits(
+	    PIXMAN_x8r8g8b8, w, h,
+	    (void*)cairo_image_surface_get_data(reference),
+	    cairo_image_surface_get_stride(reference));
+	reference_pixels = (unsigned char *) pixman_image_get_data(reference_src);
+
+	capture_src = pixman_image_create_bits(
+	    PIXMAN_x8r8g8b8, w, h,
+	    (void*)cairo_image_surface_get_data(capture),
+	    cairo_image_surface_get_stride(capture));
+	capture_pixels = (unsigned char *) pixman_image_get_data(capture_src);
+
+	/* Collect the absolute error for each color value */
+	for (x = 0; x < w; x++) {
+		for (y = 0; y < h; y++) {
+			p = &capture_pixels[(x + y * w) * 4];
+			q = &reference_pixels[(x + y * w) * 4];
+
+			for (i = 0; i < 3; i++) {
+				diff = (int) p[i] - q[i];
+				if (diff < 0)
+					diff = -diff;
+
+				error_count[i][q[i]][0] += diff;
+				error_count[i][q[i]][1]++;
+			}
+		}
+	}
+
+	/* Calculate the average absolute error for each color value */
+	for (i = 0; i < 250; i++) {
+		error_average[0][i] = i;
+
+		for (j = 1; j < 4; j++) {
+			error_average[j][i] = (double) error_count[j-1][i][0] /
+					      error_count[j-1][i][1];
+
+			if (error_average[j][i] > 60) {
+				igt_warn("Error average too high (%f)\n",
+					 error_average[j][i]);
+
+				match = false;
+				goto complete;
+			}
+		}
+	}
+
+	/*
+	 * Calculate error trend from linear fit.
+	 * A DAC-ADC chain is expected to have a linear absolute error on
+	 * most of its range
+	 */
+	for (i = 1; i < 4; i++) {
+		gsl_fit_linear((const double *) &error_average[0], 1,
+			       (const double *) &error_average[i], 1, 250,
+			       &c0, &c1, &cov00, &cov01, &cov11, &sumsq);
+
+		for (j = 0; j < 250; j++)
+			error_trend[j] = c0 + j * c1;
+
+		correlation = gsl_stats_correlation((const double *) &error_trend,
+						    1,
+						    (const double *) &error_average[i],
+						    1, 250);
+
+		if (correlation < 0.985) {
+			igt_warn("Error with reference not correlated (%f)\n",
+				 correlation);
+
+			match = false;
+			goto complete;
+		}
+	}
+
+complete:
+	pixman_image_unref(reference_src);
+	pixman_image_unref(capture_src);
+
+	return match;
+}
diff --git a/lib/igt_frame.h b/lib/igt_frame.h
index ec6a1643..1d9bbf6a 100644
--- a/lib/igt_frame.h
+++ b/lib/igt_frame.h
@@ -39,5 +39,7 @@ void igt_write_compared_frames_to_png(cairo_surface_t *reference,
 				      cairo_surface_t *capture,
 				      const char *reference_suffix,
 				      const char *capture_suffix);
+bool igt_check_analog_frame_match(cairo_surface_t *reference,
+				  cairo_surface_t *capture);
 
 #endif
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t v4 2/2] chamelium: Add support for VGA frame comparison testing

[Paul Kocialkowski]

This adds support for VGA frame comparison testing with the reference
generated from cairo. The retrieved frame from the chamelium is first
cropped, as it contains the blanking intervals, through a dedicated
helper. Another helper function asserts that the analog frame
matches or dump it to png if not.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 configure.ac        |   4 ++
 lib/igt_chamelium.c | 164 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 lib/igt_chamelium.h |   7 ++-
 tests/chamelium.c   |  57 ++++++++++++++++++
 4 files changed, 226 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7ea768e2..dec3e923 100644
--- a/configure.ac
+++ b/configure.ac
@@ -197,6 +197,10 @@ if test "x$enable_chamelium" = xyes; then
 	if test x"$glib" != xyes; then
 		AC_MSG_ERROR([Failed to find glib, required by chamelium. Use --disable-chamelium to disable chamelium support.])
 	fi
+	if test x"$gsl" != xyes; then
+		AC_MSG_ERROR([Failed to find gsl, required by chamelium. Use --disable-chamelium to disable chamelium support.])
+	fi
+
 	AC_DEFINE(HAVE_CHAMELIUM, 1, [Enable Chamelium support])
 fi
 
diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 348d2176..dcd8855f 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -937,6 +937,8 @@ static cairo_surface_t *convert_frame_dump_argb32(const struct chamelium_frame_d
 	int w = dump->width, h = dump->height;
 	uint32_t *bits_bgr = (uint32_t *) dump->bgr;
 	unsigned char *bits_argb;
+	unsigned char *bits_target;
+	int size;
 
 	image_bgr = pixman_image_create_bits(
 	    PIXMAN_b8g8r8, w, h, bits_bgr,
@@ -946,9 +948,13 @@ static cairo_surface_t *convert_frame_dump_argb32(const struct chamelium_frame_d
 
 	bits_argb = (unsigned char *) pixman_image_get_data(image_argb);
 
-	dump_surface = cairo_image_surface_create_for_data(
-	    bits_argb, CAIRO_FORMAT_ARGB32, w, h,
-	    PIXMAN_FORMAT_BPP(PIXMAN_x8r8g8b8) / 8 * w);
+	dump_surface = cairo_image_surface_create(
+	    CAIRO_FORMAT_ARGB32, w, h);
+
+	bits_target = cairo_image_surface_get_data(dump_surface);
+	size = cairo_image_surface_get_stride(dump_surface) * h;
+	memcpy(bits_target, bits_argb, size);
+	cairo_surface_mark_dirty(dump_surface);
 
 	pixman_image_unref(image_argb);
 
@@ -1054,6 +1060,154 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium *chamelium,
 }
 
 /**
+ * chamelium_assert_analog_frame_match_or_dump:
+ * @chamelium: The chamelium instance the frame dump belongs to
+ * @frame: The chamelium frame dump to match
+ * @fb: pointer to an #igt_fb structure
+ *
+ * Asserts that the provided captured frame matches the reference frame from
+ * the framebuffer. If they do not, this saves the reference and captured frames
+ * to a png file.
+ */
+void chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
+						 struct chamelium_port *port,
+						 const struct chamelium_frame_dump *frame,
+						 struct igt_fb *fb)
+{
+	cairo_surface_t *reference;
+	cairo_surface_t *capture;
+	igt_crc_t *reference_crc;
+	igt_crc_t *capture_crc;
+	char *reference_suffix;
+	char *capture_suffix;
+	bool match;
+
+	/* Grab the reference frame from framebuffer */
+	reference = igt_get_cairo_surface(chamelium->drm_fd, fb);
+
+	/* Grab the captured frame from chamelium */
+	capture = convert_frame_dump_argb32(frame);
+
+	match = igt_check_analog_frame_match(reference, capture);
+	if (!match && igt_frame_dump_is_enabled()) {
+		reference_crc = chamelium_calculate_fb_crc(chamelium->drm_fd,
+							   fb);
+		capture_crc = chamelium_get_crc_for_area(chamelium, port, 0, 0,
+							 0, 0);
+
+		reference_suffix = igt_crc_to_string_extended(reference_crc,
+							      '-', 2);
+		capture_suffix = igt_crc_to_string_extended(capture_crc, '-',
+							    2);
+
+		/* Write reference and capture frames to png */
+		igt_write_compared_frames_to_png(reference, capture,
+						 reference_suffix,
+						 capture_suffix);
+
+		free(reference_suffix);
+		free(capture_suffix);
+	}
+
+	cairo_surface_destroy(capture);
+
+	igt_assert(match);
+}
+
+
+/**
+ * chamelium_analog_frame_crop:
+ * @chamelium: The Chamelium instance to use
+ * @dump: The chamelium frame dump to crop
+ * @width: The cropped frame width
+ * @height: The cropped frame height
+ *
+ * Detects the corners of a chamelium frame and crops it to the requested
+ * width/height. This is useful for VGA frame dumps that also contain the
+ * pixels dumped during the blanking intervals.
+ *
+ * The detection is done on a brightness-threshold-basis, that is adapted
+ * to the reference frame used by i-g-t. It may not be as relevant for other
+ * frames.
+ */
+void chamelium_crop_analog_frame(struct chamelium_frame_dump *dump, int width,
+				 int height)
+{
+	unsigned char *bgr;
+	unsigned char *p;
+	unsigned char *q;
+	int top, left;
+	int x, y, xx, yy;
+	int score;
+
+	if (dump->width == width && dump->height == height)
+		return;
+
+	/* Start with the most bottom-right position. */
+	top = dump->height - height;
+	left = dump->width - width;
+
+	igt_assert(top >= 0 && left >= 0);
+
+	igt_debug("Cropping analog frame from %dx%d to %dx%d\n", dump->width,
+		  dump->height, width, height);
+
+	/* Detect the top-left corner of the frame. */
+	for (x = 0; x < dump->width; x++) {
+		for (y = 0; y < dump->height; y++) {
+			p = &dump->bgr[(x + y * dump->width) * 3];
+
+			/* Detect significantly bright pixels. */
+			if (p[0] < 50 && p[1] < 50 && p[2] < 50)
+				continue;
+
+			/*
+			 * Make sure close-by pixels are also significantly
+			 * bright.
+			 */
+			score = 0;
+			for (xx = x; xx < x + 10; xx++) {
+				for (yy = y; yy < y + 10; yy++) {
+					p = &dump->bgr[(xx + yy * dump->width) * 3];
+
+					if (p[0] > 50 && p[1] > 50 && p[2] > 50)
+						score++;
+				}
+			}
+
+			/* Not enough pixels are significantly bright. */
+			if (score < 25)
+				continue;
+
+			if (x < left)
+				left = x;
+
+			if (y < top)
+				top = y;
+
+			if (left == x || top == y)
+				continue;
+		}
+	}
+
+	igt_debug("Detected analog frame edges at %dx%d\n", left, top);
+
+	/* Crop the frame given the detected top-left corner. */
+	bgr = malloc(width * height * 3);
+
+	for (y = 0; y < height; y++) {
+		p = &dump->bgr[(left + (top + y) * dump->width) * 3];
+		q = &bgr[(y * width) * 3];
+		memcpy(q, p, width * 3);
+	}
+
+	free(dump->bgr);
+	dump->width = width;
+	dump->height = height;
+	dump->bgr = bgr;
+}
+
+/**
  * chamelium_get_frame_limit:
  * @chamelium: The Chamelium instance to use
  * @port: The port to check the frame limit on
@@ -1480,7 +1634,7 @@ igt_constructor {
 	/* Frame dumps can be large, so we need to be able to handle very large
 	 * responses
 	 *
-	 * Limit here is 10MB
+	 * Limit here is 15MB
 	 */
-	xmlrpc_limit_set(XMLRPC_XML_SIZE_LIMIT_ID, 10485760);
+	xmlrpc_limit_set(XMLRPC_XML_SIZE_LIMIT_ID, 15728640);
 }
diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
index 80afcafa..2a0fa234 100644
--- a/lib/igt_chamelium.h
+++ b/lib/igt_chamelium.h
@@ -101,7 +101,6 @@ int chamelium_get_captured_frame_count(struct chamelium *chamelium);
 int chamelium_get_frame_limit(struct chamelium *chamelium,
 			      struct chamelium_port *port,
 			      int w, int h);
-
 void chamelium_assert_frame_eq(const struct chamelium *chamelium,
 			       const struct chamelium_frame_dump *dump,
 			       struct igt_fb *fb);
@@ -109,6 +108,12 @@ void chamelium_assert_crc_eq_or_dump(struct chamelium *chamelium,
 				     igt_crc_t *reference_crc,
 				     igt_crc_t *capture_crc, struct igt_fb *fb,
 				     int index);
+void chamelium_assert_analog_frame_match_or_dump(struct chamelium *chamelium,
+						 struct chamelium_port *port,
+						 const struct chamelium_frame_dump *frame,
+						 struct igt_fb *fb);
+void chamelium_crop_analog_frame(struct chamelium_frame_dump *dump, int width,
+				 int height);
 void chamelium_destroy_frame_dump(struct chamelium_frame_dump *dump);
 
 #endif /* IGT_CHAMELIUM_H */
diff --git a/tests/chamelium.c b/tests/chamelium.c
index 34448152..33ecc2e7 100644
--- a/tests/chamelium.c
+++ b/tests/chamelium.c
@@ -399,6 +399,10 @@ enable_output(data_t *data,
 					    BROADCAST_RGB_FULL);
 
 	igt_display_commit(display);
+
+	if (chamelium_port_get_type(port) == DRM_MODE_CONNECTOR_VGA)
+		usleep(250000);
+
 	chamelium_port_wait_video_input_stable(data->chamelium, port,
 					       HOTPLUG_TIMEOUT);
 
@@ -533,6 +537,56 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
 }
 
 static void
+test_analog_frame_dump(data_t *data, struct chamelium_port *port)
+{
+	igt_display_t display;
+	igt_output_t *output;
+	igt_plane_t *primary;
+	struct igt_fb fb;
+	struct chamelium_frame_dump *frame;
+	drmModeModeInfo *mode;
+	drmModeConnector *connector;
+	int fb_id, i;
+
+	output = prepare_output(data, &display, port);
+	connector = chamelium_port_get_connector(data->chamelium, port, false);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_assert(primary);
+
+	for (i = 0; i < connector->count_modes; i++) {
+		mode = &connector->modes[i];
+
+		fb_id = igt_create_color_pattern_fb(data->drm_fd,
+						    mode->hdisplay, mode->vdisplay,
+						    DRM_FORMAT_XRGB8888,
+						    LOCAL_DRM_FORMAT_MOD_NONE,
+						    0, 0, 0, &fb);
+		igt_assert(fb_id > 0);
+
+		enable_output(data, port, output, mode, &fb);
+
+		igt_debug("Reading frame dumps from Chamelium...\n");
+
+		frame = chamelium_port_dump_pixels(data->chamelium, port, 0, 0,
+						   0, 0);
+
+		chamelium_crop_analog_frame(frame, mode->hdisplay,
+					    mode->vdisplay);
+
+		chamelium_assert_analog_frame_match_or_dump(data->chamelium,
+							    port, frame, &fb);
+
+		chamelium_destroy_frame_dump(frame);
+
+		disable_output(data, port, output);
+		igt_remove_fb(data->drm_fd, &fb);
+	}
+
+	drmModeFreeConnector(connector);
+	igt_display_fini(&display);
+}
+
+static void
 test_hpd_without_ddc(data_t *data, struct chamelium_port *port)
 {
 	struct udev_monitor *mon = igt_watch_hotplug();
@@ -775,6 +829,9 @@ igt_main
 
 		connector_subtest("vga-hpd-without-ddc", VGA)
 			test_hpd_without_ddc(&data, port);
+
+		connector_subtest("vga-frame-dump", VGA)
+			test_analog_frame_dump(&data, port);
 	}
 
 	igt_subtest_group {
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v4 0/2] Analogue/VGA frame comparison support

[Lyude Paul]

For the whole series:

Reviewed-by: Lyude <lyude@redhat.com>

I've also just pushed them, cheers!

On Thu, 2017-07-20 at 18:13 +0300, Paul Kocialkowski wrote:
> Changes since v3:
> * Squashed configure.ac changes in this series
> 
> Changes since v2:
> * Changed analogue in favor of analog
> * Integrated analog frame match fixup in the original commit
> * Rebased on top of the new revisions of the series this depends on
> 
> Changes since v1:
> * Split analogue frame comparison to igt_frame, using cairo surfaces
> * Added a chamelium helper for analogue frame checking and frame
> dumping
> 
-- 
Cheers,
	Lyude
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx