+Boyuan, forgot to Cc him when I sent. Regards, Leo
>-----Original Message----- >From: Emil Velikov [mailto:emil.l.veli...@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....@amd.com> wrote: >> From: Boyuan Zhang <boyuan.zh...@amd.com> >> >> Signed-off-by: Boyuan Zhang <boyuan.zh...@amd.com> >> Reviewed-by: Christian König <christian.koe...@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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
