[PATCH v5 2/6] ipa: simple: softisp: Extend to pass metadata
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed Mar 26 21:38:03 CET 2025
Quoting Milan Zamazal (2025-03-26 12:48:51)
> From: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> Extend the Simple IPA IPC to support returning a metadata ControlList
> when the process call has completed.
>
> A new signal from the IPA is introduced to report the metadata,
> similarly to what the hardware pipelines do.
>
> Merge the metadata reported by the ISP into any completing request to
> provide to the application. Completion of a request is delayed until
> this is done; this doesn't apply to canceled requests.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
I might have started this, but I think Milan has looked after it well,
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
> .../internal/software_isp/software_isp.h | 2 ++
> include/libcamera/ipa/soft.mojom | 1 +
> src/ipa/simple/soft_simple.cpp | 7 +---
> src/libcamera/pipeline/simple/simple.cpp | 33 ++++++++++++++++++-
> src/libcamera/software_isp/software_isp.cpp | 9 +++++
> 5 files changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index 0026cec2..1ee37dc7 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -85,12 +85,14 @@ public:
> Signal<FrameBuffer *> inputBufferReady;
> Signal<FrameBuffer *> outputBufferReady;
> Signal<uint32_t, uint32_t> ispStatsReady;
> + Signal<uint32_t, const ControlList &> metadataReady;
> Signal<const ControlList &> setSensorControls;
>
> private:
> void saveIspParams();
> void setSensorCtrls(const ControlList &sensorControls);
> void statsReady(uint32_t frame, uint32_t bufferId);
> + void saveMetadata(uint32_t frame, const ControlList &metadata);
> void inputReady(FrameBuffer *input);
> void outputReady(FrameBuffer *output);
>
> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
> index ede74413..a8e6ecda 100644
> --- a/include/libcamera/ipa/soft.mojom
> +++ b/include/libcamera/ipa/soft.mojom
> @@ -33,4 +33,5 @@ interface IPASoftInterface {
> interface IPASoftEventInterface {
> setSensorControls(libcamera.ControlList sensorControls);
> setIspParams();
> + metadataReady(uint32 frame, libcamera.ControlList metadata);
> };
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index a87c6cdd..4ef67c43 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -299,15 +299,10 @@ void IPASoftSimple::processStats(const uint32_t frame,
> int32_t again = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();
> frameContext.sensor.gain = camHelper_ ? camHelper_->gain(again) : again;
>
> - /*
> - * Software ISP currently does not produce any metadata. Use an empty
> - * ControlList for now.
> - *
> - * \todo Implement proper metadata handling
> - */
> ControlList metadata(controls::controls);
> for (auto const &algo : algorithms())
> algo->process(context_, frame, frameContext, stats_, metadata);
> + metadataReady.emit(frame, metadata);
>
> /* Sanity check */
> if (!sensorControls.contains(V4L2_CID_EXPOSURE) ||
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 2281c0a6..f34df926 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -184,6 +184,7 @@ class SimplePipelineHandler;
> struct SimpleFrameInfo {
> uint32_t frame;
> Request *request;
> + bool metadataProcessed;
> };
>
> class SimpleFrames
> @@ -207,6 +208,7 @@ void SimpleFrames::create(Request *request)
> ASSERT(inserted);
> it->second.frame = frame;
> it->second.request = request;
> + it->second.metadataProcessed = false;
> }
>
> void SimpleFrames::destroy(uint32_t frame)
> @@ -357,11 +359,13 @@ private:
> void tryPipeline(unsigned int code, const Size &size);
> static std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);
>
> + void tryCompleteRequest(Request *request);
> void completeRequest(Request *request);
> void conversionInputDone(FrameBuffer *buffer);
> void conversionOutputDone(FrameBuffer *buffer);
>
> void ispStatsReady(uint32_t frame, uint32_t bufferId);
> + void metadataReady(uint32_t frame, const ControlList &metadata);
> void setSensorControls(const ControlList &sensorControls);
> };
>
> @@ -600,6 +604,7 @@ int SimpleCameraData::init()
> swIsp_->inputBufferReady.connect(this, &SimpleCameraData::conversionInputDone);
> swIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);
> swIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);
> + swIsp_->metadataReady.connect(this, &SimpleCameraData::metadataReady);
> swIsp_->setSensorControls.connect(this, &SimpleCameraData::setSensorControls);
> }
> }
> @@ -932,6 +937,21 @@ void SimpleCameraData::clearIncompleteRequests()
> }
> }
>
> +void SimpleCameraData::tryCompleteRequest(Request *request)
> +{
> + if (request->hasPendingBuffers())
> + return;
> +
> + SimpleFrameInfo *info = frameInfo_.find(request);
> + if (!info)
> + return;
> +
> + if (!info->metadataProcessed)
> + return;
> +
> + completeRequest(request);
> +}
> +
> void SimpleCameraData::completeRequest(Request *request)
> {
> SimpleFrameInfo *info = frameInfo_.find(request);
> @@ -957,7 +977,7 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)
> /* Complete the buffer and the request. */
> Request *request = buffer->request();
> if (pipe->completeBuffer(request, buffer))
> - completeRequest(request);
> + tryCompleteRequest(request);
> }
>
> void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)
> @@ -966,6 +986,17 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)
> delayedCtrls_->get(frame));
> }
>
> +void SimpleCameraData::metadataReady(uint32_t frame, const ControlList &metadata)
> +{
> + SimpleFrameInfo *info = frameInfo_.find(frame);
> + if (!info)
> + return;
> +
> + info->request->metadata().merge(metadata);
> + info->metadataProcessed = true;
> + tryCompleteRequest(info->request);
> +}
> +
> void SimpleCameraData::setSensorControls(const ControlList &sensorControls)
> {
> delayedCtrls_->push(sensorControls);
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 4a74dcb6..6baa3fec 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -55,6 +55,11 @@ LOG_DEFINE_CATEGORY(SoftwareIsp)
> * \brief A signal emitted when the statistics for IPA are ready
> */
>
> +/**
> + * \var SoftwareIsp::metadataReady
> + * \brief A signal emitted when the metadata for IPA is ready
> + */
> +
> /**
> * \var SoftwareIsp::setSensorControls
> * \brief A signal emitted when the values to write to the sensor controls are
> @@ -141,6 +146,10 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
> }
>
> ipa_->setIspParams.connect(this, &SoftwareIsp::saveIspParams);
> + ipa_->metadataReady.connect(this,
> + [this](uint32_t frame, const ControlList &metadata) {
> + metadataReady.emit(frame, metadata);
> + });
> ipa_->setSensorControls.connect(this, &SoftwareIsp::setSensorCtrls);
As far as I can tell, all comments handled from v3:
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> debayer_->moveToThread(&ispWorkerThread_);
> --
> 2.49.0
>
More information about the libcamera-devel
mailing list