Implement 24 bit pixel formats for raw and tight

include/pixels.h

@@ -23,9 +23,9 @@
 
 struct rfb_pixel_format;
 
-void pixel32_to_cpixel(uint8_t* restrict dst,
+void pixel_to_cpixel(uint8_t* restrict dst,
                        const struct rfb_pixel_format* dst_fmt,
-                       const uint32_t* restrict src,
+                       const uint8_t* restrict src,
                        const struct rfb_pixel_format* src_fmt,
                        size_t bytes_per_cpixel, size_t len);
 

src/cursor.c

@@ -94,12 +94,14 @@ int cursor_encode(struct vec* dst, struct rfb_pixel_format* pixfmt,
 	uint8_t* dstdata = dst->data;
 	dstdata += dst->len;
 
+	int32_t src_byte_stride = image->stride * (srcfmt.bits_per_pixel / 8);
+
 	if((int32_t)width == image->stride) {
-		pixel32_to_cpixel(dstdata, pixfmt, image->addr, &srcfmt, bpp, size);
+		pixel_to_cpixel(dstdata, pixfmt, image->addr, &srcfmt, bpp, size);
 	} else {
 		for (uint32_t y = 0; y < height; ++y) {
-			pixel32_to_cpixel(dstdata + y * bpp * width, pixfmt,
+			pixel_to_cpixel(dstdata + y * bpp * width, pixfmt,
-					(uint32_t*)image->addr + y * image->stride,
+					(uint8_t*)image->addr + y * src_byte_stride,
 					&srcfmt, bpp, width);
 		}
 	}

src/damage-refinery.c

@@ -22,6 +22,7 @@
 #include <sys/param.h>
 
 #include "fb.h"
+#include "pixels.h"
 #include "damage-refinery.h"
 #include "murmurhash.h"
 
@@ -63,8 +64,9 @@ void damage_refinery_destroy(struct damage_refinery* self)
 static uint32_t damage_hash_tile(struct damage_refinery* self, uint32_t tx,
 		uint32_t ty, const struct nvnc_fb* buffer)
 {
-	uint32_t* pixels = buffer->addr;
+	uint8_t* pixels = buffer->addr;
-	int pixel_stride = buffer->stride;
+	int bpp = pixel_size_from_fourcc(buffer->fourcc_format);
+	int byte_stride = buffer->stride * bpp;
 
 	int x_start = tx * 32;
 	int x_stop = MIN((tx + 1) * 32, self->width);
@@ -72,11 +74,11 @@ static uint32_t damage_hash_tile(struct damage_refinery* self, uint32_t tx,
 	int y_stop = MIN((ty + 1) * 32, self->height);
 
 	uint32_t hash = 0;
+	int32_t xoff = x_start * bpp;
 
-	// TODO: Support different pixel sizes
 	for (int y = y_start; y < y_stop; ++y)
-		hash = murmurhash((void*)&(pixels[x_start + y * pixel_stride]),
+		hash = murmurhash((void*)&(pixels[xoff + y * byte_stride]),
-				4 * (x_stop - x_start), hash);
+				bpp * (x_stop - x_start), hash);
 
 	return hash;
 }

src/fb.c

@@ -41,6 +41,8 @@ struct nvnc_fb* nvnc_fb_new(uint16_t width, uint16_t height,
 	if (!fb)
 		return NULL;
 
+	uint32_t bpp = pixel_size_from_fourcc(fourcc_format);
+
 	fb->type = NVNC_FB_SIMPLE;
 	fb->ref = 1;
 	fb->width = width;
@@ -49,7 +51,7 @@ struct nvnc_fb* nvnc_fb_new(uint16_t width, uint16_t height,
 	fb->stride = stride;
 	fb->pts = NVNC_NO_PTS;
 
-	size_t size = height * stride * 4; /* Assume 4 byte format for now */
+	size_t size = height * stride * bpp;
 	size_t alignment = MAX(4, sizeof(void*));
 	size_t aligned_size = ALIGN_UP(size, alignment);
 

src/pixels.c

@@ -25,14 +25,13 @@
 #define XSTR(s) STR(s)
 #define STR(s) #s
 
-void pixel32_to_cpixel(uint8_t* restrict dst,
+static void pixel32_to_cpixel(uint8_t* restrict dst,
              const struct rfb_pixel_format* dst_fmt,
              const uint32_t* restrict src,
              const struct rfb_pixel_format* src_fmt,
              size_t bytes_per_cpixel, size_t len)
 {
 	assert(src_fmt->true_colour_flag);
-	assert(src_fmt->bits_per_pixel == 32);
 	assert(src_fmt->depth <= 32);
 	assert(dst_fmt->true_colour_flag);
 	assert(dst_fmt->bits_per_pixel <= 32);
@@ -152,6 +151,148 @@ void pixel32_to_cpixel(uint8_t* restrict dst,
 #undef CONVERT_PIXELS
 }
 
+void pixel_to_cpixel(uint8_t* restrict dst,
+                       const struct rfb_pixel_format* dst_fmt,
+                       const uint8_t* restrict src,
+                       const struct rfb_pixel_format* src_fmt,
+                       size_t bytes_per_cpixel, size_t len)
+{
+	if (src_fmt->bits_per_pixel == 32) {
+		pixel32_to_cpixel(dst, dst_fmt, (uint32_t*)src, src_fmt, bytes_per_cpixel, len);
+		return;
+	}
+
+	assert(src_fmt->true_colour_flag);
+	assert(src_fmt->depth <= 32);
+	assert(dst_fmt->true_colour_flag);
+	assert(dst_fmt->bits_per_pixel <= 32);
+	assert(dst_fmt->depth <= 32);
+	assert(bytes_per_cpixel <= 4 && bytes_per_cpixel >= 1);
+
+	uint32_t src_bpp = src_fmt->bits_per_pixel / 8;
+	uint32_t src_red_shift = src_fmt->red_shift;
+	uint32_t src_green_shift = src_fmt->green_shift;
+	uint32_t src_blue_shift = src_fmt->blue_shift;
+
+	uint32_t dst_red_shift = dst_fmt->red_shift;
+	uint32_t dst_green_shift = dst_fmt->green_shift;
+	uint32_t dst_blue_shift = dst_fmt->blue_shift;
+
+	uint32_t src_red_max = src_fmt->red_max;
+	uint32_t src_green_max = src_fmt->green_max;
+	uint32_t src_blue_max = src_fmt->blue_max;
+
+	uint32_t src_red_bits = POPCOUNT(src_fmt->red_max);
+	uint32_t src_green_bits = POPCOUNT(src_fmt->green_max);
+	uint32_t src_blue_bits = POPCOUNT(src_fmt->blue_max);
+
+	uint32_t dst_red_bits = POPCOUNT(dst_fmt->red_max);
+	uint32_t dst_green_bits = POPCOUNT(dst_fmt->green_max);
+	uint32_t dst_blue_bits = POPCOUNT(dst_fmt->blue_max);
+
+	uint32_t dst_endian_correction;
+
+#define CONVERT_PIXELS(cpx, px)                                                \
+	{                                                                      \
+		uint32_t r, g, b;                                              \
+		r = ((px >> src_red_shift) & src_red_max) << dst_red_bits      \
+		        >> src_red_bits << dst_red_shift;                      \
+		g = ((px >> src_green_shift) & src_green_max) << dst_green_bits\
+		        >> src_green_bits << dst_green_shift;                  \
+		b = ((px >> src_blue_shift) & src_blue_max) << dst_blue_bits   \
+		        >> src_blue_bits << dst_blue_shift;                    \
+		cpx = r | g | b;                                               \
+	}
+
+	switch (bytes_per_cpixel) {
+	case 4:
+		if (dst_fmt->big_endian_flag) {
+			while (len--) {
+				uint32_t cpx, px = 0;
+				memcpy(&px, src, src_bpp);
+				src += src_bpp;
+
+				CONVERT_PIXELS(cpx, px)
+
+				*dst++ = (cpx >> 24) & 0xff;
+				*dst++ = (cpx >> 16) & 0xff;
+				*dst++ = (cpx >> 8) & 0xff;
+				*dst++ = (cpx >> 0) & 0xff;
+			}
+		} else {
+			while (len--) {
+				uint32_t cpx, px = 0;
+				memcpy(&px, src, src_bpp);
+				src += src_bpp;
+
+				CONVERT_PIXELS(cpx, px)
+
+				*dst++ = (cpx >> 0) & 0xff;
+				*dst++ = (cpx >> 8) & 0xff;
+				*dst++ = (cpx >> 16) & 0xff;
+				*dst++ = (cpx >> 24) & 0xff;
+			}
+		}
+		break;
+	case 3:
+		if (dst_fmt->bits_per_pixel == 32 && dst_fmt->depth <= 24) {
+			uint32_t min_dst_shift = dst_red_shift;
+			if (min_dst_shift > dst_green_shift)
+				min_dst_shift = dst_green_shift;
+			if (min_dst_shift > dst_blue_shift)
+				min_dst_shift = dst_blue_shift;
+
+			dst_red_shift -= min_dst_shift;
+			dst_green_shift -= min_dst_shift;
+			dst_blue_shift -= min_dst_shift;
+		}
+
+		dst_endian_correction = dst_fmt->big_endian_flag ? 16 : 0;
+
+		while (len--) {
+			uint32_t cpx, px = 0;
+			memcpy(&px, src, src_bpp);
+			src += src_bpp;
+
+			CONVERT_PIXELS(cpx, px)
+
+			*dst++ = (cpx >> (0 ^ dst_endian_correction)) & 0xff;
+			*dst++ = (cpx >> 8) & 0xff;
+			*dst++ = (cpx >> (16 ^ dst_endian_correction)) & 0xff;
+		}
+		break;
+	case 2:
+		dst_endian_correction = dst_fmt->big_endian_flag ? 8 : 0;
+
+		while (len--) {
+			uint32_t cpx, px = 0;
+			memcpy(&px, src, src_bpp);
+			src += src_bpp;
+
+			CONVERT_PIXELS(cpx, px)
+
+			*dst++ = (cpx >> (0 ^ dst_endian_correction)) & 0xff;
+			*dst++ = (cpx >> (8 ^ dst_endian_correction)) & 0xff;
+		}
+		break;
+	case 1:
+		while (len--) {
+			uint32_t cpx, px = 0;
+			memcpy(&px, src, src_bpp);
+			src += src_bpp;
+
+			CONVERT_PIXELS(cpx, px)
+
+			*dst++ = cpx & 0xff;
+		}
+		break;
+	default:
+		abort();
+	}
+
+#undef CONVERT_PIXELS
+}
+
 /* clang-format off */
 int rfb_pixfmt_from_fourcc(struct rfb_pixel_format *dst, uint32_t src) {
 	switch (src & ~DRM_FORMAT_BIG_ENDIAN) {
@@ -215,6 +356,22 @@ bpp_32:
 		dst->green_max = 0xff;
 		dst->blue_max = 0xff;
 		break;
+	case DRM_FORMAT_BGR888:
+		dst->red_shift = 0;
+		dst->green_shift = 8;
+		dst->blue_shift = 16;
+		goto bpp_24;
+	case DRM_FORMAT_RGB888:
+		dst->red_shift = 16;
+		dst->green_shift = 8;
+		dst->blue_shift = 0;
+bpp_24:
+		dst->bits_per_pixel = 24;
+		dst->depth = 24;
+		dst->red_max = 0xff;
+		dst->green_max = 0xff;
+		dst->blue_max = 0xff;
+		break;
 	case DRM_FORMAT_RGBA4444:
 	case DRM_FORMAT_RGBX4444:
 		dst->red_shift = 12;
@@ -275,6 +432,9 @@ int pixel_size_from_fourcc(uint32_t fourcc)
 	case DRM_FORMAT_ABGR8888:
 	case DRM_FORMAT_XBGR8888:
 		return 4;
+	case DRM_FORMAT_BGR888:
+	case DRM_FORMAT_RGB888:
+		return 3;
 	case DRM_FORMAT_RGBA4444:
 	case DRM_FORMAT_RGBX4444:
 	case DRM_FORMAT_BGRA4444:
@@ -457,6 +617,8 @@ const char* drm_format_to_string(uint32_t fmt)
 	X(XRGB8888) \
 	X(ABGR8888) \
 	X(XBGR8888) \
+	X(RGB888) \
+	X(BGR888) \
 	X(RGBA4444) \
 	X(RGBX4444) \
 	X(BGRA4444) \

src/raw-encoding.c

@@ -69,7 +69,10 @@ static int raw_encode_box(struct raw_encoder_work* ctx, struct vec* dst,
 	if (rc < 0)
 		return -1;
 
-	uint32_t* b = fb->addr;
+	uint8_t* b = fb->addr;
+	int32_t src_bpp = src_fmt->bits_per_pixel / 8;
+	int32_t xoff = x_start * src_bpp;
+	int32_t src_stride = fb->stride * src_bpp;
 
 	int bpp = dst_fmt->bits_per_pixel / 8;
 
@@ -80,8 +83,8 @@ static int raw_encode_box(struct raw_encoder_work* ctx, struct vec* dst,
 	uint8_t* d = dst->data;
 
 	for (int y = y_start; y < y_start + height; ++y) {
-		pixel32_to_cpixel(d + dst->len, dst_fmt,
+		pixel_to_cpixel(d + dst->len, dst_fmt,
-		                  b + x_start + y * stride, src_fmt,
+		                  b + xoff + y * src_stride, src_fmt,
 		                  bpp, width);
 		dst->len += width * bpp;
 	}
@@ -131,7 +134,7 @@ static void raw_encoder_do_work(void* obj)
 	struct nvnc_fb* fb = ctx->fb;
 	assert(fb);
 
-	size_t bpp = nvnc_fb_get_pixel_size(fb);
+	size_t bpp = ctx->output_format.bits_per_pixel / 8;
 	size_t n_rects = pixman_region_n_rects(&ctx->damage);
 	if (n_rects > UINT16_MAX)
 		n_rects = 1;

src/tight.c

@@ -302,13 +302,14 @@ static void tight_encode_tile_basic(struct tight_encoder* self,
 	else
 		memcpy(&cfmt, &self->dfmt, sizeof(cfmt));
 
-	uint32_t* addr = nvnc_fb_get_addr(self->fb);
+	uint8_t* addr = nvnc_fb_get_addr(self->fb);
-	int32_t stride = nvnc_fb_get_stride(self->fb);
+	int32_t bpp = self->sfmt.bits_per_pixel / 8;
-
+	int32_t byte_stride = nvnc_fb_get_stride(self->fb) * bpp;
+	int32_t xoff = x * bpp;
 	// TODO: Limit width and hight to the sides
 	for (uint32_t y = y_start; y < y_start + height; ++y) {
-		void* img = addr + x + y * stride;
+		uint8_t* img = addr + xoff + y * byte_stride;
-		pixel32_to_cpixel(row, &cfmt, img, &self->sfmt,
+		pixel_to_cpixel(row, &cfmt, img, &self->sfmt,
 			bytes_per_cpixel, width);
 
 		// TODO What to do if the buffer fills up?
@@ -335,6 +336,10 @@ static enum TJPF tight_get_jpeg_pixfmt(uint32_t fourcc)
 	case DRM_FORMAT_ABGR8888:
 	case DRM_FORMAT_XBGR8888:
 		return TJPF_RGBX;
+	case DRM_FORMAT_BGR888:
+		return TJPF_RGB;
+	case DRM_FORMAT_RGB888:
+		return TJPF_BGR;
 	}
 
 	return TJPF_UNKNOWN;
@@ -360,14 +365,16 @@ static int tight_encode_tile_jpeg(struct tight_encoder* self,
 	if (!handle)
 		return -1;
 
-	uint32_t* addr = nvnc_fb_get_addr(self->fb);
+	uint8_t* addr = nvnc_fb_get_addr(self->fb);
-	int32_t stride = nvnc_fb_get_stride(self->fb);
+	int32_t bpp = self->sfmt.bits_per_pixel / 8;
-	void* img = (uint32_t*)addr + x + y * stride;
+	int32_t byte_stride = nvnc_fb_get_stride(self->fb) * bpp;
+	int32_t xoff = x * bpp;
+	uint8_t* img = addr + xoff + y * byte_stride;
 
 	enum TJSAMP subsampling = (quality == 9) ? TJSAMP_444 : TJSAMP_420;
 
 	int rc = -1;
-	rc = tjCompress2(handle, img, width, stride * 4, height, tjfmt, &buffer,
+	rc = tjCompress2(handle, img, width, byte_stride, height, tjfmt, &buffer,
 			&size, subsampling, quality, TJFLAG_FASTDCT);
 	if (rc < 0) {
 		nvnc_log(NVNC_LOG_ERROR, "Failed to encode tight JPEG box: %s",

src/zrle.c

@@ -102,7 +102,7 @@ static void zrle_encode_unichrome_tile(struct vec* dst,
 
 	vec_fast_append_8(dst, 1);
 
-	pixel32_to_cpixel(((uint8_t*)dst->data) + 1, dst_fmt, &colour, src_fmt,
+	pixel_to_cpixel(((uint8_t*)dst->data) + 1, dst_fmt, (uint8_t*)&colour, src_fmt,
 	                  bytes_per_cpixel, 1);
 
 	dst->len += bytes_per_cpixel;
@@ -135,7 +135,7 @@ static void zrle_encode_packed_tile(struct vec* dst,
 	int bytes_per_cpixel = calc_bytes_per_cpixel(dst_fmt);
 
 	uint8_t cpalette[16 * 3];
-	pixel32_to_cpixel((uint8_t*)cpalette, dst_fmt, palette, src_fmt,
+	pixel_to_cpixel((uint8_t*)cpalette, dst_fmt, (uint8_t*)palette, src_fmt,
 	                  bytes_per_cpixel, palette_size);
 
 	vec_fast_append_8(dst, 128 | palette_size);
@@ -196,7 +196,7 @@ static void zrle_encode_tile(struct vec* dst,
 
 	vec_fast_append_8(dst, 0);
 
-	pixel32_to_cpixel(((uint8_t*)dst->data) + 1, dst_fmt, src, src_fmt,
+	pixel_to_cpixel(((uint8_t*)dst->data) + 1, dst_fmt, (uint8_t*)src, src_fmt,
 	                  bytes_per_cpixel, length);
 
 	dst->len += bytes_per_cpixel * length;

test/test-pixels.c

@@ -22,10 +22,11 @@
 #define UDIV_UP(a, b) (((a) + (b) - 1) / (b))
 #define ARRAY_LEN(a) (sizeof(a) / (sizeof(a[0])))
 
-static bool test_pixel32_to_cpixel_4bpp(void)
+static bool test_pixel_to_cpixel_4bpp(void)
 {
 	uint32_t src = u32_le(0x11223344u);
 	uint32_t dst;
+	uint8_t* src_addr = (uint8_t*)&src;
 
 	struct rfb_pixel_format dstfmt = { 0 }, srcfmt = { 0 };
 
@@ -33,25 +34,63 @@ static bool test_pixel32_to_cpixel_4bpp(void)
 
 	dst = 0;
 	rfb_pixfmt_from_fourcc(&srcfmt, DRM_FORMAT_RGBA8888);
-	pixel32_to_cpixel((uint8_t*)&dst, &dstfmt, &src, &srcfmt, 4, 1);
+	pixel_to_cpixel((uint8_t*)&dst, &dstfmt, src_addr, &srcfmt, 4, 1);
 	if ((src & 0xffffff00u) != (dst & 0xffffff00u))
 		return false;
 
 	dst = 0;
 	rfb_pixfmt_from_fourcc(&dstfmt, DRM_FORMAT_ABGR8888);
-	pixel32_to_cpixel((uint8_t*)&dst, &dstfmt, &src, &srcfmt, 4, 1);
+	pixel_to_cpixel((uint8_t*)&dst, &dstfmt, src_addr, &srcfmt, 4, 1);
 	if (dst != u32_le(0x00332211u))
 		return false;
 
 	dst = 0;
 	rfb_pixfmt_from_fourcc(&dstfmt, DRM_FORMAT_ARGB8888);
-	pixel32_to_cpixel((uint8_t*)&dst, &dstfmt, &src, &srcfmt, 4, 1);
+	pixel_to_cpixel((uint8_t*)&dst, &dstfmt, src_addr, &srcfmt, 4, 1);
 	if (dst != u32_le(0x00112233u))
 		return false;
 
 	dst = 0;
 	rfb_pixfmt_from_fourcc(&dstfmt, DRM_FORMAT_BGRA8888);
-	pixel32_to_cpixel((uint8_t*)&dst, &dstfmt, &src, &srcfmt, 4, 1);
+	pixel_to_cpixel((uint8_t*)&dst, &dstfmt, src_addr, &srcfmt, 4, 1);
+	if (dst != u32_le(0x33221100u))
+		return false;
+
+	return true;
+}
+
+static bool test_pixel_to_cpixel_3bpp(void)
+{
+	//44 is extra data that should not be copied anywhere below.
+	uint32_t src = u32_le(0x44112233u);
+	uint32_t dst;
+	uint8_t* src_addr = (uint8_t*)&src;
+
+	struct rfb_pixel_format dstfmt = { 0 }, srcfmt = { 0 };
+
+	rfb_pixfmt_from_fourcc(&srcfmt, DRM_FORMAT_RGB888);
+
+	dst = 0;
+	rfb_pixfmt_from_fourcc(&dstfmt, DRM_FORMAT_RGBA8888);
+	pixel_to_cpixel((uint8_t*)&dst, &dstfmt, src_addr, &srcfmt, 4, 1);
+	if (dst != u32_le(0x11223300u))
+		return false;
+
+	dst = 0;
+	rfb_pixfmt_from_fourcc(&dstfmt, DRM_FORMAT_ABGR8888);
+	pixel_to_cpixel((uint8_t*)&dst, &dstfmt, src_addr, &srcfmt, 4, 1);
+	if (dst != u32_le(0x00332211u))
+		return false;
+
+	dst = 0;
+	rfb_pixfmt_from_fourcc(&dstfmt, DRM_FORMAT_ARGB8888);
+	pixel_to_cpixel((uint8_t*)&dst, &dstfmt, src_addr, &srcfmt, 4, 1);
+	if (dst != u32_le(0x00112233u))
+		return false;
+
+	dst = 0;
+	rfb_pixfmt_from_fourcc(&dstfmt, DRM_FORMAT_BGRA8888);
+	pixel_to_cpixel((uint8_t*)&dst, &dstfmt, src_addr, &srcfmt, 4, 1);
 	if (dst != u32_le(0x33221100u))
 		return false;
 
@@ -173,7 +212,8 @@ static bool test_rfb_pixfmt_to_string(void)
 
 int main()
 {
-	bool ok = test_pixel32_to_cpixel_4bpp() &&
+	bool ok = test_pixel_to_cpixel_4bpp() &&
+		test_pixel_to_cpixel_3bpp() &&
 		test_fourcc_to_pixman_fmt() &&
 		test_extract_alpha_mask_rgba8888() &&
 		test_drm_format_to_string() &&