[PATCH v3 2/6] libcamera: software_isp: Handle queued output buffers on stop

[Laurent Pinchart]

Hi Milan,

Thank you for the patch.

On Tue, Feb 25, 2025 at 04:06:08PM +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>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  .../internal/software_isp/software_isp.h      |  2 ++
>  src/libcamera/software_isp/software_isp.cpp   | 23 ++++++++++++++++---
>  2 files changed, 22 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..5073ce7a 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -7,6 +7,7 @@
>  
>  #pragma once
>  
> +#include <deque>
>  #include <functional>
>  #include <initializer_list>
>  #include <map>
> @@ -101,6 +102,7 @@ private:
>  
>  	std::unique_ptr<ipa::soft::IPAProxySoft> ipa_;
>  	bool running_;
> +	std::deque<FrameBuffer *> queuedOutputBuffers_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index 1a39f4d8..140cddf3 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -13,10 +13,13 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  
> +#include <libcamera/base/log.h>
> +
>  #include <libcamera/controls.h>
>  #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 +303,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 +337,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 +382,11 @@ void SoftwareIsp::inputReady(FrameBuffer *input)
>  
>  void SoftwareIsp::outputReady(FrameBuffer *output)
>  {
> -	outputBufferReady.emit(output);
> +	if (running_) {
> +		ASSERT(queuedOutputBuffers_.front() == output);
> +		queuedOutputBuffers_.pop_front();
> +		outputBufferReady.emit(output);
> +	}
>  }
>  
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart