[libcamera-devel] [PATCH v3] libcamera: v4l2PixelFormat: Replace hex with fourCC
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Mar 26 16:37:05 CET 2020
Hi Kaaira,
On 26/03/2020 09:33, Kaaira Gupta wrote:
> Print fourCC characters instead of the hex value in toString() as they are
> more informative. Also, write the tests for this in formats.cpp
>
> Signed-off-by: Kaaira Gupta <kgupta at es.iitr.ac.in>
Thank you, This looks good to me.
Only a couple of quite minor formatting points, which if there are no
further issues, and you're happy with; I can handle when applying.
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>
> Changes since v2:
> - reformatted the code
> - added cerr messages before TestFail
> - Changed test case to accomodate edge case
> - Increased buffer size of char array
> - Changed the maximum length of for loop.
>
> Changes since v1:
> - Add tests for checking this function.
> - use char[] instead of stringstream.
> - add checks for default value.
> - Print '.' for non-printable characters.
>
> src/libcamera/v4l2_videodevice.cpp | 19 ++++++++++++++++---
> test/v4l2_videodevice/formats.cpp | 20 ++++++++++++++++++++
> 2 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index b778181..bb049ec 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -336,9 +336,22 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const
> */
> std::string V4L2PixelFormat::toString() const
> {
> - char str[11];
> - snprintf(str, 11, "0x%08x", fourcc_);
> - return str;
> + if (fourcc_ == 0)
> + 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");
Probably a blank line here to make sure the return statement is distinct.
> + return ss;
> }
>
> /**
> diff --git a/test/v4l2_videodevice/formats.cpp b/test/v4l2_videodevice/formats.cpp
> index d504d17..4e02afc 100644
> --- a/test/v4l2_videodevice/formats.cpp
> +++ b/test/v4l2_videodevice/formats.cpp
> @@ -47,6 +47,26 @@ protected:
> return TestFail;
> }
>
> + std::vector<std::pair<uint32_t, const char*>> formats{
Missing space between formats and {
> + { V4L2_PIX_FMT_YUYV, "YUYV" }, { 0, "Invalid" },
> + { v4l2_fourcc( 0, 1, 2, 3 ), "...." },
Because this is a table, I would probably put each entry on it's own
line, rather than condensing to fit 80chars or such.
i.e.
{ V4L2_PIX_FMT_YUYV, "YUYV" },
{ 0, "Invalid" },
{ v4l2_fourcc( 0, 1, 2, 3 ), "...." },
...
> + { V4L2_PIX_FMT_Y16_BE, "Y16 -BE" }
> + };
> +
> + for (const auto &format : formats) {
> + if (V4L2PixelFormat(format.first).toString() != format.second) {
> + cerr << "Failed to convert V4L2PixelFormat to its fourcc"
> + << " string" << endl;
> + return TestFail;
> + }
> + }
> +
> + if (V4L2PixelFormat().toString() != "Invalid") {
> + cerr << "Failed to convert default V4L2PixelFormat to 'Invalid'"
> + << " string" << endl;
> + return TestFail;
> + }
> +
> return TestPass;
> }
> };
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list