[PATCH v2 3/5] libcamera: software_isp: Handle queued input buffers on stop

[Laurent Pinchart]

Hi Milan,

Thank you for the patch.

On Mon, Feb 24, 2025 at 07:52:33PM +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().
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  .../internal/software_isp/software_isp.h         |  1 +
>  src/libcamera/software_isp/software_isp.cpp      | 16 +++++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index f2344355..bfe34725 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 *> queuedOutputBuffers_;
> +	std::deque<FrameBuffer *> queuedInputBuffers_;

I'd move input before output.

>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 4339e547..3a605ab2 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -301,6 +301,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);
> @@ -327,6 +329,9 @@ int SoftwareIsp::start()
>  
>  /**
>   * \brief Stops the Software ISP streaming operation
> + *
> + * All pending buffers are returned back (output buffers as canceled) before
> + * this method finishes.

Shouldn't the input buffer be cancelled too ?

>   */
>  void SoftwareIsp::stop()
>  {
> @@ -342,6 +347,10 @@ void SoftwareIsp::stop()
>  		outputBufferReady.emit(buffer);
>  	}
>  	queuedOutputBuffers_.clear();
> +
> +	for (auto buffer : queuedInputBuffers_)
> +		inputBufferReady.emit(buffer);
> +	queuedInputBuffers_.clear();
>  }
>  
>  /**
> @@ -375,7 +384,12 @@ void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)
>  
>  void SoftwareIsp::inputReady(FrameBuffer *input)
>  {
> -	inputBufferReady.emit(input);
> +	if (running_) {
> +		queuedInputBuffers_.erase(find(queuedInputBuffers_.begin(),
> +					       queuedInputBuffers_.end(),
> +					       input));

Same comment as for 2/5.

> +		inputBufferReady.emit(input);
> +	}
>  }
>  
>  void SoftwareIsp::outputReady(FrameBuffer *output)

-- 
Regards,

Laurent Pinchart