Hi Emil, Please see the following information about this patch.
- Issue: For Mpeg4, the VOP and GOV headers were truncated. With the existing workaround in st/va, playback shows massive corruptions. - This Patch: Provide another way to get the truncated headers back. Massive corruptions are gone with this patch. At the same time, add an environmental variable to allow user to decide whether to use this patch. Regards, Boyuan -----Original Message----- From: Liu, Leo Sent: November-05-15 10:15 PM To: Emil Velikov; Zhang, Boyuan Cc: ML mesa-dev Subject: RE: [Mesa-dev] [PATCH] st/va: add mpeg4 startcode workaround +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
