[FFmpeg-devel] [PATCH v1] avcodec/vaapi: set more flags for VASurfaceAttrib

[Wang, Fei W]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Fu,
> Linjie
> Sent: Thursday, November 28, 2019 5:42 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>; mypopy at gmail.com
> Subject: Re: [FFmpeg-devel] [PATCH v1] avcodec/vaapi: set more flags for
> VASurfaceAttrib
> 
> Hi,
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > Wang, Fei W
> > Sent: Wednesday, November 20, 2019 10:40
> > To: mypopy at gmail.com; FFmpeg development discussions and patches
> > <ffmpeg-devel at ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v1] avcodec/vaapi: set more flags
> > for VASurfaceAttrib
> >
> >
> > > On Tue, Nov 19, 2019 at 4:24 PM Wangfei <fei.w.wang at intel.com> wrote:
> > > >
> > > > flags and value.type is needed when pass VASurfaceAttrib to driver.
> > > > Otherwise the attribute will be considered invalid in driver.
> > > >
> > > > Signed-off-by: Wangfei <fei.w.wang at intel.com>
> > > > ---
> > > >  libavcodec/vaapi_decode.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
> > > > index 69512e1d45..a93aba5a5d 100644
> > > > --- a/libavcodec/vaapi_decode.c
> > > > +++ b/libavcodec/vaapi_decode.c
> > > > @@ -350,6 +350,8 @@ static int
> > > > vaapi_decode_find_best_format(AVCodecContext *avctx,
> > > >
> > > >          ctx->pixel_format_attribute = (VASurfaceAttrib) {
> > > >              .type          = VASurfaceAttribPixelFormat,
> > > > +            .flags         = VA_SURFACE_ATTRIB_SETTABLE,
> > > > +            .value.type    = VAGenericValueTypeInteger,
> > > I can't found any description in libva about this part, do you have
> > > any other docs?
> > I checked the logic of vaCreateSurfaces implementation in
> > media-driver, it will check flags and value.type in VASurfaceAttrib
> > when using it. If leaks of the info, it will consider the attribute is
> > invalid or assert(in this case, flags checked first, so no assert occurred.):
> > https://github.com/intel/media-
> >
> driver/blob/8a8b8ae263a04cb5dd384d4625215cf296d2cc59/media_driver/li
> n
> > ux/common/ddi/media_libva.cpp#L2149
> >
> > On the other hand, there is another place in ffmpeg-vaapi that sets
> > VASurfaceAttrib, and it filled flags and value.type:
> >
> https://github.com/FFmpeg/FFmpeg/blob/d73f06270600c37c74beeceac37f5
> > 93838ced383/libavutil/hwcontext_vaapi.c#L527
> > So I believe here we need do the same thing.
> >
> > > >              .value.value.i = best_fourcc,
> > > >          };
> 
> Setting flags and type allows driver to assign the
> expected_fourcc/media_format of surface according to the best FourCC,
> instead of  determining by the VA_RT_FORMAT_ defined in
> vaapi_format_map[].
> 
> With this patch, decoder still works well even if a wrong VA_RT_FORMAT_
> was assigned.
> (For example, VA_RT_FORMAT_YUV444 assigned to VA_FOURCC_NV12,
> when decoding
> NV12 clips)
Thanks Linjie for your comments. When decoding NV12 clips(here I guess you mean 8bit depth clips), we shouldn't set  VA_RT_FORMAT_YUV444 RT format but should be  VA_RT_FORMAT_YUV420 when create surface. If it happens, I believe it should be a bug in ffmpeg.
> 
> Patch LGTM since it seems to be more robust.(but also may hide some
> mistakes on the other hand)
> 
> Thanks,
> 
> - linjie
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request at ffmpeg.org
> with subject "unsubscribe".