[Mesa-dev] [PATCH 6/7] st/va: add initial Video Post Processing support

Emil Velikov emil.l.velikov at gmail.com
Mon Oct 19 10:10:52 PDT 2015


On 17 October 2015 at 00:14, Julien Isorce <julien.isorce at gmail.com> wrote:
> Improve following functions to support VA_PROFILE_NONE profile (vpp):
> vlVaQueryConfigProfiles
> vlVaQueryConfigEntrypoints
> vlVaCreateConfig
> vlVaQueryConfigAttributes
>
> Add VADriverVTableVPP and improve following functions to support vpp:
> vlVaCreateContext
> vlVaDestroyContext
> vlVaBeginPicture
> vlVaRenderPicture
> vlVaEndPicture
>
Please split into two patches - roughly as per above grouping or
otherwise you feel is appropriate.

> Add handleVAProcPipelineParameterBufferType helper.
>
> One of the application is:
> VASurfaceNV12 -> gstvaapipostproc -> VASurfaceRGBA
>
> Signed-off-by: Julien Isorce <j.isorce at samsung.com>
> ---
>  src/gallium/state_trackers/va/config.c     | 20 +++++++
>  src/gallium/state_trackers/va/context.c    | 94 +++++++++++++++++++-----------
>  src/gallium/state_trackers/va/picture.c    | 89 +++++++++++++++++++++++++++-
>  src/gallium/state_trackers/va/surface.c    | 73 +++++++++++++++++++++++
>  src/gallium/state_trackers/va/va_private.h | 13 ++++-
>  5 files changed, 254 insertions(+), 35 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/config.c b/src/gallium/state_trackers/va/config.c
> index cfb0b25..bde6615 100644
> --- a/src/gallium/state_trackers/va/config.c
> +++ b/src/gallium/state_trackers/va/config.c
> @@ -52,6 +52,9 @@ vlVaQueryConfigProfiles(VADriverContextP ctx, VAProfile *profile_list, int *num_
>              profile_list[(*num_profiles)++] = vap;
>        }
>
> +   /* Support postprocessing through vl_compositor */
> +   profile_list[(*num_profiles)++] = VAProfileNone;
> +
>     return VA_STATUS_SUCCESS;
>  }
>
> @@ -67,6 +70,11 @@ vlVaQueryConfigEntrypoints(VADriverContextP ctx, VAProfile profile,
>
>     *num_entrypoints = 0;
>
> +   if (profile == VAProfileNone) {
> +       entrypoint_list[(*num_entrypoints)++] = VAEntrypointVideoProc;
> +       return VA_STATUS_SUCCESS;
> +   }
> +
>     p = ProfileToPipe(profile);
>     if (p == PIPE_VIDEO_PROFILE_UNKNOWN)
>        return VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
> @@ -118,6 +126,11 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile profile, VAEntrypoint entrypoin
>     if (!ctx)
>        return VA_STATUS_ERROR_INVALID_CONTEXT;
>
> +   if (profile == VAProfileNone && entrypoint == VAEntrypointVideoProc) {
> +       *config_id = PIPE_VIDEO_PROFILE_UNKNOWN;
> +       return VA_STATUS_SUCCESS;
> +   }
> +
>     p = ProfileToPipe(profile);
>     if (p == PIPE_VIDEO_PROFILE_UNKNOWN)
>        return VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
> @@ -151,6 +164,13 @@ vlVaQueryConfigAttributes(VADriverContextP ctx, VAConfigID config_id, VAProfile
>        return VA_STATUS_ERROR_INVALID_CONTEXT;
>
>     *profile = PipeToProfile(config_id);
> +
> +   if (config_id == PIPE_VIDEO_PROFILE_UNKNOWN) {
> +      *entrypoint = VAEntrypointVideoProc;
> +       *num_attribs = 0;
> +      return VA_STATUS_SUCCESS;
> +   }
> +
>     *entrypoint = VAEntrypointVLD;
>
>     *num_attribs = 1;
> diff --git a/src/gallium/state_trackers/va/context.c b/src/gallium/state_trackers/va/context.c
> index 8949d42..ddc863b 100644
> --- a/src/gallium/state_trackers/va/context.c
> +++ b/src/gallium/state_trackers/va/context.c
> @@ -87,6 +87,14 @@ static struct VADriverVTable vtable =
>     &vlVaQuerySurfaceAttributes
>  };
>
> +static struct VADriverVTableVPP vtable_vpp =
> +{
> +   VA_DRIVER_VTABLE_VPP_VERSION,
Please _never_ do such a thing. Afaict the define provides the VERSION
currently defined in the API, rather than the one implemented here.
They might align now, but things will break badly as the API gets
updated.

> +   &vlVaQueryVideoProcFilters,
> +   &vlVaQueryVideoProcFilterCaps,
> +   &vlVaQueryVideoProcPipelineCaps
> +};
> +
>  PUBLIC VAStatus
>  VA_DRIVER_INIT_FUNC(VADriverContextP ctx)
>  {
> @@ -122,6 +130,7 @@ VA_DRIVER_INIT_FUNC(VADriverContextP ctx)
>     ctx->version_major = 0;
>     ctx->version_minor = 1;
>     *ctx->vtable = vtable;
> +   *ctx->vtable_vpp = vtable_vpp;
>     ctx->max_profiles = PIPE_VIDEO_PROFILE_MPEG4_AVC_HIGH - PIPE_VIDEO_PROFILE_UNKNOWN;
>     ctx->max_entrypoints = 1;
>     ctx->max_attributes = 1;
> @@ -151,11 +160,16 @@ vlVaCreateContext(VADriverContextP ctx, VAConfigID config_id, int picture_width,
>     struct pipe_video_codec templat = {};
>     vlVaDriver *drv;
>     vlVaContext *context;
> +   int is_vpp = 0;
Drop the = 0 part.

>
>     if (!ctx)
>        return VA_STATUS_ERROR_INVALID_CONTEXT;
>
> -   if (!(picture_width && picture_height))
> +   is_vpp = config_id == PIPE_VIDEO_PROFILE_UNKNOWN &&
> +       picture_width == 0 && picture_height == 0 && flag ==0 && !render_targets
> +       && num_render_targets == 0;
Please indent - all the conditionals should start at the same column
and nuke the == 0.

> +
> +   if (!(picture_width && picture_height) && !is_vpp)
>        return VA_STATUS_ERROR_INVALID_IMAGE_FORMAT;
>
>     drv = VL_VA_DRIVER(ctx);
> @@ -163,38 +177,48 @@ vlVaCreateContext(VADriverContextP ctx, VAConfigID config_id, int picture_width,
>     if (!context)
>        return VA_STATUS_ERROR_ALLOCATION_FAILED;
>
> -   templat.profile = config_id;
> -   templat.entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;
> -   templat.chroma_format = PIPE_VIDEO_CHROMA_FORMAT_420;
> -   templat.width = picture_width;
> -   templat.height = picture_height;
> -   templat.max_references = num_render_targets;
> -   templat.expect_chunked_decode = true;
> -
> -   if (u_reduce_video_profile(templat.profile) ==
> -       PIPE_VIDEO_FORMAT_MPEG4_AVC)
> -      templat.level = u_get_h264_level(templat.width, templat.height,
> -                            &templat.max_references);
> -
> -   context->decoder = drv->pipe->create_video_codec(drv->pipe, &templat);
> -   if (!context->decoder) {
> -      FREE(context);
> -      return VA_STATUS_ERROR_ALLOCATION_FAILED;
> -   }
> -
> -   if (u_reduce_video_profile(context->decoder->profile) ==
> -         PIPE_VIDEO_FORMAT_MPEG4_AVC) {
> -      context->desc.h264.pps = CALLOC_STRUCT(pipe_h264_pps);
> -      if (!context->desc.h264.pps) {
> +   if (is_vpp) {
> +      context->decoder = NULL;
> +      if (!drv->compositor.upload) {
>           FREE(context);
> -         return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +         return VA_STATUS_ERROR_INVALID_CONTEXT;
>        }
> -      context->desc.h264.pps->sps = CALLOC_STRUCT(pipe_h264_sps);
> -      if (!context->desc.h264.pps->sps) {
> -         FREE(context->desc.h264.pps);
> +   } else {
> +      templat.profile = config_id;
> +      templat.entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;
> +      templat.chroma_format = PIPE_VIDEO_CHROMA_FORMAT_420;
> +      templat.width = picture_width;
> +      templat.height = picture_height;
> +      templat.max_references = 2;
> +      templat.expect_chunked_decode = true;
> +
> +      if (u_reduce_video_profile(templat.profile) ==
> +        PIPE_VIDEO_FORMAT_MPEG4_AVC) {
> +        templat.max_references = 16;
> +        templat.level = u_get_h264_level(templat.width, templat.height,
> +                             &templat.max_references);
> +      }
> +
Why the max_references changes ? Shouldn't those be a separate patch ?

> +      context->decoder = drv->pipe->create_video_codec(drv->pipe, &templat);
> +      if (!context->decoder) {
>           FREE(context);
>           return VA_STATUS_ERROR_ALLOCATION_FAILED;
>        }
> +
> +      if (u_reduce_video_profile(context->decoder->profile) ==
> +         PIPE_VIDEO_FORMAT_MPEG4_AVC) {
> +         context->desc.h264.pps = CALLOC_STRUCT(pipe_h264_pps);
> +         if (!context->desc.h264.pps) {
> +            FREE(context);
> +            return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +         }
> +         context->desc.h264.pps->sps = CALLOC_STRUCT(pipe_h264_sps);
> +         if (!context->desc.h264.pps->sps) {
> +            FREE(context->desc.h264.pps);
> +            FREE(context);
(Mildly related) Keeping track of all these might not fair well. Most
of st/va and other video st do this, yet (iirc) Christian did not see
any issues with opting for goto statements.

> +            return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +         }
> +      }
>     }
>
>     context->desc.base.profile = config_id;
> @@ -214,12 +238,16 @@ vlVaDestroyContext(VADriverContextP ctx, VAContextID context_id)
>
>     drv = VL_VA_DRIVER(ctx);
>     context = handle_table_get(drv->htab, context_id);
> -   if (u_reduce_video_profile(context->decoder->profile) ==
> -         PIPE_VIDEO_FORMAT_MPEG4_AVC) {
> -      FREE(context->desc.h264.pps->sps);
> -      FREE(context->desc.h264.pps);
> +
> +   if (context->decoder) {
> +      if (u_reduce_video_profile(context->decoder->profile) ==
> +            PIPE_VIDEO_FORMAT_MPEG4_AVC) {
> +         FREE(context->desc.h264.pps->sps);
> +         FREE(context->desc.h264.pps);
> +      }
> +      context->decoder->destroy(context->decoder);
>     }
> -   context->decoder->destroy(context->decoder);
> +
>     FREE(context);
>     handle_table_remove(drv->htab, context_id);
>
> diff --git a/src/gallium/state_trackers/va/picture.c b/src/gallium/state_trackers/va/picture.c
> index 9b94b39..0b42afe 100644
> --- a/src/gallium/state_trackers/va/picture.c
> +++ b/src/gallium/state_trackers/va/picture.c
> @@ -32,6 +32,7 @@
>  #include "util/u_video.h"
>
>  #include "vl/vl_vlc.h"
> +#include "vl/vl_winsys.h"
>
>  #include "va_private.h"
>
> @@ -58,6 +59,16 @@ vlVaBeginPicture(VADriverContextP ctx, VAContextID context_id, VASurfaceID rende
>        return VA_STATUS_ERROR_INVALID_SURFACE;
>
>     context->target = surf->buffer;
> +
> +   if (!context->decoder) {
> +      /* VPP */
> +      if ((context->target->buffer_format != PIPE_FORMAT_B8G8R8A8_UNORM  &&
> +           context->target->buffer_format != PIPE_FORMAT_R8G8B8A8_UNORM) ||
> +           context->target->interlaced)
> +          return VA_STATUS_ERROR_UNIMPLEMENTED;
> +      return VA_STATUS_SUCCESS;
> +   }
> +
>     context->decoder->begin_frame(context->decoder, context->target, NULL);
>
>     return VA_STATUS_SUCCESS;
> @@ -521,11 +532,79 @@ handleVASliceDataBufferType(vlVaContext *context, vlVaBuffer *buf)
>        num_buffers, (const void * const*)buffers, sizes);
>  }
>
> +static VAStatus
> +handleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext *context, vlVaBuffer *buf)
> +{
> +    struct u_rect src_rect = {0};
> +    struct u_rect dst_rect = {0};
> +    struct u_rect *dirty_area = NULL;
> +    vlVaSurface *src_surface = NULL;
> +    VAProcPipelineParameterBuffer *pipeline_param = NULL;
> +    struct pipe_surface **surfaces = NULL;
> +    struct pipe_screen *screen = NULL;
> +    struct pipe_surface *psurf = NULL;
Cough :)

> +
> +    if (!drv || !context)
> +       return VA_STATUS_ERROR_INVALID_CONTEXT;
> +
> +    if (!buf || !buf->data)
> +       return VA_STATUS_ERROR_INVALID_BUFFER;
> +
> +    if (!context->target)
> +        return VA_STATUS_ERROR_INVALID_SURFACE;
> +
> +    pipeline_param = buf->data; // cast
> +
> +    src_surface = handle_table_get(drv->htab, pipeline_param->surface);
> +    if (!src_surface || !src_surface->buffer)
> +       return VA_STATUS_ERROR_INVALID_SURFACE;
> +
> +    screen = drv->pipe->screen;
> +
> +    if(src_surface->fence) {
> +       screen->fence_finish(screen, src_surface->fence, PIPE_TIMEOUT_INFINITE);
> +       screen->fence_reference(screen, &src_surface->fence, NULL);
> +    }
> +
> +    surfaces = context->target->get_surfaces(context->target);
> +
> +    if (!surfaces)
if (!surfaces || !surfaces[0]) ?

> +        return VA_STATUS_ERROR_INVALID_SURFACE;
> +
> +    psurf = surfaces[0];
> +
> +    if (!psurf)
> +       return VA_STATUS_ERROR_INVALID_SURFACE;
> +
> +    src_rect.x0 = pipeline_param->surface_region->x;
> +    src_rect.y0 = pipeline_param->surface_region->y;
> +    src_rect.x1 = pipeline_param->surface_region->x + pipeline_param->surface_region->width;
> +    src_rect.y1 = pipeline_param->surface_region->y + pipeline_param->surface_region->height;
> +
> +    dst_rect.x0 = pipeline_param->output_region->x;
> +    dst_rect.y0 = pipeline_param->output_region->y;
> +    dst_rect.x1 = pipeline_param->output_region->x + pipeline_param->output_region->width;
> +    dst_rect.y1 = pipeline_param->output_region->y + pipeline_param->output_region->height;
> +
> +    dirty_area = vl_screen_get_dirty_area(drv->vscreen);
> +
> +    vl_compositor_clear_layers(&drv->cstate);
> +    vl_compositor_set_buffer_layer(&drv->cstate, &drv->compositor, 0, src_surface->buffer, &src_rect, NULL, VL_COMPOSITOR_WEAVE);
> +    vl_compositor_set_layer_dst_area(&drv->cstate, 0, &dst_rect);
> +    vl_compositor_render(&drv->cstate, &drv->compositor, psurf, dirty_area, true);
> +
> +    screen->fence_reference(screen, &src_surface->fence, NULL);
> +    drv->pipe->flush(drv->pipe, &src_surface->fence, 0);
> +
> +    return VA_STATUS_SUCCESS;
> +}
> +
>  VAStatus
>  vlVaRenderPicture(VADriverContextP ctx, VAContextID context_id, VABufferID *buffers, int num_buffers)
>  {
>     vlVaDriver *drv;
>     vlVaContext *context;
> +   VAStatus vaStatus = VA_STATUS_SUCCESS;
>
>     unsigned i;
>
> @@ -561,13 +640,16 @@ vlVaRenderPicture(VADriverContextP ctx, VAContextID context_id, VABufferID *buff
>        case VASliceDataBufferType:
>           handleVASliceDataBufferType(context, buf);
>           break;
> +      case VAProcPipelineParameterBufferType:
> +         vaStatus = handleVAProcPipelineParameterBufferType(drv, context, buf);
> +         break;
>
>        default:
>           break;
>        }
>     }
>
> -   return VA_STATUS_SUCCESS;
> +   return vaStatus;
>  }
>
>  VAStatus
> @@ -587,6 +669,11 @@ vlVaEndPicture(VADriverContextP ctx, VAContextID context_id)
>     if (!context)
>        return VA_STATUS_ERROR_INVALID_CONTEXT;
>
> +   if (!context->decoder) {
> +      /* VPP */
> +      return VA_STATUS_SUCCESS;
> +   }
> +
>     context->mpeg4.frame_num++;
>     context->decoder->end_frame(context->decoder, context->target, &context->desc.base);
>
> diff --git a/src/gallium/state_trackers/va/surface.c b/src/gallium/state_trackers/va/surface.c
> index eb5b8ca..bfa4803 100644
> --- a/src/gallium/state_trackers/va/surface.c
> +++ b/src/gallium/state_trackers/va/surface.c
> @@ -625,3 +625,76 @@ no_res:
>
>     return VA_STATUS_ERROR_ALLOCATION_FAILED;
>  }
> +
> +VAStatus
> +vlVaQueryVideoProcFilters(VADriverContextP ctx, VAContextID context,
> +                          VAProcFilterType *filters, unsigned int *num_filters)
> +{
> +   unsigned int num = 0;
> +
> +   if (!ctx)
> +      return VA_STATUS_ERROR_INVALID_CONTEXT;
> +
> +   if (!num_filters || !filters)
> +      return VA_STATUS_ERROR_INVALID_PARAMETER;
> +
> +   filters[num++] = VAProcFilterNone;
> +
> +   *num_filters = num;
> +
> +   return VA_STATUS_SUCCESS;
> +}
> +
> +VAStatus
> +vlVaQueryVideoProcFilterCaps(VADriverContextP ctx, VAContextID context,
> +                             VAProcFilterType type, void *filter_caps,
> +                             unsigned int *num_filter_caps)
> +{
> +   if (!ctx)
> +      return VA_STATUS_ERROR_INVALID_CONTEXT;
> +
> +   return VA_STATUS_ERROR_UNIMPLEMENTED;
Something looks fishy here. Shouldn't one return SUCCESS or  ENOTSUPP
like error, when we have FilterNone ?

> +}
> +
> +static VAProcColorStandardType vpp_input_color_standards[VAProcColorStandardCount] = {
> +   VAProcColorStandardBT601
> +};
> +
> +static VAProcColorStandardType vpp_output_color_standards[VAProcColorStandardCount] = {
> +   VAProcColorStandardBT601
> +};
> +
> +VAStatus
> +vlVaQueryVideoProcPipelineCaps(VADriverContextP ctx, VAContextID context,
> +                               VABufferID *filters, unsigned int num_filters,
> +                               VAProcPipelineCaps *pipeline_cap)
> +{
> +   unsigned int i = 0;
> +
> +   if (!ctx)
> +      return VA_STATUS_ERROR_INVALID_CONTEXT;
> +
> +   if (!pipeline_cap)
> +   return VA_STATUS_ERROR_INVALID_PARAMETER;
> +
> +   if (num_filters && !filters)
> +      return VA_STATUS_ERROR_INVALID_PARAMETER;
> +
> +   pipeline_cap->pipeline_flags = 0;
> +   pipeline_cap->filter_flags = 0;
> +   pipeline_cap->num_forward_references = 0;
> +   pipeline_cap->num_backward_references = 0;
> +   pipeline_cap->num_input_color_standards = 1;
> +   pipeline_cap->input_color_standards = vpp_input_color_standards;
> +   pipeline_cap->num_output_color_standards = 1;
> +   pipeline_cap->output_color_standards = vpp_output_color_standards;
> +
> +   for (i = 0; i < num_filters; i++) {
> +      vlVaBuffer *buf = handle_table_get(VL_VA_DRIVER(ctx)->htab, filters[i]);
> +
> +      if (!buf || buf->type >= VABufferTypeMax)
> +         return VA_STATUS_ERROR_INVALID_BUFFER;
> +   }
> +
> +   return VA_STATUS_SUCCESS;
> +}
> diff --git a/src/gallium/state_trackers/va/va_private.h b/src/gallium/state_trackers/va/va_private.h
> index 2cdd787..d3a29bc 100644
> --- a/src/gallium/state_trackers/va/va_private.h
> +++ b/src/gallium/state_trackers/va/va_private.h
> @@ -33,6 +33,7 @@
>
>  #include <va/va.h>
>  #include <va/va_backend.h>
> +#include <va/va_backend_vpp.h>
>  #include <va/va_drmcommon.h>
>
>  #include "pipe/p_video_enums.h"
> @@ -148,7 +149,8 @@ PipeToProfile(enum pipe_video_profile profile)
>     case PIPE_VIDEO_PROFILE_MPEG4_AVC_HIGH:
>        return VAProfileH264High;
>     case PIPE_VIDEO_PROFILE_MPEG4_AVC_EXTENDED:
> -       return VAProfileNone;
> +   case PIPE_VIDEO_PROFILE_UNKNOWN:
> +      return VAProfileNone;
>     default:
>        assert(0);
>        return -1;
> @@ -179,6 +181,8 @@ ProfileToPipe(VAProfile profile)
>        return PIPE_VIDEO_PROFILE_MPEG4_AVC_MAIN;
>     case VAProfileH264High:
>        return PIPE_VIDEO_PROFILE_MPEG4_AVC_HIGH;
> +   case VAProfileNone:
> +       return PIPE_VIDEO_PROFILE_UNKNOWN;
Mapping ProfileNone to PROFILE_UNKNOW and vice versa (incl. all the
uses above) seems like an abuse ?

Cheers
Emil


More information about the mesa-dev mailing list