[Mesa-dev] [PATCH 4/7] st/va: implement VaCreateSurfaces2 and VaQuerySurfaceAttributes
Julien Isorce
julien.isorce at gmail.com
Mon Oct 19 15:57:34 PDT 2015
On 19 October 2015 at 16:55, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 17 October 2015 at 00:14, Julien Isorce <julien.isorce at gmail.com>
> wrote:
> > Inspired from
> http://cgit.freedesktop.org/vaapi/intel-driver/tree/src/i965_drv_video.c
> >
> Please mention the actual usecase here and/or how you've tested this.
>
Thx for the review.
All right I'll amend commit message that this is to support gstreamer-vaapi.
Also the first advantage to use VaCreateSurfaces2 over existing
VaCreateSurfaces, is that you can select which pixel format you want for
the surface. Pixel format that you can query with new function:
vlVaQuerySurfaceAttributes
>
> > Signed-off-by: Julien Isorce <j.isorce at samsung.com>
> > ---
> > src/gallium/state_trackers/va/context.c | 5 +-
> > src/gallium/state_trackers/va/surface.c | 288
> ++++++++++++++++++++++++-----
> > src/gallium/state_trackers/va/va_private.h | 7 +
> > 3 files changed, 249 insertions(+), 51 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/va/context.c
> b/src/gallium/state_trackers/va/context.c
> > index 8b003ae..8949d42 100644
> > --- a/src/gallium/state_trackers/va/context.c
> > +++ b/src/gallium/state_trackers/va/context.c
> > @@ -81,7 +81,10 @@ static struct VADriverVTable vtable =
> > &vlVaSetDisplayAttributes,
> > &vlVaBufferInfo,
> > &vlVaLockSurface,
> > - &vlVaUnlockSurface
> > + &vlVaUnlockSurface,
> > + &vlVaGetSurfaceAttributes,
> > + &vlVaCreateSurfaces2,
> > + &vlVaQuerySurfaceAttributes
> > };
> >
> > PUBLIC VAStatus
> > diff --git a/src/gallium/state_trackers/va/surface.c
> b/src/gallium/state_trackers/va/surface.c
> > index 8d4487b..be435cb 100644
> > --- a/src/gallium/state_trackers/va/surface.c
> > +++ b/src/gallium/state_trackers/va/surface.c
> > @@ -29,6 +29,8 @@
> > #include "pipe/p_screen.h"
> > #include "pipe/p_video_codec.h"
> >
> > +#include "state_tracker/drm_driver.h"
> > +
> > #include "util/u_memory.h"
> > #include "util/u_handle_table.h"
> > #include "util/u_rect.h"
> > @@ -36,6 +38,7 @@
> > #include "util/u_surface.h"
> >
> > #include "vl/vl_compositor.h"
> > +#include "vl/vl_video_buffer.h"
> > #include "vl/vl_winsys.h"
> >
> > #include "va_private.h"
> > @@ -44,56 +47,8 @@ VAStatus
> > vlVaCreateSurfaces(VADriverContextP ctx, int width, int height, int
> format,
> > int num_surfaces, VASurfaceID *surfaces)
> > {
> > - struct pipe_video_buffer templat = {};
> > - struct pipe_screen *pscreen;
> > - vlVaDriver *drv;
> > - int i;
> > -
> > - if (!ctx)
> > - return VA_STATUS_ERROR_INVALID_CONTEXT;
> > -
> > - if (!(width && height))
> > - return VA_STATUS_ERROR_INVALID_IMAGE_FORMAT;
> > -
> > - drv = VL_VA_DRIVER(ctx);
> > - pscreen = VL_VA_PSCREEN(ctx);
> > -
> > - templat.buffer_format = pscreen->get_video_param
> > - (
> > - pscreen,
> > - PIPE_VIDEO_PROFILE_UNKNOWN,
> > - PIPE_VIDEO_ENTRYPOINT_BITSTREAM,
> > - PIPE_VIDEO_CAP_PREFERED_FORMAT
> > - );
> > - templat.chroma_format = ChromaToPipe(format);
> > - templat.width = width;
> > - templat.height = height;
> > - templat.interlaced = pscreen->get_video_param
> > - (
> > - pscreen,
> > - PIPE_VIDEO_PROFILE_UNKNOWN,
> > - PIPE_VIDEO_ENTRYPOINT_BITSTREAM,
> > - PIPE_VIDEO_CAP_PREFERS_INTERLACED
> > - );
> > -
> > - for (i = 0; i < num_surfaces; ++i) {
> > - vlVaSurface *surf = CALLOC(1, sizeof(vlVaSurface));
> > - if (!surf)
> > - goto no_res;
> > -
> > - surf->templat = templat;
> > - surf->buffer = drv->pipe->create_video_buffer(drv->pipe,
> &templat);
> > - util_dynarray_init(&surf->subpics);
> > - surfaces[i] = handle_table_add(drv->htab, surf);
> > - }
> > -
> > - return VA_STATUS_SUCCESS;
> > -
> > -no_res:
> > - if (i)
> > - vlVaDestroySurfaces(ctx, surfaces, i);
> > -
> > - return VA_STATUS_ERROR_ALLOCATION_FAILED;
> > + return vlVaCreateSurfaces2(ctx, format, width, height, surfaces,
> num_surfaces,
> > + NULL, 0);
> > }
> >
> > VAStatus
> > @@ -349,3 +304,236 @@ vlVaUnlockSurface(VADriverContextP ctx,
> VASurfaceID surface)
> >
> > return VA_STATUS_ERROR_UNIMPLEMENTED;
> > }
> > +
> > +VAStatus
> > +vlVaGetSurfaceAttributes(VADriverContextP ctx, VAConfigID config,
> > + VASurfaceAttrib *attrib_list, unsigned int
> num_attribs)
> > +{
> > + return VA_STATUS_ERROR_UNIMPLEMENTED; /* DEPRECATED */
> > +}
> Hmm implemented only to return ENOIMPL. Why ?
>
2 reasons, first that I do not know how to set it in "VADriverVTable
vtable" otherwise, mhh actually maybe I can just set NULL in place of
"&vlVaGetSurfaceAttributes" :) libva indeed checks for its presence. And
also to make it clear in mesa st/va code that this function is deprecated,
see libva/va_backend.h. It is replaced by VaQuerySurfaceAttributes.
>
> > +
> > +VAStatus
> > +vlVaQuerySurfaceAttributes(VADriverContextP ctx, VAConfigID config,
> > + VASurfaceAttrib *attrib_list, unsigned int
> *num_attribs)
> > +{
> > + VAStatus vaStatus = VA_STATUS_SUCCESS;
> > + vlVaDriver *drv = NULL;
> > + VASurfaceAttrib *attribs = NULL;
> > + struct pipe_screen *pscreen = NULL;
> > + int i = 0;
> > +
> You don't need most of these variable initializers. st/va doesn't seem
> to follow this approach either.
>
> ok
> > + if (config == VA_INVALID_ID)
> > + return VA_STATUS_ERROR_INVALID_CONFIG;
> > +
> > + if (!attrib_list && !num_attribs)
> > + return VA_STATUS_ERROR_INVALID_PARAMETER;
> > +
> > + if (attrib_list == NULL) {
> > + *num_attribs = VASurfaceAttribCount;
> > + return VA_STATUS_SUCCESS;
> > + }
> > +
> > + attribs = CALLOC(VASurfaceAttribCount, sizeof(VASurfaceAttrib));
> > +
> > + if (attribs == NULL)
> > + return VA_STATUS_ERROR_ALLOCATION_FAILED;
> > +
> Push the calloc/check further down, where it's needed ? Otherwise you
> leak on the memory on the next three return statements.
>
I see, thx.
>
> > + if (!ctx)
> > + return VA_STATUS_ERROR_INVALID_CONTEXT;
> > +
> > + drv = VL_VA_DRIVER(ctx);
> > +
> > + if (!drv)
> > + return VA_STATUS_ERROR_INVALID_CONTEXT;
> > +
> > + pscreen = VL_VA_PSCREEN(ctx);
> > +
> > + if (!pscreen)
> > + return VA_STATUS_ERROR_INVALID_CONTEXT;
> > +
> > + if (config == PIPE_VIDEO_PROFILE_UNKNOWN) {
> > + /* Assume VAEntrypointVideoProc for now. */
> > + attribs[i].type = VASurfaceAttribPixelFormat;
> > + attribs[i].value.type = VAGenericValueTypeInteger;
> > + attribs[i].flags = VA_SURFACE_ATTRIB_GETTABLE |
> VA_SURFACE_ATTRIB_SETTABLE;
> > + attribs[i].value.value.i = VA_FOURCC_BGRA;
> > + i++;
> > +
> > + attribs[i].type = VASurfaceAttribPixelFormat;
> > + attribs[i].value.type = VAGenericValueTypeInteger;
> > + attribs[i].flags = VA_SURFACE_ATTRIB_GETTABLE |
> VA_SURFACE_ATTRIB_SETTABLE;
> > + attribs[i].value.value.i = VA_FOURCC_RGBA;
> > + i++;
> > + } else {
> > + /* Assume VAEntrypointVLD for now. */
> > + attribs[i].type = VASurfaceAttribPixelFormat;
> > + attribs[i].value.type = VAGenericValueTypeInteger;
> > + attribs[i].flags = VA_SURFACE_ATTRIB_GETTABLE |
> VA_SURFACE_ATTRIB_SETTABLE;
> > + attribs[i].value.value.i = VA_FOURCC_NV12;
> > + i++;
> > + }
> > +
> > + attribs[i].type = VASurfaceAttribMemoryType;
> > + attribs[i].value.type = VAGenericValueTypeInteger;
> > + attribs[i].flags = VA_SURFACE_ATTRIB_GETTABLE |
> VA_SURFACE_ATTRIB_SETTABLE;
> > + attribs[i].value.value.i = VA_SURFACE_ATTRIB_MEM_TYPE_VA;
> > + i++;
> > +
> > + attribs[i].type = VASurfaceAttribExternalBufferDescriptor;
> > + attribs[i].value.type = VAGenericValueTypePointer;
> > + attribs[i].flags = VA_SURFACE_ATTRIB_SETTABLE;
> > + attribs[i].value.value.p = NULL; /* ignore */
> > + i++;
> > +
> > + attribs[i].type = VASurfaceAttribMaxWidth;
> > + attribs[i].value.type = VAGenericValueTypeInteger;
> > + attribs[i].flags = VA_SURFACE_ATTRIB_GETTABLE;
> > + attribs[i].value.value.i = vl_video_buffer_max_size(pscreen);
> > + i++;
> > +
> > + attribs[i].type = VASurfaceAttribMaxHeight;
> > + attribs[i].value.type = VAGenericValueTypeInteger;
> > + attribs[i].flags = VA_SURFACE_ATTRIB_GETTABLE;
> > + attribs[i].value.value.i = vl_video_buffer_max_size(pscreen);
> > + i++;
> > +
> > + if (i > *num_attribs) {
> > + *num_attribs = i;
> > + FREE(attribs);
> > + return VA_STATUS_ERROR_MAX_NUM_EXCEEDED;
> > + }
> > +
> > + *num_attribs = i;
> > + memcpy(attrib_list, attribs, i * sizeof(VASurfaceAttrib));
> > + FREE(attribs);
> > +
> > + return vaStatus;
> > +}
> > +
> > +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)
> > +{
> > + vlVaDriver *drv = NULL;
> > + VASurfaceAttribExternalBuffers *memory_attibute = NULL;
> > + VAStatus vaStatus = VA_STATUS_SUCCESS;
> > + struct pipe_video_buffer templat = {};
> > + struct pipe_screen *pscreen = NULL;
> > + int memory_type = VA_SURFACE_ATTRIB_MEM_TYPE_VA;
> > + int expected_fourcc = 0;
> > + int i = 0;
> > +
> Please drop the initializers, unless needed.
>
ok
>
> > + if (!ctx)
> > + return VA_STATUS_ERROR_INVALID_CONTEXT;
> > +
> > + if (!(width && height))
> > + return VA_STATUS_ERROR_INVALID_IMAGE_FORMAT;
> > +
> > + drv = VL_VA_DRIVER(ctx);
> > + pscreen = VL_VA_PSCREEN(ctx);
> > +
> > + for (i = 0; i < num_attribs && attrib_list; i++) {
> > + if ((attrib_list[i].type == VASurfaceAttribPixelFormat) &&
> > + (attrib_list[i].flags & VA_SURFACE_ATTRIB_SETTABLE)) {
> > + if (attrib_list[i].value.type != VAGenericValueTypeInteger)
> > + return VA_STATUS_ERROR_INVALID_PARAMETER;
> > + expected_fourcc = attrib_list[i].value.value.i;
> > + }
> > +
> > + if ((attrib_list[i].type == VASurfaceAttribMemoryType) &&
> > + (attrib_list[i].flags & VA_SURFACE_ATTRIB_SETTABLE)) {
> > +
> > + if (attrib_list[i].value.type != VAGenericValueTypeInteger)
> > + return VA_STATUS_ERROR_INVALID_PARAMETER;
> > +
> > + switch (attrib_list[i].value.value.i) {
> > + case VA_SURFACE_ATTRIB_MEM_TYPE_VA:
> > + default:{
> > + return VA_STATUS_ERROR_UNSUPPORTED_MEMORY_TYPE;}
> > + }
> > + }
> > +
> > + if ((attrib_list[i].type ==
> VASurfaceAttribExternalBufferDescriptor) &&
> > + (attrib_list[i].flags == VA_SURFACE_ATTRIB_SETTABLE)) {
> > + if (attrib_list[i].value.type != VAGenericValueTypePointer)
> > + return VA_STATUS_ERROR_INVALID_PARAMETER;
> > + memory_attibute = (VASurfaceAttribExternalBuffers
> *)attrib_list[i].value.value.p;
> > + }
> > + }
> > +
> > + /* support 420 & 422 & RGB32 format, 422 and RGB32 are only used
> > + * for post-processing (including color conversion) */
> > + if (VA_RT_FORMAT_YUV420 != format &&
> > + VA_RT_FORMAT_YUV422 != format &&
> > + VA_RT_FORMAT_YUV444 != format &&
> > + VA_RT_FORMAT_YUV411 != format &&
> > + VA_RT_FORMAT_YUV400 != format &&
> We don't have any support for 400 or 411. Why are they in the list ?
>
Just a copy past, oups :)
Thx
Julien
>
> Cheers,
> Emil
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151019/804c78ab/attachment-0001.html>
More information about the mesa-dev
mailing list