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

Christian König deathsimple at vodafone.de
Wed Jul 20 08:47:30 UTC 2016


Am 20.07.2016 um 06:12 schrieb Zhang, Boyuan:
>
> >> @@ -150,7 +167,16 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile 
> profile, VAEntrypoint entrypoin
> >>if (entrypoint != VAEntrypointVLD)
> >>         return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
> >>
> >> -   *config_id = p;
> >> +   if (entrypoint == VAEntrypointEncSlice || entrypoint == 
> VAEntrypointEncPicture)
> >> +      config->entrypoint = PIPE_VIDEO_ENTRYPOINT_ENCODE;
> >> +   else
> >> +      config->entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;
>
> >Well that doesn't make much sense here.
>
> >First we return and error if the entrypoint isn't VAEntrypointVLD and
> >then check if it's an encoding entry point.
>
> >Additional to that I already wondered if we are really going to support
> >slice level as well as picture level encoding.
>
> >I think that it should only be one of the two.
>
> >Regards,
> >Christian.
>
>
> Hi Christian,
>
>
> Sorry for the confusion, The first 2 lines of codes
>
> >>if (entrypoint != VAEntrypointVLD)
> >>         return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>
> will actually be removed in the last patch where we enable the VAAPI 
> Encode (Patch 12/12). In other word, we don't accept VAEncode 
> entrypoint until the time we enable VAAPI Encode. Therefore, we still 
> only accept VAEntrypointVLD at this patch.
>

Makes sense, but I suggest that in this case we should add at least a 
comment why this is still disabled.

And it would look better if we have an "#if 0" or something like this in 
the code which gets explicitly removed with the last patch.

>
> And we need to accept both picture level and slice level entrypoint. 
> For some application, e.g. libva h264encode test, if we don't enable 
> slice level encode, it will fail the call and report h264 encode is 
> not supported. If we enable both, it will still use picture level 
> encode. That's why I put both here.
>

The problem with slice level encoding is that we haven't fully 
implemented it. E.g. the last time I checked the h264encode test it 
would try to add an SPS and PPS in front of the slice data returned from 
our VA-API driver.

Since our VA-API driver doesn't return slice data, but rather a full 
blown elementary stream you end up with a complete mess which looks 
something like this:

SPS (added by the application), PPS (added by the application), Slice 
Header, SPS (added by the driver), PPS(added by the driver), Slice 
Header/Slice Data.

That might work in some if not most cases, but is certainly not 
complaint to the VA-API specification.

Christian.

>
> Regards,
>
> Boyuan
>
> ------------------------------------------------------------------------
> *From:* Christian König <deathsimple at vodafone.de>
> *Sent:* July 19, 2016 4:52 AM
> *To:* Zhang, Boyuan; mesa-dev at lists.freedesktop.org
> *Cc:* adf.lists at gmail.com
> *Subject:* Re: [PATCH 05/12] st/va: add encode entrypoint
> Am 19.07.2016 um 00:43 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.
> >
> > Signed-off-by: Boyuan Zhang <boyuan.zhang at amd.com>
> > ---
> >   src/gallium/state_trackers/va/config.c     | 69 
> +++++++++++++++++++++++++++---
> >   src/gallium/state_trackers/va/context.c    | 59 
> ++++++++++++++-----------
> >   src/gallium/state_trackers/va/surface.c    | 14 ++++--
> >   src/gallium/state_trackers/va/va_private.h |  5 +++
> >   4 files changed, 115 insertions(+), 32 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/va/config.c 
> b/src/gallium/state_trackers/va/config.c
> > index 9ca0aa8..7ea7e24 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,29 @@ 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;
> > +      pipe_mutex_lock(drv->mutex);
> > +      *config_id = handle_table_add(drv->htab, config);
> > +      pipe_mutex_unlock(drv->mutex);
> >         return VA_STATUS_SUCCESS;
> >      }
> >
> > @@ -150,7 +167,16 @@ vlVaCreateConfig(VADriverContextP ctx, 
> VAProfile profile, VAEntrypoint entrypoin
> >      if (entrypoint != VAEntrypointVLD)
> >         return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
> >
> > -   *config_id = p;
> > +   if (entrypoint == VAEntrypointEncSlice || entrypoint == 
> VAEntrypointEncPicture)
> > +      config->entrypoint = PIPE_VIDEO_ENTRYPOINT_ENCODE;
> > +   else
> > +      config->entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;
>
> Well that doesn't make much sense here.
>
> First we return and error if the entrypoint isn't VAEntrypointVLD and
> then check if it's an encoding entry point.
>
> Additional to that I already wondered if we are really going to support
> slice level as well as picture level encoding.
>
> I think that it should only be one of the two.
>
> Regards,
> Christian.
>
> > +
> > +   config->profile = p;
> > +
> > +   pipe_mutex_lock(drv->mutex);
> > +   *config_id = handle_table_add(drv->htab, config);
> > +   pipe_mutex_unlock(drv->mutex);
> >
> >      return VA_STATUS_SUCCESS;
> >   }
> > @@ -158,9 +184,27 @@ 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;
> > +
> > +   pipe_mutex_lock(drv->mutex);
> > +   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);
> > +   pipe_mutex_unlock(drv->mutex);
> > +
> >      return VA_STATUS_SUCCESS;
> >   }
> >
> > @@ -168,18 +212,33 @@ 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;
> > +
> > +   pipe_mutex_lock(drv->mutex);
> > +   config = handle_table_get(drv->htab, config_id);
> > +   pipe_mutex_unlock(drv->mutex);
> > +
> > +   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..8882cba 100644
> > --- a/src/gallium/state_trackers/va/context.c
> > +++ b/src/gallium/state_trackers/va/context.c
> > @@ -195,18 +195,23 @@ 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);
> > +   pipe_mutex_lock(drv->mutex);
> > +   config = handle_table_get(drv->htab, config_id);
> > +   pipe_mutex_unlock(drv->mutex);
> > +
> > +   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 +223,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 +239,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 +274,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 +302,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..8ce4143 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,13 @@ vlVaQuerySurfaceAttributes(VADriverContextP 
> ctx, VAConfigID config,
> >      if (!drv)
> >         return VA_STATUS_ERROR_INVALID_CONTEXT;
> >
> > +   pipe_mutex_lock(drv->mutex);
> > +   config = handle_table_get(drv->htab, config_id);
> > +   pipe_mutex_unlock(drv->mutex);
> > +
> > +   if (!config)
> > +      return VA_STATUS_ERROR_INVALID_CONFIG;
> > +
> >      pscreen = VL_VA_PSCREEN(ctx);
> >
> >      if (!pscreen)
> > @@ -363,7 +371,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;
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160720/9798d9b5/attachment-0001.html>


More information about the mesa-dev mailing list