<div dir="ltr">Thanks for the review Milan!<div><br></div><div>Uploaded to Gitlab: <a href="https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/commit/7e25bf9aaaa62decb441fdc1627653e4933a43e0">https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/commit/7e25bf9aaaa62decb441fdc1627653e4933a43e0</a></div><div><br></div><div>To avoid the spam, I'll upload another series of patches with other updates.</div><div><br></div><div>BR,</div><div>Harvey</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Aug 6, 2024 at 12:29 PM Milan Zamazal <<a href="mailto:mzamazal@redhat.com">mzamazal@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
thank you for the patch.<br>
<br>
Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>> writes:<br>
<br>
> As the helper function DmaBufAllocator::exportBuffers is added, we can<br>
> avoid some code duplication in SoftwareIsp as well.<br>
><br>
> Signed-off-by: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
> ---<br>
>  src/libcamera/software_isp/software_isp.cpp | 18 +-----------------<br>
>  1 file changed, 1 insertion(+), 17 deletions(-)<br>
><br>
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp<br>
> index c8748d88..5240eb3f 100644<br>
> --- a/src/libcamera/software_isp/software_isp.cpp<br>
> +++ b/src/libcamera/software_isp/software_isp.cpp<br>
> @@ -256,23 +256,7 @@ int SoftwareIsp::exportBuffers(const Stream *stream, unsigned int count,<br>
>       if (stream == nullptr)<br>
>               return -EINVAL;<br>
>  <br>
> -     for (unsigned int i = 0; i < count; i++) {<br>
> -             const std::string name = "frame-" + std::to_string(i);<br>
> -             const size_t frameSize = debayer_->frameSize();<br>
> -<br>
> -             FrameBuffer::Plane outPlane;<br>
> -             outPlane.fd = SharedFD(dmaHeap_.alloc(name.c_str(), frameSize));<br>
> -             if (!outPlane.fd.isValid()) {<br>
> -                     LOG(SoftwareIsp, Error)<br>
> -                             << "failed to allocate a dma_buf";<br>
> -                     return -ENOMEM;<br>
> -             }<br>
> -             outPlane.offset = 0;<br>
> -             outPlane.length = frameSize;<br>
> -<br>
> -             std::vector<FrameBuffer::Plane> planes{ outPlane };<br>
> -             buffers->emplace_back(std::make_unique<FrameBuffer>(std::move(planes)));<br>
> -     }<br>
> +     dmaHeap_.exportBuffers(count, { debayer_->frameSize() }, buffers);<br>
>  <br>
>       return count;<br>
<br>
I think it should be<br>
<br>
  return dmaHeap_.exportBuffers(count, { debayer_->frameSize() }, buffers);<br>
<br>
so that the right value is returned on errors.<br>
<br>
With that change:<br>
<br>
Reviewed-by: Milan Zamazal <<a href="mailto:mzamazal@redhat.com" target="_blank">mzamazal@redhat.com</a>><br>
<br>
>  }<br>
<br>
</blockquote></div><br clear="all"><div><br></div><span class="gmail_signature_prefix">-- </span><br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>BR,</div>Harvey Yang</div></div>