[PATCH v2 1/2] pipeline: Add support for dumping capture script and metadata
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Oct 7 11:28:15 CEST 2024
Quoting Paul Elder (2024-10-04 13:05:16)
> 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.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v2:
> - clean up code
> - add support for creating new dump file when restarting capture
> - document the environment variables
> ---
> Documentation/environment_variables.rst | 26 ++++++
> include/libcamera/internal/camera.h | 2 +
> include/libcamera/internal/pipeline_handler.h | 19 +++++
> src/libcamera/camera.cpp | 13 +++
> src/libcamera/pipeline_handler.cpp | 85 ++++++++++++++++++-
> 5 files changed, 144 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/environment_variables.rst b/Documentation/environment_variables.rst
> index 7da9883a8380..b0448d387847 100644
> --- a/Documentation/environment_variables.rst
> +++ b/Documentation/environment_variables.rst
> @@ -29,6 +29,32 @@ LIBCAMERA_IPA_CONFIG_PATH
>
> Example value: ``${HOME}/.libcamera/share/ipa:/opt/libcamera/vendor/share/ipa``
>
> +LIBCAMERA_DUMP_CAPTURE_SCRIPT
> + The custom destination for capture script dump output.
> +
> + The precensce of this environment variable enables capture script dumping.
s/precensce/presence/
> + All controls that are set for each request will be dumped into the file
s/dumped/written/?
> + specified by the environment variable as a capture script, which can later
> + be fed into the cam application to replay a control sequence.
Should it be specific to cam? Or can we say "can later be parsed by
applications such as the 'cam tool' to replay the control sequence."
> +
> + The file that is written to will be suffixed with a number indicating the
> + number of capture. That is, if the capture is stopped and started again, a
> + new capture script will be dumped with the suffix incremented.
> +
> + Example value: ``/home/{user}/capture_script.yaml``
Suffixed ? After the .yaml?
> +
> +LIBCAMERA_DUMP_METADATA
> + The custom destination for metadata dump output.
> +
> + This is similar to LIBCAMERA_DUMP_CAPTURE_SCRIPT, except instead of a
> + capture script with controls for each frame, the dump will consist of all
> + metadata that was returned for every frame.
> +
> + Also similar to LIBCAMERA_DUMP_CAPTURE_SCRIPT, there will be a number suffix
> + added to the filename of the dump.
> +
> + Example value: ``/home/{user}/metadata_dump.yaml``
> +
> LIBCAMERA_IPA_FORCE_ISOLATION
> When set to a non-empty string, force process isolation of all IPA modules.
>
> diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
> index 0add0428bb5d..b029d85901a7 100644
> --- a/include/libcamera/internal/camera.h
> +++ b/include/libcamera/internal/camera.h
> @@ -16,6 +16,7 @@
> #include <libcamera/base/class.h>
>
> #include <libcamera/camera.h>
> +#include <libcamera/orientation.h>
>
> namespace libcamera {
>
> @@ -65,6 +66,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..f31aced71331 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,
> + Orientation orientation);
> +
> protected:
> void registerCamera(std::shared_ptr<Camera> camera);
> void hotplugMediaDevice(MediaDevice *media);
> @@ -81,6 +87,11 @@ protected:
> CameraManager *manager_;
>
> private:
> + enum class 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,12 @@ private:
> const char *name_;
> unsigned int useCount_;
>
> + unsigned int captureCount_;
> + std::string fileCapture_;
> + std::string fileMetadata_;
> + std::unique_ptr<std::ostream> dumpCaptureScript_;
> + std::unique_ptr<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_);
> +
> 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..4df64ac90bdf 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,8 +69,10 @@ LOG_DEFINE_CATEGORY(Pipeline)
> * through the PipelineHandlerFactoryBase::create() function.
> */
> PipelineHandler::PipelineHandler(CameraManager *manager)
> - : manager_(manager), useCount_(0)
> + : manager_(manager), useCount_(0), captureCount_(0)
> {
> + fileCapture_ = utils::secure_getenv("LIBCAMERA_DUMP_CAPTURE_SCRIPT");
> + fileMetadata_ = utils::secure_getenv("LIBCAMERA_DUMP_METADATA");
> }
>
> PipelineHandler::~PipelineHandler()
> @@ -464,6 +467,8 @@ void PipelineHandler::doQueueRequest(Request *request)
>
> request->_d()->sequence_ = data->requestSequence_++;
>
> + dumpRequest(request, DumpMode::Controls);
> +
I'm not a big fan of this enum. Probably better to just do (Already
assuming there will be a helper class from below)
requestLog_->writeControlList(request, request->metadata());
> if (request->_d()->cancelled_) {
> completeRequest(request);
> return;
> @@ -555,6 +560,8 @@ void PipelineHandler::completeRequest(Request *request)
>
> request->_d()->complete();
>
> + dumpRequest(request, DumpMode::Metadata);
and
metadataLog_->writeControlList(request, request->metadata());
(Where requestLog_ and metadataLog_ would just be instances of whatever
helper handles the output)
> +
> Camera::Private *data = camera->_d();
>
> while (!data->queuedRequests_.empty()) {
> @@ -758,6 +765,82 @@ void PipelineHandler::disconnect()
> * \return The CameraManager for this pipeline handler
> */
>
> +void PipelineHandler::dumpConfiguration(const std::set<const Stream *> &streams,
> + Orientation orientation)
> +{
> + captureCount_++;
> +
> + /* These need to be done here in case capture is restarted */
Maybe this should be handled in a YamlOutput helper class then ?
Otherwise if feels a bit ugly to have the internal implementation detail
of managing files associated with the PipelineHandler.
What does this statement actually refer to though ?
Is it that when a capture is restarted is the only time you can open the file?
Or is it that you need to reopen a new file when a capture is restarted ?
Or is it that you need to re-output the configuration when a capture is restarted?
I think the comment actually means it is opening a new output file when
a capture is stopped and restarted.
I see on the review of the previous version you mentioned you're waiting
for a YamlEmitter class...
But I think that means this code 'will' later be output through a helper
- so we are better just doing that now.
But no need to go write the full YamlEmitter if that's being handled in
separately - just make a helper class private to the PipelineHandler
which will help identify what YamlEmitter will need to provide to you!
> + if (!fileCapture_.empty()) {
> + std::string file = fileCapture_ + "." + std::to_string(captureCount_);
> + LOG(Pipeline, Info) << "Dumping capture script to " << file;
> + dumpCaptureScript_ = std::make_unique<std::ofstream>(file);
> + }
> +
> + /*
> + * Metadata needs to go into a separate file because otherwise it'll
> + * flood the capture script
> + */
> + if (!fileMetadata_.empty()) {
> + std::string file = fileMetadata_ + "." + std::to_string(captureCount_);
> + LOG(Pipeline, Info) << "Dumping metadata to " << file;
> + dumpMetadata_ = std::make_unique<std::ofstream>(file);
> + *dumpMetadata_ << "frames:" << std::endl;
> + dumpMetadata_->flush();
> + }
> +
> + if (!dumpCaptureScript_)
> + return;
> +
> + std::ostream &output = *dumpCaptureScript_;
> +
> + output << "configuration:" << std::endl;
> + output << " orientation: " << orientation << std::endl;
> +
> + /* \todo Dump Sensor configuration */
> +
> + output << " streams:" << std::endl;
> + for (const auto &stream : streams) {
> + const StreamConfiguration &streamConfig = stream->configuration();
> + output << " - pixelFormat: " << streamConfig.pixelFormat << std::endl;
> + output << " size: " << streamConfig.size << std::endl;
> + output << " stride: " << streamConfig.stride << std::endl;
> + output << " frameSize: " << streamConfig.frameSize << std::endl;
> + output << " bufferCount: " << streamConfig.bufferCount << std::endl;
> + if (streamConfig.colorSpace) {
> + output << " colorSpace: " << streamConfig.colorSpace->toString() << std::endl;
> + }
> + }
> +
> + output << "frames:" << std::endl;
> + dumpCaptureScript_->flush();
And then something like flush() should be part of the output class
destructor rather than scattered after arbitrary writes. ?
> +}
> +
> +void PipelineHandler::dumpRequest(Request *request, DumpMode mode)
> +{
> + ControlList &controls =
> + mode == DumpMode::Controls ? request->controls()
> + : request->metadata();
> + std::ostream *output =
> + mode == DumpMode::Controls ? dumpCaptureScript_.get()
> + : dumpMetadata_.get();
> +
> + if (!output || controls.empty())
> + return;
> +
> + /* \todo Figure out PFC */
> + *output << " - " << request->sequence() << ":" << std::endl;
> +
> + const ControlIdMap *idMap = controls.idMap();
> + for (const auto &pair : controls) {
> + const ControlId *ctrlId = idMap->at(pair.first);
> + *output << " " << ctrlId->name() << ": " << pair.second.toString() << std::endl;
> + }
> +
> + /* \todo Investigate the overhead of flushing this frequently */
> + output->flush();
Definitely sounds like something that should just be done at destruct
time to me.
Do you expect someone to be filtering or parsing this in realtime?
> +}
> +
> /**
> * \class PipelineHandlerFactoryBase
> * \brief Base class for pipeline handler factories
> --
> 2.39.2
>
More information about the libcamera-devel
mailing list