On 24/09/2019 04:21, Fu, Linjie wrote:
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-boun...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
