[PATCH v2 2/5] libcamera: software_isp: Handle queued output buffers on stop

[Laurent Pinchart]

Hi Milan,

Thank you for the patch.

On Mon, Feb 24, 2025 at 07:52:32PM +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 output buffers from the SoftwareIsp
> slots once SoftwareIsp is stopped.  Let's track the queued output
> buffers and mark those still pending as canceled in SoftwareIsp::stop
> and return them to the pipeline handler.
> 
> Dealing with input buffers is addressed in a separate patch.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  .../internal/software_isp/software_isp.h      |  1 +
>  src/libcamera/software_isp/software_isp.cpp   | 22 ++++++++++++++++---
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index af0dcc24..f2344355 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -101,6 +101,7 @@ private:
>  
>  	std::unique_ptr<ipa::soft::IPAProxySoft> ipa_;
>  	bool running_;
> +	std::deque<FrameBuffer *> queuedOutputBuffers_;

#include <deque>

>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 1a39f4d8..4339e547 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -17,6 +17,7 @@
>  #include <libcamera/formats.h>
>  #include <libcamera/stream.h>
>  
> +#include "libcamera/internal/framebuffer.h"
>  #include "libcamera/internal/ipa_manager.h"
>  #include "libcamera/internal/software_isp/debayer_params.h"
>  
> @@ -300,8 +301,11 @@ int SoftwareIsp::queueBuffers(uint32_t frame, FrameBuffer *input,
>  			return -EINVAL;
>  	}
>  
> -	for (auto iter = outputs.begin(); iter != outputs.end(); iter++)
> -		process(frame, input, iter->second);
> +	for (auto iter = outputs.begin(); iter != outputs.end(); iter++) {
> +		FrameBuffer *const buffer = iter->second;
> +		queuedOutputBuffers_.push_back(buffer);
> +		process(frame, input, buffer);
> +	}
>  
>  	return 0;
>  }
> @@ -331,6 +335,13 @@ void SoftwareIsp::stop()
>  
>  	running_ = false;
>  	ipa_->stop();
> +
> +	for (auto buffer : queuedOutputBuffers_) {
> +		FrameMetadata &metadata = buffer->_d()->metadata();
> +		metadata.status = FrameMetadata::FrameCancelled;
> +		outputBufferReady.emit(buffer);
> +	}
> +	queuedOutputBuffers_.clear();
>  }
>  
>  /**
> @@ -369,7 +380,12 @@ void SoftwareIsp::inputReady(FrameBuffer *input)
>  
>  void SoftwareIsp::outputReady(FrameBuffer *output)
>  {
> -	outputBufferReady.emit(output);
> +	if (running_) {
> +		queuedOutputBuffers_.erase(find(queuedOutputBuffers_.begin(),

s/find/std::find/

and #include <algorithm>.

This being said, the software ISP always processes buffers in order. Do
we need to call find(), can't we just call pop_front() with an assertion
to ensure the element matches ?

> +						queuedOutputBuffers_.end(),
> +						output));
> +		outputBufferReady.emit(output);
> +	}
>  }
>  
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart