[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