[Mesa-dev] [PATCH] st/va: add mpeg4 startcode workaround

[Emil Velikov]

On 5 November 2015 at 21:00, Leo Liu <leo.liu at amd.com> wrote:
> From: Boyuan Zhang <boyuan.zhang at amd.com>
>
> Signed-off-by: Boyuan Zhang <boyuan.zhang at amd.com>
> Reviewed-by: Christian König <christian.koenig at amd.com>
> ---
>  src/gallium/state_trackers/va/buffer.c     |  24 +++++-
>  src/gallium/state_trackers/va/context.c    |   7 ++
>  src/gallium/state_trackers/va/picture.c    | 117 +++++++++++++++++------------
>  src/gallium/state_trackers/va/va_private.h |   3 +
>  4 files changed, 102 insertions(+), 49 deletions(-)
>
Guys can get some commit message please ? What is the workaround why
is it needed ?

It's a bit sad that one has to ask for such a thing.


> @@ -59,8 +70,17 @@ vlVaCreateBuffer(VADriverContextP ctx, VAContextID context, VABufferType type,
>        return VA_STATUS_ERROR_ALLOCATION_FAILED;
>     }
>
> -   if (data)
> -      memcpy(buf->data, data, size * num_elements);
> +   uint8_t* pExternalData = (uint8_t*) data;
> +   if (data) {
> +      if ((u_reduce_video_profile(pContext->desc.base.profile) == PIPE_VIDEO_FORMAT_MPEG4)
> +            && (pContext->mpeg4.vaapi_mpeg4_workaround == true)
> +            && (buf->type == VASliceDataBufferType)) {
Please follow st/va coding style - drop the explicit comparison
against true, && should be at the end of the line.


> --- a/src/gallium/state_trackers/va/context.c
> +++ b/src/gallium/state_trackers/va/context.c
> @@ -35,6 +35,8 @@
>
>  #include "va_private.h"
>
> +DEBUG_GET_ONCE_BOOL_OPTION(mpeg4, "VAAPI_MPEG4_WORKAROUND", FALSE);
> +
You do realise that defined as it, one can only use
VAAPI_MPEG4_WORKAROUND on debug mesa builds ?


> @@ -275,6 +277,11 @@ vlVaCreateContext(VADriverContextP ctx, VAConfigID config_id, int picture_width,
>              return VA_STATUS_ERROR_ALLOCATION_FAILED;
>           }
>        }
> +
> +      if (u_reduce_video_profile(context->decoder->profile) ==
> +            PIPE_VIDEO_FORMAT_MPEG4) {
u_reduce_video_profile() is called three times already. Stash it into
a local variable and keep the comparison on a single line ?


> --- a/src/gallium/state_trackers/va/picture.c
> +++ b/src/gallium/state_trackers/va/picture.c
> @@ -584,60 +584,83 @@ vlVaDecoderFixMPEG4Startcode(vlVaContext *context)

> +      for (int i = 0 ; i < VL_VA_MPEG4_BYTES_FOR_LOOKUP ; i++) {
> +         if (memcmp (p, start_code, sizeof(start_code)) == 0) {
> +            found = true;
Just use startcode_available directly ?

> +            break;
> +         }
> +         p += 1;
> +         extraSize += 1;
> +      }
> +      if (found) {
> +         startcode_available = true;


-Emil