| Message ID | 20250824-b4-v0-5-2-gpuisp-v2-a-v2-16-96f4576c814e@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > The eGL class wants to be able to teardown its sync_ data member > properly but, doing so in the destructor means we can't make the eGL > context current and thus can't tear down the sync primitive properly. > > Introduce a stop() method to the debayer class which triggers from the > softisp's stop method, allowing a controlled and appropriate tear-down > of debayer-egl and egl class related data well before the destructors > get invoked. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > src/libcamera/software_isp/debayer.h | 1 + > src/libcamera/software_isp/software_isp.cpp | 3 +++ > 2 files changed, 4 insertions(+) > > diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h > index 214bcdd3c535bae7851d6e0221ba68c19785507d..352ffb89ad9d5a32ed1bbb14253af5e3f21d508c 100644 > --- a/src/libcamera/software_isp/debayer.h > +++ b/src/libcamera/software_isp/debayer.h > @@ -47,6 +47,7 @@ public: > strideAndFrameSize(const PixelFormat &outputFormat, const Size &size) = 0; > > virtual void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params) = 0; > + void stop() {} Not virtual? > virtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0; > > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp > index e8fa8a17a11c63ebab1338a90df204f4a888c4d6..e6bf76f214280194bc20aaaed4b5bc96598436fb 100644 > --- a/src/libcamera/software_isp/software_isp.cpp > +++ b/src/libcamera/software_isp/software_isp.cpp > @@ -363,6 +363,9 @@ int SoftwareIsp::start() > */ > void SoftwareIsp::stop() > { > + debayer_->invokeMethod(&Debayer::stop, > + ConnectionTypeQueued); The line break between the arguments needed? > + > ispWorkerThread_.exit(); > ispWorkerThread_.wait();
On 26/08/2025 13:13, Milan Zamazal wrote: >> + void stop() {} > Not virtual? Didn't seem worth implementing in cpu_debayer. I don't mind changing thought. --- bod
Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > On 26/08/2025 13:13, Milan Zamazal wrote: >>> + void stop() {} >> Not virtual? > > Didn't seem worth implementing in cpu_debayer. It can be inherited there. The main problem is that debayer_->invokeMethod(&Debayer::stop, ConnectionTypeQueued); invokes a wrong method for GPU if I'm not mistaken. > I don't mind changing thought. > > --- > bod
On 26/08/2025 13:42, Milan Zamazal wrote: > Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > >> On 26/08/2025 13:13, Milan Zamazal wrote: >>>> + void stop() {} >>> Not virtual? >> >> Didn't seem worth implementing in cpu_debayer. > > It can be inherited there. The main problem is that > > debayer_->invokeMethod(&Debayer::stop, ConnectionTypeQueued); > > invokes a wrong method for GPU if I'm not mistaken. No you're right :( --- bod
diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h index 214bcdd3c535bae7851d6e0221ba68c19785507d..352ffb89ad9d5a32ed1bbb14253af5e3f21d508c 100644 --- a/src/libcamera/software_isp/debayer.h +++ b/src/libcamera/software_isp/debayer.h @@ -47,6 +47,7 @@ public: strideAndFrameSize(const PixelFormat &outputFormat, const Size &size) = 0; virtual void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params) = 0; + void stop() {} virtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0; diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index e8fa8a17a11c63ebab1338a90df204f4a888c4d6..e6bf76f214280194bc20aaaed4b5bc96598436fb 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -363,6 +363,9 @@ int SoftwareIsp::start() */ void SoftwareIsp::stop() { + debayer_->invokeMethod(&Debayer::stop, + ConnectionTypeQueued); + ispWorkerThread_.exit(); ispWorkerThread_.wait();
The eGL class wants to be able to teardown its sync_ data member properly but, doing so in the destructor means we can't make the eGL context current and thus can't tear down the sync primitive properly. Introduce a stop() method to the debayer class which triggers from the softisp's stop method, allowing a controlled and appropriate tear-down of debayer-egl and egl class related data well before the destructors get invoked. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- src/libcamera/software_isp/debayer.h | 1 + src/libcamera/software_isp/software_isp.cpp | 3 +++ 2 files changed, 4 insertions(+)