[Mesa-dev] [PATCH 2/7] st/va: properly defines VAImageFormat formats and improve VaCreateImage

[Emil Velikov]

On 17 October 2015 at 00:14, Julien Isorce <julien.isorce at gmail.com> wrote:
> Also add RGBA, RGBX and BGRX.
> Also extend ChromaToPipe and implement PipeToYCbCr.
>
> Note that gstreamer-vaapi check all the VAImageFormat fields.
>
> Signed-off-by: Julien Isorce <j.isorce at samsung.com>
> ---
>  src/gallium/state_trackers/va/image.c      | 10 ++++++--
>  src/gallium/state_trackers/va/va_private.h | 38 +++++++++++++++++++++++++++++-
>  2 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/image.c b/src/gallium/state_trackers/va/image.c
> index 022240d..c7fbe1a 100644
> --- a/src/gallium/state_trackers/va/image.c
> +++ b/src/gallium/state_trackers/va/image.c
> @@ -44,7 +44,10 @@ static const VAImageFormat formats[VL_VA_MAX_IMAGE_FORMATS] =
>     {VA_FOURCC('Y','V','1','2')},
>     {VA_FOURCC('Y','U','Y','V')},
>     {VA_FOURCC('U','Y','V','Y')},
> -   {VA_FOURCC('B','G','R','A')}
> +   {.fourcc = VA_FOURCC('B','G','R','A'), .byte_order = VA_LSB_FIRST, 32, 32, 0x00ff0000, 0x0000ff00, 0x000000ff, 0xff000000},
> +   {.fourcc = VA_FOURCC('R','G','B','A'), .byte_order = VA_LSB_FIRST, 32, 32, 0x000000ff, 0x0000ff00, 0x00ff0000, 0xff000000},
> +   {.fourcc = VA_FOURCC('B','G','R','X'), .byte_order = VA_LSB_FIRST, 32, 24, 0x00ff0000, 0x0000ff00, 0x000000ff, 0x00000000},
> +   {.fourcc = VA_FOURCC('R','G','B','X'), .byte_order = VA_LSB_FIRST, 32, 24, 0x000000ff, 0x0000ff00, 0x00ff0000, 0x00000000}
>  };
>
Please drop the explicit size of the array (VL_VA_MAX_IMAGE_FORMATS),
and do a STATIC_ASSERT upon it's size. The latter will trigger at
build time and save a few wtf. moments.

>  static void
> @@ -116,7 +119,7 @@ vlVaCreateImage(VADriverContextP ctx, VAImageFormat *format, int width, int heig
>     img->width = width;
>     img->height = height;
>     w = align(width, 2);
> -   h = align(width, 2);
> +   h = align(height, 2);
>
This is a separate fix, which just landed in master.

>     switch (format->fourcc) {
>     case VA_FOURCC('N','V','1','2'):
> @@ -149,6 +152,9 @@ vlVaCreateImage(VADriverContextP ctx, VAImageFormat *format, int width, int heig
>        break;
>
>     case VA_FOURCC('B','G','R','A'):
> +   case VA_FOURCC('R','G','B','A'):
> +   case VA_FOURCC('B','G','R','X'):
> +   case VA_FOURCC('R','G','B','X'):
>        img->num_planes = 1;
>        img->pitches[0] = w * 4;
>        img->offsets[0] = 0;
> diff --git a/src/gallium/state_trackers/va/va_private.h b/src/gallium/state_trackers/va/va_private.h
> index 1ea7be7..3479156 100644
> --- a/src/gallium/state_trackers/va/va_private.h
> +++ b/src/gallium/state_trackers/va/va_private.h
> @@ -46,7 +46,7 @@
>  #define VL_VA_DRIVER(ctx) ((vlVaDriver *)ctx->pDriverData)
>  #define VL_VA_PSCREEN(ctx) (VL_VA_DRIVER(ctx)->vscreen->pscreen)
>
> -#define VL_VA_MAX_IMAGE_FORMATS 6
> +#define VL_VA_MAX_IMAGE_FORMATS 9
>
>  static inline enum pipe_video_chroma_format
>  ChromaToPipe(int format)
> @@ -58,6 +58,8 @@ ChromaToPipe(int format)
>        return PIPE_VIDEO_CHROMA_FORMAT_422;
>     case VA_RT_FORMAT_YUV444:
>        return PIPE_VIDEO_CHROMA_FORMAT_444;
> +   case VA_RT_FORMAT_RGB32:
> +       return 0;
Use symbolic name rather than 0 ? Shouldn't the RGB formats handling
be part of other function ? (or just rename this and the ones below)

Cheers
Emil