[PATCH v3 5/6] libcamera: software_isp: Dispatch messages on stop

[Kieran Bingham]

Quoting Milan Zamazal (2025-02-25 15:06:11)
> There may be pending messages in SoftwareIsp message queue when
> SoftwareIsp stops.  The call to IPAProxySoft::stop() will dispatch them
> before SoftwareIsp::stop() finishes.  But this is dependent on
> IPAProxySoft::stop() implementation, let's break this dependency and
> dispatch messages to SoftwareIsp explicitly in SoftwareIsp::stop().
> 
> This also allows dropping `running_' flag.  Since the SoftwareIsp
> messages get processed and invoke IPA calls before the IPA proxy is set
> to ProxyStopping state and the SoftwareIsp worker thread is no longer
> running, it's guaranteed that no new messages come to SoftwareIsp and
> attempt to call the stopped IPA proxy.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>

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

> ---
>  .../internal/software_isp/software_isp.h      |  1 -
>  src/libcamera/software_isp/software_isp.cpp   | 24 ++++++++-----------
>  2 files changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index 400a4dc5..133b545c 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -101,7 +101,6 @@ private:
>         DmaBufAllocator dmaHeap_;
>  
>         std::unique_ptr<ipa::soft::IPAProxySoft> ipa_;
> -       bool running_;
>         std::deque<FrameBuffer *> queuedInputBuffers_;
>         std::deque<FrameBuffer *> queuedOutputBuffers_;
>  };
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index beac66fc..193713b9 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -14,6 +14,7 @@
>  #include <unistd.h>
>  
>  #include <libcamera/base/log.h>
> +#include <libcamera/base/thread.h>
>  
>  #include <libcamera/controls.h>
>  #include <libcamera/formats.h>
> @@ -323,7 +324,6 @@ int SoftwareIsp::start()
>         int ret = ipa_->start();
>         if (ret)
>                 return ret;
> -       running_ = true;
>  
>         ispWorkerThread_.start();
>         return 0;
> @@ -340,7 +340,8 @@ void SoftwareIsp::stop()
>         ispWorkerThread_.exit();
>         ispWorkerThread_.wait();
>  
> -       running_ = false;
> +       Thread::current()->dispatchMessages(Message::Type::InvokeMessage, this);
> +
>         ipa_->stop();
>  
>         for (auto buffer : queuedOutputBuffers_) {
> @@ -383,26 +384,21 @@ void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)
>  
>  void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)
>  {
> -       if (running_)
> -               ispStatsReady.emit(frame, bufferId);
> +       ispStatsReady.emit(frame, bufferId);
>  }
>  
>  void SoftwareIsp::inputReady(FrameBuffer *input)
>  {
> -       if (running_) {
> -               ASSERT(queuedInputBuffers_.front() == input);
> -               queuedInputBuffers_.pop_front();
> -               inputBufferReady.emit(input);
> -       }
> +       ASSERT(queuedInputBuffers_.front() == input);
> +       queuedInputBuffers_.pop_front();
> +       inputBufferReady.emit(input);
>  }
>  
>  void SoftwareIsp::outputReady(FrameBuffer *output)
>  {
> -       if (running_) {
> -               ASSERT(queuedOutputBuffers_.front() == output);
> -               queuedOutputBuffers_.pop_front();
> -               outputBufferReady.emit(output);
> -       }
> +       ASSERT(queuedOutputBuffers_.front() == output);
> +       queuedOutputBuffers_.pop_front();
> +       outputBufferReady.emit(output);
>  }
>  
>  } /* namespace libcamera */
> -- 
> 2.48.1
>