| Message ID | 1568131675-19726-1-git-send-email-linjie.fu@intel.com |
|---|---|
| State | New |
| Headers | show |
On 10/09/2019 17:07, Linjie Fu wrote: > There is no VA_RT_FORMAT_AYUV in defined in libva, and currently in > media-driver, VA_FOURCC_AYUV is used to represent VA_RT_FORMAT_AYUV. That doesn't make sense - VA_RT_FORMAT_* is a bit mask, so using VA_FOURCC_AYUV looks like a random combination of other values. > Another patch could be sent to refine the code after this issue is > addressed: > https://github.com/intel/libva/issues/335 > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavutil/hwcontext_vaapi.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c > index cf11764..724bbeb 100644 > --- a/libavutil/hwcontext_vaapi.c > +++ b/libavutil/hwcontext_vaapi.c > @@ -116,6 +116,15 @@ static const VAAPIFormatDescriptor vaapi_format_map[] = { > #endif > MAP(UYVY, YUV422, UYVY422, 0), > MAP(YUY2, YUV422, YUYV422, 0), > +#ifdef VA_FOURCC_Y210 > + MAP(Y210, YUV422_10,Y210, 0), > +#endif > +#define VA_RT_FORMAT_AYUV VA_FOURCC_AYUV > + MAP(AYUV, AYUV, AYUV, 0), > +#undef VA_RT_FORMAT_AYUV > +#ifdef VA_FOURCC_Y410 > + MAP(Y410, YUV444_10,Y410, 0), That looks suspicious - you've defined Y410 as having an alpha channel, but that render target format doesn't have one. > +#endif > MAP(411P, YUV411, YUV411P, 0), > MAP(422V, YUV422, YUV440P, 0), > MAP(444P, YUV444, YUV444P, 0), To try to clarify the intent here, the formats you are intending to use for this new decoder are: 4:2:0: * 8-bit: NV12. * 10-bit: P010. * With alpha: not supported. 4:2:2: * 8-bit: YUYV or similar? * 10-bit: Y210. * With alpha: not supported. 4:4:4: * 8-bit: the existing 444P, or not supported? * 10-bit: not supported. * 8-bit + alpha: AYUV. * 10-bit + alpha: Y410? (With the alpha channel truncated?) Can you fill in the missing parts here? - Mark
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Friday, September 13, 2019 07:48 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 4/6] lavu/hwcontext_vaapi: add > vaapi_format_map support for AYUV/Y210/Y410 > > On 10/09/2019 17:07, Linjie Fu wrote: > > There is no VA_RT_FORMAT_AYUV in defined in libva, and currently in > > media-driver, VA_FOURCC_AYUV is used to represent > VA_RT_FORMAT_AYUV. > > That doesn't make sense - VA_RT_FORMAT_* is a bit mask, so using > VA_FOURCC_AYUV looks like a random combination of other values. > > Another patch could be sent to refine the code after this issue is > > addressed: > > https://github.com/intel/libva/issues/335 > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > --- > > libavutil/hwcontext_vaapi.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c > > index cf11764..724bbeb 100644 > > --- a/libavutil/hwcontext_vaapi.c > > +++ b/libavutil/hwcontext_vaapi.c > > @@ -116,6 +116,15 @@ static const VAAPIFormatDescriptor > vaapi_format_map[] = { > > #endif > > MAP(UYVY, YUV422, UYVY422, 0), > > MAP(YUY2, YUV422, YUYV422, 0), > > +#ifdef VA_FOURCC_Y210 > > + MAP(Y210, YUV422_10,Y210, 0), > > +#endif > > +#define VA_RT_FORMAT_AYUV VA_FOURCC_AYUV > > + MAP(AYUV, AYUV, AYUV, 0), > > +#undef VA_RT_FORMAT_AYUV > > +#ifdef VA_FOURCC_Y410 > > + MAP(Y410, YUV444_10,Y410, 0), > > That looks suspicious - you've defined Y410 as having an alpha channel, but > that render target format doesn't have one. Also mentioned this in https://github.com/intel/libva/issues/335. > > > +#endif > > MAP(411P, YUV411, YUV411P, 0), > > MAP(422V, YUV422, YUV440P, 0), > > MAP(444P, YUV444, YUV444P, 0), > > To try to clarify the intent here, the formats you are intending to use for this > new decoder are: > > 4:2:0: > * 8-bit: NV12. > * 10-bit: P010. > * With alpha: not supported. > > 4:2:2: > * 8-bit: YUYV or similar? Supported FourCC is YUY2, and supported decode format is YUYV422; > * 10-bit: Y210. > * With alpha: not supported. > > 4:4:4: > * 8-bit: the existing 444P, or not supported? > * 10-bit: not supported. The planar format for decoder output is not supported in driver/hardware. > * 8-bit + alpha: AYUV. > * 10-bit + alpha: Y410? (With the alpha channel truncated?) The 10-bit+alpha format is Y410, and the alpha channel data is truncated to 2 bits. - linjie
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Mark Thompson > Sent: Friday, September 13, 2019 07:48 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 4/6] lavu/hwcontext_vaapi: add > vaapi_format_map support for AYUV/Y210/Y410 > > On 10/09/2019 17:07, Linjie Fu wrote: > > There is no VA_RT_FORMAT_AYUV in defined in libva, and currently in > > media-driver, VA_FOURCC_AYUV is used to represent > VA_RT_FORMAT_AYUV. > > That doesn't make sense - VA_RT_FORMAT_* is a bit mask, so using > VA_FOURCC_AYUV looks like a random combination of other values. > > > > diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c > > index cf11764..724bbeb 100644 > > --- a/libavutil/hwcontext_vaapi.c > > +++ b/libavutil/hwcontext_vaapi.c > > @@ -116,6 +116,15 @@ static const VAAPIFormatDescriptor > vaapi_format_map[] = { > > #endif > > MAP(UYVY, YUV422, UYVY422, 0), > > MAP(YUY2, YUV422, YUYV422, 0), > > +#ifdef VA_FOURCC_Y210 > > + MAP(Y210, YUV422_10,Y210, 0), > > +#endif > > +#define VA_RT_FORMAT_AYUV VA_FOURCC_AYUV > > + MAP(AYUV, AYUV, AYUV, 0), > > +#undef VA_RT_FORMAT_AYUV > > +#ifdef VA_FOURCC_Y410 > > + MAP(Y410, YUV444_10,Y410, 0), > > That looks suspicious - you've defined Y410 as having an alpha channel, but > that render target format doesn't have one. > Reconsider about these concerns and collect some information in driver. 1. VA_RT_FORMAT describes the sampling format, several fourcc may have the Same VA_RT_FORMAT. (NV12, YV12, I420 have VA_RT_FORMAT_YUV420) so YUV444 is good for AYUV, and YUV444_10 is good for Y410. (alpha channel is not relevant with the sampling format) 2. In media driver, mediaFMT is actually used for decode which is decided by expected_fourcc. DDI_MEDIA_FORMAT mediaFmt = DdiMedia_OsFormatToMediaFormat(expected_fourcc,format); https://github.com/intel/media-driver/blob/cf80fa0e3c81361696ada1285ea17a417114be47/media_driver/linux/common/ddi/media_libva.cpp#L2222 No declaring for a RT_FORMAT with alpha channel won't do harm to the decode procedure since mediaFMT is actually mapped by FourCC. 3. in CreateDecAttributes and MediaLibvaCaps::CheckEncRTFormat, VA_RT_FORMAT_YUV444 and VA_RT_FORMAT_YUV444_10 are used for MAIN444 and MAIN444_10 https://github.com/intel/media-driver/blob/cf80fa0e3c81361696ada1285ea17a417114be47/media_driver/linux/gen11/ddi/media_libva_caps_g11.cpp#L1189 https://github.com/intel/media-driver/blob/cf80fa0e3c81361696ada1285ea17a417114be47/media_driver/linux/common/ddi/media_libva_caps.cpp#L350 3. If no against, I would like to refine the patch for AYUV and Y410 as follow, and resend the patch. + MAP(AYUV, YUV444, AYUV, 0), +#ifdef VA_FOURCC_Y410 + MAP(Y410, YUV444_10, Y410, 0), +#endif Regards, - linjie
On 24/09/2019 04:21, Fu, Linjie wrote: >> -----Original Message----- >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of >> Mark Thompson >> Sent: Friday, September 13, 2019 07:48 >> To: ffmpeg-devel@ffmpeg.org >> Subject: Re: [FFmpeg-devel] [PATCH 4/6] lavu/hwcontext_vaapi: add >> vaapi_format_map support for AYUV/Y210/Y410 >> >> On 10/09/2019 17:07, Linjie Fu wrote: >>> There is no VA_RT_FORMAT_AYUV in defined in libva, and currently in >>> media-driver, VA_FOURCC_AYUV is used to represent >> VA_RT_FORMAT_AYUV. >> >> That doesn't make sense - VA_RT_FORMAT_* is a bit mask, so using >> VA_FOURCC_AYUV looks like a random combination of other values. >>> >>> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c >>> index cf11764..724bbeb 100644 >>> --- a/libavutil/hwcontext_vaapi.c >>> +++ b/libavutil/hwcontext_vaapi.c >>> @@ -116,6 +116,15 @@ static const VAAPIFormatDescriptor >> vaapi_format_map[] = { >>> #endif >>> MAP(UYVY, YUV422, UYVY422, 0), >>> MAP(YUY2, YUV422, YUYV422, 0), >>> +#ifdef VA_FOURCC_Y210 >>> + MAP(Y210, YUV422_10,Y210, 0), >>> +#endif >>> +#define VA_RT_FORMAT_AYUV VA_FOURCC_AYUV >>> + MAP(AYUV, AYUV, AYUV, 0), >>> +#undef VA_RT_FORMAT_AYUV >>> +#ifdef VA_FOURCC_Y410 >>> + MAP(Y410, YUV444_10,Y410, 0), >> >> That looks suspicious - you've defined Y410 as having an alpha channel, but >> that render target format doesn't have one. >> > > Reconsider about these concerns and collect some information in driver. > > 1. VA_RT_FORMAT describes the sampling format, several fourcc may have the > Same VA_RT_FORMAT. (NV12, YV12, I420 have VA_RT_FORMAT_YUV420) > so YUV444 is good for AYUV, and YUV444_10 is good for Y410. > (alpha channel is not relevant with the sampling format) > > 2. In media driver, mediaFMT is actually used for decode which is decided by expected_fourcc. > DDI_MEDIA_FORMAT mediaFmt = DdiMedia_OsFormatToMediaFormat(expected_fourcc,format); > https://github.com/intel/media-driver/blob/cf80fa0e3c81361696ada1285ea17a417114be47/media_driver/linux/common/ddi/media_libva.cpp#L2222 > > No declaring for a RT_FORMAT with alpha channel won't do harm to the decode procedure since mediaFMT > is actually mapped by FourCC. > > 3. in CreateDecAttributes and MediaLibvaCaps::CheckEncRTFormat, > VA_RT_FORMAT_YUV444 and VA_RT_FORMAT_YUV444_10 are used for MAIN444 and MAIN444_10 > > https://github.com/intel/media-driver/blob/cf80fa0e3c81361696ada1285ea17a417114be47/media_driver/linux/gen11/ddi/media_libva_caps_g11.cpp#L1189 > > https://github.com/intel/media-driver/blob/cf80fa0e3c81361696ada1285ea17a417114be47/media_driver/linux/common/ddi/media_libva_caps.cpp#L350 > > 3. If no against, I would like to refine the patch for AYUV and Y410 as follow, and resend the patch. > + MAP(AYUV, YUV444, AYUV, 0), > +#ifdef VA_FOURCC_Y410 > + MAP(Y410, YUV444_10, Y410, 0), > +#endif That render target format makes more sense to me. However, the confusion about the presence of alpha channels feels like a bad idea - you're defining formats with alpha channels, then selectively ignoring the alpha channel in some places but not others. From what you've said so far, I get the impression that the alpha channel in your AYUV and Y410 formats is not actually supported at all in the hardware you have? (That is, there is no support for decoding video with an alpha channel, such as two-layer H.265.) If that's true, I think you should be defining a 0YUV format and using that everywhere rather than AYUV. Declaring that it contains an alpha channel when it doesn't can only cause confusion later, especially around automatic conversions and when combining video. - Mark
Am Mi., 25. Sept. 2019 um 00:45 Uhr schrieb Mark Thompson <sw@jkqxz.net>:
> From what you've said so far, I get the impression that the alpha channel in your AYUV and Y410 formats is not actually supported at all in the hardware you have? (That is, there is no support for decoding video with an alpha channel, such as two-layer H.265.)
The way I understand it is that there is a bug outside of FFmpeg that
is supposed to be fixed in the future as hevc rext is supposed to
provide an alpha channel (or can provide an alpha channel).
Carl Eugen
Am Do., 19. Sept. 2019 um 09:11 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: > > 4:2:2: > > * 8-bit: YUYV or similar? > > Supported FourCC is YUY2, and supported decode format is YUYV422; Why is this not AV_PIX_FMT_YUYV422? Carl Eugen
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Carl Eugen Hoyos > Sent: Wednesday, September 25, 2019 07:33 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 4/6] lavu/hwcontext_vaapi: add > vaapi_format_map support for AYUV/Y210/Y410 > > Am Mi., 25. Sept. 2019 um 00:45 Uhr schrieb Mark Thompson > <sw@jkqxz.net>: > > From what you've said so far, I get the impression that the alpha channel in > your AYUV and Y410 formats is not actually supported at all in the hardware > you have? (That is, there is no support for decoding video with an alpha > channel, such as two-layer H.265.) > > The way I understand it is that there is a bug outside of FFmpeg that > is supposed to be fixed in the future as hevc rext is supposed to > provide an alpha channel (or can provide an alpha channel). > Thanks Carl. Yes, that's exactly what current situation is. See https://github.com/intel/media-driver/issues/719 - linjie
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Carl Eugen Hoyos > Sent: Wednesday, September 25, 2019 07:34 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 4/6] lavu/hwcontext_vaapi: add > vaapi_format_map support for AYUV/Y210/Y410 > > Am Do., 19. Sept. 2019 um 09:11 Uhr schrieb Fu, Linjie <linjie.fu@intel.com>: > > > > 4:2:2: > > > * 8-bit: YUYV or similar? > > > > Supported FourCC is YUY2, and supported decode format is YUYV422; > > Why is this not AV_PIX_FMT_YUYV422? No new pixel format is needed for 4:2:2 8 bit, here what I mean by "YUYV422" is AV_PIX_FMT_YUYV422. - linjie
diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c index cf11764..724bbeb 100644 --- a/libavutil/hwcontext_vaapi.c +++ b/libavutil/hwcontext_vaapi.c @@ -116,6 +116,15 @@ static const VAAPIFormatDescriptor vaapi_format_map[] = { #endif MAP(UYVY, YUV422, UYVY422, 0), MAP(YUY2, YUV422, YUYV422, 0), +#ifdef VA_FOURCC_Y210 + MAP(Y210, YUV422_10,Y210, 0), +#endif +#define VA_RT_FORMAT_AYUV VA_FOURCC_AYUV + MAP(AYUV, AYUV, AYUV, 0), +#undef VA_RT_FORMAT_AYUV +#ifdef VA_FOURCC_Y410 + MAP(Y410, YUV444_10,Y410, 0), +#endif MAP(411P, YUV411, YUV411P, 0), MAP(422V, YUV422, YUV440P, 0), MAP(444P, YUV444, YUV444P, 0),
There is no VA_RT_FORMAT_AYUV in defined in libva, and currently in media-driver, VA_FOURCC_AYUV is used to represent VA_RT_FORMAT_AYUV. Another patch could be sent to refine the code after this issue is addressed: https://github.com/intel/libva/issues/335 Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- libavutil/hwcontext_vaapi.c | 9 +++++++++ 1 file changed, 9 insertions(+)