| Message ID | 20191119082319.5041-1-fei.w.wang@intel.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 19, 2019 at 4:24 PM Wangfei <fei.w.wang@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@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? > .value.value.i = best_fourcc, > }; > > -- > 2.17.1 >
> On Tue, Nov 19, 2019 at 4:24 PM Wangfei <fei.w.wang@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@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/linux/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/d73f06270600c37c74beeceac37f593838ced383/libavutil/hwcontext_vaapi.c#L527 So I believe here we need do the same thing. > > .value.value.i = best_fourcc, > > }; > > > > -- > > 2.17.1 > >
Ping, any other comments of this patch? Thanks. > -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Wang, Fei W > Sent: Wednesday, November 20, 2019 10:40 AM > To: mypopy@gmail.com; FFmpeg development discussions and patches > <ffmpeg-devel@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@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@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 > nux/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, > > > }; > > > > > > -- > > > 2.17.1 > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org > with subject "unsubscribe".
Ping for the 2nd time. Thanks. > -----Original Message----- > From: Wang, Fei W > Sent: Friday, November 22, 2019 1:45 PM > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org>; mypopy@gmail.com > Subject: RE: [FFmpeg-devel] [PATCH v1] avcodec/vaapi: set more flags for > VASurfaceAttrib > > Ping, any other comments of this patch? Thanks. > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Wang, Fei W > > Sent: Wednesday, November 20, 2019 10:40 AM > > To: mypopy@gmail.com; FFmpeg development discussions and patches > > <ffmpeg-devel@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@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@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 > > nux/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, > > > > }; > > > > > > > > -- > > > > 2.17.1 > > > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Hi, > -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Wang, Fei W > Sent: Wednesday, November 20, 2019 10:40 > To: mypopy@gmail.com; FFmpeg development discussions and patches > <ffmpeg-devel@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@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@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/lin > 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) Patch LGTM since it seems to be more robust.(but also may hide some mistakes on the other hand) Thanks, - linjie
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu, > Linjie > Sent: Thursday, November 28, 2019 5:42 PM > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org>; mypopy@gmail.com > Subject: Re: [FFmpeg-devel] [PATCH v1] avcodec/vaapi: set more flags for > VASurfaceAttrib > > Hi, > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Wang, Fei W > > Sent: Wednesday, November 20, 2019 10:40 > > To: mypopy@gmail.com; FFmpeg development discussions and patches > > <ffmpeg-devel@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@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@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org > with subject "unsubscribe".
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, .value.value.i = best_fourcc, };
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@intel.com> --- libavcodec/vaapi_decode.c | 2 ++ 1 file changed, 2 insertions(+)