[FFmpeg-devel,4/6] lavu/hwcontext_vaapi: add vaapi_format_map support for AYUV/Y210/Y410

Submitted by Linjie Fu on Sept. 10, 2019, 4:07 p.m.

Details

Message ID 1568131675-19726-1-git-send-email-linjie.fu@intel.com
State New
Headers show

Commit Message

Linjie Fu Sept. 10, 2019, 4:07 p.m.
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(+)

Comments

Mark Thompson Sept. 12, 2019, 11:48 p.m.
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
Linjie Fu Sept. 19, 2019, 7:10 a.m.
> -----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
Linjie Fu Sept. 24, 2019, 3:21 a.m.
> -----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
Mark Thompson Sept. 24, 2019, 10:45 p.m.
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
Carl Eugen Hoyos Sept. 24, 2019, 11:32 p.m.
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
Carl Eugen Hoyos Sept. 24, 2019, 11:34 p.m.
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
Linjie Fu Sept. 25, 2019, 1:07 a.m.
> -----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
Linjie Fu Sept. 25, 2019, 1:11 a.m.
> -----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

Patch hide | download patch | download mbox

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),