GNOME Bugzilla – Bug 789876
video: Add 10bits variants of GRAY/NV12/NV16 packed inside LE 32 bits
Last modified: 2018-02-06 21:38:48 UTC
These new formats are widely used on Xilinx SoC. They are similar to Microsoft P010/P210 but uses a packing the reduces the overhead. Each components are packed into 32 bits words, with only 2 bit padding. The provided pack/unpack are strictly meant for testing. No performance optimization has been done.
Created attachment 362920 [details] [review] video: Add NV12_10LE32 support This adds a 10bit variant for NV12 which packs 3 10bit components into little endian 32bit words. The MSB 2 bits are padding and should be ignored. This format is used on Xilinx SoC and is identified with there with the FOURCC XV15
Created attachment 362921 [details] [review] video: Add GRAY10_LE32 support This add a 10bit variant of gray scale packed into 32bits little endian words. The MSB 2 bits are padding and should be ignored. This format is used on Xilinx SoC and is identified with the FOURCC XV10.
Created attachment 362922 [details] [review] video: Add NV16_10LE32 support This adds a 10 bit variant for NV16 packed into 32 bits little endian words. The MSB 2 bits are padding. This format is used on Xilinx SoC and identified with the FOURCC XV20.
Review of attachment 362920 [details] [review]: ::: gst-libs/gst/video/video-format.c @@ +5015,3 @@ PSTR222, PLANE012, OFFS0, SUB444, PACK_Y444_12LE), + MAKE_YUV_C_LE_FORMAT (NV12_10LE32, "raw video", + GST_MAKE_FOURCC ('X', 'V', '1', '5'), DPTH10_10_10, PSTR122, PLANE011, pstride should be set to PSTR0 as the pixel distance is not constant. Same for the other formats.
Created attachment 366891 [details] [review] video: Add NV12_10LE32 support This adds a 10bit variant for NV12 which packs 3 10bit components into little endian 32bit words. The MSB 2 bits are padding and should be ignored. This format is used on Xilinx SoC and is identified with there with the FOURCC XV15
Created attachment 366892 [details] [review] video: Add GRAY10_LE32 support This add a 10bit variant of gray scale packed into 32bits little endian words. The MSB 2 bits are padding and should be ignored. This format is used on Xilinx SoC and is identified with the FOURCC XV10.
Created attachment 366893 [details] [review] video: Add NV16_10LE32 support This adds a 10 bit variant for NV16 packed into 32 bits little endian words. The MSB 2 bits are padding. This format is used on Xilinx SoC and identified with the FOURCC XV20.
This update is to fix the PSTRIDE, for complex format this should be set to 0, because the distance between pixels it not constant and not always in bytes.
Review of attachment 366891 [details] [review]: ::: gst-libs/gst/video/video-format.h @@ +59,3 @@ * @GST_VIDEO_FORMAT_NV12: planar 4:2:0 YUV with interleaved UV plane * @GST_VIDEO_FORMAT_NV21: planar 4:2:0 YUV with interleaved VU plane + * @GST_VIDEO_FORMAT_NV12_10LE32: 10-bit varant of @GST_VIDEO_FORMAT_NV12, packet in 32bit words (MSB 2 bits padding) (Since: 1.X) typo 'variant'. Same for GST_VIDEO_FORMAT_NV16_10LE32
Review of attachment 366893 [details] [review]: ::: gst-libs/gst/video/video-format.c @@ +5487,3 @@ return GST_VIDEO_FORMAT_AYUV64; + case GST_MAKE_FOURCC ('X', 'V', '1', '0'): + return GST_VIDEO_FORMAT_GRAY10_LE32; This should be part of the previous patch. Also Xilinx uses 'DRM_FORMAT_Y10' and 'V4L2_PIX_FMT_Y10' for this format. Should we reflect that in this name?
So GST_VIDEO_FORMAT_NV12_10LE32 is working fine on the hardware but conversion with GST_VIDEO_FORMAT_NV16_10LE32 is broken.
Will fix. Y10 is wrong at least on V4L, since it means GREY_10LE16 (6 bits padding). You should report this to your kernel dev. Y or GREY is synonyms, not sure why V4L went the inconsistent way, in GST we have grey, not y8. But I'm open with it, anyone else prefer y ?
(In reply to Guillaume Desmottes from comment #11) > So GST_VIDEO_FORMAT_NV12_10LE32 is working fine on the hardware but > conversion with GST_VIDEO_FORMAT_NV16_10LE32 is broken. Please provide raw samples in both formats, specifying size, strides and offsets. Ideally two similar images.
(In reply to Nicolas Dufresne (stormer) from comment #13) > (In reply to Guillaume Desmottes from comment #11) > > So GST_VIDEO_FORMAT_NV12_10LE32 is working fine on the hardware but > > conversion with GST_VIDEO_FORMAT_NV16_10LE32 is broken. > > Please provide raw samples in both formats, specifying size, strides and > offsets. Ideally two similar images. https://people.collabora.com/~cassidy/video-format/xv15.raw https://people.collabora.com/~cassidy/video-format/xv20.raw Both files contain a single 3840x2160 frame with a stride of 5120 and the second plane starting at offset 11059200. I use those pipelines to render on my laptop: gst-launch-1.0 -v filesrc location="xv15.raw" ! rawvideoparse format=nv12-10le32 width=3840 height=2160 ! videoconvert ! imagefreeze ! xvimagesink gst-launch-1.0 -v filesrc location="xv20.raw" ! rawvideoparse format=nv16-10le32 width=3840 height=2160 ! videoconvert ! imagefreeze ! xvimagesink There first one is fine but the colors of the second are wrong. Both are properly displayed on the hardware using kmssink.
Review of attachment 366893 [details] [review]: ::: gst-libs/gst/video/video-format.c @@ +4791,3 @@ + gint uv = GET_UV_420 (y, flags); + const guint32 *restrict sy = GET_PLANE_LINE (0, y); + const guint32 *restrict suv = GET_PLANE_LINE (1, uv); Thanks, the bug was here, I need to get rid of uv and just pass y, it's not sub-sampled vertically. @@ +4886,3 @@ + gint uv = GET_UV_420 (y, flags); + guint32 *restrict dy = GET_PLANE_LINE (0, y); + guint32 *restrict duv = GET_PLANE_LINE (1, uv); ... and here
Created attachment 366957 [details] [review] video: Add NV12_10LE32 support This adds a 10bit variant for NV12 which packs 3 10bit components into little endian 32bit words. The MSB 2 bits are padding and should be ignored. This format is used on Xilinx SoC and is identified with there with the FOURCC XV15
Created attachment 366958 [details] [review] video: Add GRAY10_LE32 support This add a 10bit variant of gray scale packed into 32bits little endian words. The MSB 2 bits are padding and should be ignored. This format is used on Xilinx SoC and is identified with the FOURCC XV10.
Created attachment 366959 [details] [review] video: Add NV16_10LE32 support This adds a 10 bit variant for NV16 packed into 32 bits little endian words. The MSB 2 bits are padding. This format is used on Xilinx SoC and identified with the FOURCC XV20.
Attachment 366957 [details] pushed as 2b9725d - video: Add NV12_10LE32 support Attachment 366958 [details] pushed as f7a27d7 - video: Add GRAY10_LE32 support Attachment 366959 [details] pushed as c256e96 - video: Add NV16_10LE32 support
This seems to break the libs/video unit test.
Yes, it's on my to-do to look at.
Created attachment 367963 [details] [review] video-format: Fix 10LE32 formats packing function The source offset (soff) was not incremented for each component and then each group of 3 components were inverted. This was causing a staircase effect combined with some noise.
For the curious, I was testing 4K images over a 1080p screen, making this bug invisible. This fixes the unit test also.
Attachment 367963 [details] pushed as eb7565b - video-format: Fix 10LE32 formats packing function