[PATCH v2 1/5] libcamera: software_isp: Emit ispStatsReady only if IPA is running

[Laurent Pinchart]

Hi Milan,

Thank you for the patch.

On Mon, Feb 24, 2025 at 07:52:31PM +0100, Milan Zamazal wrote:
> Software ISP runs debayering in a separate thread and debayering may
> emit statsReady when software ISP (including the IPA) is being stopped.
> The signal waits in a queue and gets invoked later, resulting in an
> assertion error when attempting to invoke a method on the stopped IPA:
> 
>   FATAL default soft_ipa_proxy.cpp:456 assertion
>   "state_ == ProxyRunning" failed in processStatsThread()
> 
> Let's prevent this problem by forwarding the ISP stats signal from
> software ISP only when the IPA is running.  To track this,
> SoftwareISP::running_ variable is introduced.
> 
> Making processing of the other signals in SoftwareISP more robust is
> addressed in the followup patches.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  include/libcamera/internal/software_isp/software_isp.h | 1 +
>  src/libcamera/software_isp/software_isp.cpp            | 5 ++++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index 440a296d..af0dcc24 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -100,6 +100,7 @@ private:
>  	DmaBufAllocator dmaHeap_;
>  
>  	std::unique_ptr<ipa::soft::IPAProxySoft> ipa_;
> +	bool running_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 44baf200..1a39f4d8 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -315,6 +315,7 @@ int SoftwareIsp::start()
>  	int ret = ipa_->start();
>  	if (ret)
>  		return ret;
> +	running_ = true;
>  
>  	ispWorkerThread_.start();
>  	return 0;
> @@ -328,6 +329,7 @@ void SoftwareIsp::stop()
>  	ispWorkerThread_.exit();
>  	ispWorkerThread_.wait();
>  
> +	running_ = false;
>  	ipa_->stop();
>  }
>  
> @@ -356,7 +358,8 @@ void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)
>  
>  void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)
>  {
> -	ispStatsReady.emit(frame, bufferId);
> +	if (running_)
> +		ispStatsReady.emit(frame, bufferId);
>  }
>  
>  void SoftwareIsp::inputReady(FrameBuffer *input)

-- 
Regards,

Laurent Pinchart