[Mesa-dev] [PATCH 4/7] st/va: implement VaCreateSurfaces2 and VaQuerySurfaceAttributes
Emil Velikov
emil.l.velikov at gmail.com
Mon Oct 19 08:55:33 PDT 2015
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.
> 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 ?
> +
> +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.
> + 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.
> + 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.
> + 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 ?
Cheers,
Emil
More information about the mesa-dev
mailing list