From: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
To: Lyude Paul <lyude@redhat.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH i-g-t v2 2/2] chamelium: Add support for VGA frame comparison testing
Date: Wed, 19 Jul 2017 15:31:40 +0300 [thread overview]
Message-ID: <1500467500.1329.9.camel@linux.intel.com> (raw)
In-Reply-To: <1500312945.4421.8.camel@redhat.com>
On Mon, 2017-07-17 at 13:35 -0400, Lyude Paul wrote:
> Just one change for this patch below
>
> On Wed, 2017-07-12 at 17:57 +0300, Paul Kocialkowski wrote:
> > 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 analogue frame
> > matches or dump it to png if not.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> > ---
> > lib/igt_chamelium.c | 164
> > ++++++++++++++++++++++++++++++++++++++++++++++++++--
> > lib/igt_chamelium.h | 7 ++-
> > lib/igt_frame.c | 6 +-
> > tests/chamelium.c | 57 ++++++++++++++++++
> > 4 files changed, 225 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> > index df49936b..8701192e 100644
> > --- a/lib/igt_chamelium.c
> > +++ b/lib/igt_chamelium.c
> > @@ -938,6 +938,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,
> > @@ -947,9 +949,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);
> >
> > @@ -1055,6 +1061,154 @@ void chamelium_assert_crc_eq_or_dump(struct
> > chamelium *chamelium,
> > }
> >
> > /**
> > + * chamelium_assert_analogue_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_analogue_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_analogue_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_analogue_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_analogue_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 analogue 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 analogue 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..101e64b6 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_analogue_frame_match_or_dump(struct chamelium
> > *chamelium,
> > + struct
> > chamelium_port *port,
> > + const struct
> > chamelium_frame_dump *frame,
> > + struct igt_fb
> > *fb);
> > +void chamelium_crop_analogue_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/lib/igt_frame.c b/lib/igt_frame.c
> > index dc84fe01..8ca110c9 100644
> > --- a/lib/igt_frame.c
> > +++ b/lib/igt_frame.c
> > @@ -204,13 +204,13 @@ bool
> > igt_check_analogue_frame_match(cairo_surface_t *reference,
> > p = &capture_pixels[(x + y * w) * 4];
> > q = &reference_pixels[(x + y * w) * 4];
> >
> > - for (i = 1; i < 4; i++) {
> > + for (i = 0; i < 3; i++) {
> > diff = (int) p[i] - q[i];
> > if (diff < 0)
> > diff = -diff;
> >
> > - error_count[i-1][q[i-1]][0] += diff;
> > - error_count[i-1][q[i-1]][1]++;
> > + error_count[i][q[i]][0] += diff;
> > + error_count[i][q[i]][1]++;
>
> Am I missing something? This change seems like it should just be part
> of patch #1 instead of being done afterwards in patch #2.
Woops, looks like I messed this one up when crafting the new version.
Thanks for catching it!
> > }
> > }
> > }
> > diff --git a/tests/chamelium.c b/tests/chamelium.c
> > index 89a3bde0..baaa424b 100644
> > --- a/tests/chamelium.c
> > +++ b/tests/chamelium.c
> > @@ -358,6 +358,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);
> >
> > @@ -492,6 +496,56 @@ test_display_frame_dump(data_t *data, struct
> > chamelium_port *port)
> > }
> >
> > static void
> > +test_analogue_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_XRGB8
> > 8
> > 88,
> > + 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_analogue_frame(frame, mode->hdisplay,
> > + mode->vdisplay);
> > +
> > + chamelium_assert_analogue_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();
> > @@ -734,6 +788,9 @@ igt_main
> >
> > connector_subtest("vga-hpd-without-ddc", VGA)
> > test_hpd_without_ddc(&data, port);
> > +
> > + connector_subtest("vga-frame-dump", VGA)
> > + test_analogue_frame_dump(&data, port);
> > }
> >
> > igt_subtest_group {
> > --
> > 2.13.2
> >
--
Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo, Finland
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2017-07-19 12:31 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-06 13:26 [PATCH i-g-t 0/1] chamelium: Add support for VGA frame comparison testing Paul Kocialkowski
2017-07-06 13:26 ` [PATCH i-g-t] " Paul Kocialkowski
2017-07-12 14:57 ` [PATCH i-g-t v2 0/2] Analogue/VGA frame comparison support Paul Kocialkowski
2017-07-12 14:57 ` [PATCH i-g-t v2 1/2] lib/igt_frame: Add support for analogue frame comparison testing Paul Kocialkowski
2017-07-12 14:57 ` [PATCH i-g-t v2 2/2] chamelium: Add support for VGA " Paul Kocialkowski
2017-07-17 17:35 ` Lyude Paul
2017-07-19 12:31 ` Paul Kocialkowski [this message]
2017-07-17 17:38 ` [PATCH i-g-t 0/1] " Lyude Paul
2017-07-19 13:48 ` [PATCH i-g-t v3 0/2] Analogue/VGA frame comparison support Paul Kocialkowski
2017-07-19 13:48 ` [PATCH i-g-t v3 1/2] lib/igt_frame: Add support for analog frame comparison testing Paul Kocialkowski
2017-07-19 13:48 ` [PATCH i-g-t v3 2/2] chamelium: Add support for VGA " Paul Kocialkowski
2017-07-19 17:50 ` [PATCH i-g-t v3 0/2] Analogue/VGA frame comparison support Lyude Paul
2017-07-20 15:15 ` Paul Kocialkowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1500467500.1329.9.camel@linux.intel.com \
--to=paul.kocialkowski@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=lyude@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.