[PATCH 1/2] pipeline: Add support for dumping capture script and metadata

[Jacopo Mondi]

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 :)

>
> 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

>  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 ?

> +
>  	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

> +
> +	/*
> +	 * 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 ?

Thanks
  j

> +	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
>