[PATCH v1 5/8] ipa: rkisp1: Add debug metadata support to the rkisp1

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Wed, Oct 02, 2024 at 06:19:23PM +0200, Stefan Klug wrote:
> Add a DebugMetadata helper to the context and add the corresponding
> plumbing.  This is all that is needed to support debug metadata in an
> IPA.
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  src/ipa/rkisp1/ipa_context.h | 5 +++++
>  src/ipa/rkisp1/rkisp1.cpp    | 4 ++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index d52e73ad2503..7b93a9e9461d 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -17,8 +17,11 @@
>  #include <libcamera/control_ids.h>
>  #include <libcamera/controls.h>
>  #include <libcamera/geometry.h>
> +
>  #include <libcamera/ipa/core_ipa_interface.h>
>  
> +#include "libcamera/internal/debug_controls.h"
> +
>  #include <libipa/camera_sensor_helper.h>
>  #include <libipa/fc_queue.h>
>  #include <libipa/matrix.h>
> @@ -194,6 +197,8 @@ struct IPAContext {
>  
>  	ControlInfoMap::Map ctrlMap;
>  
> +	DebugMetadata debugMetadata;
> +
>  	/* Interface to the Camera Helper */
>  	std::unique_ptr<CameraSensorHelper> camHelper;
>  };
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index a579f21de56f..5b1ef0c372c6 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -117,6 +117,7 @@ const IPAHwSettings ipaHwSettingsV12{
>  const ControlInfoMap::Map rkisp1Controls{
>  	{ &controls::AwbEnable, ControlInfo(false, true) },
>  	{ &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },
> +	{ &controls::DebugMetadataEnable, ControlInfo(false, true, false) },
>  	{ &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },
>  	{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },
>  };
> @@ -326,6 +327,7 @@ void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids)
>  void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)
>  {
>  	IPAFrameContext &frameContext = context_.frameContexts.alloc(frame);
> +	context_.debugMetadata.checkForEnable(controls);
>  
>  	for (auto const &a : algorithms()) {
>  		Algorithm *algo = static_cast<Algorithm *>(a.get());
> @@ -368,6 +370,7 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
>  		context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>  
>  	ControlList metadata(controls::controls);
> +	context_.debugMetadata.assignControlList(&metadata);

What's the advantage of doing this here, instead of...

>  
>  	for (auto const &a : algorithms()) {
>  		Algorithm *algo = static_cast<Algorithm *>(a.get());
> @@ -377,6 +380,7 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
>  	}
>  
>  	setControls(frame);
> +	context_.debugMetadata.assignControlList(nullptr);

... extracting the control list from debugMetadata here and merging it
into metadata ?

>  	metadataReady.emit(frame, metadata);
>  }

-- 
Regards,

Laurent Pinchart