by Ming Qian

[permalink] [raw]

Subject: RE: [PATCH v3 03/10] media: Add YUYV64_12 video format

Hi Mirela,

>From: Mirela Rabulea (OSS) <[email protected]>
>Sent: 2023年2月21日 5:59
>To: Ming Qian <[email protected]>; [email protected]; hverkuil-
>[email protected]; [email protected]
>Cc: [email protected]; [email protected]; [email protected];
>[email protected]; X.H. Bao <[email protected]>; dl-linux-imx <linux-
>[email protected]>; [email protected]; [email protected];
>[email protected]; [email protected]
>Subject: Re: [PATCH v3 03/10] media: Add YUYV64_12 video format
>
>Hi Ming,
>
>On 20.02.2023 04:15, Ming Qian wrote:
>>> From: Mirela Rabulea (OSS) <[email protected]>
>>> Sent: 2023年2月19日 5:52
>>> To: Ming Qian <[email protected]>; [email protected]; hverkuil-
>>> [email protected]; [email protected]
>>> Cc: [email protected]; [email protected];
>>> [email protected]; [email protected]; X.H. Bao
>>> <[email protected]>; dl-linux-imx <linux- [email protected]>;
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]
>>> Subject: Re: [PATCH v3 03/10] media: Add YUYV64_12 video format
>>>
>>> Hi,
>>>
>>> On 14.02.2023 12:04, Ming Qian wrote:
>>>> YUYV64_12 is a YUV format with 12-bits per component like YUYV,
>>>> expanded to 16bits.
>>>> Data in the 12 high bits, zeros in the 4 low bits, arranged in
>>>> little endian order.
>>>>
>>>> Signed-off-by: Ming Qian <[email protected]>
>>>> ---
>>>> .../media/v4l/pixfmt-packed-yuv.rst | 42 +++++++++++++++++++
>>>> drivers/media/v4l2-core/v4l2-common.c | 1 +
>>>> drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
>>>> include/uapi/linux/videodev2.h | 1 +
>>>> 4 files changed, 45 insertions(+)
>>>>
>>>> diff --git
>>>> a/Documentation/userspace-api/media/v4l/pixfmt-packed-yuv.rst
>>>> b/Documentation/userspace-api/media/v4l/pixfmt-packed-yuv.rst
>>>> index bf283a1b5581..76fc331f126e 100644
>>>> --- a/Documentation/userspace-api/media/v4l/pixfmt-packed-yuv.rst
>>>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-packed-yuv.rst
>>>> @@ -341,6 +341,48 @@ components horizontally by 2, storing 2 pixels
>>>> in 4
>>> bytes.
>>>>
>>>> \normalsize
>>>>
>>>> +The next lists the packed YUV 4:2:2 formats with more than 8 bits
>>>> +per
>>> component.
>>>> +expand the bits per component to 16 bits, data in the high bits,
>>>> +zeros in the low bits, arranged in little endian order. storing 2
>>>> +pixels in 8
>>> bytes.
>>>> +
>>>> +.. raw:: latex
>>>> +
>>>> + \footnotesize
>>>> +
>>>> +.. tabularcolumns::
>>>>
>>> +|p{3.4cm}|p{1.2cm}|p{0.8cm}|p{0.8cm}|p{0.8cm}|p{0.8cm}|p{0.8cm}|p{0.
>>> +|8
>>>> +cm}|p{0.8cm}|p{0.8cm}|
>>>> +
>>>> +.. flat-table:: Packed YUV 4:2:2 Formats (more than 8bpc)
>>>> + :header-rows: 1
>>>> + :stub-columns: 0
>>>> +
>>>> + * - Identifier
>>>> + - Code
>>>> + - Byte 1-0
>>>> + - Byte 3-2
>>>> + - Byte 5-4
>>>> + - Byte 7-6
>>>> + - Byte 9-8
>>>> + - Byte 11-10
>>>> + - Byte 13-12
>>>> + - Byte 15-14
>>>> + * .. _V4L2-PIX-FMT-YUYV64-12:
>>>> +
>>>> + - ``V4L2_PIX_FMT_YUYV64_12``
>>>> + - 'Y212'
>>>> +
>>>> + - Y'\ :sub:`0`
>>>> + - Cb\ :sub:`0`
>>>> + - Y'\ :sub:`1`
>>>> + - Cr\ :sub:`0`
>>>> + - Y'\ :sub:`2`
>>>> + - Cb\ :sub:`2`
>>>> + - Y'\ :sub:`3`
>>>> + - Cr\ :sub:`2`
>>>> +
>>>> +.. raw:: latex
>>>> +
>>>> + \normalsize
>>>> +
>>>> **Color Sample Location:**
>>>> Chroma samples are :ref:`interstitially sited<yuv-chroma-centered>`
>>>> horizontally.
>>>> diff --git a/drivers/media/v4l2-core/v4l2-common.c
>>>> b/drivers/media/v4l2-core/v4l2-common.c
>>>> index a5e8ba370d33..8fb1c0fa8158 100644
>>>> --- a/drivers/media/v4l2-core/v4l2-common.c
>>>> +++ b/drivers/media/v4l2-core/v4l2-common.c
>>>> @@ -258,6 +258,7 @@ const struct v4l2_format_info
>>> *v4l2_format_info(u32 format)
>>>> { .format = V4L2_PIX_FMT_YVYU, .pixel_enc =
>>> V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0,
>>> 0,
>>> 0 }, .hdiv = 2, .vdiv = 1 },
>>>> { .format = V4L2_PIX_FMT_UYVY, .pixel_enc =
>>> V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0,
>>> 0,
>>> 0 }, .hdiv = 2, .vdiv = 1 },
>>>> { .format = V4L2_PIX_FMT_VYUY, .pixel_enc =
>>> V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0,
>>> 0,
>>> 0 }, .hdiv = 2, .vdiv = 1 },
>>>> + { .format = V4L2_PIX_FMT_YUYV64_12, .pixel_enc =
>>>> +V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 4,
>>>> +0, 0, 0 }, .hdiv = 2, .vdiv = 1 },
>>>>
>>>> /* YUV planar formats */
>>>> { .format = V4L2_PIX_FMT_NV12, .pixel_enc =
>>> V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2,
>>> 0,
>>> 0 }, .hdiv = 2, .vdiv = 2 },
>>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> index ec1c9ef27e88..e5002d6fe7ee 100644
>>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>> @@ -1343,6 +1343,7 @@ static void v4l_fill_fmtdesc(struct
>>>> v4l2_fmtdesc
>>> *fmt)
>>>> case V4L2_PIX_FMT_YUV420: descr = "Planar YUV 4:2:0"; break;
>>>> case V4L2_PIX_FMT_HI240: descr = "8-bit Dithered RGB (BTTV)";
>>> break;
>>>> case V4L2_PIX_FMT_M420: descr = "YUV 4:2:0 (M420)";
>>> break;
>>>> + case V4L2_PIX_FMT_YUYV64_12: descr = "12-bit Depth YUYV
>>> 4:2:2"; break;
>>>> case V4L2_PIX_FMT_NV12: descr = "Y/UV 4:2:0"; break;
>>>> case V4L2_PIX_FMT_NV21: descr = "Y/VU 4:2:0"; break;
>>>> case V4L2_PIX_FMT_NV16: descr = "Y/UV 4:2:2"; break;
>>>> diff --git a/include/uapi/linux/videodev2.h
>>>> b/include/uapi/linux/videodev2.h index 3d8f89bff33c..3712a36d6fdf
>>>> 100644
>>>> --- a/include/uapi/linux/videodev2.h
>>>> +++ b/include/uapi/linux/videodev2.h
>>>> @@ -618,6 +618,7 @@ struct v4l2_pix_format {
>>>> #define V4L2_PIX_FMT_YUVA32 v4l2_fourcc('Y', 'U', 'V', 'A') /*
>>>> 32 YUVA-8-
>>> 8-8-8 */
>>>> #define V4L2_PIX_FMT_YUVX32 v4l2_fourcc('Y', 'U', 'V', 'X') /*
>>>> 32 YUVX-8-
>>> 8-8-8 */
>>>> #define V4L2_PIX_FMT_M420 v4l2_fourcc('M', '4', '2', '0') /* 12 YUV
>4:2:0
>>> 2 lines y, 1 line uv interleaved */
>>>> +#define V4L2_PIX_FMT_YUYV64_12 v4l2_fourcc('Y', '2', '1', '2') /* 32
>YUYV
>>> 12-bit per component */
>>>
>>> It seems Y212 is already added with the same fourcc code, causing
>>> build
>>> errors:
>>>
>>> drivers/media/v4l2-core/v4l2-ioctl.c: In function ‘v4l_fill_fmtdesc’:
>>> drivers/media/v4l2-core/v4l2-ioctl.c:1455:9: error: duplicate case value
>>> 1455 | case V4L2_PIX_FMT_Y212: descr = "12-bit YUYV
>>> Packed"; break;
>>> | ^~~~
>>> drivers/media/v4l2-core/v4l2-ioctl.c:1349:9: note: previously used here
>>> 1349 | case V4L2_PIX_FMT_YUYV64_12: descr = "12-bit Depth
>>> YUYV 4:2:2"; break;
>>> | ^~~~
>>>
>>> Regards,
>>> Mirela
>>
>> Hi Mirela,
>> Would you please double check it?
>> The format V4L2_PIX_FMT_Y212 was added by the first version of
>> this patchset, and it should be replaced by
>> V4L2_PIX_FMT_YUYV64_12 in this v3 patchset.
>> And I checked my patch, and my local environment, I didn't found
>> any V4L2_PIX_FMT_Y212, and I didn't meet this build error
>>
>> Ming
>
>I applied the _v3 version of the patches over linux-next, and the build error
>comes from conflicting definition of the fourcc
>#define V4L2_PIX_FMT_YUYV64_12 v4l2_fourcc('Y', '2', '1', '2') /* 32
>YUYV 12-bit per component */
>
>with this patch, which adds:
>+#define V4L2_PIX_FMT_Y212 v4l2_fourcc('Y', '2', '1', '2') /* 32
>YUYV 4:2:2 */
>
>commit 0dc1d7a79a8d13e316d3b168e9fc57e376099c7a
>Author: Tomi Valkeinen <[email protected]>
>Date: Wed Dec 21 11:24:43 2022 +0200
>
> media: Add Y210, Y212 and Y216 formats
>
> Add Y210, Y212 and Y216 formats.
>
> Signed-off-by: Tomi Valkeinen
><[email protected]>
> Reviewed-by: Laurent Pinchart
><[email protected]>
> Acked-by: Mauro Carvalho Chehab <[email protected]>
> Acked-by: Hans Verkuil <[email protected]>
> Signed-off-by: Laurent Pinchart
><[email protected]>
>
>https://patchwork.kernel.org/project/linux-
>media/patch/20221219140139.294245-3-
>[email protected]/
>
>In this case, matching the formats defined in DRM was the stronger argument
>for the format naming scheme.
>
>Regards,
>Mirela
>

Thanks for your comment, I got it, I was working on media_tree, and this patch is not included yet.
As the format V4L2_PIX_FMT_Y212 is added, we can use it directly and drop V4L2_PIX_FMT_YUYV64_12.

I will prepare the v4 patch set that drop this patch and apply V4L2_PIX_FMT_Y212 directly after the Y212 patch in merged to media_tree.

Ming

>>
>>>>
>>>> /* two planes -- one Y, one Cr + Cb interleaved */
>>>> #define V4L2_PIX_FMT_NV12 v4l2_fourcc('N', 'V', '1', '2') /* 12 Y/CbCr
>>> 4:2:0 */