[Mesa-dev] [PATCH v4 3/9] st/va: implement VaCreateSurfaces2 and VaQuerySurfaceAttributes

[Emil Velikov]

On 29 October 2015 at 17:40, Julien Isorce <j.isorce at samsung.com> wrote:
> +
> +VAStatus
> +vlVaCreateSurfaces2(VADriverContextP ctx, unsigned int format,
> +                    unsigned int width, unsigned int height,
> +                    VASurfaceID *surfaces, unsigned int num_surfaces,
> +                    VASurfaceAttrib *attrib_list, unsigned int num_attribs)
> +{
[snip]
> +    if (VA_RT_FORMAT_YUV400 != format &&
Please drop this format.

> +        VA_RT_FORMAT_YUV420 != format &&
> +        VA_RT_FORMAT_YUV422 != format &&
> +        VA_RT_FORMAT_YUV444 != format &&
> +        VA_RT_FORMAT_RGB32  != format) {
Whereas RGB32, if it works, is by sheer luck. Because ...

[snip]
> +
> +    memset(&templat, 0, sizeof(templat));
> +
[snip]
> +    if (format != VA_RT_FORMAT_RGB32)
> +       templat.chroma_format = ChromaToPipe(format);
> +
chroma_format defaults to 0 (thanks to memset above) which likely maps
to PIPE_VIDEO_CHROMA_FORMAT_400 from the pipe_video_chroma_format
enum. With most drivers (incl the 'software implementation' in aux/vl)
only handle 420/422/444.

[snip]
> +no_res:
> +   for (i = 0; i < num_surfaces; i++) {
> +      if (surfaces[i] != VA_INVALID_ID)
Should have caught this one sooner - this looks odd for couple of reasons.
 - vlVaDestroySurfaces removes a total of i surfaces. Thus we don't
need a loop here.
 - handle_table_add() returns 0 on error and 0xffffffff
(VA_INVALID_ID) is a valid handle.

-Emil