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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxx> wrote:
Add support for drawing Y210, Y212, Y216 pixels.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>

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




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux