[PATCH v8 8/8] libcamera: software_isp: Refactor SoftwareIsp to use DmaBufAllocator::exportBuffers

Milan Zamazal mzamazal at redhat.com
Tue Aug 6 12:29:19 CEST 2024


Hi,

thank you for the patch.

Harvey Yang <chenghaoyang at chromium.org> writes:

> As the helper function DmaBufAllocator::exportBuffers is added, we can
> avoid some code duplication in SoftwareIsp as well.
>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> ---
>  src/libcamera/software_isp/software_isp.cpp | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index c8748d88..5240eb3f 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -256,23 +256,7 @@ int SoftwareIsp::exportBuffers(const Stream *stream, unsigned int count,
>  	if (stream == nullptr)
>  		return -EINVAL;
>  
> -	for (unsigned int i = 0; i < count; i++) {
> -		const std::string name = "frame-" + std::to_string(i);
> -		const size_t frameSize = debayer_->frameSize();
> -
> -		FrameBuffer::Plane outPlane;
> -		outPlane.fd = SharedFD(dmaHeap_.alloc(name.c_str(), frameSize));
> -		if (!outPlane.fd.isValid()) {
> -			LOG(SoftwareIsp, Error)
> -				<< "failed to allocate a dma_buf";
> -			return -ENOMEM;
> -		}
> -		outPlane.offset = 0;
> -		outPlane.length = frameSize;
> -
> -		std::vector<FrameBuffer::Plane> planes{ outPlane };
> -		buffers->emplace_back(std::make_unique<FrameBuffer>(std::move(planes)));
> -	}
> +	dmaHeap_.exportBuffers(count, { debayer_->frameSize() }, buffers);
>  
>  	return count;

I think it should be

  return dmaHeap_.exportBuffers(count, { debayer_->frameSize() }, buffers);

so that the right value is returned on errors.

With that change:

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

>  }



More information about the libcamera-devel mailing list