[PATCH i-g-t 1/3] lib/igt_chamelium: Add chamelium_frame_match_or_dump_frame_pair()

[Borah, Chaitanya Kumar]

Hello Swati,

> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Swati
> Sharma
> Sent: Monday, March 24, 2025 2:40 PM
> To: igt-dev at lists.freedesktop.org
> Cc: Sharma, Swati2 <swati2.sharma at intel.com>
> Subject: [PATCH i-g-t 1/3] lib/igt_chamelium: Add
> chamelium_frame_match_or_dump_frame_pair()
> 
> Add chamelium_frame_match_or_dump_frame_pair() to compare frame
> dumps captured from chamelium.
> 
> Signed-off-by: Swati Sharma <swati2.sharma at intel.com>
> ---
>  lib/igt_chamelium.c | 60
> +++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_chamelium.h |  6 +++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c index
> 620fbbf7d..44a169b29 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -2070,6 +2070,66 @@ bool chamelium_frame_match_or_dump(struct
> chamelium *chamelium,
>  	return match;
>  }
> 
> +/**
> + * chamelium_frame_match_or_dump:
> + * @chamelium: The chamelium instance the frame dump belongs to
> + * @frame0: The chamelium reference frame dump to match
> + * @frame1: The chamelium capture frame dump to match
> + * @reference_crc: CRC of chamelium reference frame
> + * @check: the type of frame matching check to use
> + *
> + * Returns bool 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.
> + */
> +bool chamelium_frame_match_or_dump_frame_pair(struct chamelium
> *chamelium,
> +					      struct chamelium_port *port,
> +					      const struct
> chamelium_frame_dump *frame0,
> +					      const struct
> chamelium_frame_dump *frame1,
> +					      igt_crc_t *reference_crc,

The asymmetry in the arguments (reference_crc and not capture_crc) is bother me a bit. 

...

> +					      enum chamelium_check check)
> +{
> +	cairo_surface_t *reference;
> +	cairo_surface_t *capture;
> +	igt_crc_t *capture_crc;
> +	bool match;
> +
> +	/* Grab the captured reference frame from chamelium */
> +	reference = convert_frame_dump_argb32(frame0);
> +
> +	/* Grab the captured frame from chamelium */
> +	capture = convert_frame_dump_argb32(frame1);
> +
> +	switch (check) {
> +	case CHAMELIUM_CHECK_ANALOG:
> +		match = igt_check_analog_frame_match(reference, capture);
> +		break;
> +	case CHAMELIUM_CHECK_CHECKERBOARD:
> +		match = igt_check_checkerboard_frame_match(reference,
> capture);
> +		break;
> +	default:
> +		igt_assert(false);
> +	}
> +
> +	if (!match && igt_frame_dump_is_enabled()) {
> +		/* Get the captured frame CRC from the Chamelium. */
> +		capture_crc = chamelium_get_crc_for_area(chamelium, port,
> 0, 0,
> +							 0, 0);

Does this mean that chamelium should be displaying the image when this comparison is done?
If so, It will defeat the purpose of using the frame dump for comparison.


> +		igt_assert(capture_crc);
> +
> +		compared_frames_dump(reference, capture, reference_crc,
> +				     capture_crc);

Since compared_frames_dump() calculates the crc if not passed by the caller we should let it.
That way we can remove the need of passing the reference_crc to this helper. (and also not calculate capture_crc)

Regards

Chaitanya


> +
> +		free(reference_crc);
> +		free(capture_crc);
> +	}
> +
> +	cairo_surface_destroy(reference);
> +	cairo_surface_destroy(capture);
> +
> +	return match;
> +}
> +
>  /**
>   * chamelium_analog_frame_crop:
>   * @chamelium: The Chamelium instance to use diff --git
> a/lib/igt_chamelium.h b/lib/igt_chamelium.h index d979de4a2..140d52c95
> 100644
> --- a/lib/igt_chamelium.h
> +++ b/lib/igt_chamelium.h
> @@ -257,6 +257,12 @@ bool chamelium_frame_match_or_dump(struct
> chamelium *chamelium,
>  				   const struct chamelium_frame_dump
> *frame,
>  				   struct igt_fb *fb,
>  				   enum chamelium_check check);
> +bool chamelium_frame_match_or_dump_frame_pair(struct chamelium
> *chamelium,
> +					      struct chamelium_port *port,
> +					      const struct
> chamelium_frame_dump *frame0,
> +					      const struct
> chamelium_frame_dump *frame1,
> +					      igt_crc_t *reference_crc,
> +					      enum chamelium_check check);
>  void chamelium_crop_analog_frame(struct chamelium_frame_dump *dump,
> int width,
>  				 int height);
>  void chamelium_destroy_frame_dump(struct chamelium_frame_dump
> *dump);
> --
> 2.25.1