[PATCH v3] libcamera: software_isp: Handle signals in the proper thread

[Kieran Bingham]

Quoting Milan Zamazal (2025-01-31 19:59:28)
> inputBufferReady ready signal in the simple pipeline is handled in the
> pipeline handler thread.  outputBufferReady and ispStatsReady signals
> should be handled there too.
> 
> Rather than relying on the user of the SoftwareIsp instance, let
> SoftwareIsp inherits Object.  SoftwareIsp serves as a signal proxy, the
> signals above are emitted from signal handlers.  This means that if
> SoftwareIsp inherits Object then the slots are invoked in SoftwareIsp
> thread.  Which is the camera manager thread because the SoftwareIsp
> instance is created there.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  .../internal/software_isp/software_isp.h         |  3 ++-
>  src/libcamera/pipeline/simple/simple.cpp         | 16 +---------------
>  2 files changed, 3 insertions(+), 16 deletions(-)
> 
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index d51b03fd..440a296d 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -18,6 +18,7 @@
>  
>  #include <libcamera/base/class.h>
>  #include <libcamera/base/log.h>
> +#include <libcamera/base/object.h>
>  #include <libcamera/base/signal.h>
>  #include <libcamera/base/thread.h>
>  
> @@ -43,7 +44,7 @@ struct StreamConfiguration;
>  
>  LOG_DECLARE_CATEGORY(SoftwareIsp)
>  
> -class SoftwareIsp
> +class SoftwareIsp : public Object
>  {
>  public:
>         SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 8ac24e6e..6e039bf3 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -537,21 +537,7 @@ int SimpleCameraData::init()
>                                 << "Failed to create software ISP, disabling software debayering";
>                         swIsp_.reset();
>                 } else {
> -                       /*
> -                        * The inputBufferReady signal is emitted from the soft ISP thread,
> -                        * and needs to be handled in the pipeline handler thread. Signals
> -                        * implement queued delivery, but this works transparently only if
> -                        * the receiver is bound to the target thread. As the
> -                        * SimpleCameraData class doesn't inherit from the Object class, it
> -                        * is not bound to any thread, and the signal would be delivered
> -                        * synchronously. Instead, connect the signal to a lambda function
> -                        * bound explicitly to the pipe, which is bound to the pipeline
> -                        * handler thread. The function then simply forwards the call to
> -                        * conversionInputDone().
> -                        */
> -                       swIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {
> -                               this->conversionInputDone(buffer);
> -                       });
> +                       swIsp_->inputBufferReady.connect(this, &SimpleCameraData::conversionInputDone);

This looks like it was easier than I thought it was going to be!


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

>                         swIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);
>                         swIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);
>                         swIsp_->setSensorControls.connect(this, &SimpleCameraData::setSensorControls);
> -- 
> 2.48.1
>