[RESEND] libcamera: software_isp: Skip stop when the worker has not started
diff mbox series

Message ID 20260907104012.20897-1-birk.skyum@pm.me
State New
Headers show
Series
  • [RESEND] libcamera: software_isp: Skip stop when the worker has not started
Related show

Commit Message

Birk Skyum Sept. 7, 2026, 10:40 a.m. UTC
The simple pipeline calls stop() if the capture device fails to start
streaming, before SoftwareIsp::start() has run. The blocking invocation
of Debayer::stop() then waits indefinitely for a worker thread that is
not running.

Return early when the worker has not started so that Camera::start()
can report the original capture error.

Bug: https://gitlab.freedesktop.org/camera/libcamera/-/issues/349
Signed-off-by: Birk Skyum <birk.skyum@pm.me>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
Resending inline with git send-email, as requested. The first inline
submission had damaged whitespace, and the subsequent resend used an
attachment. This resend preserves the code and adds Kieran's review tag.

Tested on a Lenovo Yoga Slim 7x with libcamera 0.7.2. Injecting an
ETIMEDOUT failure into VIDIOC_STREAMON causes the unpatched cam process
to hang until an eight-second timeout. With this patch, cam promptly
returns the capture-start error. This does not fix the underlying
sensor-start failure.

Original submission: https://patchwork.libcamera.org/patch/28194/
Review: https://lists.libcamera.org/pipermail/libcamera-devel/2026-September/061900.html
Reproducer: https://gist.github.com/birkskyum/5ec156ca5654d7ac2251a275718c94b3

 src/libcamera/software_isp/software_isp.cpp | 3 +++
 1 file changed, 3 insertions(+)


base-commit: 191e202178f02430b5942397c70d215cdd2056fa

Comments

Hans de Goede Sept. 14, 2026, 1 p.m. UTC | #1
On 7-Sep-26 12:40 PM, Birk Skyum wrote:
> The simple pipeline calls stop() if the capture device fails to start
> streaming, before SoftwareIsp::start() has run. The blocking invocation
> of Debayer::stop() then waits indefinitely for a worker thread that is
> not running.
> 
> Return early when the worker has not started so that Camera::start()
> can report the original capture error.
> 
> Bug: https://gitlab.freedesktop.org/camera/libcamera/-/issues/349
> Signed-off-by: Birk Skyum <birk.skyum@pm.me>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans



> ---
> Resending inline with git send-email, as requested. The first inline
> submission had damaged whitespace, and the subsequent resend used an
> attachment. This resend preserves the code and adds Kieran's review tag.
> 
> Tested on a Lenovo Yoga Slim 7x with libcamera 0.7.2. Injecting an
> ETIMEDOUT failure into VIDIOC_STREAMON causes the unpatched cam process
> to hang until an eight-second timeout. With this patch, cam promptly
> returns the capture-start error. This does not fix the underlying
> sensor-start failure.
> 
> Original submission: https://patchwork.libcamera.org/patch/28194/
> Review: https://lists.libcamera.org/pipermail/libcamera-devel/2026-September/061900.html
> Reproducer: https://gist.github.com/birkskyum/5ec156ca5654d7ac2251a275718c94b3
> 
>  src/libcamera/software_isp/software_isp.cpp | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index c73a16c..c801034 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -395,6 +395,9 @@ int SoftwareIsp::start()
>   */
>  void SoftwareIsp::stop()
>  {
> +	if (!ispWorkerThread_.isRunning())
> +		return;
> +
>  	debayer_->invokeMethod(&Debayer::stop,
>  			       ConnectionTypeBlocking);
>  
> 
> base-commit: 191e202178f02430b5942397c70d215cdd2056fa

Patch
diff mbox series

diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index c73a16c..c801034 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -395,6 +395,9 @@  int SoftwareIsp::start()
  */
 void SoftwareIsp::stop()
 {
+	if (!ispWorkerThread_.isRunning())
+		return;
+
 	debayer_->invokeMethod(&Debayer::stop,
 			       ConnectionTypeBlocking);