Re: [igt-dev] [PATCH i-g-t v3 18/21] lib/igt_frame: Add a checkerboard frame comparison method

From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
To: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Petri Latvala <petri.latvala@intel.com>,
	Eben Upton <eben@raspberrypi.org>,
	igt-dev@lists.freedesktop.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [igt-dev] [PATCH i-g-t v3 18/21] lib/igt_frame: Add a checkerboard frame comparison method
Date: Mon, 14 Jan 2019 16:04:16 +0100	[thread overview]
Message-ID: <332105994b936f46264e21fe582aeb0567362374.camel@bootlin.com> (raw)
In-Reply-To: <20190111151928.4tym2fgfk4osc4rb@flea>

Hi,

On Fri, 2019-01-11 at 16:19 +0100, Maxime Ripard wrote:
> On Fri, Jan 11, 2019 at 10:05:29AM +0100, Paul Kocialkowski wrote:
> > This introduces a new frame comparison method that was designed for
> > patterns that follow a checkerboard style. These patterns are made of
> > consecutive rectangular shapes with alternating solid colors. They are
> > currently used for some Chamelium-based tests.
> > 
> > The method is particularly adapted for cases where the edges of the
> > shapes might be blurred (e.g. due to scaling), which makes it impossible
> > to use pixel-perfect or CRC-based comparisons to decide whether the
> > captured frame matches the reference.
> > 
> > Overall, this test will first detect the edges of the pattern and later
> > exclude them from comparison. Colors are compared between the reference
> > and capture with a low threshold for error. A percentage of the faulty
> > pixels is calculated and the captured frame is considered invalid if
> > more than one percent of the pixels are erroneous.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  lib/igt_frame.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/igt_frame.h |   2 +
> >  2 files changed, 123 insertions(+)
> > 
> > diff --git a/lib/igt_frame.c b/lib/igt_frame.c
> > index 6984c02e9912..e0215660ea6e 100644
> > --- a/lib/igt_frame.c
> > +++ b/lib/igt_frame.c
> > @@ -267,3 +267,124 @@ complete:
> >  
> >  	return match;
> >  }
> > +
> > +#define XR24_COLOR_VALUE(data, stride, x, y, c) \
> > +	*((uint8_t *)(data) + (y) * (stride) + 4 * (x) + (c))
> > +
> > +/**
> > + * igt_check_checkerboard_frame_match:
> > + * @reference: The reference cairo surface
> > + * @capture: The captured cairo surface
> > + *
> > + * Checks that the reference frame matches the captured frame using a
> > + * method designed for checkerboard patterns. These patterns are made of
> > + * consecutive rectangular shapes with alternating solid colors.
> > + *
> > + * The intent of this method is to cover cases where the captured result is
> > + * pixel-perfect due to features such as scaling or YUV conversion and
> 
> is *not* pixel perfect?

Woops, good catch!

[...]

> > diff --git a/lib/igt_frame.h b/lib/igt_frame.h
> > index 11f96cbea203..f44f57d7ce73 100644
> > --- a/lib/igt_frame.h
> > +++ b/lib/igt_frame.h
> > @@ -38,5 +38,7 @@ void igt_write_compared_frames_to_png(cairo_surface_t *reference,
> >  				      const char *capture_suffix);
> >  bool igt_check_analog_frame_match(cairo_surface_t *reference,
> >  				  cairo_surface_t *capture);
> > +bool igt_check_checkerboard_frame_match(cairo_surface_t *reference,
> > +					cairo_surface_t *capture);
> 
> Looks good otherwise. I guess it's missing some comments about the
> algorithm being used though.

I tried to include comments where I felt was necessary, but might have
missed out on some aspects. Or maybe you meant that the documentation
block does not provide enough detail about the algorithm itself (as
opposed to what it does)?

> Maxmie

Cheers,

Pual

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev