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

[Mark Thompson]

On 17/10/16 15:42, Mark Thompson wrote:
> The default will only be used if the VAConfigRTFormat attribute is not
> provided by the user.
> ---
> On 17/10/16 15:21, Julien Isorce wrote:
>> Hi Mark,
>>
>> Thx for the patch. I can see it has already landed.
>>
>> I just tried it with gstreamer-vaapi and it causes problem since they create the
>> config like this for VPP:
>>
>> va_status = vaCreateConfig (filter->va_display, VAProfileNone,
>>       VAEntrypointVideoProc, NULL, 0, &filter->va_config);
>>
>> As you can see num attrivs is 0 so it makes vaQuerySurfaceAttributes to return
>> no supported format
>> because config->rt_format is 0.
>>
>> So I plan to make a patch that looks like this:
>>
>>
>> --- 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 || 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 || 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 whatever test application you are using ?
>>
>> Not that the intel va driver always return the full list for vpp.
>>
>> Cheers
>> Julien
> 
> Hi Julien,
> 
> On vaCreateConfig(), va.h says:
> /**
>  * Create a configuration for the decode pipeline
>  * it passes in the attribute list that specifies the attributes it cares
>  * about, with the rest taking default values.
>  */
> 
> I think that means that it should be fixed there instead?  That is, if we don't
> pass the render target format attribute, just assume a default value which I
> suppose should be VA_RT_FORMAT_YUV420.
> 
> So, something like the enclosing patch?  (Not tested.)

Hmm.  The i965 driver uses all of the possible values (depending on the
profile/entrypoint) ored together here - thats probably a more compatible choice
than YUV420 only.

Thanks,

- Mark


That is, add:

>  src/gallium/state_trackers/va/config.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/gallium/state_trackers/va/config.c
> b/src/gallium/state_trackers/va/config.c
> index 2f96eb6..f2a89b7 100644
> --- a/src/gallium/state_trackers/va/config.c
> +++ b/src/gallium/state_trackers/va/config.c
> @@ -182,6 +182,9 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile profile,
> VAEntrypoint entrypoin
>     if (!config)
>        return VA_STATUS_ERROR_ALLOCATION_FAILED;
> 
> +   // Default value, overwritten by the VAConfigRTformat attribute if present.
> +   config->rt_format = VA_RT_FORMAT_YUV420;
> +
>     if (profile == VAProfileNone && entrypoint == VAEntrypointVideoProc) {
>        config->entrypoint = VAEntrypointVideoProc;
>        config->profile = PIPE_VIDEO_PROFILE_UNKNOWN;

config->rt_format |= VA_RT_FORMAT_RGB32;