| Message ID | 20250224185235.43381-2-mzamazal@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series |
|
| Related | show |
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@redhat.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@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)
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)
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@redhat.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(-)