[libcamera-devel] [PATCH v3] libcamera: PixelFormat: Replace hex with fourcc and modifiers

[Kaaira Gupta]

Print fourCC characters instead of the hex values in toString() as they
are easier to comprehend. Also, print the corresponding modifiers of the
DRM formats so that it can be more specific.

Describe the modifiers for MIPI vendor as it is mostly used in
libcamera. Print digits for the rest.

Signed-off-by: Kaaira Gupta <kgupta at es.iitr.ac.in>
---

Changes since v2:
	- Remove description for all vendors except for MIPI
	- Change commit message to reflect this change.
	- Change tests accordingly.

Changes since v1:
	- Replaced magic numbers with expressive values.
	- Re-wrote ARM vendor's modifiers
	- Re-wrote the vendors' map with a macro.
	- Changed the copyrights in test file.
	- Changed the tests.

 src/libcamera/pixelformats.cpp | 65 ++++++++++++++++++++++++++++++++--
 test/meson.build               |  1 +
 test/pixel-format.cpp          | 47 ++++++++++++++++++++++++
 3 files changed, 110 insertions(+), 3 deletions(-)
 create mode 100644 test/pixel-format.cpp

diff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp
index 1330dc5..e16f5ba 100644
--- a/src/libcamera/pixelformats.cpp
+++ b/src/libcamera/pixelformats.cpp
@@ -6,6 +6,11 @@
  */
 
 #include <libcamera/pixelformats.h>
+#include <map>
+#include <string.h>
+
+#define PIXELFORMAT_VENDOR(vendor)		    \
+	{ DRM_FORMAT_MOD_VENDOR_## vendor, #vendor }
 
 /**
  * \file pixelformats.h
@@ -108,9 +113,63 @@ bool PixelFormat::operator<(const PixelFormat &other) const
  */
 std::string PixelFormat::toString() const
 {
-	char str[11];
-	snprintf(str, 11, "0x%08x", fourcc_);
-	return str;
+	if (fourcc_ == DRM_FORMAT_INVALID)
+		return "<INVALID>";
+
+	char ss[8] = { static_cast<char>(fourcc_ & 0x7f),
+		       static_cast<char>((fourcc_ >> 8) & 0x7f),
+		       static_cast<char>((fourcc_ >> 16) & 0x7f),
+		       static_cast<char>((fourcc_ >> 24) & 0x7f) };
+
+	for (unsigned int i = 0; i < 4; i++) {
+		if (!isprint(ss[i]))
+			ss[i] = '.';
+	}
+
+	if (fourcc_ & (1 << 31))
+		strcat(ss, "-BE");
+
+	std::string modifier;
+
+	if (modifier_ == DRM_FORMAT_MOD_INVALID) {
+		modifier = " - <INVALID> modifier";
+		return ss + modifier;
+	}
+
+	/* Map vendors with their Ids: */
+	std::map<long int, std::string> vendors = {
+		PIXELFORMAT_VENDOR(NONE),
+		PIXELFORMAT_VENDOR(MIPI)
+	};
+
+	/* Get the vendor name using its Id */
+	long int vendorCode = (modifier_ >> 56) & 0xff;
+	std::string vendor;
+	std::string vendorSpecification;
+
+	switch (vendorCode) {
+	case DRM_FORMAT_MOD_VENDOR_NONE: {
+		vendor = vendors[vendorCode];
+		if (modifier_ == DRM_FORMAT_MOD_LINEAR)
+			vendorSpecification = "Linear Layout";
+		break;
+	}
+	case DRM_FORMAT_MOD_VENDOR_MIPI: {
+		vendor = vendors[vendorCode];
+		if (modifier_ == MIPI_FORMAT_MOD_CSI2_PACKED)
+			vendorSpecification = "CSI-2 packed";
+		break;
+	}
+	default: {
+		vendor = std::to_string(vendorCode);
+		vendorSpecification = std::to_string(modifier_ & 0xff);
+	}
+	}
+
+	modifier = vendor + " : " + vendorSpecification;
+	std::string formatString(ss);
+
+	return formatString + " - " + modifier;
 }
 
 } /* namespace libcamera */
diff --git a/test/meson.build b/test/meson.build
index bd7da14..591d848 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -33,6 +33,7 @@ internal_tests = [
     ['message',                         'message.cpp'],
     ['object',                          'object.cpp'],
     ['object-invoke',                   'object-invoke.cpp'],
+    ['pixel-format',                    'pixel-format.cpp'],
     ['signal-threads',                  'signal-threads.cpp'],
     ['threads',                         'threads.cpp'],
     ['timer',                           'timer.cpp'],
diff --git a/test/pixel-format.cpp b/test/pixel-format.cpp
new file mode 100644
index 0000000..edada56
--- /dev/null
+++ b/test/pixel-format.cpp
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2020, Kaaira Gupta
+ * libcamera pixel format handling test
+ */
+
+#include <iostream>
+#include <vector>
+
+#include "libcamera/pixelformats.h"
+
+#include "test.h"
+
+using namespace std;
+using namespace libcamera;
+
+class PixelFormatTest : public Test
+{
+protected:
+	int run()
+	{
+		std::vector<std::pair<PixelFormat, const char *>> formats{
+			{ PixelFormat(DRM_FORMAT_SRGGB8, DRM_FORMAT_MOD_INVALID), "RGGB - <INVALID> modifier" },
+			{ PixelFormat(DRM_FORMAT_SRGGB8, DRM_FORMAT_MOD_LINEAR), "RGGB - NONE : Linear Layout" },
+			{ PixelFormat(DRM_FORMAT_C8, DRM_FORMAT_MOD_SAMSUNG_64_32_TILE ), "C8   - 4 : 1" },
+			{ PixelFormat(DRM_FORMAT_BIG_ENDIAN,MIPI_FORMAT_MOD_CSI2_PACKED),"....-BE - MIPI : CSI-2 packed"}
+		};
+		for (const auto &format : formats) {
+			if ((format.first).toString() != format.second) {
+				cerr << "Failed to convert PixelFormat "
+				     << format.first.fourcc() << " to string"
+				     << endl;
+				return TestFail;
+			}
+		}
+
+		if (PixelFormat().toString() != "<INVALID>") {
+			cerr << "Failed to convert default PixelFormat to string"
+			     << endl;
+			return TestFail;
+		}
+
+		return TestPass;
+	}
+};
+
+TEST_REGISTER(PixelFormatTest)
-- 
2.17.1