[v8,17/26] libcamera: software_isp: debayer_egl: Make DebayerEGL an environment option
diff mbox series

Message ID 20251212002937.3118-18-bryan.odonoghue@linaro.org
State Superseded
Headers show
Series
  • Add GLES 2.0 GPUISP to libcamera
Related show

Commit Message

Bryan O'Donoghue Dec. 12, 2025, 12:29 a.m. UTC 
If GPUISP support is available make it so an environment variable can
switch it on.

Given we don't have full feature parity with CPUISP just yet on pixel
format output, we should default to CPUISP mode giving the user the option
to switch on GPUISP by setting LIBCAMERA_SOFTISP_MODE=gpu

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 src/libcamera/software_isp/software_isp.cpp | 24 ++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

Comments

Robert Mader Dec. 14, 2025, 1:30 p.m. UTC | #1 
Hi

On 12.12.25 01:29, Bryan O'Donoghue wrote:
> If GPUISP support is available make it so an environment variable can
> switch it on.
>
> Given we don't have full feature parity with CPUISP just yet on pixel
> format output, we should default to CPUISP mode giving the user the option
> to switch on GPUISP by setting LIBCAMERA_SOFTISP_MODE=gpu
>
> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   src/libcamera/software_isp/software_isp.cpp | 24 ++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index b31a374d8..efe44e393 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -15,6 +15,7 @@
>   
>   #include <libcamera/base/log.h>
>   #include <libcamera/base/thread.h>
> +#include <libcamera/base/utils.h>
>   
>   #include <libcamera/controls.h>
>   #include <libcamera/formats.h>
> @@ -25,6 +26,9 @@
>   #include "libcamera/internal/software_isp/debayer_params.h"
>   
>   #include "debayer_cpu.h"
> +#if HAVE_DEBAYER_EGL
> +#include "debayer_egl.h"
> +#endif
>   
>   /**
>    * \file software_isp.cpp
> @@ -117,7 +121,25 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
>   	}
>   	stats->statsReady.connect(this, &SoftwareIsp::statsReady);
>   
> -	debayer_ = std::make_unique<DebayerCpu>(std::move(stats), configuration);
> +#if HAVE_DEBAYER_EGL
> +	std::optional<std::string> softISPMode = configuration.envOption("LIBCAMERA_SOFTISP_MODE", { "software_isp", "mode" });
> +
> +	if (softISPMode && softISPMode == "gpu") {
> +		debayer_ = std::make_unique<DebayerEGL>(std::move(stats), configuration);
> +		if (!debayer_) {
> +			LOG(SoftwareIsp, Error) << "Failed to instantiate GPUISP";
> +			return;
If we fail to initialize EGL / DebayerEGL, it would probably be better 
to fall back to CPU instead of returning.
> +		}
> +	}
> +#endif
> +	if (!debayer_)
> +		debayer_ = std::make_unique<DebayerCpu>(std::move(stats), configuration);
> +
> +	if (!debayer_) {
> +		LOG(SoftwareIsp, Error) << "Failed to create Debayer object";
> +		return;
> +	}
> +
>   	debayer_->inputBufferReady.connect(this, &SoftwareIsp::inputReady);
>   	debayer_->outputBufferReady.connect(this, &SoftwareIsp::outputReady);
>
Barnabás Pőcze Dec. 18, 2025, 9:52 a.m. UTC | #2 
Hi

2025. 12. 12. 1:29 keltezéssel, Bryan O'Donoghue írta:
> If GPUISP support is available make it so an environment variable can
> switch it on.
> 
> Given we don't have full feature parity with CPUISP just yet on pixel
> format output, we should default to CPUISP mode giving the user the option
> to switch on GPUISP by setting LIBCAMERA_SOFTISP_MODE=gpu
> 
> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   src/libcamera/software_isp/software_isp.cpp | 24 ++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index b31a374d8..efe44e393 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -15,6 +15,7 @@
>   
>   #include <libcamera/base/log.h>
>   #include <libcamera/base/thread.h>
> +#include <libcamera/base/utils.h>
>   
>   #include <libcamera/controls.h>
>   #include <libcamera/formats.h>
> @@ -25,6 +26,9 @@
>   #include "libcamera/internal/software_isp/debayer_params.h"
>   
>   #include "debayer_cpu.h"
> +#if HAVE_DEBAYER_EGL
> +#include "debayer_egl.h"
> +#endif
>   
>   /**
>    * \file software_isp.cpp
> @@ -117,7 +121,25 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
>   	}
>   	stats->statsReady.connect(this, &SoftwareIsp::statsReady);
>   
> -	debayer_ = std::make_unique<DebayerCpu>(std::move(stats), configuration);
> +#if HAVE_DEBAYER_EGL
> +	std::optional<std::string> softISPMode = configuration.envOption("LIBCAMERA_SOFTISP_MODE", { "software_isp", "mode" });
> +
> +	if (softISPMode && softISPMode == "gpu") {
> +		debayer_ = std::make_unique<DebayerEGL>(std::move(stats), configuration);
> +		if (!debayer_) {

`std::make_unique()` never returns `nullptr`.


> +			LOG(SoftwareIsp, Error) << "Failed to instantiate GPUISP";
> +			return;
> +		}
> +	}
> +#endif
> +	if (!debayer_)
> +		debayer_ = std::make_unique<DebayerCpu>(std::move(stats), configuration);
> +
> +	if (!debayer_) {
> +		LOG(SoftwareIsp, Error) << "Failed to create Debayer object";
> +		return;
> +	}
> +
>   	debayer_->inputBufferReady.connect(this, &SoftwareIsp::inputReady);
>   	debayer_->outputBufferReady.connect(this, &SoftwareIsp::outputReady);
>

Patch
diff mbox series 

diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index b31a374d8..efe44e393 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -15,6 +15,7 @@ 
 
 #include <libcamera/base/log.h>
 #include <libcamera/base/thread.h>
+#include <libcamera/base/utils.h>
 
 #include <libcamera/controls.h>
 #include <libcamera/formats.h>
@@ -25,6 +26,9 @@ 
 #include "libcamera/internal/software_isp/debayer_params.h"
 
 #include "debayer_cpu.h"
+#if HAVE_DEBAYER_EGL
+#include "debayer_egl.h"
+#endif
 
 /**
  * \file software_isp.cpp
@@ -117,7 +121,25 @@  SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
 	}
 	stats->statsReady.connect(this, &SoftwareIsp::statsReady);
 
-	debayer_ = std::make_unique<DebayerCpu>(std::move(stats), configuration);
+#if HAVE_DEBAYER_EGL
+	std::optional<std::string> softISPMode = configuration.envOption("LIBCAMERA_SOFTISP_MODE", { "software_isp", "mode" });
+
+	if (softISPMode && softISPMode == "gpu") {
+		debayer_ = std::make_unique<DebayerEGL>(std::move(stats), configuration);
+		if (!debayer_) {
+			LOG(SoftwareIsp, Error) << "Failed to instantiate GPUISP";
+			return;
+		}
+	}
+#endif
+	if (!debayer_)
+		debayer_ = std::make_unique<DebayerCpu>(std::move(stats), configuration);
+
+	if (!debayer_) {
+		LOG(SoftwareIsp, Error) << "Failed to create Debayer object";
+		return;
+	}
+
 	debayer_->inputBufferReady.connect(this, &SoftwareIsp::inputReady);
 	debayer_->outputBufferReady.connect(this, &SoftwareIsp::outputReady);