| Message ID | 20250611013245.133785-14-bryan.odonoghue@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Bryan, Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > OpenGL requires both configure() and process() to operate on the same > thread. As preparation for that, move current CPU configure into the > WorkerThread with a ConnectionTypeBlocking invocation of > &DebayerCpu::configure. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > src/libcamera/software_isp/software_isp.cpp | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp > index 7bee8f06..e8fa8a17 100644 > --- a/src/libcamera/software_isp/software_isp.cpp > +++ b/src/libcamera/software_isp/software_isp.cpp > @@ -263,7 +263,15 @@ int SoftwareIsp::configure(const StreamConfiguration &inputCfg, > debayer_->moveToThread(&ispWorkerThread_); > ispWorkerThread_.start(); > > - return debayer_->configure(inputCfg, outputCfgs, ccmEnabled_); > + ret = debayer_->invokeMethod(&Debayer::configure, > + ConnectionTypeBlocking, inputCfg, > + outputCfgs, ccmEnabled_); > + if (ret) { > + ispWorkerThread_.exit(); > + ispWorkerThread_.wait(); > + } > + > + return ret; > } > > /** > @@ -386,7 +394,7 @@ void SoftwareIsp::stop() > void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output) > { > ipa_->computeParams(frame); > - debayer_->invokeMethod(&DebayerCpu::process, > + debayer_->invokeMethod(&Debayer::process, > ConnectionTypeQueued, frame, input, output, debayerParams_);s Is this change related? > }
On 17/06/2025 12:26, Milan Zamazal wrote: >> /** >> @@ -386,7 +394,7 @@ void SoftwareIsp::stop() >> void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output) >> { >> ipa_->computeParams(frame); >> - debayer_->invokeMethod(&DebayerCpu::process, >> + debayer_->invokeMethod(&Debayer::process, >> ConnectionTypeQueued, frame, input, output, debayerParams_);s > Is this change related? Yes, its maybe not clear from the commit log but, Mesa/OpenGL operates without handles => the "handle" or pointer to context is stored in a thread-specific magic variable. This means we need to make both configure and process operate on the same thread becuase what will happen otherwise is the GL context won't be known by one of the threads and instead of crashing or giving a meaningful error message it will just fail silently with no output image. --- bod
Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > On 17/06/2025 12:26, Milan Zamazal wrote: >>> /** >>> @@ -386,7 +394,7 @@ void SoftwareIsp::stop() >>> void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output) >>> { >>> ipa_->computeParams(frame); >>> - debayer_->invokeMethod(&DebayerCpu::process, >>> + debayer_->invokeMethod(&Debayer::process, >>> ConnectionTypeQueued, frame, input, output, debayerParams_);s >> Is this change related? > > Yes, its maybe not clear from the commit log but, Mesa/OpenGL operates without handles => the "handle" or pointer to context is stored in a > thread-specific magic variable. But DebayerCpu -> Debayer should be in the next patch (Make the debayer_ object of type class Debayer not DebayerCpu), shouldn't it? > This means we need to make both configure and process operate on the same thread becuase what will happen otherwise is the GL context won't be > known by one of the threads and instead of crashing or giving a meaningful error message it will just fail silently with no output image. Oh, this must be really frustrating.
diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index 7bee8f06..e8fa8a17 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -263,7 +263,15 @@ int SoftwareIsp::configure(const StreamConfiguration &inputCfg, debayer_->moveToThread(&ispWorkerThread_); ispWorkerThread_.start(); - return debayer_->configure(inputCfg, outputCfgs, ccmEnabled_); + ret = debayer_->invokeMethod(&Debayer::configure, + ConnectionTypeBlocking, inputCfg, + outputCfgs, ccmEnabled_); + if (ret) { + ispWorkerThread_.exit(); + ispWorkerThread_.wait(); + } + + return ret; } /** @@ -386,7 +394,7 @@ void SoftwareIsp::stop() void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output) { ipa_->computeParams(frame); - debayer_->invokeMethod(&DebayerCpu::process, + debayer_->invokeMethod(&Debayer::process, ConnectionTypeQueued, frame, input, output, debayerParams_); }
OpenGL requires both configure() and process() to operate on the same thread. As preparation for that, move current CPU configure into the WorkerThread with a ConnectionTypeBlocking invocation of &DebayerCpu::configure. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- src/libcamera/software_isp/software_isp.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)