[Mesa-dev] [PATCH] st/va: change frame_idx from array to hash table
Andres Gomez
agomez at igalia.com
Tue Nov 28 13:40:00 UTC 2017
Forgot to include mesa-stable ...
On Tue, 2017-11-28 at 15:32 +0200, Andres Gomez wrote:
> Julien, this looks like a good candidate to nominate for inclusion in
> the 17.2 stable queue.
>
> What do you think?
>
> On Tue, 2017-07-25 at 15:31 +0100, Julien Isorce wrote:
> > The picture_is was assumed to be a frame number so in 0-31.
> > But the vaapi client gstreamer-vaapi uses the surfaces handles
> > as identifier which are unsigned int.
> >
> > This bug can happen when using a lot of vaapi surfaces within
> > the same process. Indeed Mesa/st/va increments a counter for the
> > surface ID: mesa/util/u_handle_table.c::handle_table_add which
> > starts from 0 and incremented by 1 at each call.
> > So when creating above 32 surfaces it was a problem.
> >
> > The following bug contains a test that reproduces the problem
> > by running a couple of vaapih264enc in the same process. The above
> > also explains why there was no pb when running them in separated
> > processes.
> >
> > https://bugzilla.gnome.org/show_bug.cgi?id=785085
> > ---
> > src/gallium/include/pipe/p_video_state.h | 4 +++-
> > src/gallium/state_trackers/va/context.c | 9 +++++++--
> > src/gallium/state_trackers/va/picture.c | 11 ++++++++---
> > src/gallium/state_trackers/va/va_private.h | 13 +++++++++++++
> > 4 files changed, 31 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/gallium/include/pipe/p_video_state.h b/src/gallium/include/pipe/p_video_state.h
> > index 39b3905..53f9ab3 100644
> > --- a/src/gallium/include/pipe/p_video_state.h
> > +++ b/src/gallium/include/pipe/p_video_state.h
> > @@ -32,6 +32,7 @@
> > #include "pipe/p_format.h"
> > #include "pipe/p_state.h"
> > #include "pipe/p_screen.h"
> > +#include "util/u_hash_table.h"
> > #include "util/u_inlines.h"
> >
> > #ifdef __cplusplus
> > @@ -407,7 +408,8 @@ struct pipe_h264_enc_picture_desc
> > bool not_referenced;
> > bool is_idr;
> > bool enable_vui;
> > - unsigned int frame_idx[32];
> > + struct util_hash_table *frame_idx;
> > +
> > };
> >
> > struct pipe_h265_sps
> > diff --git a/src/gallium/state_trackers/va/context.c b/src/gallium/state_trackers/va/context.c
> > index 186f5066..f2cb37a 100644
> > --- a/src/gallium/state_trackers/va/context.c
> > +++ b/src/gallium/state_trackers/va/context.c
> > @@ -280,8 +280,10 @@ vlVaCreateContext(VADriverContextP ctx, VAConfigID config_id, int picture_width,
> >
> > context->desc.base.profile = config->profile;
> > context->desc.base.entry_point = config->entrypoint;
> > - if (config->entrypoint == PIPE_VIDEO_ENTRYPOINT_ENCODE)
> > + if (config->entrypoint == PIPE_VIDEO_ENTRYPOINT_ENCODE) {
> > context->desc.h264enc.rate_ctrl.rate_ctrl_method = config->rc;
> > + context->desc.h264enc.frame_idx = util_hash_table_create(handle_hash, handle_compare);
> > + }
> >
> > mtx_lock(&drv->mutex);
> > *context_id = handle_table_add(drv->htab, context);
> > @@ -308,7 +310,10 @@ vlVaDestroyContext(VADriverContextP ctx, VAContextID context_id)
> > }
> >
> > if (context->decoder) {
> > - if (context->desc.base.entry_point != PIPE_VIDEO_ENTRYPOINT_ENCODE) {
> > + if (context->desc.base.entry_point == PIPE_VIDEO_ENTRYPOINT_ENCODE) {
> > + if (context->desc.h264enc.frame_idx)
> > + util_hash_table_destroy (context->desc.h264enc.frame_idx);
> > + } else {
> > if (u_reduce_video_profile(context->decoder->profile) ==
> > PIPE_VIDEO_FORMAT_MPEG4_AVC) {
> > FREE(context->desc.h264.pps->sps);
> > diff --git a/src/gallium/state_trackers/va/picture.c b/src/gallium/state_trackers/va/picture.c
> > index 20fe750..338e090 100644
> > --- a/src/gallium/state_trackers/va/picture.c
> > +++ b/src/gallium/state_trackers/va/picture.c
> > @@ -427,7 +427,10 @@ handleVAEncPictureParameterBufferType(vlVaDriver *drv, vlVaContext *context, vlV
> > PIPE_USAGE_STREAM, coded_buf->size);
> > context->coded_buf = coded_buf;
> >
> > - context->desc.h264enc.frame_idx[h264->CurrPic.picture_id] = h264->frame_num;
> > + util_hash_table_set(context->desc.h264enc.frame_idx,
> > + UINT_TO_PTR(h264->CurrPic.picture_id),
> > + UINT_TO_PTR(h264->frame_num));
> > +
> > if (context->desc.h264enc.is_idr)
> > context->desc.h264enc.picture_type = PIPE_H264_ENC_PICTURE_TYPE_IDR;
> > else
> > @@ -455,11 +458,13 @@ handleVAEncSliceParameterBufferType(vlVaDriver *drv, vlVaContext *context, vlVaB
> > for (int i = 0; i < 32; i++) {
> > if (h264->RefPicList0[i].picture_id != VA_INVALID_ID) {
> > if (context->desc.h264enc.ref_idx_l0 == VA_INVALID_ID)
> > - context->desc.h264enc.ref_idx_l0 = context->desc.h264enc.frame_idx[h264->RefPicList0[i].picture_id];
> > + context->desc.h264enc.ref_idx_l0 = PTR_TO_UINT(util_hash_table_get(context->desc.h264enc.frame_idx,
> > + UINT_TO_PTR(h264->RefPicList0[i].picture_id)));
> > }
> > if (h264->RefPicList1[i].picture_id != VA_INVALID_ID && h264->slice_type == 1) {
> > if (context->desc.h264enc.ref_idx_l1 == VA_INVALID_ID)
> > - context->desc.h264enc.ref_idx_l1 = context->desc.h264enc.frame_idx[h264->RefPicList1[i].picture_id];
> > + context->desc.h264enc.ref_idx_l1 = PTR_TO_UINT(util_hash_table_get(context->desc.h264enc.frame_idx,
> > + UINT_TO_PTR(h264->RefPicList1[i].picture_id)));
> > }
> > }
> >
> > diff --git a/src/gallium/state_trackers/va/va_private.h b/src/gallium/state_trackers/va/va_private.h
> > index 9c32c08..dee0c2b 100644
> > --- a/src/gallium/state_trackers/va/va_private.h
> > +++ b/src/gallium/state_trackers/va/va_private.h
> > @@ -52,6 +52,19 @@
> > #define VL_VA_MAX_IMAGE_FORMATS 11
> > #define VL_VA_ENC_GOP_COEFF 16
> >
> > +#define UINT_TO_PTR(x) ((void*)(uintptr_t)(x))
> > +#define PTR_TO_UINT(x) ((unsigned)((intptr_t)(x)))
> > +
> > +static inline unsigned handle_hash(void *key)
> > +{
> > + return PTR_TO_UINT(key);
> > +}
> > +
> > +static inline int handle_compare(void *key1, void *key2)
> > +{
> > + return PTR_TO_UINT(key1) != PTR_TO_UINT(key2);
> > +}
> > +
> > static inline enum pipe_video_chroma_format
> > ChromaToPipe(int format)
> > {
--
Br,
Andres
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171128/840d7de9/attachment-0001.sig>
More information about the mesa-dev
mailing list