[igt-dev] [PATCH i-g-t 1/2] lib/igt_infoframe: add support for AVI InfoFrames

[Arkadiusz Hiler]

On Mon, Jul 22, 2019 at 06:01:26PM +0300, Simon Ser wrote:
> This commit adds partial support for AVI InfoFrames. Only bytes 1 and 2 of the
> InfoFrame payload are parsed.
> 
> These bytes contain (among other things) information about the aspect ratio of
> the frames.
> 
> Signed-off-by: Simon Ser <simon.ser at intel.com>
> ---
>  lib/igt_infoframe.c | 30 ++++++++++++++++++++++++++++
>  lib/igt_infoframe.h | 48 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/lib/igt_infoframe.c b/lib/igt_infoframe.c
> index 7e4fb45881a6..d8a2e609573c 100644
> --- a/lib/igt_infoframe.c
> +++ b/lib/igt_infoframe.c
> @@ -27,6 +27,7 @@
>  
>  #include <string.h>
>  
> +#include "igt_core.h"
>  #include "igt_infoframe.h"
>  
>  /**
> @@ -61,6 +62,35 @@ static const int sample_sizes[] = {
>  
>  static const size_t sample_sizes_len = sizeof(sample_sizes) / sizeof(sample_sizes[0]);
>  
> +bool infoframe_avi_parse(struct infoframe_avi *infoframe, int version,
> +			 const uint8_t *buf, size_t buf_size)
> +{
> +	memset(infoframe, 0, sizeof(*infoframe));
> +
> +	switch (version) {
> +	case 2:
> +	case 3:
> +	case 4:
> +		break; /* supported */
> +	default:
> +		igt_debug("Unsuppported AVI InfoFrame version: %d\n", version);
> +		return false;
> +	}
> +
> +	if (buf_size < 13)
> +		return false;
> +
> +	infoframe->rgb_ycbcr = buf[0] >> 5;
> +	infoframe->scan = buf[0] & 0x3;
> +
> +	infoframe->colorimetry = buf[1] >> 7;

Shouldn't it be >> 6? It's 2 last bits.

> +	infoframe->picture_aspect_ratio = (buf[1] >> 4) & 0x3;
> +	infoframe->active_aspect_ratio = buf[1] & 0xF;
> +	infoframe->vic = buf[3];
> +
> +	return true;
> +}
> +
>  bool infoframe_audio_parse(struct infoframe_audio *infoframe, int version,
>  			   const uint8_t *buf, size_t buf_size)
>  {
> diff --git a/lib/igt_infoframe.h b/lib/igt_infoframe.h
> index 35daa3ea169d..056cdad6072a 100644
> --- a/lib/igt_infoframe.h
> +++ b/lib/igt_infoframe.h
> @@ -32,6 +32,52 @@
>  #include <stddef.h>
>  #include <stdint.h>
>  
> +enum infoframe_avi_rgb_ycbcr {
> +	INFOFRAME_AVI_RGB = 0,
> +	INFOFRAME_AVI_YCBCR422 = 1,
> +	INFOFRAME_AVI_YCBCR444 = 2,
> +	INFOFRAME_AVI_YCBCR420 = 3,
> +	INFOFRAME_AVI_IDO_DEFINED = 7,
> +};
> +
> +enum infoframe_avi_scan {
> +	INFOFRAME_AVI_SCAN_UNSPECIFIED = 0,
> +	INFOFRAME_AVI_OVERSCAN = 1,
> +	INFOFRAME_AVI_UNDERSCAN = 2,
> +};
> +
> +enum infoframe_avi_colorimetry {
> +	INFOFRAME_AVI_COLORIMETRY_UNSPECIFIED = 0,
> +	INFOFRAME_AVI_SMPTE_170M = 1,
> +	INFOFRAME_AVI_ITUR_BT709 = 2,
> +	INFOFRAME_AVI_COLORIMETRY_EXTENDED = 3,
> +};
> +
> +enum infoframe_avi_picture_aspect_ratio {
> +	INFOFRAME_AVI_PIC_AR_UNSPECIFIED = 0,
> +	INFOFRAME_AVI_PIC_AR_4_3 = 1,
> +	INFOFRAME_AVI_PIC_AR_16_9 = 2,
> +};
> +
> +enum infoframe_avi_active_aspect_ratio {
> +	INFOFRAME_AVI_ACT_AR_PIC = 0, /* same as picture aspect ratio */

Shouldn't this be 8? I believe it is 0b1000.

With the above addressed
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>