[libcamera-devel] [PATCH v4] libcamera: PixelFormat: Replace hex with fourcc and modifiers
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Jun 2 12:35:43 CEST 2020
Hi Kaaira,
On 28/05/2020 14:05, Kaaira Gupta wrote:
> Print fourCC characters instead of the hex values in toString() as they
> are easier to comprehend. Also, print the corresponding modifiers for
> MIPI vendor as it is mostly used in libcamera.
>
> Signed-off-by: Kaaira Gupta <kgupta at es.iitr.ac.in>
> ---
>
> Changes since v3:
> -shortened the texts.
> -Removed default case as well.
> -changed commit message and tests to reflect the changes.
>
> 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 | 28 +++++++++++++++++---
> test/meson.build | 1 +
> test/pixel-format.cpp | 47 ++++++++++++++++++++++++++++++++++
> 3 files changed, 73 insertions(+), 3 deletions(-)
> create mode 100644 test/pixel-format.cpp
>
> diff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp
> index 1330dc5..16aa7e4 100644
> --- a/src/libcamera/pixelformats.cpp
> +++ b/src/libcamera/pixelformats.cpp
> @@ -6,6 +6,7 @@
> */
>
> #include <libcamera/pixelformats.h>
> +#include <string.h>
>
> /**
> * \file pixelformats.h
> @@ -108,9 +109,30 @@ 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 fourcc[5] = { 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(fourcc[i]))
> + fourcc[i] = '.';
> + }
> +
> + std::string formatString(fourcc);
> +
> + if (fourcc_ & (1 << 31))
> + formatString += "-BE";
> +
> + if (modifier_ == DRM_FORMAT_MOD_INVALID)
> + formatString += ":<INVALID> modifier";
I think this is the only part I find slightly distasteful, as I don't
think we need to explicitly mention the " modifier".
With this removed
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
So if you're happy, I can simply remove the extra word when applying.
> + else if (modifier_ == MIPI_FORMAT_MOD_CSI2_PACKED)
> + formatString += ":packed";
> +
> + return formatString;
> }
>
> } /* 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..84062b7
> --- /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" },
> + { PixelFormat(DRM_FORMAT_C8, DRM_FORMAT_MOD_SAMSUNG_64_32_TILE), "C8 " },
> + { PixelFormat(DRM_FORMAT_BIG_ENDIAN, MIPI_FORMAT_MOD_CSI2_PACKED), "....-BE: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)
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list