[1/3] ipa: simple: softisp: Extend to pass metadata
diff mbox series

Message ID 20240617232525.878530-2-kieran.bingham@ideasonboard.com
State New
Headers show
Series
  • ipa: simple: Introduce metadata reporting
Related show

Commit Message

Kieran Bingham June 17, 2024, 11:25 p.m. UTC 
Extend the Simple IPA IPC to support returning a metadata controllist
when the process call has completed.

For efficiency, use the existing signal setIspParams() directly
to avoid having an extra async callback to return the metadata.

Merge the metadata reported by the ISP into any completing
request to provide to the application.

This should be further extended or improved to make use of the frame
context structures so that the metadata is tied to a specific frame and
request completion.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 include/libcamera/internal/software_isp/software_isp.h | 5 ++++-
 include/libcamera/ipa/soft.mojom                       | 2 +-
 src/ipa/simple/soft_simple.cpp                         | 4 +++-
 src/libcamera/pipeline/simple/simple.cpp               | 7 +++++--
 src/libcamera/software_isp/software_isp.cpp            | 6 ++++--
 5 files changed, 17 insertions(+), 7 deletions(-)

Comments

Kieran Bingham June 17, 2024, 11:34 p.m. UTC | #1 
Quoting Kieran Bingham (2024-06-18 00:25:23)
> Extend the Simple IPA IPC to support returning a metadata controllist
> when the process call has completed.
> 
> For efficiency, use the existing signal setIspParams() directly
> to avoid having an extra async callback to return the metadata.
> 
> Merge the metadata reported by the ISP into any completing
> request to provide to the application.
> 
> This should be further extended or improved to make use of the frame
> context structures so that the metadata is tied to a specific frame and
> request completion.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  include/libcamera/internal/software_isp/software_isp.h | 5 ++++-
>  include/libcamera/ipa/soft.mojom                       | 2 +-
>  src/ipa/simple/soft_simple.cpp                         | 4 +++-
>  src/libcamera/pipeline/simple/simple.cpp               | 7 +++++--
>  src/libcamera/software_isp/software_isp.cpp            | 6 ++++--
>  5 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index c5338c05b99b..2631f40ef22a 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -80,8 +80,10 @@ public:
>         Signal<> ispStatsReady;
>         Signal<const ControlList &> setSensorControls;
>  
> +       const ControlList &metadata() { return metadata_; }
> +

And of course this fails CI because I failed to document this new
addition:

https://gitlab.freedesktop.org/camera/libcamera/-/jobs/60009854

/builds/camera/libcamera/include/libcamera/internal/software_isp/software_isp.h:83: error: Member metadata() (function) of class libcamera::SoftwareIsp is not documented. (warning treated as error, aborting now)

--
Kieran
Laurent Pinchart June 17, 2024, 11:37 p.m. UTC | #2 
Hi Kieran,

Thank you for the patch.

On Tue, Jun 18, 2024 at 12:25:23AM +0100, Kieran Bingham wrote:
> Extend the Simple IPA IPC to support returning a metadata controllist

s/controllist/ControlList/

> when the process call has completed.
> 
> For efficiency, use the existing signal setIspParams() directly
> to avoid having an extra async callback to return the metadata.
> 
> Merge the metadata reported by the ISP into any completing
> request to provide to the application.
> 
> This should be further extended or improved to make use of the frame
> context structures so that the metadata is tied to a specific frame and
> request completion.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  include/libcamera/internal/software_isp/software_isp.h | 5 ++++-
>  include/libcamera/ipa/soft.mojom                       | 2 +-
>  src/ipa/simple/soft_simple.cpp                         | 4 +++-
>  src/libcamera/pipeline/simple/simple.cpp               | 7 +++++--
>  src/libcamera/software_isp/software_isp.cpp            | 6 ++++--
>  5 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index c5338c05b99b..2631f40ef22a 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -80,8 +80,10 @@ public:
>  	Signal<> ispStatsReady;
>  	Signal<const ControlList &> setSensorControls;
>  
> +	const ControlList &metadata() { return metadata_; }
> +
>  private:
> -	void saveIspParams();
> +	void saveIspParams(const ControlList &metadata);
>  	void setSensorCtrls(const ControlList &sensorControls);
>  	void statsReady();
>  	void inputReady(FrameBuffer *input);
> @@ -92,6 +94,7 @@ private:
>  	SharedMemObject<DebayerParams> sharedParams_;
>  	DebayerParams debayerParams_;
>  	DmaBufAllocator dmaHeap_;
> +	ControlList metadata_;
>  
>  	std::unique_ptr<ipa::soft::IPAProxySoft> ipa_;
>  };
> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
> index 3aa2066ebaa0..1a0221df7660 100644
> --- a/include/libcamera/ipa/soft.mojom
> +++ b/include/libcamera/ipa/soft.mojom
> @@ -24,5 +24,5 @@ interface IPASoftInterface {
>  
>  interface IPASoftEventInterface {
>  	setSensorControls(libcamera.ControlList sensorControls);
> -	setIspParams();
> +	setIspParams(libcamera.ControlList metadata);
>  };
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index b7746ce09206..09c7b575301e 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -250,6 +250,8 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
>  	SwIspStats::Histogram histogram = stats_->yHistogram;
>  	if (ignoreUpdates_ > 0)
>  		blackLevel_.update(histogram);
> +
> +	ControlList metadata(controls::controls);
>  	const uint8_t blackLevel = blackLevel_.get();
>  
>  	/*
> @@ -303,7 +305,7 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
>  		params_->blue[i] = gammaTable_[idx];
>  	}
>  
> -	setIspParams.emit();
> +	setIspParams.emit(metadata);
>  
>  	/* \todo Switch to the libipa/algorithm.h API someday. */
>  
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index eb36578e320e..accdb744d61b 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -860,10 +860,13 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
>  			return;
>  		}
>  
> -		if (converter_)
> +		if (converter_) {
>  			converter_->queueBuffers(buffer, conversionQueue_.front());
> -		else
> +		} else {
>  			swIsp_->queueBuffers(buffer, conversionQueue_.front());
> +			if (request)
> +				request->metadata().merge(swIsp_->metadata());

You're shoving metadata in a random request here, that really makes me
cringe. It's not just that there's no frame context tracking on the IPA
side, the pipeline handler side is wrong too.

> +		}
>  
>  		conversionQueue_.pop();
>  		return;
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 20fb6f48fdf9..efbbea2d2279 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -68,7 +68,8 @@ LOG_DEFINE_CATEGORY(SoftwareIsp)
>  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)
>  	: dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |
>  		   DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
> -		   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf)
> +		   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf),
> +	  metadata_(controls::controls)
>  {
>  	/*
>  	 * debayerParams_ must be initialized because the initial value is used for
> @@ -349,9 +350,10 @@ void SoftwareIsp::process(FrameBuffer *input, FrameBuffer *output)
>  			       ConnectionTypeQueued, input, output, debayerParams_);
>  }
>  
> -void SoftwareIsp::saveIspParams()
> +void SoftwareIsp::saveIspParams(const ControlList &metadata)
>  {
>  	debayerParams_ = *sharedParams_;
> +	metadata_.merge(metadata, ControlList::MergePolicy::OverwriteExisting);
>  }
>  
>  void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)
Kieran Bingham June 19, 2024, 10:10 p.m. UTC | #3 
Quoting Laurent Pinchart (2024-06-18 00:37:14)
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Tue, Jun 18, 2024 at 12:25:23AM +0100, Kieran Bingham wrote:
> > Extend the Simple IPA IPC to support returning a metadata controllist
> 
> s/controllist/ControlList/
> 
> > when the process call has completed.
> > 
> > For efficiency, use the existing signal setIspParams() directly
> > to avoid having an extra async callback to return the metadata.
> > 
> > Merge the metadata reported by the ISP into any completing
> > request to provide to the application.
> > 
> > This should be further extended or improved to make use of the frame
> > context structures so that the metadata is tied to a specific frame and
> > request completion.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  include/libcamera/internal/software_isp/software_isp.h | 5 ++++-
> >  include/libcamera/ipa/soft.mojom                       | 2 +-
> >  src/ipa/simple/soft_simple.cpp                         | 4 +++-
> >  src/libcamera/pipeline/simple/simple.cpp               | 7 +++++--
> >  src/libcamera/software_isp/software_isp.cpp            | 6 ++++--
> >  5 files changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> > index c5338c05b99b..2631f40ef22a 100644
> > --- a/include/libcamera/internal/software_isp/software_isp.h
> > +++ b/include/libcamera/internal/software_isp/software_isp.h
> > @@ -80,8 +80,10 @@ public:
> >       Signal<> ispStatsReady;
> >       Signal<const ControlList &> setSensorControls;
> >  
> > +     const ControlList &metadata() { return metadata_; }
> > +
> >  private:
> > -     void saveIspParams();
> > +     void saveIspParams(const ControlList &metadata);
> >       void setSensorCtrls(const ControlList &sensorControls);
> >       void statsReady();
> >       void inputReady(FrameBuffer *input);
> > @@ -92,6 +94,7 @@ private:
> >       SharedMemObject<DebayerParams> sharedParams_;
> >       DebayerParams debayerParams_;
> >       DmaBufAllocator dmaHeap_;
> > +     ControlList metadata_;
> >  
> >       std::unique_ptr<ipa::soft::IPAProxySoft> ipa_;
> >  };
> > diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
> > index 3aa2066ebaa0..1a0221df7660 100644
> > --- a/include/libcamera/ipa/soft.mojom
> > +++ b/include/libcamera/ipa/soft.mojom
> > @@ -24,5 +24,5 @@ interface IPASoftInterface {
> >  
> >  interface IPASoftEventInterface {
> >       setSensorControls(libcamera.ControlList sensorControls);
> > -     setIspParams();
> > +     setIspParams(libcamera.ControlList metadata);
> >  };
> > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> > index b7746ce09206..09c7b575301e 100644
> > --- a/src/ipa/simple/soft_simple.cpp
> > +++ b/src/ipa/simple/soft_simple.cpp
> > @@ -250,6 +250,8 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
> >       SwIspStats::Histogram histogram = stats_->yHistogram;
> >       if (ignoreUpdates_ > 0)
> >               blackLevel_.update(histogram);
> > +
> > +     ControlList metadata(controls::controls);
> >       const uint8_t blackLevel = blackLevel_.get();
> >  
> >       /*
> > @@ -303,7 +305,7 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
> >               params_->blue[i] = gammaTable_[idx];
> >       }
> >  
> > -     setIspParams.emit();
> > +     setIspParams.emit(metadata);
> >  
> >       /* \todo Switch to the libipa/algorithm.h API someday. */
> >  
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index eb36578e320e..accdb744d61b 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -860,10 +860,13 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
> >                       return;
> >               }
> >  
> > -             if (converter_)
> > +             if (converter_) {
> >                       converter_->queueBuffers(buffer, conversionQueue_.front());
> > -             else
> > +             } else {
> >                       swIsp_->queueBuffers(buffer, conversionQueue_.front());
> > +                     if (request)
> > +                             request->metadata().merge(swIsp_->metadata());
> 
> You're shoving metadata in a random request here, that really makes me
> cringe. It's not just that there's no frame context tracking on the IPA
> side, the pipeline handler side is wrong too.

Oh, yes - I'm aware ;-)

There's lots more to do on the pipeline handler to fix up managing
controls and synchronisation.

I know Milan has a branch with some work that gets closer to this on
the IPA side already too, so I'm looking forward to that getting out.

As I said to Robert on IRC when I sent these, it was a case of 'no point
them sitting on my PC when they could be a starting point/reference'.

What I want to make obvious or easier - is that we can put more data in
here such as timing or measurements of the internal performances to be
able to give constant monitoring of the 'cost' of the SoftISP.



> > +             }
> >  
> >               conversionQueue_.pop();
> >               return;
> > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> > index 20fb6f48fdf9..efbbea2d2279 100644
> > --- a/src/libcamera/software_isp/software_isp.cpp
> > +++ b/src/libcamera/software_isp/software_isp.cpp
> > @@ -68,7 +68,8 @@ LOG_DEFINE_CATEGORY(SoftwareIsp)
> >  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)
> >       : dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |
> >                  DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
> > -                DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf)
> > +                DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf),
> > +       metadata_(controls::controls)
> >  {
> >       /*
> >        * debayerParams_ must be initialized because the initial value is used for
> > @@ -349,9 +350,10 @@ void SoftwareIsp::process(FrameBuffer *input, FrameBuffer *output)
> >                              ConnectionTypeQueued, input, output, debayerParams_);
> >  }
> >  
> > -void SoftwareIsp::saveIspParams()
> > +void SoftwareIsp::saveIspParams(const ControlList &metadata)
> >  {
> >       debayerParams_ = *sharedParams_;
> > +     metadata_.merge(metadata, ControlList::MergePolicy::OverwriteExisting);
> >  }
> >  
> >  void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart June 19, 2024, 10:16 p.m. UTC | #4 
On Wed, Jun 19, 2024 at 11:10:44PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-06-18 00:37:14)
> > On Tue, Jun 18, 2024 at 12:25:23AM +0100, Kieran Bingham wrote:
> > > Extend the Simple IPA IPC to support returning a metadata controllist
> > 
> > s/controllist/ControlList/
> > 
> > > when the process call has completed.
> > > 
> > > For efficiency, use the existing signal setIspParams() directly
> > > to avoid having an extra async callback to return the metadata.
> > > 
> > > Merge the metadata reported by the ISP into any completing
> > > request to provide to the application.
> > > 
> > > This should be further extended or improved to make use of the frame
> > > context structures so that the metadata is tied to a specific frame and
> > > request completion.
> > > 
> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > ---
> > >  include/libcamera/internal/software_isp/software_isp.h | 5 ++++-
> > >  include/libcamera/ipa/soft.mojom                       | 2 +-
> > >  src/ipa/simple/soft_simple.cpp                         | 4 +++-
> > >  src/libcamera/pipeline/simple/simple.cpp               | 7 +++++--
> > >  src/libcamera/software_isp/software_isp.cpp            | 6 ++++--
> > >  5 files changed, 17 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> > > index c5338c05b99b..2631f40ef22a 100644
> > > --- a/include/libcamera/internal/software_isp/software_isp.h
> > > +++ b/include/libcamera/internal/software_isp/software_isp.h
> > > @@ -80,8 +80,10 @@ public:
> > >       Signal<> ispStatsReady;
> > >       Signal<const ControlList &> setSensorControls;
> > >  
> > > +     const ControlList &metadata() { return metadata_; }
> > > +
> > >  private:
> > > -     void saveIspParams();
> > > +     void saveIspParams(const ControlList &metadata);
> > >       void setSensorCtrls(const ControlList &sensorControls);
> > >       void statsReady();
> > >       void inputReady(FrameBuffer *input);
> > > @@ -92,6 +94,7 @@ private:
> > >       SharedMemObject<DebayerParams> sharedParams_;
> > >       DebayerParams debayerParams_;
> > >       DmaBufAllocator dmaHeap_;
> > > +     ControlList metadata_;
> > >  
> > >       std::unique_ptr<ipa::soft::IPAProxySoft> ipa_;
> > >  };
> > > diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
> > > index 3aa2066ebaa0..1a0221df7660 100644
> > > --- a/include/libcamera/ipa/soft.mojom
> > > +++ b/include/libcamera/ipa/soft.mojom
> > > @@ -24,5 +24,5 @@ interface IPASoftInterface {
> > >  
> > >  interface IPASoftEventInterface {
> > >       setSensorControls(libcamera.ControlList sensorControls);
> > > -     setIspParams();
> > > +     setIspParams(libcamera.ControlList metadata);
> > >  };
> > > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> > > index b7746ce09206..09c7b575301e 100644
> > > --- a/src/ipa/simple/soft_simple.cpp
> > > +++ b/src/ipa/simple/soft_simple.cpp
> > > @@ -250,6 +250,8 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
> > >       SwIspStats::Histogram histogram = stats_->yHistogram;
> > >       if (ignoreUpdates_ > 0)
> > >               blackLevel_.update(histogram);
> > > +
> > > +     ControlList metadata(controls::controls);
> > >       const uint8_t blackLevel = blackLevel_.get();
> > >  
> > >       /*
> > > @@ -303,7 +305,7 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
> > >               params_->blue[i] = gammaTable_[idx];
> > >       }
> > >  
> > > -     setIspParams.emit();
> > > +     setIspParams.emit(metadata);
> > >  
> > >       /* \todo Switch to the libipa/algorithm.h API someday. */
> > >  
> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > > index eb36578e320e..accdb744d61b 100644
> > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > @@ -860,10 +860,13 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
> > >                       return;
> > >               }
> > >  
> > > -             if (converter_)
> > > +             if (converter_) {
> > >                       converter_->queueBuffers(buffer, conversionQueue_.front());
> > > -             else
> > > +             } else {
> > >                       swIsp_->queueBuffers(buffer, conversionQueue_.front());
> > > +                     if (request)
> > > +                             request->metadata().merge(swIsp_->metadata());
> > 
> > You're shoving metadata in a random request here, that really makes me
> > cringe. It's not just that there's no frame context tracking on the IPA
> > side, the pipeline handler side is wrong too.
> 
> Oh, yes - I'm aware ;-)
> 
> There's lots more to do on the pipeline handler to fix up managing
> controls and synchronisation.
> 
> I know Milan has a branch with some work that gets closer to this on
> the IPA side already too, so I'm looking forward to that getting out.
> 
> As I said to Robert on IRC when I sent these, it was a case of 'no point
> them sitting on my PC when they could be a starting point/reference'.
> 
> What I want to make obvious or easier - is that we can put more data in
> here such as timing or measurements of the internal performances to be
> able to give constant monitoring of the 'cost' of the SoftISP.

I like that :-)

> > > +             }
> > >  
> > >               conversionQueue_.pop();
> > >               return;
> > > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> > > index 20fb6f48fdf9..efbbea2d2279 100644
> > > --- a/src/libcamera/software_isp/software_isp.cpp
> > > +++ b/src/libcamera/software_isp/software_isp.cpp
> > > @@ -68,7 +68,8 @@ LOG_DEFINE_CATEGORY(SoftwareIsp)
> > >  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)
> > >       : dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |
> > >                  DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
> > > -                DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf)
> > > +                DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf),
> > > +       metadata_(controls::controls)
> > >  {
> > >       /*
> > >        * debayerParams_ must be initialized because the initial value is used for
> > > @@ -349,9 +350,10 @@ void SoftwareIsp::process(FrameBuffer *input, FrameBuffer *output)
> > >                              ConnectionTypeQueued, input, output, debayerParams_);
> > >  }
> > >  
> > > -void SoftwareIsp::saveIspParams()
> > > +void SoftwareIsp::saveIspParams(const ControlList &metadata)
> > >  {
> > >       debayerParams_ = *sharedParams_;
> > > +     metadata_.merge(metadata, ControlList::MergePolicy::OverwriteExisting);
> > >  }
> > >  
> > >  void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)

Patch
diff mbox series 

diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
index c5338c05b99b..2631f40ef22a 100644
--- a/include/libcamera/internal/software_isp/software_isp.h
+++ b/include/libcamera/internal/software_isp/software_isp.h
@@ -80,8 +80,10 @@  public:
 	Signal<> ispStatsReady;
 	Signal<const ControlList &> setSensorControls;
 
+	const ControlList &metadata() { return metadata_; }
+
 private:
-	void saveIspParams();
+	void saveIspParams(const ControlList &metadata);
 	void setSensorCtrls(const ControlList &sensorControls);
 	void statsReady();
 	void inputReady(FrameBuffer *input);
@@ -92,6 +94,7 @@  private:
 	SharedMemObject<DebayerParams> sharedParams_;
 	DebayerParams debayerParams_;
 	DmaBufAllocator dmaHeap_;
+	ControlList metadata_;
 
 	std::unique_ptr<ipa::soft::IPAProxySoft> ipa_;
 };
diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
index 3aa2066ebaa0..1a0221df7660 100644
--- a/include/libcamera/ipa/soft.mojom
+++ b/include/libcamera/ipa/soft.mojom
@@ -24,5 +24,5 @@  interface IPASoftInterface {
 
 interface IPASoftEventInterface {
 	setSensorControls(libcamera.ControlList sensorControls);
-	setIspParams();
+	setIspParams(libcamera.ControlList metadata);
 };
diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
index b7746ce09206..09c7b575301e 100644
--- a/src/ipa/simple/soft_simple.cpp
+++ b/src/ipa/simple/soft_simple.cpp
@@ -250,6 +250,8 @@  void IPASoftSimple::processStats(const ControlList &sensorControls)
 	SwIspStats::Histogram histogram = stats_->yHistogram;
 	if (ignoreUpdates_ > 0)
 		blackLevel_.update(histogram);
+
+	ControlList metadata(controls::controls);
 	const uint8_t blackLevel = blackLevel_.get();
 
 	/*
@@ -303,7 +305,7 @@  void IPASoftSimple::processStats(const ControlList &sensorControls)
 		params_->blue[i] = gammaTable_[idx];
 	}
 
-	setIspParams.emit();
+	setIspParams.emit(metadata);
 
 	/* \todo Switch to the libipa/algorithm.h API someday. */
 
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index eb36578e320e..accdb744d61b 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -860,10 +860,13 @@  void SimpleCameraData::bufferReady(FrameBuffer *buffer)
 			return;
 		}
 
-		if (converter_)
+		if (converter_) {
 			converter_->queueBuffers(buffer, conversionQueue_.front());
-		else
+		} else {
 			swIsp_->queueBuffers(buffer, conversionQueue_.front());
+			if (request)
+				request->metadata().merge(swIsp_->metadata());
+		}
 
 		conversionQueue_.pop();
 		return;
diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index 20fb6f48fdf9..efbbea2d2279 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -68,7 +68,8 @@  LOG_DEFINE_CATEGORY(SoftwareIsp)
 SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)
 	: dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |
 		   DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
-		   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf)
+		   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf),
+	  metadata_(controls::controls)
 {
 	/*
 	 * debayerParams_ must be initialized because the initial value is used for
@@ -349,9 +350,10 @@  void SoftwareIsp::process(FrameBuffer *input, FrameBuffer *output)
 			       ConnectionTypeQueued, input, output, debayerParams_);
 }
 
-void SoftwareIsp::saveIspParams()
+void SoftwareIsp::saveIspParams(const ControlList &metadata)
 {
 	debayerParams_ = *sharedParams_;
+	metadata_.merge(metadata, ControlList::MergePolicy::OverwriteExisting);
 }
 
 void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)