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

[Liu, Leo]

+Boyuan, forgot to Cc him when I sent.

Regards,
Leo


>-----Original Message-----
>From: Emil Velikov [mailto:emil.l.velikov at gmail.com]
>Sent: Thursday, November 05, 2015 7:00 PM
>To: Liu, Leo
>Cc: ML mesa-dev
>Subject: Re: [Mesa-dev] [PATCH] st/va: add mpeg4 startcode workaround
>
>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