[RFC,v3,04/17] libcamera: software_isp: Separate allocation of the parameters buffer
diff mbox series

Message ID 20260604095105.68798-7-mzamazal@redhat.com
State Superseded
Headers show
Series
  • Untitled series #5978
Related show

Commit Message

Milan Zamazal June 4, 2026, 9:50 a.m. UTC
There will be some additions to that code in followup patches and let's
not blow the code in SoftwareIsp constructor even more.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 .../internal/software_isp/software_isp.h        |  1 +
 src/libcamera/software_isp/software_isp.cpp     | 17 ++++++++++++-----
 2 files changed, 13 insertions(+), 5 deletions(-)

Comments

Kieran Bingham June 4, 2026, 6:41 p.m. UTC | #1
Quoting Milan Zamazal (2026-06-04 10:50:48)
> There will be some additions to that code in followup patches and let's
> not blow the code in SoftwareIsp constructor even more.
> 

I'm not sure we would normally put code into a function if it performs
one thing and is only used once, but maybe it helps later, so I'll
already tentatively put this here as I don't think it harms anything but
I'm not sure if it's essential yet:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>  .../internal/software_isp/software_isp.h        |  1 +
>  src/libcamera/software_isp/software_isp.cpp     | 17 ++++++++++++-----
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index 85c9059e2..f97d25f0a 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -90,6 +90,7 @@ public:
>  private:
>         void saveIspParams(const uint32_t paramsBufferId);
>         void paramsBufferReady(const uint32_t paramsBufferId);
> +       bool allocateParamsBuffers();
>         void setSensorCtrls(const ControlList &sensorControls);
>         void statsReady(uint32_t frame, uint32_t bufferId);
>         void inputReady(FrameBuffer *input);
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 3d1dec21d..5dca10aee 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -90,12 +90,8 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
>                 LOG(SoftwareIsp, Error) << "Failed to create DmaBufAllocator object";
>                 return;
>         }
> -
> -       sharedParams_ = SharedMemObject<DebayerParams>("softIsp_params");
> -       if (!sharedParams_) {
> -               LOG(SoftwareIsp, Error) << "Failed to create shared memory for parameters";
> +       if (!allocateParamsBuffers())
>                 return;
> -       }
>  
>         const CameraManager &cm = *pipe->cameraManager();
>  
> @@ -181,6 +177,17 @@ SoftwareIsp::~SoftwareIsp()
>         debayer_.reset();
>  }
>  
> +bool SoftwareIsp::allocateParamsBuffers()
> +{
> +       sharedParams_ = SharedMemObject<DebayerParams>("softIsp_params");
> +       if (!sharedParams_) {
> +               LOG(SoftwareIsp, Error) << "Failed to create shared memory for parameters";
> +               return false;
> +       }
> +
> +       return true;
> +}
> +
>  /**
>   * \fn int SoftwareIsp::loadConfiguration([[maybe_unused]] const std::string &filename)
>   * \brief Load a configuration from a file
> -- 
> 2.54.0
>

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 85c9059e2..f97d25f0a 100644
--- a/include/libcamera/internal/software_isp/software_isp.h
+++ b/include/libcamera/internal/software_isp/software_isp.h
@@ -90,6 +90,7 @@  public:
 private:
 	void saveIspParams(const uint32_t paramsBufferId);
 	void paramsBufferReady(const uint32_t paramsBufferId);
+	bool allocateParamsBuffers();
 	void setSensorCtrls(const ControlList &sensorControls);
 	void statsReady(uint32_t frame, uint32_t bufferId);
 	void inputReady(FrameBuffer *input);
diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index 3d1dec21d..5dca10aee 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -90,12 +90,8 @@  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
 		LOG(SoftwareIsp, Error) << "Failed to create DmaBufAllocator object";
 		return;
 	}
-
-	sharedParams_ = SharedMemObject<DebayerParams>("softIsp_params");
-	if (!sharedParams_) {
-		LOG(SoftwareIsp, Error) << "Failed to create shared memory for parameters";
+	if (!allocateParamsBuffers())
 		return;
-	}
 
 	const CameraManager &cm = *pipe->cameraManager();
 
@@ -181,6 +177,17 @@  SoftwareIsp::~SoftwareIsp()
 	debayer_.reset();
 }
 
+bool SoftwareIsp::allocateParamsBuffers()
+{
+	sharedParams_ = SharedMemObject<DebayerParams>("softIsp_params");
+	if (!sharedParams_) {
+		LOG(SoftwareIsp, Error) << "Failed to create shared memory for parameters";
+		return false;
+	}
+
+	return true;
+}
+
 /**
  * \fn int SoftwareIsp::loadConfiguration([[maybe_unused]] const std::string &filename)
  * \brief Load a configuration from a file