[PATCH v2] libcamera: software_isp: Replace malloc() with std::vector<>

[Milan Zamazal]

Hi Laurent,

thank you for the patch.

Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:

> libcamera is implemented in C++, use std::vector<> to manage the
> dynamically allocated line buffers instead of malloc() and free(). This
> simplifies the code and improves memory safety by ensuring no allocation
> will be leaked.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Milan Zamazal <mzamazal at redhat.com>

(I thought that malloc was used here to be able to detect allocation
failures when exceptions are not used but it's probably not an issue
with total size of line buffers not being much more than ~100 kB
usually).

> ---
> Changes since v1:
>
> - Use std::vector instead of new[]() and delete[]()
> ---
>  src/libcamera/software_isp/debayer_cpu.cpp | 31 +++++++++-------------
>  src/libcamera/software_isp/debayer_cpu.h   |  2 +-
>  2 files changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index f8d2677d657a..077f7f4bc45b 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -49,16 +49,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats)
>  	/* Initialize color lookup tables */
>  	for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)
>  		red_[i] = green_[i] = blue_[i] = i;
> -
> -	for (unsigned int i = 0; i < kMaxLineBuffers; i++)
> -		lineBuffers_[i] = nullptr;
>  }
>  
> -DebayerCpu::~DebayerCpu()
> -{
> -	for (unsigned int i = 0; i < kMaxLineBuffers; i++)
> -		free(lineBuffers_[i]);
> -}
> +DebayerCpu::~DebayerCpu() = default;
>  
>  #define DECLARE_SRC_POINTERS(pixel_t)                            \
>  	const pixel_t *prev = (const pixel_t *)src[0] + xShift_; \
> @@ -526,13 +519,10 @@ int DebayerCpu::configure(const StreamConfiguration &inputCfg,
>  	lineBufferPadding_ = inputConfig_.patternSize.width * inputConfig_.bpp / 8;
>  	lineBufferLength_ = window_.width * inputConfig_.bpp / 8 +
>  			    2 * lineBufferPadding_;
> -	for (unsigned int i = 0;
> -	     i < (inputConfig_.patternSize.height + 1) && enableInputMemcpy_;
> -	     i++) {
> -		free(lineBuffers_[i]);
> -		lineBuffers_[i] = (uint8_t *)malloc(lineBufferLength_);
> -		if (!lineBuffers_[i])
> -			return -ENOMEM;
> +
> +	if (enableInputMemcpy_) {
> +		for (unsigned int i = 0; i <= inputConfig_.patternSize.height; i++)
> +			lineBuffers_[i].resize(lineBufferLength_);
>  	}
>  
>  	measuredFrames_ = 0;
> @@ -587,9 +577,10 @@ void DebayerCpu::setupInputMemcpy(const uint8_t *linePointers[])
>  		return;
>  
>  	for (unsigned int i = 0; i < patternHeight; i++) {
> -		memcpy(lineBuffers_[i], linePointers[i + 1] - lineBufferPadding_,
> +		memcpy(lineBuffers_[i].data(),
> +		       linePointers[i + 1] - lineBufferPadding_,
>  		       lineBufferLength_);
> -		linePointers[i + 1] = lineBuffers_[i] + lineBufferPadding_;
> +		linePointers[i + 1] = lineBuffers_[i].data() + lineBufferPadding_;
>  	}
>  
>  	/* Point lineBufferIndex_ to first unused lineBuffer */
> @@ -614,9 +605,11 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[])
>  	if (!enableInputMemcpy_)
>  		return;
>  
> -	memcpy(lineBuffers_[lineBufferIndex_], linePointers[patternHeight] - lineBufferPadding_,
> +	memcpy(lineBuffers_[lineBufferIndex_].data(),
> +	       linePointers[patternHeight] - lineBufferPadding_,
>  	       lineBufferLength_);
> -	linePointers[patternHeight] = lineBuffers_[lineBufferIndex_] + lineBufferPadding_;
> +	linePointers[patternHeight] = lineBuffers_[lineBufferIndex_].data()
> +				    + lineBufferPadding_;
>  
>  	lineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1);
>  }
> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
> index 1dac64351f1d..8237a64be733 100644
> --- a/src/libcamera/software_isp/debayer_cpu.h
> +++ b/src/libcamera/software_isp/debayer_cpu.h
> @@ -146,7 +146,7 @@ private:
>  	DebayerInputConfig inputConfig_;
>  	DebayerOutputConfig outputConfig_;
>  	std::unique_ptr<SwStatsCpu> stats_;
> -	uint8_t *lineBuffers_[kMaxLineBuffers];
> +	std::vector<uint8_t> lineBuffers_[kMaxLineBuffers];
>  	unsigned int lineBufferLength_;
>  	unsigned int lineBufferPadding_;
>  	unsigned int lineBufferIndex_;
>
> base-commit: 19bbca3c0b376ba0183f5db53472c8c46cd402b5