[libcamera-devel] [PATCH] cam: convert to libfmt
Naushir Patuck
naush at raspberrypi.com
Tue May 10 11:49:14 CEST 2022
Hi Tomi,
On Tue, 10 May 2022 at 08:16, Tomi Valkeinen via libcamera-devel <
libcamera-devel at lists.libcamera.org> wrote:
> This is just a conversation starter, not for merging. I really like
> libfmt, and I really hate the C++ stream-based string formatting. libfmt
> is the first library I add to any new C++ project I make.
>
> You can find more information about libfmt from:
>
> https://github.com/fmtlib/fmt
> https://fmt.dev/latest/index.html
>
> This patch is just a crude conversion with ugly macros to showcase what
> the formatting code might look like.
>
> libfmt pages suggest that libfmt would be faster and smaller (exe size)
> than iostreams, but for the size it didn't seem to be true in cam's case
> as the tests below show. However, simple prints did reduce the exe size,
> but the few more complex ones increased the size.
>
> Size tests with gcc 11.2.0-19ubuntu1
>
> - Without libfmt
>
> debug 3523400
> debug lto 3269368
> release 223056
> release lto 172280
>
> - With libfmt
>
> debug 4424256
> debug lto 4143840
> release 303952
> release lto 252640
>
> Above shows that cam's size clearly increases with libfmt. However, the
> increase really comes only from one case, the use of fmt::memory_buffer
> and std::back_inserter. Converting that code to use fmt::format() and
> naive string append gives:
>
> release with string append 233680
> release lto with string append 186936
>
> Also, if I add another use of fmt::memory_buffer and std::back_inserter
> to another file, I see much less increase in the size:
>
> release lto with two uses of memory_buffer, back_inserter 256736
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>
For what it's worth, I absolutely loathe formatting in std iostream, and
libfmt is a wonderful alternative.
It also is close enough to the C++20 std::format implementation that
eventual porting would be low effort. So I am all for this change :)
Regards,
Naush
> ---
> src/cam/camera_session.cpp | 105 ++++++++++++++++---------------------
> src/cam/drm.cpp | 68 ++++++++----------------
> src/cam/event_loop.cpp | 9 ++--
> src/cam/file_sink.cpp | 31 +++++------
> src/cam/image.cpp | 15 +++---
> src/cam/kms_sink.cpp | 38 ++++++--------
> src/cam/main.cpp | 28 +++++-----
> src/cam/meson.build | 5 +-
> src/cam/options.cpp | 66 ++++++++++-------------
> src/cam/stream_options.cpp | 18 +++----
> 10 files changed, 165 insertions(+), 218 deletions(-)
>
> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> index efffafbf..7843c3fd 100644
> --- a/src/cam/camera_session.cpp
> +++ b/src/cam/camera_session.cpp
> @@ -5,10 +5,9 @@
> * camera_session.cpp - Camera capture session
> */
>
> -#include <iomanip>
> -#include <iostream>
> #include <limits.h>
> -#include <sstream>
> +#include <fmt/format.h>
> +#include <fmt/ostream.h>
>
> #include <libcamera/control_ids.h>
> #include <libcamera/property_ids.h>
> @@ -22,6 +21,9 @@
> #include "main.h"
> #include "stream_options.h"
>
> +#define PR(...) fmt::print(__VA_ARGS__)
> +#define EPR(...) fmt::print(stderr, __VA_ARGS__)
> +
> using namespace libcamera;
>
> CameraSession::CameraSession(CameraManager *cm,
> @@ -40,13 +42,12 @@ CameraSession::CameraSession(CameraManager *cm,
> camera_ = cm->get(cameraId);
>
> if (!camera_) {
> - std::cerr << "Camera " << cameraId << " not found" <<
> std::endl;
> + EPR("Camera {} not found\n", cameraId);
> return;
> }
>
> if (camera_->acquire()) {
> - std::cerr << "Failed to acquire camera " << cameraId
> - << std::endl;
> + EPR("Failed to acquire camera {}", cameraId);
> return;
> }
>
> @@ -55,15 +56,14 @@ CameraSession::CameraSession(CameraManager *cm,
> std::unique_ptr<CameraConfiguration> config =
> camera_->generateConfiguration(roles);
> if (!config || config->size() != roles.size()) {
> - std::cerr << "Failed to get default stream configuration"
> - << std::endl;
> + EPR("Failed to get default stream configuration\n");
> return;
> }
>
> /* Apply configuration if explicitly requested. */
> if (StreamKeyValueParser::updateConfiguration(config.get(),
>
> options_[OptStream])) {
> - std::cerr << "Failed to update configuration" << std::endl;
> + EPR("Failed to update configuration\n");
> return;
> }
>
> @@ -72,20 +72,17 @@ CameraSession::CameraSession(CameraManager *cm,
> #ifdef HAVE_KMS
> if (options_.isSet(OptDisplay)) {
> if (options_.isSet(OptFile)) {
> - std::cerr << "--display and --file options are
> mutually exclusive"
> - << std::endl;
> + EPR("--display and --file options are mutually
> exclusive\n");
> return;
> }
>
> if (roles.size() != 1) {
> - std::cerr << "Display doesn't support multiple
> streams"
> - << std::endl;
> + EPR("Display doesn't support multiple streams\n");
> return;
> }
>
> if (roles[0] != StreamRole::Viewfinder) {
> - std::cerr << "Display requires a viewfinder stream"
> - << std::endl;
> + EPR("Display requires a viewfinder stream\n");
> return;
> }
> }
> @@ -97,15 +94,14 @@ CameraSession::CameraSession(CameraManager *cm,
>
> case CameraConfiguration::Adjusted:
> if (strictFormats) {
> - std::cout << "Adjusting camera configuration
> disallowed by --strict-formats argument"
> - << std::endl;
> + PR("Adjusting camera configuration disallowed by
> --strict-formats argument\n");
> return;
> }
> - std::cout << "Camera configuration adjusted" << std::endl;
> + PR("Camera configuration adjusted\n");
> break;
>
> case CameraConfiguration::Invalid:
> - std::cout << "Camera configuration invalid" << std::endl;
> + PR("Camera configuration invalid\n");
> return;
> }
>
> @@ -121,8 +117,7 @@ CameraSession::~CameraSession()
> void CameraSession::listControls() const
> {
> for (const auto &[id, info] : camera_->controls()) {
> - std::cout << "Control: " << id->name() << ": "
> - << info.toString() << std::endl;
> + PR("Control: {}: {}}n", id->name(), info.toString());
> }
> }
>
> @@ -131,8 +126,7 @@ void CameraSession::listProperties() const
> for (const auto &[key, value] : camera_->properties()) {
> const ControlId *id = properties::properties.at(key);
>
> - std::cout << "Property: " << id->name() << " = "
> - << value.toString() << std::endl;
> + PR("Property: {} = {}\n", id->name(), value.toString());
> }
> }
>
> @@ -140,17 +134,15 @@ void CameraSession::infoConfiguration() const
> {
> unsigned int index = 0;
> for (const StreamConfiguration &cfg : *config_) {
> - std::cout << index << ": " << cfg.toString() << std::endl;
> + PR("{}: {}\n", index, cfg.toString());
>
> const StreamFormats &formats = cfg.formats();
> for (PixelFormat pixelformat : formats.pixelformats()) {
> - std::cout << " * Pixelformat: "
> - << pixelformat << " "
> - << formats.range(pixelformat).toString()
> - << std::endl;
> + PR(" * Pixelformat: {} {}\n", pixelformat,
> + formats.range(pixelformat).toString());
>
> for (const Size &size : formats.sizes(pixelformat))
> - std::cout << " - " << size << std::endl;
> + PR(" - {}\n", size);
> }
>
> index++;
> @@ -168,7 +160,7 @@ int CameraSession::start()
>
> ret = camera_->configure(config_.get());
> if (ret < 0) {
> - std::cout << "Failed to configure camera" << std::endl;
> + PR("Failed to configure camera\n");
> return ret;
> }
>
> @@ -197,8 +189,7 @@ int CameraSession::start()
> if (sink_) {
> ret = sink_->configure(*config_);
> if (ret < 0) {
> - std::cout << "Failed to configure frame sink"
> - << std::endl;
> + PR("Failed to configure frame sink\n");
> return ret;
> }
>
> @@ -214,12 +205,12 @@ void CameraSession::stop()
> {
> int ret = camera_->stop();
> if (ret)
> - std::cout << "Failed to stop capture" << std::endl;
> + PR("Failed to stop capture\n");
>
> if (sink_) {
> ret = sink_->stop();
> if (ret)
> - std::cout << "Failed to stop frame sink" <<
> std::endl;
> + PR("Failed to stop frame sink\n");
> }
>
> sink_.reset();
> @@ -238,7 +229,7 @@ int CameraSession::startCapture()
> for (StreamConfiguration &cfg : *config_) {
> ret = allocator_->allocate(cfg.stream());
> if (ret < 0) {
> - std::cerr << "Can't allocate buffers" << std::endl;
> + EPR("Can't allocate buffers\n");
> return -ENOMEM;
> }
>
> @@ -254,7 +245,7 @@ int CameraSession::startCapture()
> for (unsigned int i = 0; i < nbuffers; i++) {
> std::unique_ptr<Request> request =
> camera_->createRequest();
> if (!request) {
> - std::cerr << "Can't create request" << std::endl;
> + EPR("Can't create request\n");
> return -ENOMEM;
> }
>
> @@ -266,8 +257,7 @@ int CameraSession::startCapture()
>
> ret = request->addBuffer(stream, buffer.get());
> if (ret < 0) {
> - std::cerr << "Can't set buffer for request"
> - << std::endl;
> + EPR("Can't set buffer for request\n");
> return ret;
> }
>
> @@ -281,14 +271,14 @@ int CameraSession::startCapture()
> if (sink_) {
> ret = sink_->start();
> if (ret) {
> - std::cout << "Failed to start frame sink" <<
> std::endl;
> + PR("Failed to start frame sink\n");
> return ret;
> }
> }
>
> ret = camera_->start();
> if (ret) {
> - std::cout << "Failed to start capture" << std::endl;
> + PR("Failed to start capture\n");
> if (sink_)
> sink_->stop();
> return ret;
> @@ -297,7 +287,7 @@ int CameraSession::startCapture()
> for (std::unique_ptr<Request> &request : requests_) {
> ret = queueRequest(request.get());
> if (ret < 0) {
> - std::cerr << "Can't queue request" << std::endl;
> + EPR("Can't queue request\n");
> camera_->stop();
> if (sink_)
> sink_->stop();
> @@ -306,13 +296,11 @@ int CameraSession::startCapture()
> }
>
> if (captureLimit_)
> - std::cout << "cam" << cameraIndex_
> - << ": Capture " << captureLimit_ << " frames"
> - << std::endl;
> + PR("cam{}: Capture {} frames\n", cameraIndex_,
> + captureLimit_);
> else
> - std::cout << "cam" << cameraIndex_
> - << ": Capture until user interrupts by SIGINT"
> - << std::endl;
> + PR("cam{}: Capture until user interrupts by SIGINT\n",
> + cameraIndex_);
>
> return 0;
> }
> @@ -364,23 +352,23 @@ void CameraSession::processRequest(Request *request)
>
> bool requeue = true;
>
> - std::stringstream info;
> - info << ts / 1000000000 << "."
> - << std::setw(6) << std::setfill('0') << ts / 1000 % 1000000
> - << " (" << std::fixed << std::setprecision(2) << fps << "
> fps)";
> + auto sbuf = fmt::memory_buffer();
> + fmt::format_to(std::back_inserter(sbuf), "{}.{:06} ({:.2f} fps)",
> + ts / 1000000000,
> + ts / 1000 % 1000000,
> + fps);
>
> for (const auto &[stream, buffer] : buffers) {
> const FrameMetadata &metadata = buffer->metadata();
>
> - info << " " << streamNames_[stream]
> - << " seq: " << std::setw(6) << std::setfill('0') <<
> metadata.sequence
> - << " bytesused: ";
> + fmt::format_to(std::back_inserter(sbuf), " {} seq: {:06}
> bytesused: ",
> + streamNames_[stream], metadata.sequence);
>
> unsigned int nplane = 0;
> for (const FrameMetadata::Plane &plane :
> metadata.planes()) {
> - info << plane.bytesused;
> + fmt::format_to(std::back_inserter(sbuf), "{}",
> plane.bytesused);
> if (++nplane < metadata.planes().size())
> - info << "/";
> + fmt::format_to(std::back_inserter(sbuf),
> "/");
> }
> }
>
> @@ -389,14 +377,13 @@ void CameraSession::processRequest(Request *request)
> requeue = false;
> }
>
> - std::cout << info.str() << std::endl;
> + PR("{}\n", fmt::to_string(sbuf));
>
> if (printMetadata_) {
> const ControlList &requestMetadata = request->metadata();
> for (const auto &[key, value] : requestMetadata) {
> const ControlId *id = controls::controls.at(key);
> - std::cout << "\t" << id->name() << " = "
> - << value.toString() << std::endl;
> + PR("\t{} = {}\n", id->name(), value.toString());
> }
> }
>
> diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp
> index 46e34eb5..84919ab3 100644
> --- a/src/cam/drm.cpp
> +++ b/src/cam/drm.cpp
> @@ -10,12 +10,12 @@
> #include <algorithm>
> #include <errno.h>
> #include <fcntl.h>
> -#include <iostream>
> #include <set>
> #include <string.h>
> #include <sys/ioctl.h>
> #include <sys/stat.h>
> #include <sys/types.h>
> +#include <fmt/core.h>
>
> #include <libcamera/framebuffer.h>
> #include <libcamera/geometry.h>
> @@ -25,6 +25,9 @@
>
> #include "event_loop.h"
>
> +#define PR(...) fmt::print(__VA_ARGS__)
> +#define EPR(...) fmt::print(stderr, __VA_ARGS__)
> +
> namespace DRM {
>
> Object::Object(Device *dev, uint32_t id, Type type)
> @@ -178,9 +181,7 @@ Connector::Connector(Device *dev, const
> drmModeConnector *connector)
> {
> auto typeName = connectorTypeNames.find(connector->connector_type);
> if (typeName == connectorTypeNames.end()) {
> - std::cerr
> - << "Invalid connector type "
> - << connector->connector_type << std::endl;
> + EPR("Invalid connector type {}}n",
> connector->connector_type);
> typeName =
> connectorTypeNames.find(DRM_MODE_CONNECTOR_Unknown);
> }
>
> @@ -213,9 +214,7 @@ Connector::Connector(Device *dev, const
> drmModeConnector *connector)
> return e.id() ==
> encoderId;
> });
> if (encoder == encoders.end()) {
> - std::cerr
> - << "Encoder " << encoderId << " not found"
> - << std::endl;
> + EPR("Encoder {} not found\n", encoderId);
> continue;
> }
>
> @@ -296,9 +295,7 @@ FrameBuffer::~FrameBuffer()
>
> if (ret == -1) {
> ret = -errno;
> - std::cerr
> - << "Failed to close GEM object: "
> - << strerror(-ret) << std::endl;
> + EPR("Failed to close GEM object: {}\n",
> strerror(-ret));
> }
> }
>
> @@ -408,9 +405,8 @@ int Device::init()
> fd_ = open(name, O_RDWR | O_CLOEXEC);
> if (fd_ < 0) {
> ret = -errno;
> - std::cerr
> - << "Failed to open DRM/KMS device " << name << ": "
> - << strerror(-ret) << std::endl;
> + EPR("Failed to open DRM/KMS device {}: {}\n", name,
> + strerror(-ret));
> return ret;
> }
>
> @@ -421,9 +417,7 @@ int Device::init()
> ret = drmSetClientCap(fd_, DRM_CLIENT_CAP_ATOMIC, 1);
> if (ret < 0) {
> ret = -errno;
> - std::cerr
> - << "Failed to enable atomic capability: "
> - << strerror(-ret) << std::endl;
> + EPR("Failed to enable atomic capability: {}\n",
> strerror(-ret));
> return ret;
> }
>
> @@ -448,9 +442,7 @@ int Device::getResources()
> };
> if (!resources) {
> ret = -errno;
> - std::cerr
> - << "Failed to get DRM/KMS resources: "
> - << strerror(-ret) << std::endl;
> + EPR("Failed to get DRM/KMS resources: {}\n",
> strerror(-ret));
> return ret;
> }
>
> @@ -458,9 +450,7 @@ int Device::getResources()
> drmModeCrtc *crtc = drmModeGetCrtc(fd_,
> resources->crtcs[i]);
> if (!crtc) {
> ret = -errno;
> - std::cerr
> - << "Failed to get CRTC: " << strerror(-ret)
> - << std::endl;
> + EPR("Failed to get CRTC: {}\n", strerror(-ret));
> return ret;
> }
>
> @@ -476,9 +466,7 @@ int Device::getResources()
> drmModeGetEncoder(fd_, resources->encoders[i]);
> if (!encoder) {
> ret = -errno;
> - std::cerr
> - << "Failed to get encoder: " <<
> strerror(-ret)
> - << std::endl;
> + EPR("Failed to get encoder: {}\n", strerror(-ret));
> return ret;
> }
>
> @@ -494,9 +482,7 @@ int Device::getResources()
> drmModeGetConnector(fd_, resources->connectors[i]);
> if (!connector) {
> ret = -errno;
> - std::cerr
> - << "Failed to get connector: " <<
> strerror(-ret)
> - << std::endl;
> + EPR("Failed to get connector: {}\n",
> strerror(-ret));
> return ret;
> }
>
> @@ -513,9 +499,7 @@ int Device::getResources()
> };
> if (!planes) {
> ret = -errno;
> - std::cerr
> - << "Failed to get DRM/KMS planes: "
> - << strerror(-ret) << std::endl;
> + EPR("Failed to get DRM/KMS planes: {}\n", strerror(-ret));
> return ret;
> }
>
> @@ -524,9 +508,7 @@ int Device::getResources()
> drmModeGetPlane(fd_, planes->planes[i]);
> if (!plane) {
> ret = -errno;
> - std::cerr
> - << "Failed to get plane: " <<
> strerror(-ret)
> - << std::endl;
> + EPR("Failed to get plane: {}\n", strerror(-ret));
> return ret;
> }
>
> @@ -556,9 +538,7 @@ int Device::getResources()
> drmModePropertyRes *property = drmModeGetProperty(fd_, id);
> if (!property) {
> ret = -errno;
> - std::cerr
> - << "Failed to get property: " <<
> strerror(-ret)
> - << std::endl;
> + EPR("Failed to get property: {}\n",
> strerror(-ret));
> continue;
> }
>
> @@ -573,9 +553,8 @@ int Device::getResources()
> for (auto &object : objects_) {
> ret = object.second->setup();
> if (ret < 0) {
> - std::cerr
> - << "Failed to setup object " <<
> object.second->id()
> - << ": " << strerror(-ret) << std::endl;
> + EPR("Failed to setup object {}: {}\n",
> + object.second->id(), strerror(-ret));
> return ret;
> }
> }
> @@ -616,9 +595,8 @@ std::unique_ptr<FrameBuffer> Device::createFrameBuffer(
> ret = drmPrimeFDToHandle(fd_, plane.fd.get(),
> &handle);
> if (ret < 0) {
> ret = -errno;
> - std::cerr
> - << "Unable to import framebuffer
> dmabuf: "
> - << strerror(-ret) << std::endl;
> + EPR("Unable to import framebuffer dmabuf:
> {}\n",
> + strerror(-ret));
> return nullptr;
> }
>
> @@ -636,9 +614,7 @@ std::unique_ptr<FrameBuffer> Device::createFrameBuffer(
> strides.data(), offsets, &fb->id_, 0);
> if (ret < 0) {
> ret = -errno;
> - std::cerr
> - << "Failed to add framebuffer: "
> - << strerror(-ret) << std::endl;
> + EPR("Failed to add framebuffer: {}\n", strerror(-ret));
> return nullptr;
> }
>
> diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp
> index e25784c0..87aaf59a 100644
> --- a/src/cam/event_loop.cpp
> +++ b/src/cam/event_loop.cpp
> @@ -10,7 +10,10 @@
> #include <assert.h>
> #include <event2/event.h>
> #include <event2/thread.h>
> -#include <iostream>
> +#include <fmt/core.h>
> +
> +#define PR(...) fmt::print(__VA_ARGS__)
> +#define EPR(...) fmt::print(stderr, __VA_ARGS__)
>
> EventLoop *EventLoop::instance_ = nullptr;
>
> @@ -71,13 +74,13 @@ void EventLoop::addEvent(int fd, EventType type,
> event->event_ = event_new(base_, fd, events,
> &EventLoop::Event::dispatch,
> event.get());
> if (!event->event_) {
> - std::cerr << "Failed to create event for fd " << fd <<
> std::endl;
> + EPR("Failed to create event for fd {}\n", fd);
> return;
> }
>
> int ret = event_add(event->event_, nullptr);
> if (ret < 0) {
> - std::cerr << "Failed to add event for fd " << fd <<
> std::endl;
> + EPR("Failed to add event for fd {}\n", fd);
> return;
> }
>
> diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp
> index 45213d4a..86e2118c 100644
> --- a/src/cam/file_sink.cpp
> +++ b/src/cam/file_sink.cpp
> @@ -7,11 +7,12 @@
>
> #include <assert.h>
> #include <fcntl.h>
> -#include <iomanip>
> -#include <iostream>
> -#include <sstream>
> #include <string.h>
> #include <unistd.h>
> +#include <fmt/core.h>
> +
> +#define PR(...) fmt::print(__VA_ARGS__)
> +#define EPR(...) fmt::print(stderr, __VA_ARGS__)
>
> #include <libcamera/camera.h>
>
> @@ -70,10 +71,10 @@ void FileSink::writeBuffer(const Stream *stream,
> FrameBuffer *buffer)
>
> pos = filename.find_first_of('#');
> if (pos != std::string::npos) {
> - std::stringstream ss;
> - ss << streamNames_[stream] << "-" << std::setw(6)
> - << std::setfill('0') << buffer->metadata().sequence;
> - filename.replace(pos, 1, ss.str());
> + auto s = fmt::format("{}-{:06}",
> + streamNames_[stream],
> + buffer->metadata().sequence);
> + filename.replace(pos, 1, s);
> }
>
> fd = open(filename.c_str(), O_CREAT | O_WRONLY |
> @@ -81,8 +82,7 @@ void FileSink::writeBuffer(const Stream *stream,
> FrameBuffer *buffer)
> S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
> S_IWOTH);
> if (fd == -1) {
> ret = -errno;
> - std::cerr << "failed to open file " << filename << ": "
> - << strerror(-ret) << std::endl;
> + EPR("failed to open file {}: {}\n", filename,
> strerror(-ret));
> return;
> }
>
> @@ -95,20 +95,17 @@ void FileSink::writeBuffer(const Stream *stream,
> FrameBuffer *buffer)
> unsigned int length = std::min<unsigned
> int>(meta.bytesused, data.size());
>
> if (meta.bytesused > data.size())
> - std::cerr << "payload size " << meta.bytesused
> - << " larger than plane size " <<
> data.size()
> - << std::endl;
> + EPR("payload size {} larger than plane size {}\n",
> + meta.bytesused, data.size());
>
> ret = ::write(fd, data.data(), length);
> if (ret < 0) {
> ret = -errno;
> - std::cerr << "write error: " << strerror(-ret)
> - << std::endl;
> + EPR("write error: {}\n", strerror(-ret));
> break;
> } else if (ret != (int)length) {
> - std::cerr << "write error: only " << ret
> - << " bytes written instead of "
> - << length << std::endl;
> + EPR("write error: only {} bytes written instead of
> {}\n",
> + ret, length);
> break;
> }
> }
> diff --git a/src/cam/image.cpp b/src/cam/image.cpp
> index fe2cc6da..73bcf915 100644
> --- a/src/cam/image.cpp
> +++ b/src/cam/image.cpp
> @@ -9,11 +9,14 @@
>
> #include <assert.h>
> #include <errno.h>
> -#include <iostream>
> #include <map>
> #include <string.h>
> #include <sys/mman.h>
> #include <unistd.h>
> +#include <fmt/core.h>
> +
> +#define PR(...) fmt::print(__VA_ARGS__)
> +#define EPR(...) fmt::print(stderr, __VA_ARGS__)
>
> using namespace libcamera;
>
> @@ -49,10 +52,8 @@ std::unique_ptr<Image> Image::fromFrameBuffer(const
> FrameBuffer *buffer, MapMode
>
> if (plane.offset > length ||
> plane.offset + plane.length > length) {
> - std::cerr << "plane is out of buffer: buffer
> length="
> - << length << ", plane offset=" <<
> plane.offset
> - << ", plane length=" << plane.length
> - << std::endl;
> + EPR("plane is out of buffer: buffer length={},
> plane offset={}, plane length={}\n",
> + length, plane.offset, plane.length);
> return nullptr;
> }
> size_t &mapLength = mappedBuffers[fd].mapLength;
> @@ -68,8 +69,8 @@ std::unique_ptr<Image> Image::fromFrameBuffer(const
> FrameBuffer *buffer, MapMode
> MAP_SHARED, fd, 0);
> if (address == MAP_FAILED) {
> int error = -errno;
> - std::cerr << "Failed to mmap plane: "
> - << strerror(-error) << std::endl;
> + EPR("Failed to mmap plane: {}\n",
> + strerror(-error));
> return nullptr;
> }
>
> diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp
> index 7add81a6..823b75e4 100644
> --- a/src/cam/kms_sink.cpp
> +++ b/src/cam/kms_sink.cpp
> @@ -10,10 +10,13 @@
> #include <array>
> #include <algorithm>
> #include <assert.h>
> -#include <iostream>
> #include <memory>
> #include <stdint.h>
> #include <string.h>
> +#include <fmt/core.h>
> +
> +#define PR(...) fmt::print(__VA_ARGS__)
> +#define EPR(...) fmt::print(stderr, __VA_ARGS__)
>
> #include <libcamera/camera.h>
> #include <libcamera/formats.h>
> @@ -54,11 +57,9 @@ KMSSink::KMSSink(const std::string &connectorName)
>
> if (!connector_) {
> if (!connectorName.empty())
> - std::cerr
> - << "Connector " << connectorName << " not
> found"
> - << std::endl;
> + EPR("Connector {} not found\n", connectorName);
> else
> - std::cerr << "No connected connector found" <<
> std::endl;
> + EPR("No connected connector found\n");
> return;
> }
>
> @@ -119,7 +120,7 @@ int KMSSink::configure(const
> libcamera::CameraConfiguration &config)
> mode.vdisplay ==
> cfg.size.height;
> });
> if (iter == modes.end()) {
> - std::cerr << "No mode matching " << cfg.size << std::endl;
> + EPR("No mode matching {}\n", cfg.size);
> return -EINVAL;
> }
>
> @@ -192,17 +193,12 @@ int KMSSink::configurePipeline(const
> libcamera::PixelFormat &format)
> {
> const int ret = selectPipeline(format);
> if (ret) {
> - std::cerr
> - << "Unable to find display pipeline for format "
> - << format << std::endl;
> -
> + EPR("Unable to find display pipeline for format {}\n",
> format);
> return ret;
> }
>
> - std::cout
> - << "Using KMS plane " << plane_->id() << ", CRTC " <<
> crtc_->id()
> - << ", connector " << connector_->name()
> - << " (" << connector_->id() << ")" << std::endl;
> + PR("Using KMS plane {}, CRTC {}, connector {} ({})\n",
> + plane_->id(), crtc_->id(), connector_->name(),
> connector_->id());
>
> return 0;
> }
> @@ -228,9 +224,8 @@ int KMSSink::start()
>
> ret = request->commit(DRM::AtomicRequest::FlagAllowModeset);
> if (ret < 0) {
> - std::cerr
> - << "Failed to disable CRTCs and planes: "
> - << strerror(-ret) << std::endl;
> + EPR("Failed to disable CRTCs and planes: {}\n",
> + strerror(-ret));
> return ret;
> }
>
> @@ -250,9 +245,7 @@ int KMSSink::stop()
>
> int ret = request.commit(DRM::AtomicRequest::FlagAllowModeset);
> if (ret < 0) {
> - std::cerr
> - << "Failed to stop display pipeline: "
> - << strerror(-ret) << std::endl;
> + EPR("Failed to stop display pipeline: {}\n",
> strerror(-ret));
> return ret;
> }
>
> @@ -312,9 +305,8 @@ bool KMSSink::processRequest(libcamera::Request
> *camRequest)
> if (!queued_) {
> int ret = drmRequest->commit(flags);
> if (ret < 0) {
> - std::cerr
> - << "Failed to commit atomic request: "
> - << strerror(-ret) << std::endl;
> + EPR("Failed to commit atomic request: {}\n",
> + strerror(-ret));
> /* \todo Implement error handling */
> }
>
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index c7f664b9..03615dc9 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -6,10 +6,9 @@
> */
>
> #include <atomic>
> -#include <iomanip>
> -#include <iostream>
> #include <signal.h>
> #include <string.h>
> +#include <fmt/core.h>
>
> #include <libcamera/libcamera.h>
> #include <libcamera/property_ids.h>
> @@ -78,8 +77,7 @@ int CamApp::init(int argc, char **argv)
>
> ret = cm_->start();
> if (ret) {
> - std::cout << "Failed to start camera manager: "
> - << strerror(-ret) << std::endl;
> + fmt::print("Failed to start camera manager: {}\n", -ret);
> return ret;
> }
>
> @@ -173,12 +171,12 @@ int CamApp::parseOptions(int argc, char *argv[])
>
> void CamApp::cameraAdded(std::shared_ptr<Camera> cam)
> {
> - std::cout << "Camera Added: " << cam->id() << std::endl;
> + fmt::print("Camera Added: {}\n", cam->id());
> }
>
> void CamApp::cameraRemoved(std::shared_ptr<Camera> cam)
> {
> - std::cout << "Camera Removed: " << cam->id() << std::endl;
> + fmt::print("Camera Removed: {}\n", cam->id());
> }
>
> void CamApp::captureDone()
> @@ -193,11 +191,11 @@ int CamApp::run()
>
> /* 1. List all cameras. */
> if (options_.isSet(OptList)) {
> - std::cout << "Available cameras:" << std::endl;
> + fmt::print("Available cameras:\n");
>
> unsigned int index = 1;
> for (const std::shared_ptr<Camera> &cam : cm_->cameras()) {
> - std::cout << index << ": " <<
> cameraName(cam.get()) << std::endl;
> + fmt::print("{}: {}\n", cameraName(cam.get()),
> index);
> index++;
> }
> }
> @@ -215,12 +213,12 @@ int CamApp::run()
> index,
>
> camera.children());
> if (!session->isValid()) {
> - std::cout << "Failed to create camera
> session" << std::endl;
> + fmt::print("Failed to create camera
> session\n");
> return -EINVAL;
> }
>
> - std::cout << "Using camera " <<
> session->camera()->id()
> - << " as cam" << index << std::endl;
> + fmt::print("Using camera{} as cam{}\n",
> + session->camera()->id(), index);
>
> session->captureDone.connect(this,
> &CamApp::captureDone);
>
> @@ -250,7 +248,7 @@ int CamApp::run()
>
> ret = session->start();
> if (ret) {
> - std::cout << "Failed to start camera session" <<
> std::endl;
> + fmt::print("Failed to start camera session\n");
> return ret;
> }
>
> @@ -259,8 +257,8 @@ int CamApp::run()
>
> /* 5. Enable hotplug monitoring. */
> if (options_.isSet(OptMonitor)) {
> - std::cout << "Monitoring new hotplug and unplug events" <<
> std::endl;
> - std::cout << "Press Ctrl-C to interrupt" << std::endl;
> + fmt::print("Monitoring new hotplug and unplug events\n");
> + fmt::print("Press Ctrl-C to interrupt\n");
>
> cm_->cameraAdded.connect(this, &CamApp::cameraAdded);
> cm_->cameraRemoved.connect(this, &CamApp::cameraRemoved);
> @@ -323,7 +321,7 @@ std::string CamApp::cameraName(const Camera *camera)
>
> void signalHandler([[maybe_unused]] int signal)
> {
> - std::cout << "Exiting" << std::endl;
> + fmt::print("Exiting");
> CamApp::instance()->quit();
> }
>
> diff --git a/src/cam/meson.build b/src/cam/meson.build
> index 5bab8c9e..2b47383d 100644
> --- a/src/cam/meson.build
> +++ b/src/cam/meson.build
> @@ -7,6 +7,8 @@ if not libevent.found()
> subdir_done()
> endif
>
> +libfmt_dep = dependency('fmt')
> +
> cam_enabled = true
>
> cam_sources = files([
> @@ -25,7 +27,7 @@ cam_cpp_args = []
> libdrm = dependency('libdrm', required : false)
>
> if libdrm.found()
> - cam_cpp_args += [ '-DHAVE_KMS' ]
> + cam_cpp_args += [ '-DHAVE_KMS', ]
> cam_sources += files([
> 'drm.cpp',
> 'kms_sink.cpp'
> @@ -38,6 +40,7 @@ cam = executable('cam', cam_sources,
> libcamera_public,
> libdrm,
> libevent,
> + libfmt_dep,
> ],
> cpp_args : cam_cpp_args,
> install : true)
> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> index 4f7e8691..c9979385 100644
> --- a/src/cam/options.cpp
> +++ b/src/cam/options.cpp
> @@ -7,9 +7,11 @@
>
> #include <assert.h>
> #include <getopt.h>
> -#include <iomanip>
> -#include <iostream>
> #include <string.h>
> +#include <fmt/core.h>
> +
> +#define PR(...) fmt::print(__VA_ARGS__)
> +#define EPR(...) fmt::print(stderr, __VA_ARGS__)
>
> #include "options.h"
>
> @@ -390,26 +392,23 @@ KeyValueParser::Options KeyValueParser::parse(const
> char *arguments)
> continue;
>
> if (optionsMap_.find(key) == optionsMap_.end()) {
> - std::cerr << "Invalid option " << key << std::endl;
> + EPR("Invalid option {}\n", key);
> return options;
> }
>
> OptionArgument arg = optionsMap_[key].argument;
> if (value.empty() && arg == ArgumentRequired) {
> - std::cerr << "Option " << key << " requires an
> argument"
> - << std::endl;
> + EPR("Option {} requires an argument\n", key);
> return options;
> } else if (!value.empty() && arg == ArgumentNone) {
> - std::cerr << "Option " << key << " takes no
> argument"
> - << std::endl;
> + EPR("Option {} takes no argument\n", key);
> return options;
> }
>
> const Option &option = optionsMap_[key];
> if (!options.parseValue(key, option, value.c_str())) {
> - std::cerr << "Failed to parse '" << value << "' as
> "
> - << option.typeName() << " for option "
> << key
> - << std::endl;
> + EPR("Failed to parse '{}' as {} for option {}\n",
> + value, option.typeName(), key);
> return options;
> }
> }
> @@ -453,16 +452,16 @@ void KeyValueParser::usage(int indent)
> argument += "]";
> }
>
> - std::cerr << std::setw(indent) << argument;
> + EPR("{:{}}", argument, indent);
>
> for (const char *help = option.help, *end = help; end;) {
> end = strchr(help, '\n');
> if (end) {
> - std::cerr << std::string(help, end - help
> + 1);
> - std::cerr << std::setw(indent) << " ";
> + EPR(std::string(help, end - help + 1));
> + EPR("{:{}}", "", indent);
> help = end + 1;
> } else {
> - std::cerr << help << std::endl;
> + EPR("{}\n", help);
> }
> }
> }
> @@ -929,10 +928,10 @@ OptionsParser::Options OptionsParser::parse(int
> argc, char **argv)
>
> if (c == '?' || c == ':') {
> if (c == '?')
> - std::cerr << "Invalid option ";
> + EPR("Invalid option ");
> else
> - std::cerr << "Missing argument for option
> ";
> - std::cerr << argv[optind - 1] << std::endl;
> + EPR("Missing argument for option ");
> + EPR("{}\n", argv[optind - 1]);
>
> usage();
> return options;
> @@ -946,8 +945,7 @@ OptionsParser::Options OptionsParser::parse(int argc,
> char **argv)
> }
>
> if (optind < argc) {
> - std::cerr << "Invalid non-option argument '" <<
> argv[optind]
> - << "'" << std::endl;
> + EPR("Invalid non-option argument '{}'\n", argv[optind]);
> usage();
> return options;
> }
> @@ -992,14 +990,9 @@ void OptionsParser::usage()
>
> indent = (indent + 7) / 8 * 8;
>
> - std::cerr << "Options:" << std::endl;
> -
> - std::ios_base::fmtflags f(std::cerr.flags());
> - std::cerr << std::left;
> + EPR("Options:\n");
>
> usageOptions(options_, indent);
> -
> - std::cerr.flags(f);
> }
>
> void OptionsParser::usageOptions(const std::list<Option> &options,
> @@ -1036,16 +1029,16 @@ void OptionsParser::usageOptions(const
> std::list<Option> &options,
> if (option.isArray)
> argument += " ...";
>
> - std::cerr << std::setw(indent) << argument;
> + EPR("{:{}}", argument, indent);
>
> - for (const char *help = option.help, *end = help; end; ) {
> + for (const char *help = option.help, *end = help; end;) {
> end = strchr(help, '\n');
> if (end) {
> - std::cerr << std::string(help, end - help
> + 1);
> - std::cerr << std::setw(indent) << " ";
> + EPR(std::string(help, end - help + 1));
> + EPR("{:{}}", "", indent);
> help = end + 1;
> } else {
> - std::cerr << help << std::endl;
> + EPR("{}\n", help);
> }
> }
>
> @@ -1060,8 +1053,8 @@ void OptionsParser::usageOptions(const
> std::list<Option> &options,
> return;
>
> for (const Option *option : parentOptions) {
> - std::cerr << std::endl << "Options valid in the context of
> "
> - << option->optionName() << ":" << std::endl;
> + EPR("\nOptions valid in the context of {}:\n",
> + option->optionName());
> usageOptions(option->children, indent);
> }
> }
> @@ -1125,15 +1118,14 @@ bool OptionsParser::parseValue(const Option
> &option, const char *arg,
>
> std::tie(options, error) = childOption(option.parent, options);
> if (error) {
> - std::cerr << "Option " << option.optionName() << "
> requires a "
> - << error->optionName() << " context" <<
> std::endl;
> + EPR("Option {} requires a {} context\n",
> + option.optionName(), error->optionName());
> return false;
> }
>
> if (!options->parseValue(option.opt, option, arg)) {
> - std::cerr << "Can't parse " << option.typeName()
> - << " argument for option " << option.optionName()
> - << std::endl;
> + EPR("Can't parse {} argument for option {}\n",
> + option.typeName(), option.optionName());
> return false;
> }
>
> diff --git a/src/cam/stream_options.cpp b/src/cam/stream_options.cpp
> index 150bd27c..666862eb 100644
> --- a/src/cam/stream_options.cpp
> +++ b/src/cam/stream_options.cpp
> @@ -6,7 +6,10 @@
> */
> #include "stream_options.h"
>
> -#include <iostream>
> +#include <fmt/core.h>
> +
> +#define PR(...) fmt::print(__VA_ARGS__)
> +#define EPR(...) fmt::print(stderr, __VA_ARGS__)
>
> using namespace libcamera;
>
> @@ -30,8 +33,7 @@ KeyValueParser::Options
> StreamKeyValueParser::parse(const char *arguments)
>
> if (options.valid() && options.isSet("role") &&
> !parseRole(&role, options)) {
> - std::cerr << "Unknown stream role "
> - << options["role"].toString() << std::endl;
> + EPR("Unknown stream role {}\n",
> options["role"].toString());
> options.invalidate();
> }
>
> @@ -64,7 +66,7 @@ int
> StreamKeyValueParser::updateConfiguration(CameraConfiguration *config,
> const OptionValue &values)
> {
> if (!config) {
> - std::cerr << "No configuration provided" << std::endl;
> + EPR("No configuration provided\n");
> return -EINVAL;
> }
>
> @@ -75,12 +77,8 @@ int
> StreamKeyValueParser::updateConfiguration(CameraConfiguration *config,
> const std::vector<OptionValue> &streamParameters =
> values.toArray();
>
> if (config->size() != streamParameters.size()) {
> - std::cerr
> - << "Number of streams in configuration "
> - << config->size()
> - << " does not match number of streams parsed "
> - << streamParameters.size()
> - << std::endl;
> + EPR("Number of streams in configuration {} does not match
> number of streams parsed {}\n",
> + config->size(), streamParameters.size());
> return -EINVAL;
> }
>
> --
> 2.34.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220510/793b28fc/attachment-0001.htm>
More information about the libcamera-devel
mailing list