| Message ID | 20241004120517.3572281-2-paul.elder@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
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@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 >
On Mon, Oct 07, 2024 at 10:28:15AM +0100, Kieran Bingham wrote: > 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@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." Let's make it generic. We should also document the capture script format as part of libcamera now that libcamera knows about capture scripts. It can be done later. > > + > > + 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`` Example value: ``/home/${USER}/capture_script.yaml`` or Example value: ``${HOME}/capture_script.yaml`` > > Suffixed ? After the .yaml? I feel there's so room for improvement here. One option would be to reuse the same scheme as cam when capturing to files, with a '#' place holder that gets replaced with a counter. > > + > > +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 */ \todo Same below. As this hard to handle though, can't we do it now ? > > + 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. If you want to start a new paragraph, add a blank line. Otherwise, drop the line break. > > + * This needs to be here as PipelineHandler::start is a virtual function s/start/start()/ > > + * so it is impractical to add the dumping there. Same here, blank line. > > + * TODO Pass the sensor configuration, once it is supported > > + */ > > + d->pipe_->invokeMethod(&PipelineHandler::dumpConfiguration, > > + ConnectionTypeBlocking, d->activeStreams_, d->orientation_); We have PipelineHandler::stop() implemented in the base class and PipelineHandler::stopDevice() implemented in subclasses. How about doing the same for start, and drop PipelineHandler::dumpConfiguration() ? That would avoid going back and forth between threads. > > + > > 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()); request->controls() I suppose. > > > > 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? I'm mostly concerned about a crash, but maybe that's not a big deal. > > +} > > + > > /** > > * \class PipelineHandlerFactoryBase > > * \brief Base class for pipeline handler factories
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. + All controls that are set for each request will be dumped into the file + specified by the environment variable as a capture script, which can later + be fed into the cam application to replay a 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`` + +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); + if (request->_d()->cancelled_) { completeRequest(request); return; @@ -555,6 +560,8 @@ void PipelineHandler::completeRequest(Request *request) request->_d()->complete(); + dumpRequest(request, DumpMode::Metadata); + 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 */ + 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(); +} + +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(); +} + /** * \class PipelineHandlerFactoryBase * \brief Base class for pipeline handler factories
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@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(-)