[libcamera-devel] [PATCH] libcamera: utils: Add hex stream output helper
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sun Oct 13 22:02:20 CEST 2019
Hi Laurent,
Thanks for your work.
On 2019-10-13 22:23:46 +0300, Laurent Pinchart wrote:
> Add a utils::hex() function that simplifies writing hexadecimal values
> to an ostream. The function handles the '0x' prefix, the field width and
> the fill character automatically. Use it through the libcamera code
> base, and add a test.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
I really like this patch!
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/android/camera_device.cpp | 2 +-
> src/libcamera/camera_sensor.cpp | 6 ++--
> src/libcamera/controls.cpp | 6 ++--
> src/libcamera/include/utils.h | 40 ++++++++++++++++++++++
> src/libcamera/stream.cpp | 7 ++--
> src/libcamera/utils.cpp | 39 ++++++++++++++++++++++
> src/libcamera/v4l2_subdevice.cpp | 6 ++--
> src/libcamera/v4l2_videodevice.cpp | 6 ++--
> test/camera-sensor.cpp | 3 +-
> test/meson.build | 1 +
> test/utils.cpp | 53 ++++++++++++++++++++++++++++++
> 11 files changed, 147 insertions(+), 22 deletions(-)
> create mode 100644 test/utils.cpp
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index bf991d5933cd..c7c9b3fd1724 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -640,7 +640,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> << ", direction: " << stream->stream_type
> << ", width: " << stream->width
> << ", height: " << stream->height
> - << ", format: " << std::hex << stream->format;
> + << ", format: " << utils::hex(stream->format);
> }
>
> /* Hardcode viewfinder role, collecting sizes from the stream config. */
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 9e8b44a23850..1b8e8c0e07da 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -14,6 +14,7 @@
> #include <math.h>
>
> #include "formats.h"
> +#include "utils.h"
> #include "v4l2_subdevice.h"
>
> /**
> @@ -79,9 +80,8 @@ int CameraSensor::init()
>
> if (entity_->function() != MEDIA_ENT_F_CAM_SENSOR) {
> LOG(CameraSensor, Error)
> - << "Invalid sensor function 0x"
> - << std::hex << std::setfill('0') << std::setw(8)
> - << entity_->function();
> + << "Invalid sensor function "
> + << utils::hex(entity_->function());
> return -EINVAL;
> }
>
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index e46aa438a06e..6a0301f3a2ae 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -549,8 +549,7 @@ const ControlValue &ControlList::get(unsigned int id) const
> const auto ctrl = idmap_->find(id);
> if (ctrl == idmap_->end()) {
> LOG(Controls, Error)
> - << std::hex << std::setfill('0')
> - << "Control 0x" << std::setw(8) << id
> + << "Control " << utils::hex(id)
> << " is not supported";
> return zero;
> }
> @@ -579,8 +578,7 @@ void ControlList::set(unsigned int id, const ControlValue &value)
> const auto ctrl = idmap_->find(id);
> if (ctrl == idmap_->end()) {
> LOG(Controls, Error)
> - << std::hex << std::setfill('0')
> - << "Control 0x" << std::setw(8) << id
> + << "Control 0x" << utils::hex(id)
> << " is not supported";
> return;
> }
> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
> index 52eee8ac2804..3efb11c119c2 100644
> --- a/src/libcamera/include/utils.h
> +++ b/src/libcamera/include/utils.h
> @@ -10,6 +10,7 @@
> #include <algorithm>
> #include <chrono>
> #include <memory>
> +#include <ostream>
> #include <string>
> #include <sys/time.h>
>
> @@ -63,6 +64,45 @@ using time_point = std::chrono::steady_clock::time_point;
> struct timespec duration_to_timespec(const duration &value);
> std::string time_point_to_string(const time_point &time);
>
> +#ifndef __DOXYGEN__
> +struct _hex {
> + uint64_t v;
> + unsigned int w;
> +};
> +
> +std::basic_ostream<char, std::char_traits<char>> &
> +operator<<(std::basic_ostream<char, std::char_traits<char>> &stream, const _hex &h);
> +#endif
> +
> +template<typename T>
> +_hex hex(T value, unsigned int width = 0);
> +
> +#ifndef __DOXYGEN__
> +template<>
> +inline _hex hex<int32_t>(int32_t value, unsigned int width)
> +{
> + return { static_cast<uint64_t>(value), width ? width : 8 };
> +}
> +
> +template<>
> +inline _hex hex<uint32_t>(uint32_t value, unsigned int width)
> +{
> + return { static_cast<uint64_t>(value), width ? width : 8 };
> +}
> +
> +template<>
> +inline _hex hex<int64_t>(int64_t value, unsigned int width)
> +{
> + return { static_cast<uint64_t>(value), width ? width : 16 };
> +}
> +
> +template<>
> +inline _hex hex<uint64_t>(uint64_t value, unsigned int width)
> +{
> + return { static_cast<uint64_t>(value), width ? width : 16 };
> +}
> +#endif
> +
> } /* namespace utils */
>
> } /* namespace libcamera */
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index c28b4cd669b2..610920d1e5b3 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -16,6 +16,7 @@
> #include <libcamera/request.h>
>
> #include "log.h"
> +#include "utils.h"
>
> /**
> * \file stream.h
> @@ -367,11 +368,7 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
> std::string StreamConfiguration::toString() const
> {
> std::stringstream ss;
> -
> - ss.fill(0);
> - ss << size.toString() << "-0x" << std::hex << std::setw(8)
> - << pixelFormat;
> -
> + ss << size.toString() << "-" << utils::hex(pixelFormat);
> return ss.str();
> }
>
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index 928db254ec67..d632f6e66638 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -143,6 +143,45 @@ std::string time_point_to_string(const time_point &time)
> return ossTimestamp.str();
> }
>
> +std::basic_ostream<char, std::char_traits<char>> &
> +operator<<(std::basic_ostream<char, std::char_traits<char>> &stream, const _hex &h)
> +{
> + stream << "0x";
> +
> + std::ostream::fmtflags flags = stream.setf(std::ios_base::hex,
> + std::ios_base::basefield);
> + std::streamsize width = stream.width(h.w);
> + char fill = stream.fill('0');
> +
> + stream << h.v;
> +
> + stream.flags(flags);
> + stream.width(width);
> + stream.fill(fill);
> +
> + return stream;
> +}
> +
> +/**
> + * \fn hex(T value, unsigned int width)
> + * \brief Write an hexadecimal value to an output string
> + * \param value The value
> + * \param width The width
> + *
> + * Return an object of unspecified type such that, if \a os is the name of an
> + * output stream of type std::ostream, and T is an integer type, then the
> + * expression
> + *
> + * \code{.cpp}
> + * os << utils::hex(value)
> + * \endcode
> + *
> + * will output the \a value to the stream in hexadecimal form with the base
> + * prefix and the filling character set to '0'. The field width is set to \a
> + * width if specified to a non-zero value, or to the native width of type T
> + * otherwise. The \a os stream configuration is not modified.
> + */
> +
> } /* namespace utils */
>
> } /* namespace libcamera */
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index a188298de34c..f2bcd7f73c5c 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -21,6 +21,7 @@
> #include "log.h"
> #include "media_device.h"
> #include "media_object.h"
> +#include "utils.h"
>
> /**
> * \file v4l2_subdevice.h
> @@ -76,10 +77,7 @@ LOG_DECLARE_CATEGORY(V4L2)
> const std::string V4L2SubdeviceFormat::toString() const
> {
> std::stringstream ss;
> -
> - ss.fill(0);
> - ss << size.toString() << "-0x" << std::hex << std::setw(4) << mbus_code;
> -
> + ss << size.toString() << "-" << utils::hex(mbus_code, 4);
> return ss.str();
> }
>
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index eb4e44deb4a5..208ab54199b1 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -23,6 +23,7 @@
> #include "log.h"
> #include "media_device.h"
> #include "media_object.h"
> +#include "utils.h"
>
> /**
> * \file v4l2_videodevice.h
> @@ -239,10 +240,7 @@ LOG_DECLARE_CATEGORY(V4L2)
> const std::string V4L2DeviceFormat::toString() const
> {
> std::stringstream ss;
> -
> - ss.fill(0);
> - ss << size.toString() << "-0x" << std::hex << std::setw(8) << fourcc;
> -
> + ss << size.toString() << "-" << utils::hex(fourcc);
> return ss.str();
> }
>
> diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp
> index 9fe59cc98d79..27c190fe7ace 100644
> --- a/test/camera-sensor.cpp
> +++ b/test/camera-sensor.cpp
> @@ -13,6 +13,7 @@
> #include "camera_sensor.h"
> #include "device_enumerator.h"
> #include "media_device.h"
> +#include "utils.h"
> #include "v4l2_subdevice.h"
>
> #include "test.h"
> @@ -91,7 +92,7 @@ protected:
> if (format.mbus_code != MEDIA_BUS_FMT_SBGGR10_1X10 ||
> format.size != Size(4096, 2160)) {
> cerr << "Failed to get a suitable format, expected 4096x2160-0x"
> - << std::hex << MEDIA_BUS_FMT_SBGGR10_1X10
> + << utils::hex(MEDIA_BUS_FMT_SBGGR10_1X10)
> << ", got " << format.toString() << endl;
> return TestFail;
> }
> diff --git a/test/meson.build b/test/meson.build
> index 84722cceb35d..cf5eb84d20b2 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -30,6 +30,7 @@ internal_tests = [
> ['threads', 'threads.cpp'],
> ['timer', 'timer.cpp'],
> ['timer-thread', 'timer-thread.cpp'],
> + ['utils', 'utils.cpp'],
> ]
>
> foreach t : public_tests
> diff --git a/test/utils.cpp b/test/utils.cpp
> new file mode 100644
> index 000000000000..9fe0d4775b73
> --- /dev/null
> +++ b/test/utils.cpp
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2018, Google Inc.
> + *
> + * utils.cpp - Miscellaneous utility tests
> + */
> +
> +#include <iostream>
> +#include <sstream>
> +
> +#include "test.h"
> +#include "utils.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +class UtilsTest : public Test
> +{
> +protected:
> + int run()
> + {
> + std::ostringstream os;
> + std::string ref;
> +
> + os << utils::hex(static_cast<int32_t>(0x42)) << " ";
> + ref += "0x00000042 ";
> + os << utils::hex(static_cast<uint32_t>(0x42)) << " ";
> + ref += "0x00000042 ";
> + os << utils::hex(static_cast<int64_t>(0x42)) << " ";
> + ref += "0x0000000000000042 ";
> + os << utils::hex(static_cast<uint64_t>(0x42)) << " ";
> + ref += "0x0000000000000042 ";
> + os << utils::hex(static_cast<int32_t>(0x42), 4) << " ";
> + ref += "0x0042 ";
> + os << utils::hex(static_cast<uint32_t>(0x42), 1) << " ";
> + ref += "0x42 ";
> + os << utils::hex(static_cast<int64_t>(0x42), 4) << " ";
> + ref += "0x0042 ";
> + os << utils::hex(static_cast<uint64_t>(0x42), 1) << " ";
> + ref += "0x42 ";
> +
> + std::string s = os.str();
> + if (s != ref) {
> + cerr << "utils::hex() test failed, expected '" << ref
> + << "', got '" << s << "'";
> + return TestFail;
> + }
> +
> + return TestPass;
> + }
> +};
> +
> +TEST_REGISTER(UtilsTest)
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list