[Mesa-dev] [PATCH 05/11] st/va: add encode entrypoint

Christian König deathsimple at vodafone.de
Mon Jul 18 13:38:11 UTC 2016


Am 16.07.2016 um 00:41 schrieb Boyuan Zhang:
> VAAPI passes PIPE_VIDEO_ENTRYPOINT_ENCODE as entry point for encoding case. We will save this encode entry point in config. config_id was used as profile previously. Now, config has both profile and entrypoint field, and config_id is used to get the config object. Later on, we pass this entrypoint to context->templat.entrypoint instead of always hardcoded to PIPE_VIDEO_ENTRYPOINT_BITSTREAM for decoding case previously.

Thanks that makes it a lot more clearer what's going on here.

Patch looks good to me in general, but you're missing one little thing 
and that is thread safety!

E.g. you need to lock/unlock the drv->mutex around accessing the handle 
table.

Additional to that one more question below.

>
> Signed-off-by: Boyuan Zhang <boyuan.zhang at amd.com>
> ---
>   src/gallium/state_trackers/va/config.c     | 61 +++++++++++++++++++++++++++---
>   src/gallium/state_trackers/va/context.c    | 57 ++++++++++++++++------------
>   src/gallium/state_trackers/va/surface.c    | 12 ++++--
>   src/gallium/state_trackers/va/va_private.h |  5 +++
>   4 files changed, 103 insertions(+), 32 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/config.c b/src/gallium/state_trackers/va/config.c
> index 9ca0aa8..73704a1 100644
> --- a/src/gallium/state_trackers/va/config.c
> +++ b/src/gallium/state_trackers/va/config.c
> @@ -34,6 +34,8 @@
>   
>   #include "va_private.h"
>   
> +#include "util/u_handle_table.h"
> +
>   DEBUG_GET_ONCE_BOOL_OPTION(mpeg4, "VAAPI_MPEG4_ENABLED", false)
>   
>   VAStatus
> @@ -128,14 +130,27 @@ VAStatus
>   vlVaCreateConfig(VADriverContextP ctx, VAProfile profile, VAEntrypoint entrypoint,
>                    VAConfigAttrib *attrib_list, int num_attribs, VAConfigID *config_id)
>   {
> +   vlVaDriver *drv;
> +   vlVaConfig *config;
>      struct pipe_screen *pscreen;
>      enum pipe_video_profile p;
>   
>      if (!ctx)
>         return VA_STATUS_ERROR_INVALID_CONTEXT;
>   
> +   drv = VL_VA_DRIVER(ctx);
> +
> +   if (!drv)
> +      return VA_STATUS_ERROR_INVALID_CONTEXT;
> +
> +   config = CALLOC(1, sizeof(vlVaConfig));
> +   if (!config)
> +      return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +
>      if (profile == VAProfileNone && entrypoint == VAEntrypointVideoProc) {
> -      *config_id = PIPE_VIDEO_PROFILE_UNKNOWN;
> +      config->entrypoint = VAEntrypointVideoProc;
> +      config->profile = PIPE_VIDEO_PROFILE_UNKNOWN;
> +      *config_id = handle_table_add(drv->htab, config);
>         return VA_STATUS_SUCCESS;
>      }
>   
> @@ -150,7 +165,14 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile profile, VAEntrypoint entrypoin
>      if (entrypoint != VAEntrypointVLD)
>         return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>   
> -   *config_id = p;
> +   if (entrypoint == VAEntrypointEncSlice || entrypoint == VAEntrypointEncPicture)

Are we really implementing both slice and picture level encoding?

Regards,
Christian.

> +      config->entrypoint = PIPE_VIDEO_ENTRYPOINT_ENCODE;
> +   else
> +      config->entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;
> +
> +   config->profile = p;
> +
> +   *config_id = handle_table_add(drv->htab, config);
>   
>      return VA_STATUS_SUCCESS;
>   }
> @@ -158,9 +180,25 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile profile, VAEntrypoint entrypoin
>   VAStatus
>   vlVaDestroyConfig(VADriverContextP ctx, VAConfigID config_id)
>   {
> +   vlVaDriver *drv;
> +   vlVaConfig *config;
> +
>      if (!ctx)
>         return VA_STATUS_ERROR_INVALID_CONTEXT;
>   
> +   drv = VL_VA_DRIVER(ctx);
> +
> +   if (!drv)
> +      return VA_STATUS_ERROR_INVALID_CONTEXT;
> +
> +   config = handle_table_get(drv->htab, config_id);
> +
> +   if (!config)
> +      return VA_STATUS_ERROR_INVALID_CONFIG;
> +
> +   FREE(config);
> +   handle_table_remove(drv->htab, config_id);
> +
>      return VA_STATUS_SUCCESS;
>   }
>   
> @@ -168,18 +206,31 @@ VAStatus
>   vlVaQueryConfigAttributes(VADriverContextP ctx, VAConfigID config_id, VAProfile *profile,
>                             VAEntrypoint *entrypoint, VAConfigAttrib *attrib_list, int *num_attribs)
>   {
> +   vlVaDriver *drv;
> +   vlVaConfig *config;
> +
>      if (!ctx)
>         return VA_STATUS_ERROR_INVALID_CONTEXT;
>   
> -   *profile = PipeToProfile(config_id);
> +   drv = VL_VA_DRIVER(ctx);
> +
> +   if (!drv)
> +      return VA_STATUS_ERROR_INVALID_CONTEXT;
> +
> +   config = handle_table_get(drv->htab, config_id);
> +
> +   if (!config)
> +      return VA_STATUS_ERROR_INVALID_CONFIG;
> +
> +   *profile = PipeToProfile(config->profile);
>   
> -   if (config_id == PIPE_VIDEO_PROFILE_UNKNOWN) {
> +   if (config->profile == PIPE_VIDEO_PROFILE_UNKNOWN) {
>         *entrypoint = VAEntrypointVideoProc;
>         *num_attribs = 0;
>         return VA_STATUS_SUCCESS;
>      }
>   
> -   *entrypoint = VAEntrypointVLD;
> +   *entrypoint = config->entrypoint;
>   
>      *num_attribs = 1;
>      attrib_list[0].type = VAConfigAttribRTFormat;
> diff --git a/src/gallium/state_trackers/va/context.c b/src/gallium/state_trackers/va/context.c
> index 402fbb2..b4334f4 100644
> --- a/src/gallium/state_trackers/va/context.c
> +++ b/src/gallium/state_trackers/va/context.c
> @@ -195,18 +195,21 @@ vlVaCreateContext(VADriverContextP ctx, VAConfigID config_id, int picture_width,
>   {
>      vlVaDriver *drv;
>      vlVaContext *context;
> +   vlVaConfig *config;
>      int is_vpp;
>   
>      if (!ctx)
>         return VA_STATUS_ERROR_INVALID_CONTEXT;
>   
> -   is_vpp = config_id == PIPE_VIDEO_PROFILE_UNKNOWN && !picture_width &&
> +   drv = VL_VA_DRIVER(ctx);
> +   config = handle_table_get(drv->htab, config_id);
> +
> +   is_vpp = config->profile == PIPE_VIDEO_PROFILE_UNKNOWN && !picture_width &&
>               !picture_height && !flag && !render_targets && !num_render_targets;
>   
>      if (!(picture_width && picture_height) && !is_vpp)
>         return VA_STATUS_ERROR_INVALID_IMAGE_FORMAT;
>   
> -   drv = VL_VA_DRIVER(ctx);
>      context = CALLOC(1, sizeof(vlVaContext));
>      if (!context)
>         return VA_STATUS_ERROR_ALLOCATION_FAILED;
> @@ -218,8 +221,8 @@ vlVaCreateContext(VADriverContextP ctx, VAConfigID config_id, int picture_width,
>            return VA_STATUS_ERROR_INVALID_CONTEXT;
>         }
>      } else {
> -      context->templat.profile = config_id;
> -      context->templat.entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;
> +      context->templat.profile = config->profile;
> +      context->templat.entrypoint = config->entrypoint;
>         context->templat.chroma_format = PIPE_VIDEO_CHROMA_FORMAT_420;
>         context->templat.width = picture_width;
>         context->templat.height = picture_height;
> @@ -234,16 +237,18 @@ vlVaCreateContext(VADriverContextP ctx, VAConfigID config_id, int picture_width,
>   
>         case PIPE_VIDEO_FORMAT_MPEG4_AVC:
>            context->templat.max_references = 0;
> -         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);
> -            return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +         if (config->entrypoint != PIPE_VIDEO_ENTRYPOINT_ENCODE) {
> +            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);
> +               return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +            }
>            }
>            break;
>   
> @@ -267,7 +272,9 @@ vlVaCreateContext(VADriverContextP ctx, VAConfigID config_id, int picture_width,
>         }
>      }
>   
> -   context->desc.base.profile = config_id;
> +   context->desc.base.profile = config->profile;
> +   context->desc.base.entry_point = config->entrypoint;
> +
>      pipe_mutex_lock(drv->mutex);
>      *context_id = handle_table_add(drv->htab, context);
>      pipe_mutex_unlock(drv->mutex);
> @@ -293,15 +300,17 @@ vlVaDestroyContext(VADriverContextP ctx, VAContextID context_id)
>      }
>   
>      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);
> -      }
> -      if (u_reduce_video_profile(context->decoder->profile) ==
> -            PIPE_VIDEO_FORMAT_HEVC) {
> -         FREE(context->desc.h265.pps->sps);
> -         FREE(context->desc.h265.pps);
> +      if (context->desc.base.entry_point != PIPE_VIDEO_ENTRYPOINT_ENCODE) {
> +         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 (u_reduce_video_profile(context->decoder->profile) ==
> +               PIPE_VIDEO_FORMAT_HEVC) {
> +            FREE(context->desc.h265.pps->sps);
> +            FREE(context->desc.h265.pps);
> +         }
>         }
>         context->decoder->destroy(context->decoder);
>      }
> diff --git a/src/gallium/state_trackers/va/surface.c b/src/gallium/state_trackers/va/surface.c
> index 3e74353..c581bb9 100644
> --- a/src/gallium/state_trackers/va/surface.c
> +++ b/src/gallium/state_trackers/va/surface.c
> @@ -319,17 +319,18 @@ vlVaUnlockSurface(VADriverContextP ctx, VASurfaceID surface)
>   }
>   
>   VAStatus
> -vlVaQuerySurfaceAttributes(VADriverContextP ctx, VAConfigID config,
> +vlVaQuerySurfaceAttributes(VADriverContextP ctx, VAConfigID config_id,
>                              VASurfaceAttrib *attrib_list, unsigned int *num_attribs)
>   {
>      vlVaDriver *drv;
> +   vlVaConfig *config;
>      VASurfaceAttrib *attribs;
>      struct pipe_screen *pscreen;
>      int i, j;
>   
>      STATIC_ASSERT(ARRAY_SIZE(vpp_surface_formats) <= VL_VA_MAX_IMAGE_FORMATS);
>   
> -   if (config == VA_INVALID_ID)
> +   if (config_id == VA_INVALID_ID)
>         return VA_STATUS_ERROR_INVALID_CONFIG;
>   
>      if (!attrib_list && !num_attribs)
> @@ -348,6 +349,11 @@ vlVaQuerySurfaceAttributes(VADriverContextP ctx, VAConfigID config,
>      if (!drv)
>         return VA_STATUS_ERROR_INVALID_CONTEXT;
>   
> +   config = handle_table_get(drv->htab, config_id);
> +
> +   if (!config)
> +      return VA_STATUS_ERROR_INVALID_CONFIG;
> +
>      pscreen = VL_VA_PSCREEN(ctx);
>   
>      if (!pscreen)
> @@ -363,7 +369,7 @@ vlVaQuerySurfaceAttributes(VADriverContextP ctx, VAConfigID config,
>   
>      /* vlVaCreateConfig returns PIPE_VIDEO_PROFILE_UNKNOWN
>       * only for VAEntrypointVideoProc. */
> -   if (config == PIPE_VIDEO_PROFILE_UNKNOWN) {
> +   if (config->profile == PIPE_VIDEO_PROFILE_UNKNOWN) {
>         for (j = 0; j < ARRAY_SIZE(vpp_surface_formats); ++j) {
>            attribs[i].type = VASurfaceAttribPixelFormat;
>            attribs[i].value.type = VAGenericValueTypeInteger;
> diff --git a/src/gallium/state_trackers/va/va_private.h b/src/gallium/state_trackers/va/va_private.h
> index d91de44..723983d 100644
> --- a/src/gallium/state_trackers/va/va_private.h
> +++ b/src/gallium/state_trackers/va/va_private.h
> @@ -244,6 +244,11 @@ typedef struct {
>   } vlVaContext;
>   
>   typedef struct {
> +   VAEntrypoint entrypoint;
> +   enum pipe_video_profile profile;
> +} vlVaConfig;
> +
> +typedef struct {
>      VABufferType type;
>      unsigned int size;
>      unsigned int num_elements;



More information about the mesa-dev mailing list