Ack. And I'll move this bit at the end just before the other FREE. Thx
-----Original Message-----
From: Christian König [mailto:deathsim...@vodafone.de]
Sent: 29 October 2015 12:22
To: Julien Isorce; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 1/2] st/va: implement VaDeriveImage
Patch #1:
> + if (buf->data)
> + FREE(buf->data);
FREE() usually does a NULL check anyway.
Apart from that minor nitpick the patch is Reviewed-by: Christian König
<christian.koe...@amd.com>
Regards,
Christian.
On 29.10.2015 12:47, Julien Isorce wrote:
> And apply relatives change to:
> vlVaBufferSetNumElements
> vlVaCreateBuffer
> vlVaMapBuffer
> vlVaUnmapBuffer
> vlVaDestroyBuffer
> vlVaPutImage
>
> It is unfortunate that there is no proper va buffer type and struct
> for this. Only possible to use VAImageBufferType which is normally
> used for normal user data array.
> On of the consequences is that it is only possible VaDeriveImage is
> only useful on surfaces backed with contiguous planes.
> Implementation inspired from cgit.freedesktop.org/vaapi/intel-driver
>
> Signed-off-by: Julien Isorce <j.iso...@samsung.com>
> ---
> src/gallium/state_trackers/va/buffer.c | 47 +++++++++++++--
> src/gallium/state_trackers/va/image.c | 92
> +++++++++++++++++++++++++++++-
> src/gallium/state_trackers/va/va_private.h | 5 ++
> 3 files changed, 138 insertions(+), 6 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/buffer.c
> b/src/gallium/state_trackers/va/buffer.c
> index 8f9ba44..b619d0b 100644
> --- a/src/gallium/state_trackers/va/buffer.c
> +++ b/src/gallium/state_trackers/va/buffer.c
> @@ -26,8 +26,11 @@
> *
>
> **********************************************************************
> ****/
>
> +#include "pipe/p_screen.h"
> #include "util/u_memory.h"
> #include "util/u_handle_table.h"
> +#include "util/u_transfer.h"
> +#include "vl/vl_winsys.h"
>
> #include "va_private.h"
>
> @@ -73,6 +76,10 @@ vlVaBufferSetNumElements(VADriverContextP ctx, VABufferID
> buf_id,
> return VA_STATUS_ERROR_INVALID_CONTEXT;
>
> buf = handle_table_get(VL_VA_DRIVER(ctx)->htab, buf_id);
> +
> + if (!buf || buf->derived_surface.resource)
> + return VA_STATUS_ERROR_INVALID_BUFFER;
> +
> buf->data = REALLOC(buf->data, buf->size * buf->num_elements,
> buf->size * num_elements);
> buf->num_elements = num_elements; @@ -86,16 +93,32 @@
> vlVaBufferSetNumElements(VADriverContextP ctx, VABufferID buf_id,
> VAStatus
> vlVaMapBuffer(VADriverContextP ctx, VABufferID buf_id, void **pbuff)
> {
> + vlVaDriver *drv;
> vlVaBuffer *buf;
>
> if (!ctx)
> return VA_STATUS_ERROR_INVALID_CONTEXT;
>
> - buf = handle_table_get(VL_VA_DRIVER(ctx)->htab, buf_id);
> + if (!pbuff)
> + return VA_STATUS_ERROR_INVALID_BUFFER;
> +
> + drv = VL_VA_DRIVER(ctx);
> +
> + buf = handle_table_get(drv->htab, buf_id);
> if (!buf)
> return VA_STATUS_ERROR_INVALID_BUFFER;
>
> - *pbuff = buf->data;
> + if (buf->derived_surface.resource) {
> + *pbuff = pipe_buffer_map(drv->pipe, buf->derived_surface.resource,
> + PIPE_TRANSFER_WRITE,
> + &buf->derived_surface.transfer);
> +
> + if (!buf->derived_surface.transfer || !*pbuff)
> + return VA_STATUS_ERROR_INVALID_BUFFER;
> +
> + } else {
> + *pbuff = buf->data;
> + }
>
> return VA_STATUS_SUCCESS;
> }
> @@ -103,16 +126,25 @@ vlVaMapBuffer(VADriverContextP ctx, VABufferID buf_id,
> void **pbuff)
> VAStatus
> vlVaUnmapBuffer(VADriverContextP ctx, VABufferID buf_id)
> {
> + vlVaDriver *drv;
> vlVaBuffer *buf;
>
> if (!ctx)
> return VA_STATUS_ERROR_INVALID_CONTEXT;
>
> - buf = handle_table_get(VL_VA_DRIVER(ctx)->htab, buf_id);
> + drv = VL_VA_DRIVER(ctx);
> +
> + buf = handle_table_get(drv->htab, buf_id);
> if (!buf)
> return VA_STATUS_ERROR_INVALID_BUFFER;
>
> - /* Nothing to do here */
> + if (buf->derived_surface.resource) {
> + if (!buf->derived_surface.transfer)
> + return VA_STATUS_ERROR_INVALID_BUFFER;
> +
> + pipe_buffer_unmap(drv->pipe, buf->derived_surface.transfer);
> + buf->derived_surface.transfer = NULL;
> + }
>
> return VA_STATUS_SUCCESS;
> }
> @@ -129,7 +161,12 @@ vlVaDestroyBuffer(VADriverContextP ctx, VABufferID
> buf_id)
> if (!buf)
> return VA_STATUS_ERROR_INVALID_BUFFER;
>
> - FREE(buf->data);
> + if (buf->data)
> + FREE(buf->data);
> +
> + if (buf->derived_surface.resource)
> + pipe_resource_reference(&buf->derived_surface.resource, NULL);
> +
> FREE(buf);
> handle_table_remove(VL_VA_DRIVER(ctx)->htab, buf_id);
>
> diff --git a/src/gallium/state_trackers/va/image.c
> b/src/gallium/state_trackers/va/image.c
> index 8e64673..f266ce8 100644
> --- a/src/gallium/state_trackers/va/image.c
> +++ b/src/gallium/state_trackers/va/image.c
> @@ -184,10 +184,95 @@ vlVaCreateImage(VADriverContextP ctx, VAImageFormat
> *format, int width, int heig
> VAStatus
> vlVaDeriveImage(VADriverContextP ctx, VASurfaceID surface, VAImage *image)
> {
> + vlVaDriver *drv;
> + vlVaSurface *surf;
> + vlVaBuffer *img_buf;
> + VAImage *img;
> + struct pipe_surface **surfaces;
> + int w;
> + int h;
> + int i;
> +
> if (!ctx)
> return VA_STATUS_ERROR_INVALID_CONTEXT;
>
> - return VA_STATUS_ERROR_UNIMPLEMENTED;
> + drv = VL_VA_DRIVER(ctx);
> +
> + if (!drv)
> + return VA_STATUS_ERROR_INVALID_CONTEXT;
> +
> + surf = handle_table_get(drv->htab, surface);
> +
> + if (!surf || !surf->buffer || surf->buffer->interlaced)
> + return VA_STATUS_ERROR_INVALID_SURFACE;
> +
> + surfaces = surf->buffer->get_surfaces(surf->buffer);
> + if (!surfaces || !surfaces[0]->texture)
> + return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +
> + img = CALLOC(1, sizeof(VAImage));
> + if (!img)
> + return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +
> + img->format.fourcc = PipeToYCbCr(surf->buffer->buffer_format);
> + img->buf = VA_INVALID_ID;
> + img->width = surf->buffer->width;
> + img->height = surf->buffer->height;
> + img->num_palette_entries = 0;
> + img->entry_bytes = 0;
> + w = align(surf->buffer->width, 2);
> + h = align(surf->buffer->height, 2);
> +
> + for (i = 0; i < ARRAY_SIZE(formats); ++i) {
> + if (img->format.fourcc == formats[i].fourcc)
> + img->format = formats[i];
> + }
> +
> + switch (img->format.fourcc) {
> + case VA_FOURCC('U','Y','V','Y'):
> + case VA_FOURCC('Y','U','Y','V'):
> + img->num_planes = 1;
> + img->pitches[0] = w * 2;
> + img->offsets[0] = 0;
> + img->data_size = w * h * 2;
> + break;
> +
> + case VA_FOURCC('B','G','R','A'):
> + case VA_FOURCC('R','G','B','A'):
> + case VA_FOURCC('B','G','R','X'):
> + case VA_FOURCC('R','G','B','X'):
> + img->num_planes = 1;
> + img->pitches[0] = w * 4;
> + img->offsets[0] = 0;
> + img->data_size = w * h * 4;
> + break;
> +
> + default:
> + /* VaDeriveImage is designed for contiguous planes. */
> + FREE(img);
> + return VA_STATUS_ERROR_INVALID_IMAGE_FORMAT;
> + }
> +
> + img_buf = CALLOC(1, sizeof(vlVaBuffer));
> + if (!img_buf) {
> + FREE(img);
> + return VA_STATUS_ERROR_ALLOCATION_FAILED;
> + }
> +
> + img->image_id = handle_table_add(drv->htab, img);
> +
> + img_buf->type = VAImageBufferType;
> + img_buf->size = image->data_size;
> + img_buf->num_elements = 1;
> + img_buf->derived_surface.fence = surf->fence;
> +
> + pipe_resource_reference(&img_buf->derived_surface.resource,
> + surfaces[0]->texture);
> +
> + img->buf = handle_table_add(VL_VA_DRIVER(ctx)->htab, img_buf);
> +
> + *image = *img;
> +
> + return VA_STATUS_SUCCESS;
> }
>
> VAStatus
> @@ -342,6 +427,11 @@ vlVaPutImage(VADriverContextP ctx, VASurfaceID surface,
> VAImageID image,
> if (!img_buf)
> return VA_STATUS_ERROR_INVALID_BUFFER;
>
> + if (img_buf->derived_surface.resource) {
> + /* Attempting to transfer derived image to surface */
> + return VA_STATUS_ERROR_UNIMPLEMENTED;
> + }
> +
> format = YCbCrToPipe(vaimage->format.fourcc);
> if (format == PIPE_FORMAT_NONE)
> return VA_STATUS_ERROR_OPERATION_FAILED; diff --git
> a/src/gallium/state_trackers/va/va_private.h
> b/src/gallium/state_trackers/va/va_private.h
> index b062357..40363b3 100644
> --- a/src/gallium/state_trackers/va/va_private.h
> +++ b/src/gallium/state_trackers/va/va_private.h
> @@ -236,6 +236,11 @@ typedef struct {
> unsigned int size;
> unsigned int num_elements;
> void *data;
> + struct {
> + struct pipe_resource *resource;
> + struct pipe_transfer *transfer;
> + struct pipe_fence_handle *fence;
> + } derived_surface;
> } vlVaBuffer;
>
> typedef struct {
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
