[Mesa-dev] [PATCH] st/va: Default to YUV420 RT format when creating a config

[Julien Isorce]

On 17 October 2016 at 16:27, Mark Thompson <sw at jkqxz.net> wrote:

> On 17/10/16 16:13, Julien Isorce wrote:
> > Hi Mark,
> >
> > Yes I actually saw that too in the intel driver though I think it does
> not add
> > VA_RT_FORMAT_RGB32 ? Or I missed something ?
> > Maybe this is a bug. In any case yes as said before " the intel va
> driver always
> > return the full list for vpp." from vaQuerySurfaceAttributes
> > no matter the format selected when creating the config.
>
> Looks like the lack of RGB32 has already been noted and fixed:
> <https://cgit.freedesktop.org/vaapi/intel-driver/commit/?id=
> e748bc7f0565d59a7ec2ba038e76a0a1de19c15c>.
>

Great, thx for the link.


>
> > So combining all it should probably be something like this:
> >
> > diff --git a/src/gallium/state_trackers/va/config.c
> > b/src/gallium/state_trackers/va/config.c
> > index 2f96eb6..11afc81 100644
> > --- a/src/gallium/state_trackers/va/config.c
> > +++ b/src/gallium/state_trackers/va/config.c
> > @@ -185,6 +185,8 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile
> profile,
> > VAEntrypoint entrypoin
> >     if (profile == VAProfileNone && entrypoint == VAEntrypointVideoProc)
> {
> >        config->entrypoint = VAEntrypointVideoProc;
> >        config->profile = PIPE_VIDEO_PROFILE_UNKNOWN;
> > +      config->rt_format = VA_RT_FORMAT_YUV420 | VA_RT_FORMAT_RGB32;
> > +
> >        for (int i = 0; i < num_attribs; i++) {
> >           if (attrib_list[i].type == VAConfigAttribRTFormat) {
> >              if (attrib_list[i].value & (VA_RT_FORMAT_YUV420 |
> > VA_RT_FORMAT_RGB32)) {
>
> Please set a default for the non-VideoProc case too, so that codecs have
> the
> same behaviour.
>

Oups


>
> > diff --git a/src/gallium/state_trackers/va/surface.c
> > b/src/gallium/state_trackers/va/surface.c
> > index 5e92980..f8513d9 100644
> > --- a/src/gallium/state_trackers/va/surface.c
> > +++ b/src/gallium/state_trackers/va/surface.c
> > @@ -419,7 +419,7 @@ vlVaQuerySurfaceAttributes(VADriverContextP ctx,
> VAConfigID
> > config_id,
> >     /* vlVaCreateConfig returns PIPE_VIDEO_PROFILE_UNKNOWN
> >      * only for VAEntrypointVideoProc. */
> >     if (config->profile == PIPE_VIDEO_PROFILE_UNKNOWN) {
> > -      if (config->rt_format == VA_RT_FORMAT_RGB32) {
> > +      if (config->rt_format & VA_RT_FORMAT_RGB32) {
> >           for (j = 0; j < ARRAY_SIZE(vpp_surface_formats); ++j) {
> >              attribs[i].type = VASurfaceAttribPixelFormat;
> >              attribs[i].value.type = VAGenericValueTypeInteger;
> > @@ -427,7 +427,8 @@ vlVaQuerySurfaceAttributes(VADriverContextP ctx,
> VAConfigID
> > config_id,
> >              attribs[i].value.value.i =
> > PipeFormatToVaFourcc(vpp_surface_formats[j]);
> >              i++;
> >           }
> > -      } else if (config->rt_format == VA_RT_FORMAT_YUV420) {
> > +      }
> > +      if (config->rt_format & VA_RT_FORMAT_YUV420) {
> >           attribs[i].type = VASurfaceAttribPixelFormat;
> >           attribs[i].value.type = VAGenericValueTypeInteger;
> >           attribs[i].flags = VA_SURFACE_ATTRIB_GETTABLE |
> > VA_SURFACE_ATTRIB_SETTABLE;
> >
> > Will it be ok for your case ?
> >
> > Cheers
> > Julien
>
> Yeah, that seems right to me (with the small change above).
>

I sent the patch to mesa-dev few minutes ago, please have a look, thx.

Cheers
Julien


>
> Thanks,
>
> - Mark
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161017/772be448/attachment.html>