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

2015-10-20 Thread Julien Isorce
On 17 October 2015 at 00:25, Ilia Mirkin  wrote:

> Not sure how VA specifies things, but if the RGBA8 stuff is supposed
> to be in CPU-endian as packed 32-bit ints, I think you're meant to use
> PIPE_FORMAT_RGBA_UNORM and so on. However if it's always supposed
> to be little-endian or array-based, then the way you have it is fine.
>

Great question.

>From what I can read, I think in VA there is no fixed assumption,  for
example VA_FOURCC('A','B','G','R') exists, I think currently mesa st/va
just only supports VA_LSB_FIRST (but VA_MSB_FIRST exists in libva headers).
Initially I just extended existing code:

case VA_FOURCC('B','G','R','A'):
   return PIPE_FORMAT_B8G8R8A8_UNORM;

I also compared with intel vaapi driver:
http://cgit.freedesktop.org/vaapi/intel-driver/tree/src/i965_drv_video.c#n291
It seems they only use LSB case. Though MSB is handled in unmaintained
vaapi driver bridge for vdpau:
http://cgit.freedesktop.org/vaapi/vdpau-driver/tree/src/vdpau_image.c#n57

As you raised the question, I decided to ask on libva irc channel to poke
some maintainers. But actually you replied to me :) (for the record, you
replied there is not a ton of big endian intel cpu).

Well in any case libva defines for example VA_FOURCC('A','B','G','R') and
VA_FOURCC('B','G','R','A'), so I guess it means I should not use
PIPE_FORMAT_RGBA_UNORM-likes so I guess the patch is ok.

I think for now mesa st/va does not work on big endian cpu when it is about
using "RGBs" formats. And after my patch it still does not.

Though I can add them if you want but it will be untested :)

Julien



>
>   -ilia
>
> On Fri, Oct 16, 2015 at 7:14 PM, Julien Isorce 
> 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 
> > ---
> >  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, 0x00ff, 0xff00, 0x00ff, 0xff00},
> > +   {.fourcc = VA_FOURCC('R','G','B','A'), .byte_order = VA_LSB_FIRST,
> 32, 32, 0x00ff, 0xff00, 0x00ff, 0xff00},
> > +   {.fourcc = VA_FOURCC('B','G','R','X'), .byte_order = VA_LSB_FIRST,
> 32, 24, 0x00ff, 0xff00, 0x00ff, 0x},
> > +   {.fourcc = VA_FOURCC('R','G','B','X'), .byte_order = VA_LSB_FIRST,
> 32, 24, 0x00ff, 0xff00, 0x00ff, 0x}
> >  };
> >
> >  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);
> >
> > 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;
> > default:
> >assert(0);
> >return PIPE_VIDEO_CHROMA_FORMAT_420;
> > @@ -80,12 +82,46 @@ YCbCrToPipe(unsigned format)
> >return PIPE_FORMAT_UYVY;
> > case VA_FOURCC('B','G','R','A'):
> >return PIPE_FORMAT_B8G8R8A8_UNORM;
> > +   case 

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

2015-10-19 Thread Emil Velikov
On 17 October 2015 at 00:14, Julien Isorce  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 
> ---
>  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, 0x00ff, 0xff00, 0x00ff, 0xff00},
> +   {.fourcc = VA_FOURCC('R','G','B','A'), .byte_order = VA_LSB_FIRST, 32, 
> 32, 0x00ff, 0xff00, 0x00ff, 0xff00},
> +   {.fourcc = VA_FOURCC('B','G','R','X'), .byte_order = VA_LSB_FIRST, 32, 
> 24, 0x00ff, 0xff00, 0x00ff, 0x},
> +   {.fourcc = VA_FOURCC('R','G','B','X'), .byte_order = VA_LSB_FIRST, 32, 
> 24, 0x00ff, 0xff00, 0x00ff, 0x}
>  };
>
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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2015-10-16 Thread Julien Isorce
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 
---
 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, 
0x00ff, 0xff00, 0x00ff, 0xff00},
+   {.fourcc = VA_FOURCC('R','G','B','A'), .byte_order = VA_LSB_FIRST, 32, 32, 
0x00ff, 0xff00, 0x00ff, 0xff00},
+   {.fourcc = VA_FOURCC('B','G','R','X'), .byte_order = VA_LSB_FIRST, 32, 24, 
0x00ff, 0xff00, 0x00ff, 0x},
+   {.fourcc = VA_FOURCC('R','G','B','X'), .byte_order = VA_LSB_FIRST, 32, 24, 
0x00ff, 0xff00, 0x00ff, 0x}
 };
 
 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);
 
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;
default:
   assert(0);
   return PIPE_VIDEO_CHROMA_FORMAT_420;
@@ -80,12 +82,46 @@ YCbCrToPipe(unsigned format)
   return PIPE_FORMAT_UYVY;
case VA_FOURCC('B','G','R','A'):
   return PIPE_FORMAT_B8G8R8A8_UNORM;
+   case VA_FOURCC('R','G','B','A'):
+  return PIPE_FORMAT_R8G8B8A8_UNORM;
+   case VA_FOURCC('B','G','R','X'):
+  return PIPE_FORMAT_B8G8R8X8_UNORM;
+   case VA_FOURCC('R','G','B','X'):
+  return PIPE_FORMAT_R8G8B8X8_UNORM;
default:
   assert(0);
   return PIPE_FORMAT_NONE;
}
 }
 
+static inline unsigned
+PipeToYCbCr(enum pipe_format p_format)
+{
+   switch (p_format) {
+   case PIPE_FORMAT_NV12:
+  return VA_FOURCC('N','V','1','2');
+   case PIPE_FORMAT_IYUV:
+  return VA_FOURCC('I','4','2','0');
+   case PIPE_FORMAT_YV12:
+  return VA_FOURCC('Y','V','1','2');
+   case PIPE_FORMAT_UYVY:
+  return VA_FOURCC('U','Y','V','Y');
+   case PIPE_FORMAT_YUYV:
+  return VA_FOURCC('Y','U','Y','V');
+   case PIPE_FORMAT_B8G8R8A8_UNORM:
+  return VA_FOURCC('B','G','R','A');
+   case PIPE_FORMAT_R8G8B8A8_UNORM:
+  return VA_FOURCC('R','G','B','A');
+   case PIPE_FORMAT_B8G8R8X8_UNORM:
+  return VA_FOURCC('B','G','R','X');
+   case PIPE_FORMAT_R8G8B8X8_UNORM:
+  return VA_FOURCC('R','G','B','X');
+   default:
+  assert(0);
+  return -1;
+   }
+}
+
 static inline VAProfile
 PipeToProfile(enum pipe_video_profile profile)
 {
-- 
1.9.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2015-10-16 Thread Ilia Mirkin
Not sure how VA specifies things, but if the RGBA8 stuff is supposed
to be in CPU-endian as packed 32-bit ints, I think you're meant to use
PIPE_FORMAT_RGBA_UNORM and so on. However if it's always supposed
to be little-endian or array-based, then the way you have it is fine.

  -ilia

On Fri, Oct 16, 2015 at 7:14 PM, Julien Isorce  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 
> ---
>  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, 0x00ff, 0xff00, 0x00ff, 0xff00},
> +   {.fourcc = VA_FOURCC('R','G','B','A'), .byte_order = VA_LSB_FIRST, 32, 
> 32, 0x00ff, 0xff00, 0x00ff, 0xff00},
> +   {.fourcc = VA_FOURCC('B','G','R','X'), .byte_order = VA_LSB_FIRST, 32, 
> 24, 0x00ff, 0xff00, 0x00ff, 0x},
> +   {.fourcc = VA_FOURCC('R','G','B','X'), .byte_order = VA_LSB_FIRST, 32, 
> 24, 0x00ff, 0xff00, 0x00ff, 0x}
>  };
>
>  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);
>
> 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;
> default:
>assert(0);
>return PIPE_VIDEO_CHROMA_FORMAT_420;
> @@ -80,12 +82,46 @@ YCbCrToPipe(unsigned format)
>return PIPE_FORMAT_UYVY;
> case VA_FOURCC('B','G','R','A'):
>return PIPE_FORMAT_B8G8R8A8_UNORM;
> +   case VA_FOURCC('R','G','B','A'):
> +  return PIPE_FORMAT_R8G8B8A8_UNORM;
> +   case VA_FOURCC('B','G','R','X'):
> +  return PIPE_FORMAT_B8G8R8X8_UNORM;
> +   case VA_FOURCC('R','G','B','X'):
> +  return PIPE_FORMAT_R8G8B8X8_UNORM;
> default:
>assert(0);
>return PIPE_FORMAT_NONE;
> }
>  }
>
> +static inline unsigned
> +PipeToYCbCr(enum pipe_format p_format)
> +{
> +   switch (p_format) {
> +   case PIPE_FORMAT_NV12:
> +  return VA_FOURCC('N','V','1','2');
> +   case PIPE_FORMAT_IYUV:
> +  return VA_FOURCC('I','4','2','0');
> +   case PIPE_FORMAT_YV12:
> +  return VA_FOURCC('Y','V','1','2');
> +   case PIPE_FORMAT_UYVY:
> +  return VA_FOURCC('U','Y','V','Y');
> +   case PIPE_FORMAT_YUYV:
> +  return VA_FOURCC('Y','U','Y','V');
> +   case PIPE_FORMAT_B8G8R8A8_UNORM:
> +  return VA_FOURCC('B','G','R','A');
> +   case PIPE_FORMAT_R8G8B8A8_UNORM:
> +  return VA_FOURCC('R','G','B','A');
> +   case PIPE_FORMAT_B8G8R8X8_UNORM:
> +  return VA_FOURCC('B','G','R','X');
> +   case PIPE_FORMAT_R8G8B8X8_UNORM:
> +  return VA_FOURCC('R','G','B','X');
> +   default:
> +  assert(0);
> +  return -1;
> +   }
> +}
> +
>  static inline VAProfile
>  PipeToProfile(enum pipe_video_profile profile)
>  {
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___