[{"id":30625,"web_url":"https://patchwork.libcamera.org/comment/30625/","msgid":"<87o766ndvk.fsf@redhat.com>","date":"2024-08-06T10:29:19","subject":"Re: [PATCH v8 8/8] libcamera: software_isp: Refactor SoftwareIsp to\n\tuse DmaBufAllocator::exportBuffers","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi,\n\nthank you for the patch.\n\nHarvey Yang writes:\n\n> As the helper function DmaBufAllocator::exportBuffers is added, we can\n> avoid some code duplication in SoftwareIsp as well.\n>\n> Signed-off-by: Harvey Yang \n> ---\n> src/libcamera/software_isp/software_isp.cpp | 18 +-----------------\n> 1 file changed, 1 insertion(+), 17 deletions(-)\n>\n> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> index c8748d88..5240eb3f 100644\n> --- a/src/libcamera/software_isp/software_isp.cpp\n> +++ b/src/libcamera/software_isp/software_isp.cpp\n> @@ -256,23 +256,7 @@ int SoftwareIsp::exportBuffers(const Stream *stream, unsigned int count,\n> \tif (stream == nullptr)\n> \t\treturn -EINVAL;\n> \n> -\tfor (unsigned int i = 0; i < count; i++) {\n> -\t\tconst std::string name = \"frame-\" + std::to_string(i);\n> -\t\tconst size_t frameSize = debayer_->frameSize();\n> -\n> -\t\tFrameBuffer::Plane outPlane;\n> -\t\toutPlane.fd = SharedFD(dmaHeap_.alloc(name.c_str(), frameSize));\n> -\t\tif (!outPlane.fd.isValid()) {\n> -\t\t\tLOG(SoftwareIsp, Error)\n> -\t\t\t\t<< \"failed to allocate a dma_buf\";\n> -\t\t\treturn -ENOMEM;\n> -\t\t}\n> -\t\toutPlane.offset = 0;\n> -\t\toutPlane.length = frameSize;\n> -\n> -\t\tstd::vector planes{ outPlane };\n> -\t\tbuffers->emplace_back(std::make_unique(std::move(planes)));\n> -\t}\n> +\tdmaHeap_.exportBuffers(count, { debayer_->frameSize() }, buffers);\n> \n> \treturn count;\n\nI think it should be\n\n return dmaHeap_.exportBuffers(count, { debayer_->frameSize() }, buffers);\n\nso that the right value is returned on errors.\n\nWith that change:\n\nReviewed-by: Milan Zamazal \n\n> }","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6F977BE173\n\tfor ;\n\tTue, 6 Aug 2024 10:29:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BF8216338D;\n\tTue, 6 Aug 2024 12:29:27 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5D9DB6195D\n\tfor ;\n\tTue, 6 Aug 2024 12:29:26 +0200 (CEST)","from mail-wm1-f71.google.com (mail-wm1-f71.google.com\n\t[209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-601-DIhnFFPcPoaulpN3C56TxQ-1; Tue, 06 Aug 2024 06:29:22 -0400","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-428e0d30911so4131145e9.2\n\tfor ;\n\tTue, 06 Aug 2024 03:29:22 -0700 (PDT)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-4282b89aa7bsm238826965e9.5.2024.08.06.03.29.20\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 06 Aug 2024 03:29:20 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"arL8uFkN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1722940165;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=SKN9Kb6F8f6Lh3DbUhs08L0CGqYrTkDIu90uT4DZlB8=;\n\tb=arL8uFkNWAGikzVB1usXqlkLnhS4/0VOw1XOHczG+8iSu5T14fS9G4rGa0sGfoMv87i9YX\n\t9u95RVkdsSP5bFYIC54eMFLUXjYdsyZamNWD8pVWAA29zj4d3hxVLf7ULwW88mmvpWQl5w\n\taCXFshBOv0aduV1nuhSBAUP1Ps0RqsY=","X-MC-Unique":"DIhnFFPcPoaulpN3C56TxQ-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1722940161; x=1723544961;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=SKN9Kb6F8f6Lh3DbUhs08L0CGqYrTkDIu90uT4DZlB8=;\n\tb=tZk9/1JrtKyQfmRZ1rlbKDuXP99QPbKVesuYPGi8WNkrnjLG1ZG5DVnfj/5cihPwPG\n\tScbCQXf/bCJWWQfyqgZTDOffAxrMp+64+ru9aKVox51qVTjYea7VlLJyyh5TBM0o5a7/\n\tJmv0RXFkExzJ9gaUdY2mfbJ8z4eQoD73sgEomt/FT3dWhP8LS7y1tGUH7CupA/WRNiAU\n\tFOn7Crrtw1Qh2uD9fkrqnZxgp7Yk83HmRJTd5R3XsFejz5AnPrW03N2CIEvFQUeT2jrz\n\tIl3TMNXMbTt0K3NBMz/q1JQ1qT4GEgL+/L4rsZYmEySwex/mWeUaDgNALY3UkHYmqHi2\n\tbOPg==","X-Gm-Message-State":"AOJu0Yw6CRXKXJEwBOqy+VOh+zClf9f3gqJ54Vn4br/nxA7ZRIV55UoO\n\t33n9yJPgBCTueIoexJiUZjfuwRHkym9VK0rT9Tp7OU1wXNYSuQcg7njhKlyRlxUTiCkV18SieE3\n\tr9FrFp/g5T5ohdFiBo6GfY4g5V8FPx4neIY9GsVoP9TC69QICJSBmgAtUFO0TdWtkopkzxNQ=","X-Received":["by 2002:a05:600c:4fc9:b0:426:8ee5:5d24 with SMTP id\n\t5b1f17b1804b1-428e6b2f79fmr107204965e9.20.1722940161632; \n\tTue, 06 Aug 2024 03:29:21 -0700 (PDT)","by 2002:a05:600c:4fc9:b0:426:8ee5:5d24 with SMTP id\n\t5b1f17b1804b1-428e6b2f79fmr107204715e9.20.1722940161119; \n\tTue, 06 Aug 2024 03:29:21 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHkOlAPNSr3ug/J0TieKcsS3N2lwUMVg6II/RMVwu87SMhlKP0cYN9B6Re74vAPsvcDdUOAmQ==","From":"Milan Zamazal ","To":"Harvey Yang ","Cc":"libcamera-devel@lists.libcamera.org,\n\tHarvey Yang ","Subject":"Re: [PATCH v8 8/8] libcamera: software_isp: Refactor SoftwareIsp to\n\tuse DmaBufAllocator::exportBuffers","In-Reply-To":"<20240805135104.139932-9-chenghaoyang@google.com> (Harvey Yang's\n\tmessage of \"Mon, 5 Aug 2024 13:48:39 +0000\")","References":"<20240805135104.139932-1-chenghaoyang@google.com>\n\t<20240805135104.139932-9-chenghaoyang@google.com>","Date":"Tue, 06 Aug 2024 12:29:19 +0200","Message-ID":"<87o766ndvk.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30626,"web_url":"https://patchwork.libcamera.org/comment/30626/","msgid":"","date":"2024-08-06T11:43:48","subject":"Re: [PATCH v8 8/8] libcamera: software_isp: Refactor SoftwareIsp to\n\tuse DmaBufAllocator::exportBuffers","submitter":{"id":148,"url":"https://patchwork.libcamera.org/api/people/148/","name":"Cheng-Hao Yang","email":"chenghaoyang@google.com"},"content":"Thanks for the review Milan!\n\nUploaded to Gitlab:\nhttps://gitlab.freedesktop.org/chenghaoyang/libcamera/-/commit/7e25bf9aaaa62decb441fdc1627653e4933a43e0\n\nTo avoid the spam, I'll upload another series of patches with other updates.\n\nBR,\nHarvey\n\nOn Tue, Aug 6, 2024 at 12:29 PM Milan Zamazal wrote:\n\n> Hi,\n>\n> thank you for the patch.\n>\n> Harvey Yang writes:\n>\n> > As the helper function DmaBufAllocator::exportBuffers is added, we can\n> > avoid some code duplication in SoftwareIsp as well.\n> >\n> > Signed-off-by: Harvey Yang \n> > ---\n> > src/libcamera/software_isp/software_isp.cpp | 18 +-----------------\n> > 1 file changed, 1 insertion(+), 17 deletions(-)\n> >\n> > diff --git a/src/libcamera/software_isp/software_isp.cpp\n> b/src/libcamera/software_isp/software_isp.cpp\n> > index c8748d88..5240eb3f 100644\n> > --- a/src/libcamera/software_isp/software_isp.cpp\n> > +++ b/src/libcamera/software_isp/software_isp.cpp\n> > @@ -256,23 +256,7 @@ int SoftwareIsp::exportBuffers(const Stream\n> *stream, unsigned int count,\n> > if (stream == nullptr)\n> > return -EINVAL;\n> >\n> > - for (unsigned int i = 0; i < count; i++) {\n> > - const std::string name = \"frame-\" + std::to_string(i);\n> > - const size_t frameSize = debayer_->frameSize();\n> > -\n> > - FrameBuffer::Plane outPlane;\n> > - outPlane.fd = SharedFD(dmaHeap_.alloc(name.c_str(),\n> frameSize));\n> > - if (!outPlane.fd.isValid()) {\n> > - LOG(SoftwareIsp, Error)\n> > - << \"failed to allocate a dma_buf\";\n> > - return -ENOMEM;\n> > - }\n> > - outPlane.offset = 0;\n> > - outPlane.length = frameSize;\n> > -\n> > - std::vector planes{ outPlane };\n> > -\n> buffers->emplace_back(std::make_unique(std::move(planes)));\n> > - }\n> > + dmaHeap_.exportBuffers(count, { debayer_->frameSize() }, buffers);\n> >\n> > return count;\n>\n> I think it should be\n>\n> return dmaHeap_.exportBuffers(count, { debayer_->frameSize() }, buffers);\n>\n> so that the right value is returned on errors.\n>\n> With that change:\n>\n> Reviewed-by: Milan Zamazal \n>\n> > }\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9A51FC323E\n\tfor ;\n\tTue, 6 Aug 2024 11:44:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B68D66337C;\n\tTue, 6 Aug 2024 13:44:28 +0200 (CEST)","from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com\n\t[IPv6:2a00:1450:4864:20::12f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4435A6195D\n\tfor ;\n\tTue, 6 Aug 2024 13:44:26 +0200 (CEST)","by mail-lf1-x12f.google.com with SMTP id\n\t2adb3069b0e04-52f04b4abdcso802244e87.2\n\tfor ;\n\tTue, 06 Aug 2024 04:44:26 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"iceIM32F\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=google.com; s=20230601; t=1722944665; x=1723549465;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=WAcFd/QbggIvLjnJc4FQ7WMY2zrQZb9UFJsidwjQsuE=;\n\tb=iceIM32FAzZkTKt+A7o3fH0jaPWYNT85GAGjCDqhAy9HinZCkaFwVwofaMPdagOqjF\n\t9F/05hihmTeZMKy3TKno0MeDm/SHC1VxpqkM/2Bb3rFVuUlkvQmDsfFt7p7ahYtK3+ej\n\tZU/TMF22fRbIlIfteJnB0OewAvc/06COP6DrJGc9mZZND+jHrBdJFUf0yP0sXweJTKcC\n\tEk2wS1kPW7jheBTb2ODJKhu9ilLMmrKawe9Z6h5XrKDokuAq5EYY3mgTyAtqYwJQNpby\n\tB41xTTLwBAffJgmVYa4XCv9Q1bgUri4vPxE1XFOpLf4mJ+fAWrxMvzt1tDDqyQnr6bEz\n\t75/w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1722944665; x=1723549465;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=WAcFd/QbggIvLjnJc4FQ7WMY2zrQZb9UFJsidwjQsuE=;\n\tb=AAzJ3pIJK6C9N0YgORR9ZbWrSuMJrL7iL6qTGUICxENXJyFVPyO3SM+Osrjswr/Bt7\n\tlNxaqZMbmXg3kZPII92K39TQ6OEm5DmMVauarFkFiMOBWVeRYC5zJ6vWZwpQCBAev9cq\n\tgqLFIgKwhCkYAJiUfV1yyt7O8GoX8LsqJDR0hFhaEVjyGdWwskkBRbNR3N3vk4LVAjyh\n\twy7S6U9GeKUiYbfSd6glmetFyal7JOUTvfU8wnE5LDsZSGMEVAwSXNfS4pjB1yvi7dfS\n\tefPPdBCXyf8DVNV76rX9LcaeeAecTGb/mEDso5J4NR60gXfgx7KFtEXDpEi8SCdlCnJh\n\tdUQw==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCWmnad5tX/IPhHwG8GqrBlOxkKGDvAj+6t/8n9TorNNfEMorbUM76R4/gdVAQPxM92pzBGeBgc0t6rHcZ0amuIxR4tyln5l2tHMaqPLtmJWPDKCoQ==","X-Gm-Message-State":"AOJu0YwKmJRzB/tKkCMTK/eTOfNx93lBgXH21f9p0YcDRpGX2ripwi+w\n\t26S2YtqQRgafA5dCYyxkmN/fewtF2L6Qrm8dxwccZpxucww1g/qv62ytNXvdufYxLGbDyy7AB0x\n\ttwrTcCZatUN07wC5MweXbSmX2xVjQpyiELZtg","X-Google-Smtp-Source":"AGHT+IFLywS/O755uZDEuxMhF4kwHVArFtWUww36mjW8CxE6ev3dbPsFY6Lhr7fPBdmnJM0AKafB2YClUP9Bo7B0lEA=","X-Received":"by 2002:a05:6512:2201:b0:52e:76e8:e18e with SMTP id\n\t2adb3069b0e04-530bb363022mr9511210e87.7.1722944664880;\n\tTue, 06 Aug 2024 04:44:24 -0700 (PDT)","MIME-Version":"1.0","References":"<20240805135104.139932-1-chenghaoyang@google.com>\n\t<20240805135104.139932-9-chenghaoyang@google.com>\n\t<87o766ndvk.fsf@redhat.com>","In-Reply-To":"<87o766ndvk.fsf@redhat.com>","From":"Cheng-Hao Yang ","Date":"Tue, 6 Aug 2024 13:43:48 +0200","Message-ID":"","Subject":"Re: [PATCH v8 8/8] libcamera: software_isp: Refactor SoftwareIsp to\n\tuse DmaBufAllocator::exportBuffers","To":"Milan Zamazal ","Cc":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"000000000000b8a19d061f02503c\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]