[v3,15/22] libcamera: software_isp: Start the ISP thread in configure
diff mbox series

Message ID 20251202133157.661407-16-bryan.odonoghue@linaro.org
State Superseded
Headers show
Series
  • GPUISP precursor series
Related show

Commit Message

Bryan O'Donoghue Dec. 2, 2025, 1:31 p.m. UTC 
EGL is not thread-safe and in fact associates invisible handles with the
threadid of the calling context.

As a result we need to make Deabyer::configure() and Debayer::process() in SoftISP execute on
the same thread.

When we call Debayer::configure() in the egl class this will setup and egl context
for us which is associated with the calling thread context. Hence when
Debayer::process(); runs it must run in the same thread as
Debayer::configure() or the hidden Gegl context handles will not point
to the same place.

Move start thread into configure() as a first step towards this.

Co-developed-by: Robert Mader <robert.mader@collabora.com>
Signed-off-by: Robert Mader <robert.mader@collabora.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 src/libcamera/software_isp/software_isp.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Milan Zamazal Dec. 3, 2025, 2:31 p.m. UTC | #1 
Hi,

nack until
https://lists.libcamera.org/pipermail/libcamera-devel/2025-December/055423.html
is clarified.

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> EGL is not thread-safe and in fact associates invisible handles with the
> threadid of the calling context.
>
> As a result we need to make Deabyer::configure() and Debayer::process() in SoftISP execute on
> the same thread.
>
> When we call Debayer::configure() in the egl class this will setup and egl context
> for us which is associated with the calling thread context. Hence when
> Debayer::process(); runs it must run in the same thread as
> Debayer::configure() or the hidden Gegl context handles will not point
> to the same place.
>
> Move start thread into configure() as a first step towards this.
>
> Co-developed-by: Robert Mader <robert.mader@collabora.com>
> Signed-off-by: Robert Mader <robert.mader@collabora.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  src/libcamera/software_isp/software_isp.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 928a2520c..afa4eb7a8 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -265,6 +265,8 @@ int SoftwareIsp::configure(const StreamConfiguration &inputCfg,
>  	if (ret < 0)
>  		return ret;
>  
> +	ispWorkerThread_.start();
> +
>  	return debayer_->configure(inputCfg, outputCfgs, ccmEnabled_);
>  }
>  
> @@ -346,7 +348,6 @@ int SoftwareIsp::start()
>  	if (ret)
>  		return ret;
>  
> -	ispWorkerThread_.start();
>  	return 0;
>  }

Patch
diff mbox series 

diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index 928a2520c..afa4eb7a8 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -265,6 +265,8 @@  int SoftwareIsp::configure(const StreamConfiguration &inputCfg,
 	if (ret < 0)
 		return ret;
 
+	ispWorkerThread_.start();
+
 	return debayer_->configure(inputCfg, outputCfgs, ccmEnabled_);
 }
 
@@ -346,7 +348,6 @@  int SoftwareIsp::start()
 	if (ret)
 		return ret;
 
-	ispWorkerThread_.start();
 	return 0;
 }