[libcamera-devel] [PATCH] cam: convert to libfmt
Eric Curtin
ecurtin at redhat.com
Tue May 10 14:59:37 CEST 2022
On Tue, 10 May 2022 at 11:10, Kieran Bingham via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Quoting Naushir Patuck (2022-05-10 10:49:14)
> > 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 :)
I am all in for this change also, personally I would have changed to
printf for now
to have one less dependency (also an easy port to C++20 std::format). The
dependency list is getting large, I noticed a build of mine failing
recently because
I didn't have libyaml.
But std::format and libfmt are quite fast and anything is better than
streams so +1.
>
> I've never used (yet) {fmt}, but I've only heard good things about
> performance, and of course it's headed into the standard, so I also
> think there is some good merit to be found in this development.
>
> --
> Kieran
>
>
> >
> > 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
> > >
> > >
>
More information about the libcamera-devel
mailing list