[PATCH 1/2] pipeline: Add support for dumping capture script and metadata
Paul Elder
paul.elder at ideasonboard.com
Wed Oct 2 12:20:48 CEST 2024
On Wed, Oct 02, 2024 at 08:28:26AM +0200, Jacopo Mondi wrote:
> Hi Paul
>
> On Tue, Oct 01, 2024 at 02:09:06AM GMT, Paul Elder wrote:
> > Add support for dumping capture scripts and metadata. The capture
> > scripts can then be fed into the cam application and a capture can thus
> > be "replayed". Metadata can also be dumped.
> >
> > Camera configuration is also dumped to the capture script. The cam
> > application currently does not support loading configuration from the
> > capture script, but support for that will be added in a subsequent
> > patch.
> >
> > These can be enabled by a new environment variable.
>
> Which needs to be documented :)
I... opened the tab ready to write it then forgot to write it :)
>
> >
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> > include/libcamera/internal/camera.h | 3 +
> > include/libcamera/internal/pipeline_handler.h | 16 ++++
> > src/libcamera/camera.cpp | 13 +++
> > src/libcamera/pipeline_handler.cpp | 93 ++++++++++++++++++-
> > 4 files changed, 124 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
> > index 0add0428bb5d..a42f03d4c755 100644
> > --- a/include/libcamera/internal/camera.h
> > +++ b/include/libcamera/internal/camera.h
> > @@ -19,6 +19,8 @@
> >
> > namespace libcamera {
> >
> > +enum class Orientation;
> > +
> > class CameraControlValidator;
> > class PipelineHandler;
> > class Stream;
> > @@ -65,6 +67,7 @@ private:
> > std::string id_;
> > std::set<Stream *> streams_;
> > std::set<const Stream *> activeStreams_;
> > + Orientation orientation_;
> >
> > bool disconnected_;
> > std::atomic<State> state_;
> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> > index 0d38080369c5..fb3914185a01 100644
> > --- a/include/libcamera/internal/pipeline_handler.h
> > +++ b/include/libcamera/internal/pipeline_handler.h
> > @@ -9,6 +9,7 @@
> >
> > #include <memory>
> > #include <queue>
> > +#include <set>
> > #include <string>
> > #include <sys/types.h>
> > #include <vector>
> > @@ -20,6 +21,8 @@
> >
> > namespace libcamera {
> >
> > +enum class Orientation;
> > +
> > class Camera;
> > class CameraConfiguration;
> > class CameraManager;
> > @@ -68,6 +71,9 @@ public:
> >
> > CameraManager *cameraManager() const { return manager_; }
> >
> > + void dumpConfiguration(const std::set<const Stream *> &streams,
> > + const Orientation &orientation);
> > +
>
> To define the expected configuration format, I would make 2/2 precede 1/2
Indeed 2/2 going first doesn't actually break anything. Sure.
>
> > protected:
> > void registerCamera(std::shared_ptr<Camera> camera);
> > void hotplugMediaDevice(MediaDevice *media);
> > @@ -81,6 +87,11 @@ protected:
> > CameraManager *manager_;
> >
> > private:
> > + enum DumpMode {
> > + Controls,
> > + Metadata,
> > + };
> > +
> > void unlockMediaDevices();
> >
> > void mediaDeviceDisconnected(MediaDevice *media);
> > @@ -89,6 +100,8 @@ private:
> > void doQueueRequest(Request *request);
> > void doQueueRequests();
> >
> > + void dumpRequest(Request *request, DumpMode mode);
> > +
> > std::vector<std::shared_ptr<MediaDevice>> mediaDevices_;
> > std::vector<std::weak_ptr<Camera>> cameras_;
> >
> > @@ -97,6 +110,9 @@ private:
> > const char *name_;
> > unsigned int useCount_;
> >
> > + std::ostream *dumpCaptureScript_;
> > + std::ostream *dumpMetadata_;
> > +
> > friend class PipelineHandlerFactoryBase;
> > };
> >
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index a86f552a47bc..1282f99d839b 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -1215,6 +1215,9 @@ int Camera::configure(CameraConfiguration *config)
> > d->activeStreams_.insert(stream);
> > }
> >
> > + /* TODO Save sensor configuration for dumping it to capture script */
> > + d->orientation_ = config->orientation;
> > +
> > d->setState(Private::CameraConfigured);
> >
> > return 0;
> > @@ -1356,6 +1359,16 @@ int Camera::start(const ControlList *controls)
> >
> > ASSERT(d->requestSequence_ == 0);
> >
> > + /*
> > + * Invoke method in blocking mode to avoid the risk of writing after
> > + * streaming has started.
> > + * This needs to be here as PipelineHandler::start is a virtual function
> > + * so it is impractical to add the dumping there.
> > + * TODO Pass the sensor configuration, once it is supported
> > + */
> > + d->pipe_->invokeMethod(&PipelineHandler::dumpConfiguration,
> > + ConnectionTypeBlocking, d->activeStreams_, d->orientation_);
>
> Can't you pass in the CameraConfiguration which containts the
> StreamConfigurations and the the Orientation already ?
CameraConfiguration can't be copied because it has virtual functions.
And CameraConfiguration is only available in Camera::configure(). So, no.
>
> > +
> > ret = d->pipe_->invokeMethod(&PipelineHandler::start,
> > ConnectionTypeBlocking, this, controls);
> > if (ret)
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index e5940469127e..7002b4323bdd 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -8,6 +8,7 @@
> > #include "libcamera/internal/pipeline_handler.h"
> >
> > #include <chrono>
> > +#include <fstream>
> > #include <sys/stat.h>
> > #include <sys/sysmacros.h>
> >
> > @@ -68,14 +69,36 @@ LOG_DEFINE_CATEGORY(Pipeline)
> > * through the PipelineHandlerFactoryBase::create() function.
> > */
> > PipelineHandler::PipelineHandler(CameraManager *manager)
> > - : manager_(manager), useCount_(0)
> > + : manager_(manager), useCount_(0),
> > + dumpCaptureScript_(nullptr), dumpMetadata_(nullptr)
> > {
> > + /* TODO Print notification that we're dumping capture script */
> > + const char *file = utils::secure_getenv("LIBCAMERA_DUMP_CAPTURE_SCRIPT");
> > + if (!file)
> > + return;
> > +
> > + dumpCaptureScript_ = new std::ofstream(file);
>
> I was really expecting to be operating on File instances
Oh ig we could do that too. I copied a few ideas from the logger.
>
> > +
> > + /*
> > + * Metadata needs to go into a separate file because otherwise it'll
> > + * flood the capture script
> > + */
> > + dumpMetadata_ = new std::ofstream(std::string(file) + ".metadata");
> > + std::string str = "frames:\n";
> > + dumpMetadata_->write(str.c_str(), str.size());
>
> and use the libyaml emitter for this instead of raw writes
>
> Have you considered it and decided it was better to use raw writes ?
No I hadn't. I'll keep it raw for now though until you make the
YamlEmitter :)
Thanks,
Paul
>
> > + dumpMetadata_->flush();
> > }
> >
> > PipelineHandler::~PipelineHandler()
> > {
> > for (std::shared_ptr<MediaDevice> media : mediaDevices_)
> > media->release();
> > +
> > + if (dumpCaptureScript_)
> > + delete dumpCaptureScript_;
> > +
> > + if (dumpMetadata_)
> > + delete dumpMetadata_;
> > }
> >
> > /**
> > @@ -464,6 +487,8 @@ void PipelineHandler::doQueueRequest(Request *request)
> >
> > request->_d()->sequence_ = data->requestSequence_++;
> >
> > + dumpRequest(request, DumpMode::Controls);
> > +
> > if (request->_d()->cancelled_) {
> > completeRequest(request);
> > return;
> > @@ -555,6 +580,8 @@ void PipelineHandler::completeRequest(Request *request)
> >
> > request->_d()->complete();
> >
> > + dumpRequest(request, DumpMode::Metadata);
> > +
> > Camera::Private *data = camera->_d();
> >
> > while (!data->queuedRequests_.empty()) {
> > @@ -758,6 +785,70 @@ void PipelineHandler::disconnect()
> > * \return The CameraManager for this pipeline handler
> > */
> >
> > +void PipelineHandler::dumpConfiguration(const std::set<const Stream *> &streams,
> > + const Orientation &orientation)
> > +{
> > + if (!dumpCaptureScript_)
> > + return;
> > +
> > + std::stringstream ss;
> > + ss << "configuration:" << std::endl;
> > + ss << " orientation: " << orientation << std::endl;
> > +
> > + /* TODO Dump Sensor configuration */
> > +
> > + ss << " streams:" << std::endl;
> > + for (const auto &stream : streams) {
> > + const StreamConfiguration &streamConfig = stream->configuration();
> > + ss << " - pixelFormat: " << streamConfig.pixelFormat << std::endl;
> > + ss << " size: " << streamConfig.size << std::endl;
> > + ss << " stride: " << streamConfig.stride << std::endl;
> > + ss << " frameSize: " << streamConfig.frameSize << std::endl;
> > + ss << " bufferCount: " << streamConfig.bufferCount << std::endl;
> > + if (streamConfig.colorSpace)
> > + ss << " colorSpace: " << streamConfig.colorSpace->toString() << std::endl;
> > + }
> > +
> > + dumpCaptureScript_->write(ss.str().c_str(), ss.str().size());
> > +
> > + std::string str = "frames:\n";
> > + dumpCaptureScript_->write(str.c_str(), str.size());
> > + dumpCaptureScript_->flush();
> > +}
> > +
> > +void PipelineHandler::dumpRequest(Request *request, DumpMode mode)
> > +{
> > + ControlList &controls =
> > + mode == DumpMode::Controls ? request->controls()
> > + : request->metadata();
> > + std::ostream *output =
> > + mode == DumpMode::Controls ? dumpCaptureScript_
> > + : dumpMetadata_;
> > +
> > + if (!output || controls.empty())
> > + return;
> > +
> > + std::stringstream ss;
> > + /* TODO Figure out PFC */
> > + ss << " - " << request->sequence() << ":" << std::endl;
> > +
> > + const ControlIdMap *idMap = controls.idMap();
> > + for (const auto &pair : controls) {
> > + const ControlId *ctrlId = idMap->at(pair.first);
> > + /* TODO Prettify enums (probably by upgrading ControlValue::toString()) */
> > + ss << " " << ctrlId->name() << ": " << pair.second.toString() << std::endl;
> > + }
> > +
> > + /*
> > + * TODO Investigate the overhead of flushing this frequently
> > + * Controls aren't going to be queued too frequently so it should be
> > + * fine to dump controls every frame. Metadata on the other hand needs
> > + * to be investigated.
> > + */
> > + output->write(ss.str().c_str(), ss.str().size());
> > + output->flush();
> > +}
> > +
> > /**
> > * \class PipelineHandlerFactoryBase
> > * \brief Base class for pipeline handler factories
> > --
> > 2.39.2
> >
More information about the libcamera-devel
mailing list