[PATCH kms++ v2 0/4] Support Y210, Y212, Y216

[PATCH kms++ v2 0/4] Support Y210, Y212, Y216

[Tomi Valkeinen]

From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Hi,

These kms++ patches add support for Y21x formats.

Changes in v2:

- Add Y212 and Y216 too
- Fix Y210 shifts (must be in the msbs)
- Format the pixelformats list a bit better
- Have dst as the first parameter in the write functions

 Tomi

Tomi Valkeinen (4):
  kms++: PixelFormats: Fix formatting
  kms++: PixelFormats: Add Y21x formats
  kms++util: Add endian.h
  kms++util: Add Y21x drawing support

 kms++/inc/kms++/pixelformats.h   |  4 ++
 kms++/src/pixelformats.cpp       | 92 +++++++++-----------------------
 kms++util/inc/kms++util/endian.h | 46 ++++++++++++++++
 kms++util/src/drawing.cpp        | 63 ++++++++++++++++++++++
 4 files changed, 139 insertions(+), 66 deletions(-)
 create mode 100644 kms++util/inc/kms++util/endian.h

-- 
2.34.1

[PATCH kms++ v2 1/4] kms++: PixelFormats: Fix formatting

[Tomi Valkeinen]

Fix formatting for some pixel formats.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 kms++/src/pixelformats.cpp | 76 +++++---------------------------------
 1 file changed, 10 insertions(+), 66 deletions(-)

diff --git a/kms++/src/pixelformats.cpp b/kms++/src/pixelformats.cpp
index d739efd..1f10f3a 100644
--- a/kms++/src/pixelformats.cpp
+++ b/kms++/src/pixelformats.cpp
@@ -32,109 +32,53 @@ static const map<PixelFormat, PixelFormatInfo> format_info_array = {
 	{ PixelFormat::NV12, {
 				     PixelColorType::YUV,
 				     2,
-				     { {
-					       8,
-					       1,
-					       1,
-				       },
-				       { 8, 2, 2 } },
+				     { { 8, 1, 1 }, { 8, 2, 2 } },
 			     } },
 	{ PixelFormat::NV21, {
 				     PixelColorType::YUV,
 				     2,
-				     { {
-					       8,
-					       1,
-					       1,
-				       },
-				       { 8, 2, 2 } },
+				     { { 8, 1, 1 }, { 8, 2, 2 } },
 			     } },
 	{ PixelFormat::NV16, {
 				     PixelColorType::YUV,
 				     2,
-				     { {
-					       8,
-					       1,
-					       1,
-				       },
-				       { 8, 2, 1 } },
+				     { { 8, 1, 1 }, { 8, 2, 1 } },
 			     } },
 	{ PixelFormat::NV61, {
 				     PixelColorType::YUV,
 				     2,
-				     { {
-					       8,
-					       1,
-					       1,
-				       },
-				       { 8, 2, 1 } },
+				     { { 8, 1, 1 }, { 8, 2, 1 } },
 			     } },
 	/* YUV planar */
 	{ PixelFormat::YUV420, {
 				       PixelColorType::YUV,
 				       3,
-				       { {
-						 8,
-						 1,
-						 1,
-					 },
-					 { 8, 2, 2 },
-					 { 8, 2, 2 } },
+				       { { 8, 1, 1 }, { 8, 2, 2 }, { 8, 2, 2 } },
 			       } },
 	{ PixelFormat::YVU420, {
 				       PixelColorType::YUV,
 				       3,
-				       { {
-						 8,
-						 1,
-						 1,
-					 },
-					 { 8, 2, 2 },
-					 { 8, 2, 2 } },
+				       { { 8, 1, 1 }, { 8, 2, 2 }, { 8, 2, 2 } },
 			       } },
 	{ PixelFormat::YUV422, {
 				       PixelColorType::YUV,
 				       3,
-				       { {
-						 8,
-						 1,
-						 1,
-					 },
-					 { 8, 2, 1 },
-					 { 8, 2, 1 } },
+				       { { 8, 1, 1 }, { 8, 2, 1 }, { 8, 2, 1 } },
 			       } },
 	{ PixelFormat::YVU422, {
 				       PixelColorType::YUV,
 				       3,
-				       { {
-						 8,
-						 1,
-						 1,
-					 },
-					 { 8, 2, 1 },
-					 { 8, 2, 1 } },
+				       { { 8, 1, 1 }, { 8, 2, 1 }, { 8, 2, 1 } },
 			       } },
 	{ PixelFormat::YUV444, {
 				       PixelColorType::YUV,
 				       3,
-				       { {
-						 8,
-						 1,
-						 1,
-					 },
-					 { 8, 1, 1 },
-					 { 8, 1, 1 } },
+				       { { 8, 1, 1 }, { 8, 1, 1 }, { 8, 1, 1 } },
 			       } },
 	{ PixelFormat::YVU444, {
 				       PixelColorType::YUV,
 				       3,
-				       { {
-						 8,
-						 1,
-						 1,
-					 },
-					 { 8, 1, 1 },
-					 { 8, 1, 1 } },
+				       { { 8, 1, 1 }, { 8, 1, 1 }, { 8, 1, 1 } },
 			       } },
 	/* RGB8 */
 	{ PixelFormat::RGB332, {
-- 
2.34.1

[PATCH kms++ v2 2/4] kms++: PixelFormats: Add Y21x formats

[Tomi Valkeinen]

Add Y210, Y212, Y216 pixel formats.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 kms++/inc/kms++/pixelformats.h |  4 ++++
 kms++/src/pixelformats.cpp     | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/kms++/inc/kms++/pixelformats.h b/kms++/inc/kms++/pixelformats.h
index 8ebb76b..e334ee6 100644
--- a/kms++/inc/kms++/pixelformats.h
+++ b/kms++/inc/kms++/pixelformats.h
@@ -31,6 +31,10 @@ enum class PixelFormat : uint32_t {
 	YVYU = MakeFourCC("YVYU"),
 	VYUY = MakeFourCC("VYUY"),
 
+	Y210 = MakeFourCC("Y210"),
+	Y212 = MakeFourCC("Y212"),
+	Y216 = MakeFourCC("Y216"),
+
 	XRGB8888 = MakeFourCC("XR24"),
 	XBGR8888 = MakeFourCC("XB24"),
 	RGBX8888 = MakeFourCC("RX24"),
diff --git a/kms++/src/pixelformats.cpp b/kms++/src/pixelformats.cpp
index 1f10f3a..5f13ef4 100644
--- a/kms++/src/pixelformats.cpp
+++ b/kms++/src/pixelformats.cpp
@@ -28,6 +28,22 @@ static const map<PixelFormat, PixelFormatInfo> format_info_array = {
 				     1,
 				     { { 16, 2, 1 } },
 			     } },
+	{ PixelFormat::Y210, {
+				     PixelColorType::YUV,
+				     1,
+				     { { 32, 2, 1 } },
+			     } },
+	{ PixelFormat::Y212, {
+				     PixelColorType::YUV,
+				     1,
+				     { { 32, 2, 1 } },
+			     } },
+	{ PixelFormat::Y216, {
+				     PixelColorType::YUV,
+				     1,
+				     { { 32, 2, 1 } },
+			     } },
+
 	/* YUV semi-planar */
 	{ PixelFormat::NV12, {
 				     PixelColorType::YUV,
-- 
2.34.1

Re: [PATCH kms++ v2 2/4] kms++: PixelFormats: Add Y21x formats

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Mon, Dec 05, 2022 at 10:03:37AM +0200, Tomi Valkeinen wrote:
> Add Y210, Y212, Y216 pixel formats.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  kms++/inc/kms++/pixelformats.h |  4 ++++
>  kms++/src/pixelformats.cpp     | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/kms++/inc/kms++/pixelformats.h b/kms++/inc/kms++/pixelformats.h
> index 8ebb76b..e334ee6 100644
> --- a/kms++/inc/kms++/pixelformats.h
> +++ b/kms++/inc/kms++/pixelformats.h
> @@ -31,6 +31,10 @@ enum class PixelFormat : uint32_t {
>  	YVYU = MakeFourCC("YVYU"),
>  	VYUY = MakeFourCC("VYUY"),
>  
> +	Y210 = MakeFourCC("Y210"),
> +	Y212 = MakeFourCC("Y212"),
> +	Y216 = MakeFourCC("Y216"),
> +
>  	XRGB8888 = MakeFourCC("XR24"),
>  	XBGR8888 = MakeFourCC("XB24"),
>  	RGBX8888 = MakeFourCC("RX24"),
> diff --git a/kms++/src/pixelformats.cpp b/kms++/src/pixelformats.cpp
> index 1f10f3a..5f13ef4 100644
> --- a/kms++/src/pixelformats.cpp
> +++ b/kms++/src/pixelformats.cpp
> @@ -28,6 +28,22 @@ static const map<PixelFormat, PixelFormatInfo> format_info_array = {
>  				     1,
>  				     { { 16, 2, 1 } },
>  			     } },
> +	{ PixelFormat::Y210, {
> +				     PixelColorType::YUV,
> +				     1,
> +				     { { 32, 2, 1 } },
> +			     } },
> +	{ PixelFormat::Y212, {
> +				     PixelColorType::YUV,
> +				     1,
> +				     { { 32, 2, 1 } },
> +			     } },
> +	{ PixelFormat::Y216, {
> +				     PixelColorType::YUV,
> +				     1,
> +				     { { 32, 2, 1 } },
> +			     } },
> +
>  	/* YUV semi-planar */
>  	{ PixelFormat::NV12, {
>  				     PixelColorType::YUV,

-- 
Regards,

Laurent Pinchart

[PATCH kms++ v2 3/4] kms++util: Add endian.h

[Tomi Valkeinen]

Add simple endianness supporting write function, and, for now, only one
shortcut helper, write16le().

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 kms++util/inc/kms++util/endian.h | 46 ++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 kms++util/inc/kms++util/endian.h

diff --git a/kms++util/inc/kms++util/endian.h b/kms++util/inc/kms++util/endian.h
new file mode 100644
index 0000000..0f7aecd
--- /dev/null
+++ b/kms++util/inc/kms++util/endian.h
@@ -0,0 +1,46 @@
+#pragma once
+
+#include <type_traits>
+#include <byteswap.h>
+#include <stdint.h>
+
+static_assert((__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) ||
+	      (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__),
+	      "Unable to detect endianness");
+
+enum class endian {
+	little = __ORDER_LITTLE_ENDIAN__,
+	big = __ORDER_BIG_ENDIAN__,
+	native = __BYTE_ORDER__
+};
+
+template<typename T>
+constexpr T byteswap(T value) noexcept
+{
+	static_assert(std::is_integral<T>(), "Type is not integral");
+	static_assert(sizeof(T) == 2 ||
+		      sizeof(T) == 4 ||
+		      sizeof(T) == 8,
+		      "Illegal value size");
+
+	switch (sizeof(T)) {
+		case 2: return bswap_16(value);
+		case 4: return bswap_32(value);
+		case 8: return bswap_64(value);
+	}
+}
+
+template<endian E, typename T>
+static void write_endian(T* dst, T val)
+{
+	if constexpr (E != endian::native)
+		val = byteswap(val);
+
+	*dst = val;
+}
+
+[[maybe_unused]]
+static void write16le(uint16_t* dst, uint16_t val)
+{
+	write_endian<endian::little, uint16_t>(dst, val);
+}
-- 
2.34.1

Re: [PATCH kms++ v2 3/4] kms++util: Add endian.h

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Mon, Dec 05, 2022 at 10:03:38AM +0200, Tomi Valkeinen wrote:
> Add simple endianness supporting write function, and, for now, only one
> shortcut helper, write16le().
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  kms++util/inc/kms++util/endian.h | 46 ++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 kms++util/inc/kms++util/endian.h
> 
> diff --git a/kms++util/inc/kms++util/endian.h b/kms++util/inc/kms++util/endian.h
> new file mode 100644
> index 0000000..0f7aecd
> --- /dev/null
> +++ b/kms++util/inc/kms++util/endian.h
> @@ -0,0 +1,46 @@
> +#pragma once
> +
> +#include <type_traits>
> +#include <byteswap.h>
> +#include <stdint.h>
> +
> +static_assert((__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) ||
> +	      (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__),
> +	      "Unable to detect endianness");
> +
> +enum class endian {
> +	little = __ORDER_LITTLE_ENDIAN__,
> +	big = __ORDER_BIG_ENDIAN__,
> +	native = __BYTE_ORDER__
> +};
> +
> +template<typename T>
> +constexpr T byteswap(T value) noexcept
> +{
> +	static_assert(std::is_integral<T>(), "Type is not integral");
> +	static_assert(sizeof(T) == 2 ||
> +		      sizeof(T) == 4 ||
> +		      sizeof(T) == 8,
> +		      "Illegal value size");
> +
> +	switch (sizeof(T)) {
> +		case 2: return bswap_16(value);
> +		case 4: return bswap_32(value);
> +		case 8: return bswap_64(value);
> +	}
> +}
> +
> +template<endian E, typename T>
> +static void write_endian(T* dst, T val)
> +{
> +	if constexpr (E != endian::native)
> +		val = byteswap(val);
> +
> +	*dst = val;
> +}
> +
> +[[maybe_unused]]
> +static void write16le(uint16_t* dst, uint16_t val)
> +{
> +	write_endian<endian::little, uint16_t>(dst, val);
> +}

-- 
Regards,

Laurent Pinchart

[PATCH kms++ v2 4/4] kms++util: Add Y21x drawing support

[Tomi Valkeinen]

Add support for drawing Y210, Y212, Y216 pixels.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
 kms++util/src/drawing.cpp | 63 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/kms++util/src/drawing.cpp b/kms++util/src/drawing.cpp
index 79e0d90..5764b08 100644
--- a/kms++util/src/drawing.cpp
+++ b/kms++util/src/drawing.cpp
@@ -3,6 +3,7 @@
 
 #include <kms++/kms++.h>
 #include <kms++util/kms++util.h>
+#include <kms++util/endian.h>
 
 using namespace std;
 
@@ -179,6 +180,62 @@ static void draw_yuv422_packed_macropixel(IFramebuffer& buf, unsigned x, unsigne
 	}
 }
 
+static void draw_y2xx_packed_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
+					  YUV yuv1, YUV yuv2)
+{
+	const uint32_t macro_size = 4;
+	uint16_t* p = (uint16_t*)(buf.map(0) + buf.stride(0) * y + x * macro_size);
+
+	switch (buf.format()) {
+	case PixelFormat::Y210: {
+		// XXX naive expansion to 10 bits, similar to 10-bit funcs in class RGB
+		uint16_t y0 = yuv1.y << 2;
+		uint16_t y1 = yuv2.y << 2;
+		uint16_t cb = ((yuv1.u  << 2) + (yuv2.u << 2)) / 2;
+		uint16_t cr = ((yuv1.v  << 2) + (yuv2.v << 2)) / 2;
+
+		// The 10 bits occupy the msb, so we shift left by 16-10 = 6
+		write16le(&p[0], y0 << 6);
+		write16le(&p[1], cb << 6);
+		write16le(&p[2], y1 << 6);
+		write16le(&p[3], cr << 6);
+		break;
+	}
+
+	case PixelFormat::Y212: {
+		// XXX naive expansion to 12 bits
+		uint16_t y0 = yuv1.y << 4;
+		uint16_t y1 = yuv2.y << 4;
+		uint16_t cb = ((yuv1.u  << 4) + (yuv2.u << 4)) / 2;
+		uint16_t cr = ((yuv1.v  << 4) + (yuv2.v << 4)) / 2;
+
+		// The 10 bits occupy the msb, so we shift left by 16-12 = 4
+		write16le(&p[0], y0 << 4);
+		write16le(&p[1], cb << 4);
+		write16le(&p[2], y1 << 4);
+		write16le(&p[3], cr << 4);
+		break;
+	}
+
+	case PixelFormat::Y216: {
+		// XXX naive expansion to 16 bits
+		uint16_t y0 = yuv1.y << 8;
+		uint16_t y1 = yuv2.y << 8;
+		uint16_t cb = ((yuv1.u  << 8) + (yuv2.u << 8)) / 8;
+		uint16_t cr = ((yuv1.v  << 8) + (yuv2.v << 8)) / 8;
+
+		write16le(&p[0], y0);
+		write16le(&p[1], cb);
+		write16le(&p[2], y1);
+		write16le(&p[3], cr);
+		break;
+	}
+
+	default:
+		throw std::invalid_argument("invalid pixelformat");
+	}
+}
+
 static void draw_yuv422_semiplanar_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
 					      YUV yuv1, YUV yuv2)
 {
@@ -257,6 +314,12 @@ void draw_yuv422_macropixel(IFramebuffer& buf, unsigned x, unsigned y, YUV yuv1,
 		draw_yuv422_packed_macropixel(buf, x, y, yuv1, yuv2);
 		break;
 
+	case PixelFormat::Y210:
+	case PixelFormat::Y212:
+	case PixelFormat::Y216:
+		draw_y2xx_packed_macropixel(buf, x, y, yuv1, yuv2);
+		break;
+
 	case PixelFormat::NV16:
 	case PixelFormat::NV61:
 		draw_yuv422_semiplanar_macropixel(buf, x, y, yuv1, yuv2);
-- 
2.34.1

Re: [PATCH kms++ v2 4/4] kms++util: Add Y21x drawing support

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Mon, Dec 05, 2022 at 10:03:39AM +0200, Tomi Valkeinen wrote:
> Add support for drawing Y210, Y212, Y216 pixels.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
>  kms++util/src/drawing.cpp | 63 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/kms++util/src/drawing.cpp b/kms++util/src/drawing.cpp
> index 79e0d90..5764b08 100644
> --- a/kms++util/src/drawing.cpp
> +++ b/kms++util/src/drawing.cpp
> @@ -3,6 +3,7 @@
>  
>  #include <kms++/kms++.h>
>  #include <kms++util/kms++util.h>
> +#include <kms++util/endian.h>
>  
>  using namespace std;
>  
> @@ -179,6 +180,62 @@ static void draw_yuv422_packed_macropixel(IFramebuffer& buf, unsigned x, unsigne
>  	}
>  }
>  
> +static void draw_y2xx_packed_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
> +					  YUV yuv1, YUV yuv2)
> +{
> +	const uint32_t macro_size = 4;
> +	uint16_t* p = (uint16_t*)(buf.map(0) + buf.stride(0) * y + x * macro_size);
> +
> +	switch (buf.format()) {
> +	case PixelFormat::Y210: {
> +		// XXX naive expansion to 10 bits, similar to 10-bit funcs in class RGB
> +		uint16_t y0 = yuv1.y << 2;
> +		uint16_t y1 = yuv2.y << 2;
> +		uint16_t cb = ((yuv1.u  << 2) + (yuv2.u << 2)) / 2;
> +		uint16_t cr = ((yuv1.v  << 2) + (yuv2.v << 2)) / 2;
> +
> +		// The 10 bits occupy the msb, so we shift left by 16-10 = 6
> +		write16le(&p[0], y0 << 6);
> +		write16le(&p[1], cb << 6);
> +		write16le(&p[2], y1 << 6);
> +		write16le(&p[3], cr << 6);
> +		break;
> +	}
> +
> +	case PixelFormat::Y212: {
> +		// XXX naive expansion to 12 bits
> +		uint16_t y0 = yuv1.y << 4;
> +		uint16_t y1 = yuv2.y << 4;
> +		uint16_t cb = ((yuv1.u  << 4) + (yuv2.u << 4)) / 2;
> +		uint16_t cr = ((yuv1.v  << 4) + (yuv2.v << 4)) / 2;
> +
> +		// The 10 bits occupy the msb, so we shift left by 16-12 = 4
> +		write16le(&p[0], y0 << 4);
> +		write16le(&p[1], cb << 4);
> +		write16le(&p[2], y1 << 4);
> +		write16le(&p[3], cr << 4);
> +		break;
> +	}
> +
> +	case PixelFormat::Y216: {
> +		// XXX naive expansion to 16 bits
> +		uint16_t y0 = yuv1.y << 8;
> +		uint16_t y1 = yuv2.y << 8;
> +		uint16_t cb = ((yuv1.u  << 8) + (yuv2.u << 8)) / 8;
> +		uint16_t cr = ((yuv1.v  << 8) + (yuv2.v << 8)) / 8;
> +
> +		write16le(&p[0], y0);
> +		write16le(&p[1], cb);
> +		write16le(&p[2], y1);
> +		write16le(&p[3], cr);
> +		break;

These three cases end up all shifting left by 8 bits. It looks like you
could simplify the code by merging all implementations into one.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	}
> +
> +	default:
> +		throw std::invalid_argument("invalid pixelformat");
> +	}
> +}
> +
>  static void draw_yuv422_semiplanar_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
>  					      YUV yuv1, YUV yuv2)
>  {
> @@ -257,6 +314,12 @@ void draw_yuv422_macropixel(IFramebuffer& buf, unsigned x, unsigned y, YUV yuv1,
>  		draw_yuv422_packed_macropixel(buf, x, y, yuv1, yuv2);
>  		break;
>  
> +	case PixelFormat::Y210:
> +	case PixelFormat::Y212:
> +	case PixelFormat::Y216:
> +		draw_y2xx_packed_macropixel(buf, x, y, yuv1, yuv2);
> +		break;
> +
>  	case PixelFormat::NV16:
>  	case PixelFormat::NV61:
>  		draw_yuv422_semiplanar_macropixel(buf, x, y, yuv1, yuv2);

-- 
Regards,

Laurent Pinchart

Re: [PATCH kms++ v2 4/4] kms++util: Add Y21x drawing support

[Tomi Valkeinen]

On 05/12/2022 10:17, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Mon, Dec 05, 2022 at 10:03:39AM +0200, Tomi Valkeinen wrote:
>> Add support for drawing Y210, Y212, Y216 pixels.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
>> ---
>>   kms++util/src/drawing.cpp | 63 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 63 insertions(+)
>>
>> diff --git a/kms++util/src/drawing.cpp b/kms++util/src/drawing.cpp
>> index 79e0d90..5764b08 100644
>> --- a/kms++util/src/drawing.cpp
>> +++ b/kms++util/src/drawing.cpp
>> @@ -3,6 +3,7 @@
>>   
>>   #include <kms++/kms++.h>
>>   #include <kms++util/kms++util.h>
>> +#include <kms++util/endian.h>
>>   
>>   using namespace std;
>>   
>> @@ -179,6 +180,62 @@ static void draw_yuv422_packed_macropixel(IFramebuffer& buf, unsigned x, unsigne
>>   	}
>>   }
>>   
>> +static void draw_y2xx_packed_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
>> +					  YUV yuv1, YUV yuv2)
>> +{
>> +	const uint32_t macro_size = 4;
>> +	uint16_t* p = (uint16_t*)(buf.map(0) + buf.stride(0) * y + x * macro_size);
>> +
>> +	switch (buf.format()) {
>> +	case PixelFormat::Y210: {
>> +		// XXX naive expansion to 10 bits, similar to 10-bit funcs in class RGB
>> +		uint16_t y0 = yuv1.y << 2;
>> +		uint16_t y1 = yuv2.y << 2;
>> +		uint16_t cb = ((yuv1.u  << 2) + (yuv2.u << 2)) / 2;
>> +		uint16_t cr = ((yuv1.v  << 2) + (yuv2.v << 2)) / 2;
>> +
>> +		// The 10 bits occupy the msb, so we shift left by 16-10 = 6
>> +		write16le(&p[0], y0 << 6);
>> +		write16le(&p[1], cb << 6);
>> +		write16le(&p[2], y1 << 6);
>> +		write16le(&p[3], cr << 6);
>> +		break;
>> +	}
>> +
>> +	case PixelFormat::Y212: {
>> +		// XXX naive expansion to 12 bits
>> +		uint16_t y0 = yuv1.y << 4;
>> +		uint16_t y1 = yuv2.y << 4;
>> +		uint16_t cb = ((yuv1.u  << 4) + (yuv2.u << 4)) / 2;
>> +		uint16_t cr = ((yuv1.v  << 4) + (yuv2.v << 4)) / 2;
>> +
>> +		// The 10 bits occupy the msb, so we shift left by 16-12 = 4
>> +		write16le(&p[0], y0 << 4);
>> +		write16le(&p[1], cb << 4);
>> +		write16le(&p[2], y1 << 4);
>> +		write16le(&p[3], cr << 4);
>> +		break;
>> +	}
>> +
>> +	case PixelFormat::Y216: {
>> +		// XXX naive expansion to 16 bits
>> +		uint16_t y0 = yuv1.y << 8;
>> +		uint16_t y1 = yuv2.y << 8;
>> +		uint16_t cb = ((yuv1.u  << 8) + (yuv2.u << 8)) / 8;
>> +		uint16_t cr = ((yuv1.v  << 8) + (yuv2.v << 8)) / 8;
>> +
>> +		write16le(&p[0], y0);
>> +		write16le(&p[1], cb);
>> +		write16le(&p[2], y1);
>> +		write16le(&p[3], cr);
>> +		break;
> 
> These three cases end up all shifting left by 8 bits. It looks like you
> could simplify the code by merging all implementations into one.

That's true. I'd like to expand the RGB and YUV classes to contain 
16-bits per component pixels. Then this code will change a bit, as 
instead of shifting we'll need to zero-out the lsbs in Y210 and Y212.

  Tomi

Re: [PATCH kms++ v2 4/4] kms++util: Add Y21x drawing support

[Geert Uytterhoeven]

Hi Tomi,

On Mon, Dec 5, 2022 at 9:07 AM Tomi Valkeinen
<tomi.valkeinen+renesas@ideasonboard.com> wrote:
> Add support for drawing Y210, Y212, Y216 pixels.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Thanks for your patch!

> --- a/kms++util/src/drawing.cpp
> +++ b/kms++util/src/drawing.cpp

> @@ -179,6 +180,62 @@ static void draw_yuv422_packed_macropixel(IFramebuffer& buf, unsigned x, unsigne
>         }
>  }
>
> +static void draw_y2xx_packed_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
> +                                         YUV yuv1, YUV yuv2)
> +{
> +       const uint32_t macro_size = 4;
> +       uint16_t* p = (uint16_t*)(buf.map(0) + buf.stride(0) * y + x * macro_size);
> +
> +       switch (buf.format()) {
> +       case PixelFormat::Y210: {
> +               // XXX naive expansion to 10 bits, similar to 10-bit funcs in class RGB
> +               uint16_t y0 = yuv1.y << 2;

"yuv1.y << 2 | yuv1.y >> 6" etc...

> +               uint16_t y1 = yuv2.y << 2;
> +               uint16_t cb = ((yuv1.u  << 2) + (yuv2.u << 2)) / 2;
> +               uint16_t cr = ((yuv1.v  << 2) + (yuv2.v << 2)) / 2;
> +
> +               // The 10 bits occupy the msb, so we shift left by 16-10 = 6
> +               write16le(&p[0], y0 << 6);
> +               write16le(&p[1], cb << 6);
> +               write16le(&p[2], y1 << 6);
> +               write16le(&p[3], cr << 6);
> +               break;
> +       }
> +
> +       case PixelFormat::Y212: {
> +               // XXX naive expansion to 12 bits
> +               uint16_t y0 = yuv1.y << 4;

"yuv1.y << 4 | yuv1.y >> 4" etc.

> +               uint16_t y1 = yuv2.y << 4;
> +               uint16_t cb = ((yuv1.u  << 4) + (yuv2.u << 4)) / 2;
> +               uint16_t cr = ((yuv1.v  << 4) + (yuv2.v << 4)) / 2;
> +
> +               // The 10 bits occupy the msb, so we shift left by 16-12 = 4
> +               write16le(&p[0], y0 << 4);
> +               write16le(&p[1], cb << 4);
> +               write16le(&p[2], y1 << 4);
> +               write16le(&p[3], cr << 4);
> +               break;
> +       }
> +
> +       case PixelFormat::Y216: {
> +               // XXX naive expansion to 16 bits
> +               uint16_t y0 = yuv1.y << 8;

"yuv1.y << 8 | yuv1.y" etc.


> +               uint16_t y1 = yuv2.y << 8;
> +               uint16_t cb = ((yuv1.u  << 8) + (yuv2.u << 8)) / 8;

Why divide by 8 instead of 2?

> +               uint16_t cr = ((yuv1.v  << 8) + (yuv2.v << 8)) / 8;
> +
> +               write16le(&p[0], y0);
> +               write16le(&p[1], cb);
> +               write16le(&p[2], y1);
> +               write16le(&p[3], cr);
> +               break;
> +       }
> +
> +       default:
> +               throw std::invalid_argument("invalid pixelformat");
> +       }
> +}
> +

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH kms++ v2 4/4] kms++util: Add Y21x drawing support

[Tomi Valkeinen]

Hi,

On 05/12/2022 10:34, Geert Uytterhoeven wrote:
> Hi Tomi,
> 
> On Mon, Dec 5, 2022 at 9:07 AM Tomi Valkeinen
> <tomi.valkeinen+renesas@ideasonboard.com> wrote:
>> Add support for drawing Y210, Y212, Y216 pixels.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> Thanks for your patch!
> 
>> --- a/kms++util/src/drawing.cpp
>> +++ b/kms++util/src/drawing.cpp
> 
>> @@ -179,6 +180,62 @@ static void draw_yuv422_packed_macropixel(IFramebuffer& buf, unsigned x, unsigne
>>          }
>>   }
>>
>> +static void draw_y2xx_packed_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
>> +                                         YUV yuv1, YUV yuv2)
>> +{
>> +       const uint32_t macro_size = 4;
>> +       uint16_t* p = (uint16_t*)(buf.map(0) + buf.stride(0) * y + x * macro_size);
>> +
>> +       switch (buf.format()) {
>> +       case PixelFormat::Y210: {
>> +               // XXX naive expansion to 10 bits, similar to 10-bit funcs in class RGB
>> +               uint16_t y0 = yuv1.y << 2;
> 
> "yuv1.y << 2 | yuv1.y >> 6" etc...

Yes, the current code leaves the lsbs zero. That's done in a few other 
places too. I want to do the proper expansion in some separate class, or 
rather, I want the RGB and YUV classes to contain 16-bit components so 
that we'll just downshift instead of expand. But I have not gotten there 
yet. It's been on my todo-list only for years now...

>> +               uint16_t y1 = yuv2.y << 2;
>> +               uint16_t cb = ((yuv1.u  << 2) + (yuv2.u << 2)) / 2;
>> +               uint16_t cr = ((yuv1.v  << 2) + (yuv2.v << 2)) / 2;
>> +
>> +               // The 10 bits occupy the msb, so we shift left by 16-10 = 6
>> +               write16le(&p[0], y0 << 6);
>> +               write16le(&p[1], cb << 6);
>> +               write16le(&p[2], y1 << 6);
>> +               write16le(&p[3], cr << 6);
>> +               break;
>> +       }
>> +
>> +       case PixelFormat::Y212: {
>> +               // XXX naive expansion to 12 bits
>> +               uint16_t y0 = yuv1.y << 4;
> 
> "yuv1.y << 4 | yuv1.y >> 4" etc.
> 
>> +               uint16_t y1 = yuv2.y << 4;
>> +               uint16_t cb = ((yuv1.u  << 4) + (yuv2.u << 4)) / 2;
>> +               uint16_t cr = ((yuv1.v  << 4) + (yuv2.v << 4)) / 2;
>> +
>> +               // The 10 bits occupy the msb, so we shift left by 16-12 = 4
>> +               write16le(&p[0], y0 << 4);
>> +               write16le(&p[1], cb << 4);
>> +               write16le(&p[2], y1 << 4);
>> +               write16le(&p[3], cr << 4);
>> +               break;
>> +       }
>> +
>> +       case PixelFormat::Y216: {
>> +               // XXX naive expansion to 16 bits
>> +               uint16_t y0 = yuv1.y << 8;
> 
> "yuv1.y << 8 | yuv1.y" etc.
> 
> 
>> +               uint16_t y1 = yuv2.y << 8;
>> +               uint16_t cb = ((yuv1.u  << 8) + (yuv2.u << 8)) / 8;
> 
> Why divide by 8 instead of 2?

Oops, good that someone is awake! That was a mistake.

  Tomi