[Mesa-dev] [PATCH v2 1/2] st/va: if h264 then delay decoder creation until max_references is known

[Emil Velikov]

Hi Julien,

On 25 November 2015 at 09:07, Julien Isorce <julien.isorce at gmail.com> wrote:
> From: Julien Isorce <julien.isorce at gmail.com>
>
> In general max_references cannot be based on num_render_targets.
>
> This patch allow to allocate accurate sizes for buffers.
> For other codecs it is a fixed value to 2.
>
> This is similar behaviour as vaapi/vdpau-driver.
>
> XXX: do the same for HEVC
>
> Signed-off-by: Julien Isorce <j.isorce at samsung.com>
> ---
>  src/gallium/state_trackers/va/context.c      | 41 ++++++++++++++--------------
>  src/gallium/state_trackers/va/picture.c      | 37 ++++++++++++++++++-------
>  src/gallium/state_trackers/va/picture_h264.c | 29 +++++++++++++++++++-
>  src/gallium/state_trackers/va/va_private.h   |  4 +--
>  4 files changed, 78 insertions(+), 33 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/context.c b/src/gallium/state_trackers/va/context.c
> index f0051e5..985007b 100644
> --- a/src/gallium/state_trackers/va/context.c
> +++ b/src/gallium/state_trackers/va/context.c

> +      /* Can only create decoders for which max_references is known. */
> +      if (u_reduce_video_profile(context->templat.profile) !=
> +         PIPE_VIDEO_FORMAT_MPEG4_AVC) {
> +         context->decoder = drv->pipe->create_video_codec(drv->pipe,
> +            &context->templat);
> +         if (!context->decoder) {
> +            FREE(context);
> +            return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +         }
>        }
What is the benefit of delaying only h264 ? Won't this make things
less obvious and you'll have more subtle checks throughout ?

> --- a/src/gallium/state_trackers/va/picture.c
> +++ b/src/gallium/state_trackers/va/picture.c
> @@ -60,6 +60,12 @@ vlVaBeginPicture(VADriverContextP ctx, VAContextID context_id, VASurfaceID rende
>
>     context->target = surf->buffer;
>     if (!context->decoder) {
> +      /* Decoder creation is delayed until max_references is set. */
> +      if (u_reduce_video_profile(context->templat.profile) ==
> +          PIPE_VIDEO_FORMAT_MPEG4_AVC)
> +         return context->templat.max_references == 0 ?
> +            VA_STATUS_SUCCESS : VA_STATUS_ERROR_INVALID_CONTEXT;
> +
Can we really hit the VA_STATUS_ERROR_INVALID_CONTEXT case here ? The
only way that I can think of is consecutive BeginPicture calls without
a EndPicture - surely that can cause issues even without this patch.

-Emil