[Mesa-dev] [PATCH 05/12] st/va: add encode entrypoint
Christian König
deathsimple at vodafone.de
Thu Jul 21 09:45:45 UTC 2016
Am 20.07.2016 um 20:00 schrieb Zhang, Boyuan:
>
> >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.
>
> Sure, I agree. I will submit a new patch set to add this and other
> minor changes.
>
> >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.
>
> I just tried to disable slice encoding support, and even Gstreamer is
> not working. It will give error message saying “unsupported HW profile”.
>
> On the other hand, by exposing slice encoding, Gstreamer will still
> work as “Frame-in, Frame-out” mode. I didn’t see that Gstreamer will
> add extra headers. And by dumping the output 264 output, it seems
> gstreamer is using picture encoding. However, as my test shown,
> gstreamer will not work at all if we don’t expose slice encoding. Do
> you have any suggestions how we should do for this situation?
>
Good question. From the GStreamer source it looks like VA-API was
switched from returning only slice data to a full blown elementary
stream at some point.
Picture encoding seems to be only used for JPEG as far as I can see.
Anyway let's keep it like this for now and fix all the fallout we will
run into later on.
Regards,
Christian.
> Regards,
>
> Boyuan
>
> *From:*Christian König [mailto:deathsimple at vodafone.de]
> *Sent:* July-20-16 4:48 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 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>
> <mailto:deathsimple at vodafone.de>
> *Sent:* July 19, 2016 4:52 AM
> *To:* Zhang, Boyuan; mesa-dev at lists.freedesktop.org
> <mailto:mesa-dev at lists.freedesktop.org>
> *Cc:* adf.lists at gmail.com <mailto: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>
> <mailto: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/20160721/089beecb/attachment-0001.html>
More information about the mesa-dev
mailing list