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

[Laurent Pinchart]

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
> 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/

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