[PATCH v3 3/6] libcamera: software_isp: Handle queued input buffers on stop

[Kieran Bingham]

Quoting Laurent Pinchart (2025-02-26 00:03:47)
> Hi Milan,
> 
> Thank you for the patch.
> 
> On Tue, Feb 25, 2025 at 04:06:09PM +0100, Milan Zamazal wrote:
> > When SoftwareIsp stops, input and output buffers queued to it may not
> > yet be fully processed.  They will be eventually returned but stop means
> > stop, there should be no processing related actions invoked afterwards.
> > 
> > Let's stop forwarding processed input buffers from SoftwareIsp slots
> > when SoftwareIsp is stopped.  Let's track the queued input buffers and
> > return them back for capture in SoftwareIsp::stop().
> > 
> > The returned input buffers are marked as canceled.  This is not

s/canceled/cancelled/

> > necessary at the moment but it gives the pipeline handlers chance to
> > deal with this if they need to.
> > 
> > Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> > ---
> >  .../internal/software_isp/software_isp.h       |  1 +
> >  src/libcamera/software_isp/software_isp.cpp    | 18 +++++++++++++++++-
> >  2 files changed, 18 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> > index 5073ce7a..400a4dc5 100644
> > --- a/include/libcamera/internal/software_isp/software_isp.h
> > +++ b/include/libcamera/internal/software_isp/software_isp.h
> > @@ -102,6 +102,7 @@ private:
> >  
> >       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 140cddf3..beac66fc 100644
> > --- a/src/libcamera/software_isp/software_isp.cpp
> > +++ b/src/libcamera/software_isp/software_isp.cpp
> > @@ -303,6 +303,8 @@ int SoftwareIsp::queueBuffers(uint32_t frame, FrameBuffer *input,
> >                       return -EINVAL;
> >       }
> >  
> > +     queuedInputBuffers_.push_back(input);
> > +
> >       for (auto iter = outputs.begin(); iter != outputs.end(); iter++) {
> >               FrameBuffer *const buffer = iter->second;
> >               queuedOutputBuffers_.push_back(buffer);
> > @@ -329,6 +331,9 @@ int SoftwareIsp::start()
> >  
> >  /**
> >   * \brief Stops the Software ISP streaming operation
> > + *
> > + * All pending buffers are returned back as canceled before this method
> > + * finishes.
> 
> s/emthod finishes/function returns/
> 

Will fix.


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

> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> >   */
> >  void SoftwareIsp::stop()
> >  {
> > @@ -344,6 +349,13 @@ void SoftwareIsp::stop()
> >               outputBufferReady.emit(buffer);
> >       }
> >       queuedOutputBuffers_.clear();
> > +
> > +     for (auto buffer : queuedInputBuffers_) {
> > +             FrameMetadata &metadata = buffer->_d()->metadata();
> > +             metadata.status = FrameMetadata::FrameCancelled;
> > +             inputBufferReady.emit(buffer);
> > +     }
> > +     queuedInputBuffers_.clear();
> >  }
> >  
> >  /**
> > @@ -377,7 +389,11 @@ void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)
> >  
> >  void SoftwareIsp::inputReady(FrameBuffer *input)
> >  {
> > -     inputBufferReady.emit(input);
> > +     if (running_) {
> > +             ASSERT(queuedInputBuffers_.front() == input);
> > +             queuedInputBuffers_.pop_front();
> > +             inputBufferReady.emit(input);
> > +     }
> >  }
> >  
> >  void SoftwareIsp::outputReady(FrameBuffer *output)
> 
> -- 
> Regards,
> 
> Laurent Pinchart