[v5,10/18] libcamera: software_isp: Call Algorithm::queueRequest
diff mbox series

Message ID 20240830072554.180672-11-mzamazal@redhat.com
State Superseded
Headers show
Series
  • Software ISP refactoring
Related show

Commit Message

Milan Zamazal Aug. 30, 2024, 7:25 a.m. UTC
This patch adds Algorithm::queueRequest call for the defined algorithms.
As there are currently no control knobs in software ISP nor the
corresponding queueRequest call chain, the patch also introduces the
queueRequest methods and calls from the pipeline to the IPA.

This is preparation only since there are currently no Algorithm based
algorithms defined and no current software ISP algorithms support
control knobs.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 include/libcamera/internal/software_isp/software_isp.h |  1 +
 include/libcamera/ipa/soft.mojom                       |  1 +
 src/ipa/simple/soft_simple.cpp                         |  9 +++++++++
 src/libcamera/pipeline/simple/simple.cpp               |  2 ++
 src/libcamera/software_isp/software_isp.cpp            | 10 ++++++++++
 5 files changed, 23 insertions(+)

Comments

Laurent Pinchart Sept. 1, 2024, 8:27 p.m. UTC | #1
Hi Milan,

Thank you for the patch.

On Fri, Aug 30, 2024 at 09:25:46AM +0200, Milan Zamazal wrote:
> This patch adds Algorithm::queueRequest call for the defined algorithms.
> As there are currently no control knobs in software ISP nor the
> corresponding queueRequest call chain, the patch also introduces the
> queueRequest methods and calls from the pipeline to the IPA.
> 
> This is preparation only since there are currently no Algorithm based
> algorithms defined and no current software ISP algorithms support
> control knobs.
> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
>  include/libcamera/internal/software_isp/software_isp.h |  1 +
>  include/libcamera/ipa/soft.mojom                       |  1 +
>  src/ipa/simple/soft_simple.cpp                         |  9 +++++++++
>  src/libcamera/pipeline/simple/simple.cpp               |  2 ++
>  src/libcamera/software_isp/software_isp.cpp            | 10 ++++++++++
>  5 files changed, 23 insertions(+)
> 
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index c5d5a46f..a3e3a9da 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -73,6 +73,7 @@ public:
>  	int start();
>  	void stop();
>  
> +	void queueRequest(const uint32_t frame, const ControlList &controls);
>  	int queueBuffers(uint32_t frame, FrameBuffer *input,
>  			 const std::map<const Stream *, FrameBuffer *> &outputs);
>  
> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
> index 6b049d20..0ca2869c 100644
> --- a/include/libcamera/ipa/soft.mojom
> +++ b/include/libcamera/ipa/soft.mojom
> @@ -23,6 +23,7 @@ interface IPASoftInterface {
>  	configure(IPAConfigInfo configInfo)
>  		=> (int32 ret);
>  
> +	[async] queueRequest(uint32 frame, libcamera.ControlList sensorControls);
>  	[async] processStats(uint32 frame,
>                               uint32 bufferId,
>                               libcamera.ControlList sensorControls);
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index 0ea62266..eb3bbd92 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -78,6 +78,7 @@ public:
>  	int start() override;
>  	void stop() override;
>  
> +	void queueRequest(const uint32_t frame, const ControlList &controls) override;
>  	void processStats(const uint32_t frame, const uint32_t bufferId,
>  			  const ControlList &sensorControls) override;
>  
> @@ -270,6 +271,14 @@ void IPASoftSimple::stop()
>  {
>  }
>  
> +void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)
> +{
> +	IPAFrameContext &frameContext = context_.frameContexts.alloc(frame);
> +
> +	for (auto const &algo : algorithms())
> +		algo->queueRequest(context_, frame, frameContext, controls);
> +}
> +
>  void IPASoftSimple::processStats([[maybe_unused]] const uint32_t frame,
>  				 [[maybe_unused]] const uint32_t bufferId,
>  				 const ControlList &sensorControls)
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 834b33d9..ed50ab52 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -1428,6 +1428,8 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
>  	if (data->useConversion_)
>  		data->conversionQueue_.push(std::move(buffers));
>  
> +	data->swIsp_->queueRequest(request->sequence(), request->controls());

This will crash if the soft ISP isn't enabled. With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
>  	return 0;
>  }
>  
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 8f1c0f65..eda18947 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -278,6 +278,16 @@ int SoftwareIsp::exportBuffers(const Stream *stream, unsigned int count,
>  	return count;
>  }
>  
> +/**
> + * \brief Queue a request and process the control list from the application
> + * \param[in] frame The number of the frame which will be processed next
> + * \param[in] controls The controls for the \a frame
> + */
> +void SoftwareIsp::queueRequest(const uint32_t frame, const ControlList &controls)
> +{
> +	ipa_->queueRequest(frame, controls);
> +}
> +
>  /**
>   * \brief Queue buffers to Software ISP
>   * \param[in] frame The frame number

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 c5d5a46f..a3e3a9da 100644
--- a/include/libcamera/internal/software_isp/software_isp.h
+++ b/include/libcamera/internal/software_isp/software_isp.h
@@ -73,6 +73,7 @@  public:
 	int start();
 	void stop();
 
+	void queueRequest(const uint32_t frame, const ControlList &controls);
 	int queueBuffers(uint32_t frame, FrameBuffer *input,
 			 const std::map<const Stream *, FrameBuffer *> &outputs);
 
diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom
index 6b049d20..0ca2869c 100644
--- a/include/libcamera/ipa/soft.mojom
+++ b/include/libcamera/ipa/soft.mojom
@@ -23,6 +23,7 @@  interface IPASoftInterface {
 	configure(IPAConfigInfo configInfo)
 		=> (int32 ret);
 
+	[async] queueRequest(uint32 frame, libcamera.ControlList sensorControls);
 	[async] processStats(uint32 frame,
                              uint32 bufferId,
                              libcamera.ControlList sensorControls);
diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
index 0ea62266..eb3bbd92 100644
--- a/src/ipa/simple/soft_simple.cpp
+++ b/src/ipa/simple/soft_simple.cpp
@@ -78,6 +78,7 @@  public:
 	int start() override;
 	void stop() override;
 
+	void queueRequest(const uint32_t frame, const ControlList &controls) override;
 	void processStats(const uint32_t frame, const uint32_t bufferId,
 			  const ControlList &sensorControls) override;
 
@@ -270,6 +271,14 @@  void IPASoftSimple::stop()
 {
 }
 
+void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)
+{
+	IPAFrameContext &frameContext = context_.frameContexts.alloc(frame);
+
+	for (auto const &algo : algorithms())
+		algo->queueRequest(context_, frame, frameContext, controls);
+}
+
 void IPASoftSimple::processStats([[maybe_unused]] const uint32_t frame,
 				 [[maybe_unused]] const uint32_t bufferId,
 				 const ControlList &sensorControls)
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 834b33d9..ed50ab52 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -1428,6 +1428,8 @@  int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
 	if (data->useConversion_)
 		data->conversionQueue_.push(std::move(buffers));
 
+	data->swIsp_->queueRequest(request->sequence(), request->controls());
+
 	return 0;
 }
 
diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index 8f1c0f65..eda18947 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -278,6 +278,16 @@  int SoftwareIsp::exportBuffers(const Stream *stream, unsigned int count,
 	return count;
 }
 
+/**
+ * \brief Queue a request and process the control list from the application
+ * \param[in] frame The number of the frame which will be processed next
+ * \param[in] controls The controls for the \a frame
+ */
+void SoftwareIsp::queueRequest(const uint32_t frame, const ControlList &controls)
+{
+	ipa_->queueRequest(frame, controls);
+}
+
 /**
  * \brief Queue buffers to Software ISP
  * \param[in] frame The frame number