[FFmpeg-devel] [PATCH 1/8] lavu/pix_fmt: add new pixel format y210

[Carl Eugen Hoyos]

Am 14.01.2020 um 03:38 schrieb Fu, Linjie <linjie.fu at intel.com>:

>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>> Carl Eugen Hoyos
>> Sent: Monday, January 13, 2020 00:41
>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel at ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavu/pix_fmt: add new pixel format
>> y210
>> 
>>> Am So., 12. Jan. 2020 um 08:57 Uhr schrieb Fu, Linjie <linjie.fu at intel.com>:
>>> 
>>>> -----Original Message-----
>>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>>> Mark Thompson
>>>> Sent: Thursday, January 9, 2020 05:47
>>>> To: ffmpeg-devel at ffmpeg.org
>>>> Subject: Re: [FFmpeg-devel] [PATCH 1/8] lavu/pix_fmt: add new pixel
>> format
>>>> y210
>>>> 
>>>>> On 29/12/2019 16:28, Linjie Fu wrote:
>>>>> Add some packed 4:2:2 10-bit pixel formats for hardware decode
>> support
>>>>> in VAAPI and QSV.
>>>>> 
>>>>> Signed-off-by: Linjie Fu <linjie.fu at intel.com>
>>>>> ---
>>>>> libavutil/pixdesc.c              | 23 +++++++++++++++++++++++
>>>>> libavutil/pixfmt.h               |  5 +++++
>>>>> libavutil/version.h              |  2 +-
>>>>> tests/ref/fate/sws-pixdesc-query |  7 +++++++
>>>>> 4 files changed, 36 insertions(+), 1 deletion(-)
>>>>> 
>>>>> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
>>>>> index 05dd4a1..1e118ef 100644
>>>>> --- a/libavutil/pixdesc.c
>>>>> +++ b/libavutil/pixdesc.c
>>>>> @@ -205,6 +205,29 @@ static const AVPixFmtDescriptor
>>>> av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
>>>>>             { 0, 4, 1, 0, 8, 3, 7, 2 },        /* V */
>>>>>         },
>>>>>     },
>>>>> +    [AV_PIX_FMT_Y210LE] = {
>>>>> +        .name = "y210le",
>>>>> +        .nb_components = 3,
>>>>> +        .log2_chroma_w = 1,
>>>>> +        .log2_chroma_h = 0,
>>>>> +        .comp = {
>>>>> +            { 0, 4, 0, 6, 10, 3, 9, 1 },        /* Y */
>>>>> +            { 0, 8, 2, 6, 10, 7, 9, 3 },        /* U */
>>>>> +            { 0, 8, 6, 6, 10, 7, 9, 7 },        /* V */
>>>>> +        },
>>>>> +    },
>>>>> +    [AV_PIX_FMT_Y210BE] = {
>>>>> +        .name = "y210be",
>>>>> +        .nb_components = 3,
>>>>> +        .log2_chroma_w = 1,
>>>>> +        .log2_chroma_h = 0,
>>>>> +        .comp = {
>>>>> +            { 0, 4, 0, 6, 10, 3, 9, 1 },        /* Y */
>>>>> +            { 0, 8, 2, 6, 10, 7, 9, 3 },        /* U */
>>>>> +            { 0, 8, 6, 6, 10, 7, 9, 7 },        /* V */
>>>>> +        },
>>>>> +        .flags = AV_PIX_FMT_FLAG_BE,
>>>>> +    },
>>>>>     [AV_PIX_FMT_RGB24] = {
>>>>>         .name = "rgb24",
>>>>>         .nb_components = 3,
>>>>> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
>>>>> index 37ecebd..7ffa5a0 100644
>>>>> --- a/libavutil/pixfmt.h
>>>>> +++ b/libavutil/pixfmt.h
>>>>> @@ -348,6 +348,9 @@ enum AVPixelFormat {
>>>>>     AV_PIX_FMT_NV24,      ///< planar YUV 4:4:4, 24bpp, 1 plane for Y
>> and 1
>>>> plane for the UV components, which are interleaved (first byte U and the
>>>> following byte V)
>>>>>     AV_PIX_FMT_NV42,      ///< as above, but U and V bytes are
>> swapped
>>>>> 
>>>>> +    AV_PIX_FMT_Y210BE,    ///< packed YUV 4:2:2, 32bpp, Y0 Cb Y1 Cr,
>> big-
>>>> endian
>>>>> +    AV_PIX_FMT_Y210LE,    ///< packed YUV 4:2:2, 32bpp, Y0 Cb Y1 Cr,
>> little-
>>>> endian
>>>> 
>>>> These comments should be clear that the data are in the high bits (like
>> P010),
>>>> rather than in the low bits (like most formats used by software codecs).
>>>> 
>>>> Being consistent with other comments would write 20bpp rather than
>> 32bpp,
>>>> though I'm not sure how much information that number is really adding.
>>>> 
>>> Would update the comments like:
>>> packed YUV 4:2:2 like YUYV422, 20bpp, data in the high bits, zeros in the
>> low bits, big-endian
>> 
>> Please do not write "zeros in the low bits", this is just confusing.
>> 
> 
> P010LE/P010BE are using similar comments, and there is identical description in the doc:
> "The 10-bit formats also use 16 bits for each channel, with the lowest 6 bits set to zero"
> https://docs.microsoft.com/en-us/windows/win32/medfound/10-bit-and-16-bit-yuv-video-formats
> 
> Hi Carl, would you please help to elaborate more on how to refine the comments?

Just remove „zeros in the low bits“.

Carl Eugen